[PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate

Icenowy Zheng posted 3 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate
Posted by Icenowy Zheng 4 months, 1 week ago
The DPU pixel clock rate corresponds to the required dot clock of the
display mode, so it needs to be tweakable.

Add support to change it, by adding generic divider setting code,
arming the code to the dpu0/dpu1 clocks, and setting the pixel clock
connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to propagate
it to the dividers.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
Changes in v2:
- Dropped round_rate() because of deprecation.
- Changed the logic of determine_rate() to early return if the divider
  could be changed.

 drivers/clk/thead/clk-th1520-ap.c | 64 ++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index 0b5458af8c550..b220a8ed22607 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -55,6 +55,7 @@ struct ccu_gate {
 
 struct ccu_div {
 	u32			enable;
+	u32			div_en;
 	struct ccu_div_internal	div;
 	struct ccu_internal	mux;
 	struct ccu_common	common;
@@ -198,6 +199,56 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
 	return rate;
 }
 
+static int ccu_div_determine_rate(struct clk_hw *hw,
+				  struct clk_rate_request *req)
+{
+	struct ccu_div *cd = hw_to_ccu_div(hw);
+	unsigned int val;
+
+	if (cd->div_en)
+		return divider_determine_rate(hw, req, NULL,
+					      cd->div.width, cd->div.flags);
+
+	regmap_read(cd->common.map, cd->common.cfg0, &val);
+	val = val >> cd->div.shift;
+	val &= GENMASK(cd->div.width - 1, 0);
+	return divider_ro_determine_rate(hw, req, NULL, cd->div.width,
+					 cd->div.flags, val);
+}
+
+static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
+				      unsigned long parent_rate)
+{
+	struct ccu_div *cd = hw_to_ccu_div(hw);
+	int val = divider_get_val(rate, parent_rate, NULL,
+				  cd->div.width, cd->div.flags);
+	unsigned int curr_val, reg_val;
+
+	if (val < 0)
+		return val;
+
+	regmap_read(cd->common.map, cd->common.cfg0, &reg_val);
+	curr_val = reg_val;
+	curr_val = curr_val >> cd->div.shift;
+	curr_val &= GENMASK(cd->div.width - 1, 0);
+
+	if (!cd->div_en && curr_val != val)
+		return -EINVAL;
+
+	reg_val &= ~cd->div_en;
+	regmap_write(cd->common.map, cd->common.cfg0, reg_val);
+	udelay(1);
+
+	reg_val &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd->div.shift);
+	reg_val |= val << cd->div.shift;
+	regmap_write(cd->common.map, cd->common.cfg0, reg_val);
+
+	reg_val |= cd->div_en;
+	regmap_write(cd->common.map, cd->common.cfg0, reg_val);
+
+	return 0;
+}
+
 static u8 ccu_div_get_parent(struct clk_hw *hw)
 {
 	struct ccu_div *cd = hw_to_ccu_div(hw);
@@ -240,7 +291,8 @@ static const struct clk_ops ccu_div_ops = {
 	.get_parent	= ccu_div_get_parent,
 	.set_parent	= ccu_div_set_parent,
 	.recalc_rate	= ccu_div_recalc_rate,
-	.determine_rate	= clk_hw_determine_rate_no_reparent,
+	.set_rate	= ccu_div_set_rate,
+	.determine_rate = ccu_div_determine_rate,
 };
 
 static void ccu_pll_disable(struct clk_hw *hw)
@@ -784,6 +836,7 @@ static struct ccu_div venc_clk = {
 };
 
 static struct ccu_div dpu0_clk = {
+	.div_en		= BIT(8),
 	.div		= TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED),
 	.common		= {
 		.clkid          = CLK_DPU0,
@@ -791,7 +844,7 @@ static struct ccu_div dpu0_clk = {
 		.hw.init	= CLK_HW_INIT_PARENTS_HW("dpu0",
 					      dpu0_pll_clk_parent,
 					      &ccu_div_ops,
-					      0),
+					      CLK_SET_RATE_UNGATE),
 	},
 };
 
@@ -800,6 +853,7 @@ static const struct clk_parent_data dpu0_clk_pd[] = {
 };
 
 static struct ccu_div dpu1_clk = {
+	.div_en		= BIT(8),
 	.div		= TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED),
 	.common		= {
 		.clkid          = CLK_DPU1,
@@ -807,7 +861,7 @@ static struct ccu_div dpu1_clk = {
 		.hw.init	= CLK_HW_INIT_PARENTS_HW("dpu1",
 					      dpu1_pll_clk_parent,
 					      &ccu_div_ops,
-					      0),
+					      CLK_SET_RATE_UNGATE),
 	},
 };
 
@@ -891,9 +945,9 @@ static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd,
 static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk",
 		video_pll_clk_pd, 0x0, BIT(4), 0);
 static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk",
-		dpu0_clk_pd, 0x0, BIT(5), 0);
+		dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT);
 static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk",
-		dpu1_clk_pd, 0x0, BIT(6), 0);
+		dpu1_clk_pd, 0x0, BIT(6), CLK_SET_RATE_PARENT);
 static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk", video_pll_clk_pd, 0x0,
 		BIT(7), 0);
 static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk", video_pll_clk_pd, 0x0,
-- 
2.50.1
Re: [PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate
Posted by Troy Mitchell 4 months ago
On Wed, Aug 13, 2025 at 03:27:01PM +0800, Icenowy Zheng wrote:
> The DPU pixel clock rate corresponds to the required dot clock of the
> display mode, so it needs to be tweakable.
> 
> Add support to change it, by adding generic divider setting code,
> arming the code to the dpu0/dpu1 clocks, and setting the pixel clock
> connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to propagate
> it to the dividers.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
> Changes in v2:
> - Dropped round_rate() because of deprecation.
> - Changed the logic of determine_rate() to early return if the divider
>   could be changed.
> 
>  drivers/clk/thead/clk-th1520-ap.c | 64 ++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> index 0b5458af8c550..b220a8ed22607 100644
> --- a/drivers/clk/thead/clk-th1520-ap.c
> +++ b/drivers/clk/thead/clk-th1520-ap.c
> @@ -55,6 +55,7 @@ struct ccu_gate {
>  
...
> +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +				      unsigned long parent_rate)
> +{
> +	struct ccu_div *cd = hw_to_ccu_div(hw);
> +	int val = divider_get_val(rate, parent_rate, NULL,
> +				  cd->div.width, cd->div.flags);
> +	unsigned int curr_val, reg_val;
> +
> +	if (val < 0)
> +		return val;
> +
> +	regmap_read(cd->common.map, cd->common.cfg0, &reg_val);
> +	curr_val = reg_val;
uh? remove this line.

                - Troy

> +	curr_val = curr_val >> cd->div.shift;
> +	curr_val &= GENMASK(cd->div.width - 1, 0);
> +
> +	if (!cd->div_en && curr_val != val)
> +		return -EINVAL;
> +
> +	reg_val &= ~cd->div_en;
> +	regmap_write(cd->common.map, cd->common.cfg0, reg_val);
> +	udelay(1);
> +
> +	reg_val &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd->div.shift);
> +	reg_val |= val << cd->div.shift;
> +	regmap_write(cd->common.map, cd->common.cfg0, reg_val);
> +
> +	reg_val |= cd->div_en;
> +	regmap_write(cd->common.map, cd->common.cfg0, reg_val);
> +
> +	return 0;
> +}
> +
>  static u8 ccu_div_get_parent(struct clk_hw *hw)
>  {
>  	struct ccu_div *cd = hw_to_ccu_div(hw);
> @@ -240,7 +291,8 @@ static const struct clk_ops ccu_div_ops = {
>  	.get_parent	= ccu_div_get_parent,
>  	.set_parent	= ccu_div_set_parent,
>  	.recalc_rate	= ccu_div_recalc_rate,
> -	.determine_rate	= clk_hw_determine_rate_no_reparent,
> +	.set_rate	= ccu_div_set_rate,
> +	.determine_rate = ccu_div_determine_rate,
>  };
>  
>  static void ccu_pll_disable(struct clk_hw *hw)
> @@ -784,6 +836,7 @@ static struct ccu_div venc_clk = {
>  };
>  
>  static struct ccu_div dpu0_clk = {
> +	.div_en		= BIT(8),
>  	.div		= TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED),
>  	.common		= {
>  		.clkid          = CLK_DPU0,
> @@ -791,7 +844,7 @@ static struct ccu_div dpu0_clk = {
>  		.hw.init	= CLK_HW_INIT_PARENTS_HW("dpu0",
>  					      dpu0_pll_clk_parent,
>  					      &ccu_div_ops,
> -					      0),
> +					      CLK_SET_RATE_UNGATE),
>  	},
>  };
>  
> @@ -800,6 +853,7 @@ static const struct clk_parent_data dpu0_clk_pd[] = {
>  };
>  
>  static struct ccu_div dpu1_clk = {
> +	.div_en		= BIT(8),
>  	.div		= TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED),
>  	.common		= {
>  		.clkid          = CLK_DPU1,
> @@ -807,7 +861,7 @@ static struct ccu_div dpu1_clk = {
>  		.hw.init	= CLK_HW_INIT_PARENTS_HW("dpu1",
>  					      dpu1_pll_clk_parent,
>  					      &ccu_div_ops,
> -					      0),
> +					      CLK_SET_RATE_UNGATE),
>  	},
>  };
>  
> @@ -891,9 +945,9 @@ static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd,
>  static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk",
>  		video_pll_clk_pd, 0x0, BIT(4), 0);
>  static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk",
> -		dpu0_clk_pd, 0x0, BIT(5), 0);
> +		dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT);
>  static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk",
> -		dpu1_clk_pd, 0x0, BIT(6), 0);
> +		dpu1_clk_pd, 0x0, BIT(6), CLK_SET_RATE_PARENT);
>  static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk", video_pll_clk_pd, 0x0,
>  		BIT(7), 0);
>  static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk", video_pll_clk_pd, 0x0,
> -- 
> 2.50.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate
Posted by Icenowy Zheng 4 months ago
在 2025-08-15星期五的 10:23 +0800,Troy Mitchell写道:
> On Wed, Aug 13, 2025 at 03:27:01PM +0800, Icenowy Zheng wrote:
> > The DPU pixel clock rate corresponds to the required dot clock of
> > the
> > display mode, so it needs to be tweakable.
> > 
> > Add support to change it, by adding generic divider setting code,
> > arming the code to the dpu0/dpu1 clocks, and setting the pixel
> > clock
> > connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to
> > propagate
> > it to the dividers.
> > 
> > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > ---
> > Changes in v2:
> > - Dropped round_rate() because of deprecation.
> > - Changed the logic of determine_rate() to early return if the
> > divider
> >   could be changed.
> > 
> >  drivers/clk/thead/clk-th1520-ap.c | 64
> > ++++++++++++++++++++++++++++---
> >  1 file changed, 59 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/clk/thead/clk-th1520-ap.c
> > b/drivers/clk/thead/clk-th1520-ap.c
> > index 0b5458af8c550..b220a8ed22607 100644
> > --- a/drivers/clk/thead/clk-th1520-ap.c
> > +++ b/drivers/clk/thead/clk-th1520-ap.c
> > @@ -55,6 +55,7 @@ struct ccu_gate {
> >  
> ...
> > +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> > +                                     unsigned long parent_rate)
> > +{
> > +       struct ccu_div *cd = hw_to_ccu_div(hw);
> > +       int val = divider_get_val(rate, parent_rate, NULL,
> > +                                 cd->div.width, cd->div.flags);
> > +       unsigned int curr_val, reg_val;
> > +
> > +       if (val < 0)
> > +               return val;
> > +
> > +       regmap_read(cd->common.map, cd->common.cfg0, &reg_val);
> > +       curr_val = reg_val;
> uh? remove this line.
> 
>                 - Troy
> 
> > +       curr_val = curr_val >> cd->div.shift;

Ooooops, I am silly enough...
Will change this to `curr_val = reg_val >> cd->div.shift;` instead in
the next revision.

> > +       curr_val &= GENMASK(cd->div.width - 1, 0);
> > +
> > +       if (!cd->div_en && curr_val != val)
> > +               return -EINVAL;
> > +
> > +       reg_val &= ~cd->div_en;
> > +       regmap_write(cd->common.map, cd->common.cfg0, reg_val);
> > +       udelay(1);
> > +
> > +       reg_val &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd-
> > >div.shift);
> > +       reg_val |= val << cd->div.shift;
> > +       regmap_write(cd->common.map, cd->common.cfg0, reg_val);
> > +
> > +       reg_val |= cd->div_en;
> > +       regmap_write(cd->common.map, cd->common.cfg0, reg_val);
> > +
> > +       return 0;
> > +}
> > +
> >  static u8 ccu_div_get_parent(struct clk_hw *hw)
> >  {
> >         struct ccu_div *cd = hw_to_ccu_div(hw);
> > @@ -240,7 +291,8 @@ static const struct clk_ops ccu_div_ops = {
> >         .get_parent     = ccu_div_get_parent,
> >         .set_parent     = ccu_div_set_parent,
> >         .recalc_rate    = ccu_div_recalc_rate,
> > -       .determine_rate = clk_hw_determine_rate_no_reparent,
> > +       .set_rate       = ccu_div_set_rate,
> > +       .determine_rate = ccu_div_determine_rate,
> >  };
> >  
> >  static void ccu_pll_disable(struct clk_hw *hw)
> > @@ -784,6 +836,7 @@ static struct ccu_div venc_clk = {
> >  };
> >  
> >  static struct ccu_div dpu0_clk = {
> > +       .div_en         = BIT(8),
> >         .div            = TH_CCU_DIV_FLAGS(0, 8,
> > CLK_DIVIDER_ONE_BASED),
> >         .common         = {
> >                 .clkid          = CLK_DPU0,
> > @@ -791,7 +844,7 @@ static struct ccu_div dpu0_clk = {
> >                 .hw.init        = CLK_HW_INIT_PARENTS_HW("dpu0",
> >                                               dpu0_pll_clk_parent,
> >                                               &ccu_div_ops,
> > -                                             0),
> > +                                             CLK_SET_RATE_UNGATE),
> >         },
> >  };
> >  
> > @@ -800,6 +853,7 @@ static const struct clk_parent_data
> > dpu0_clk_pd[] = {
> >  };
> >  
> >  static struct ccu_div dpu1_clk = {
> > +       .div_en         = BIT(8),
> >         .div            = TH_CCU_DIV_FLAGS(0, 8,
> > CLK_DIVIDER_ONE_BASED),
> >         .common         = {
> >                 .clkid          = CLK_DPU1,
> > @@ -807,7 +861,7 @@ static struct ccu_div dpu1_clk = {
> >                 .hw.init        = CLK_HW_INIT_PARENTS_HW("dpu1",
> >                                               dpu1_pll_clk_parent,
> >                                               &ccu_div_ops,
> > -                                             0),
> > +                                             CLK_SET_RATE_UNGATE),
> >         },
> >  };
> >  
> > @@ -891,9 +945,9 @@ static CCU_GATE(CLK_GPU_CORE, gpu_core_clk,
> > "gpu-core-clk", video_pll_clk_pd,
> >  static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk",
> >                 video_pll_clk_pd, 0x0, BIT(4), 0);
> >  static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk",
> > -               dpu0_clk_pd, 0x0, BIT(5), 0);
> > +               dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT);
> >  static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk",
> > -               dpu1_clk_pd, 0x0, BIT(6), 0);
> > +               dpu1_clk_pd, 0x0, BIT(6), CLK_SET_RATE_PARENT);
> >  static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk",
> > video_pll_clk_pd, 0x0,
> >                 BIT(7), 0);
> >  static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk",
> > video_pll_clk_pd, 0x0,
> > -- 
> > 2.50.1
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate
Posted by Drew Fustini 4 months ago
On Wed, Aug 13, 2025 at 03:27:01PM +0800, Icenowy Zheng wrote:
> The DPU pixel clock rate corresponds to the required dot clock of the
> display mode, so it needs to be tweakable.
> 
> Add support to change it, by adding generic divider setting code,
> arming the code to the dpu0/dpu1 clocks, and setting the pixel clock
> connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to propagate
> it to the dividers.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
> Changes in v2:
> - Dropped round_rate() because of deprecation.
> - Changed the logic of determine_rate() to early return if the divider
>   could be changed.
> 
>  drivers/clk/thead/clk-th1520-ap.c | 64 ++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> index 0b5458af8c550..b220a8ed22607 100644
> --- a/drivers/clk/thead/clk-th1520-ap.c
> +++ b/drivers/clk/thead/clk-th1520-ap.c
> @@ -55,6 +55,7 @@ struct ccu_gate {
>  
>  struct ccu_div {
>  	u32			enable;
> +	u32			div_en;
>  	struct ccu_div_internal	div;
>  	struct ccu_internal	mux;
>  	struct ccu_common	common;
> @@ -198,6 +199,56 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
>  	return rate;
>  }
>  
> +static int ccu_div_determine_rate(struct clk_hw *hw,
> +				  struct clk_rate_request *req)
> +{
> +	struct ccu_div *cd = hw_to_ccu_div(hw);
> +	unsigned int val;
> +
> +	if (cd->div_en)
> +		return divider_determine_rate(hw, req, NULL,
> +					      cd->div.width, cd->div.flags);
> +
> +	regmap_read(cd->common.map, cd->common.cfg0, &val);
> +	val = val >> cd->div.shift;
> +	val &= GENMASK(cd->div.width - 1, 0);
> +	return divider_ro_determine_rate(hw, req, NULL, cd->div.width,
> +					 cd->div.flags, val);
> +}
> +
> +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +				      unsigned long parent_rate)

This should line up with open parenthesis. Other than that nit:

Reviewed-by: Drew Fustini <fustini@kernel.org>

If no other issues arise on this series, then I can just fix it up when
applying.

Thanks,
Drew