Re: [PATCH RESEND v3 3/3] drm/stm: dsi: expose DSI PHY internal clock

2024-06-21 Thread Yannick FERTRE

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

2024-01-29 Thread Raphael Gallais-Pou
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);