Re: [PATCH RESEND v3 3/3] drm/stm: dsi: expose DSI PHY internal clock
Hi Raphael, thanks for the patch. Acked-by: Yannick Fertre Tested-by: Yannick Fertre BR Le 29/01/2024 à 11:41, Raphael Gallais-Pou a écrit : DSISRC __ __\_ |\ pll4_p_ck ->| 1 |dsi_k ck_dsi_phy ->| 0 | |/ A DSI clock is missing in the clock framework. Looking at the clk_summary, it appears that 'ck_dsi_phy' is not implemented. Since the DSI kernel clock is based on the internal DSI pll. The common clock driver can not directly expose this 'ck_dsi_phy' clock because it does not contain any common registers with the DSI. Thus it needs to be done directly within the DSI phy driver. Signed-off-by: Raphael Gallais-Pou --- Changes in v3: - Fix smatch warning: .../dw_mipi_dsi-stm.c:719 dw_mipi_dsi_stm_probe() warn: 'dsi->pclk' from clk_prepare_enable() not released on lines: 719. --- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 247 ++ 1 file changed, 216 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index 82fff9e84345..b20123854c4a 100644 --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c @@ -7,7 +7,9 @@ */ #include +#include #include +#include #include #include #include @@ -77,9 +79,12 @@ enum dsi_color { struct dw_mipi_dsi_stm { void __iomem *base; + struct device *dev; struct clk *pllref_clk; struct clk *pclk; + struct clk_hw txbyte_clk; struct dw_mipi_dsi *dsi; + struct dw_mipi_dsi_plat_data pdata; u32 hw_version; int lane_min_kbps; int lane_max_kbps; @@ -196,29 +201,198 @@ static int dsi_pll_get_params(struct dw_mipi_dsi_stm *dsi, return 0; } -static int dw_mipi_dsi_phy_init(void *priv_data) +#define clk_to_dw_mipi_dsi_stm(clk) \ + container_of(clk, struct dw_mipi_dsi_stm, txbyte_clk) + +static void dw_mipi_dsi_clk_disable(struct clk_hw *clk) { - struct dw_mipi_dsi_stm *dsi = priv_data; + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk); + + DRM_DEBUG_DRIVER("\n"); + + /* Disable the DSI PLL */ + dsi_clear(dsi, DSI_WRPCR, WRPCR_PLLEN); + + /* Disable the regulator */ + dsi_clear(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN); +} + +static int dw_mipi_dsi_clk_enable(struct clk_hw *clk) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk); u32 val; int ret; + DRM_DEBUG_DRIVER("\n"); + /* Enable the regulator */ dsi_set(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN); - ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_RRS, -SLEEP_US, TIMEOUT_US); + ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val & WISR_RRS, + SLEEP_US, TIMEOUT_US); if (ret) DRM_DEBUG_DRIVER("!TIMEOUT! waiting REGU, let's continue\n"); /* Enable the DSI PLL & wait for its lock */ dsi_set(dsi, DSI_WRPCR, WRPCR_PLLEN); - ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_PLLLS, -SLEEP_US, TIMEOUT_US); + ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val & WISR_PLLLS, + SLEEP_US, TIMEOUT_US); if (ret) DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n"); return 0; } +static int dw_mipi_dsi_clk_is_enabled(struct clk_hw *hw) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw); + + return dsi_read(dsi, DSI_WRPCR) & WRPCR_PLLEN; +} + +static unsigned long dw_mipi_dsi_clk_recalc_rate(struct clk_hw *hw, +unsigned long parent_rate) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw); + unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz; + u32 val; + + DRM_DEBUG_DRIVER("\n"); + + pll_in_khz = (unsigned int)(parent_rate / 1000); + + val = dsi_read(dsi, DSI_WRPCR); + + idf = (val & WRPCR_IDF) >> 11; + if (!idf) + idf = 1; + ndiv = (val & WRPCR_NDIV) >> 2; + odf = int_pow(2, (val & WRPCR_ODF) >> 16); + + /* Get the adjusted pll out value */ + pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf); + + return (unsigned long)pll_out_khz * 1000; +} + +static long dw_mipi_dsi_clk_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw); + unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz; + int ret; + + DRM_DEBUG_DRIVER("\n"); + + pll_in_khz = (unsigned int)(*parent_rate / 1000); + + /* Compute best pll parameters */ +
[PATCH RESEND v3 3/3] drm/stm: dsi: expose DSI PHY internal clock
DSISRC __ __\_ |\ pll4_p_ck ->| 1 |dsi_k ck_dsi_phy ->| 0 | |/ A DSI clock is missing in the clock framework. Looking at the clk_summary, it appears that 'ck_dsi_phy' is not implemented. Since the DSI kernel clock is based on the internal DSI pll. The common clock driver can not directly expose this 'ck_dsi_phy' clock because it does not contain any common registers with the DSI. Thus it needs to be done directly within the DSI phy driver. Signed-off-by: Raphael Gallais-Pou --- Changes in v3: - Fix smatch warning: .../dw_mipi_dsi-stm.c:719 dw_mipi_dsi_stm_probe() warn: 'dsi->pclk' from clk_prepare_enable() not released on lines: 719. --- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 247 ++ 1 file changed, 216 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index 82fff9e84345..b20123854c4a 100644 --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c @@ -7,7 +7,9 @@ */ #include +#include #include +#include #include #include #include @@ -77,9 +79,12 @@ enum dsi_color { struct dw_mipi_dsi_stm { void __iomem *base; + struct device *dev; struct clk *pllref_clk; struct clk *pclk; + struct clk_hw txbyte_clk; struct dw_mipi_dsi *dsi; + struct dw_mipi_dsi_plat_data pdata; u32 hw_version; int lane_min_kbps; int lane_max_kbps; @@ -196,29 +201,198 @@ static int dsi_pll_get_params(struct dw_mipi_dsi_stm *dsi, return 0; } -static int dw_mipi_dsi_phy_init(void *priv_data) +#define clk_to_dw_mipi_dsi_stm(clk) \ + container_of(clk, struct dw_mipi_dsi_stm, txbyte_clk) + +static void dw_mipi_dsi_clk_disable(struct clk_hw *clk) { - struct dw_mipi_dsi_stm *dsi = priv_data; + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk); + + DRM_DEBUG_DRIVER("\n"); + + /* Disable the DSI PLL */ + dsi_clear(dsi, DSI_WRPCR, WRPCR_PLLEN); + + /* Disable the regulator */ + dsi_clear(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN); +} + +static int dw_mipi_dsi_clk_enable(struct clk_hw *clk) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk); u32 val; int ret; + DRM_DEBUG_DRIVER("\n"); + /* Enable the regulator */ dsi_set(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN); - ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_RRS, -SLEEP_US, TIMEOUT_US); + ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val & WISR_RRS, + SLEEP_US, TIMEOUT_US); if (ret) DRM_DEBUG_DRIVER("!TIMEOUT! waiting REGU, let's continue\n"); /* Enable the DSI PLL & wait for its lock */ dsi_set(dsi, DSI_WRPCR, WRPCR_PLLEN); - ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_PLLLS, -SLEEP_US, TIMEOUT_US); + ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val & WISR_PLLLS, + SLEEP_US, TIMEOUT_US); if (ret) DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n"); return 0; } +static int dw_mipi_dsi_clk_is_enabled(struct clk_hw *hw) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw); + + return dsi_read(dsi, DSI_WRPCR) & WRPCR_PLLEN; +} + +static unsigned long dw_mipi_dsi_clk_recalc_rate(struct clk_hw *hw, +unsigned long parent_rate) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw); + unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz; + u32 val; + + DRM_DEBUG_DRIVER("\n"); + + pll_in_khz = (unsigned int)(parent_rate / 1000); + + val = dsi_read(dsi, DSI_WRPCR); + + idf = (val & WRPCR_IDF) >> 11; + if (!idf) + idf = 1; + ndiv = (val & WRPCR_NDIV) >> 2; + odf = int_pow(2, (val & WRPCR_ODF) >> 16); + + /* Get the adjusted pll out value */ + pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf); + + return (unsigned long)pll_out_khz * 1000; +} + +static long dw_mipi_dsi_clk_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw); + unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz; + int ret; + + DRM_DEBUG_DRIVER("\n"); + + pll_in_khz = (unsigned int)(*parent_rate / 1000); + + /* Compute best pll parameters */ + idf = 0; + ndiv = 0; + odf = 0; + + ret = dsi_pll_get_params(dsi, pll_in_khz, rate / 1000, +&idf, &ndiv, &odf);