Re: [PATCH v3 1/1] clk: sunxi-ng: h616: Reparent GPU clock during frequency changes

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne četrtek, 20. februar 2025 ob 16:41:54 Srednjeevropski standardni čas je Jernej Škrabec napisal(a):
> Dne četrtek, 20. februar 2025 ob 12:38:08 Srednjeevropski standardni čas je Philippe Simons napisal(a):
> > The H616 manual does not state that the GPU PLL supports
> > dynamic frequency configuration, so we must take extra care when changing
> > the frequency. Currently any attempt to do device DVFS on the GPU lead
> > to panfrost various ooops, and GPU hangs.
> > 
> > The manual describes the algorithm for changing the PLL
> > frequency, which the CPU PLL notifier code already support, so we reuse
> > that to reparent the GPU clock to GPU1 clock during frequency
> > changes.
> > 
> > Signed-off-by: Philippe Simons <simons.philippe@gmail.com>
> > Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/clk/sunxi-ng/ccu-sun50i-h616.c | 36 +++++++++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> Changelog is missing here. What's changed?
> 
> In any case, this patch isn't useful on its own. What about PPU and GPU DT node?

In the light of separate PPU & GPU DT series, this patch is gtg.

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
 
> > 
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> > index 190816c35..6050cbfa9 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> > @@ -328,10 +328,16 @@ static SUNXI_CCU_M_WITH_MUX_GATE(gpu0_clk, "gpu0", gpu0_parents, 0x670,
> >  				       24, 1,	/* mux */
> >  				       BIT(31),	/* gate */
> >  				       CLK_SET_RATE_PARENT);
> > +
> > +/*
> > + * This clk is needed as a temporary fall back during GPU PLL freq changes.
> > + * Set CLK_IS_CRITICAL flag to prevent from being disabled.
> > + */
> > +#define SUN50I_H616_GPU_CLK1_REG        0x674
> >  static SUNXI_CCU_M_WITH_GATE(gpu1_clk, "gpu1", "pll-periph0-2x", 0x674,
> >  					0, 2,	/* M */
> >  					BIT(31),/* gate */
> > -					0);
> > +					CLK_IS_CRITICAL);
> >  
> >  static SUNXI_CCU_GATE(bus_gpu_clk, "bus-gpu", "psi-ahb1-ahb2",
> >  		      0x67c, BIT(0), 0);
> > @@ -1120,6 +1126,19 @@ static struct ccu_pll_nb sun50i_h616_pll_cpu_nb = {
> >  	.lock		= BIT(28),
> >  };
> >  
> > +static struct ccu_mux_nb sun50i_h616_gpu_nb = {
> > +	.common		= &gpu0_clk.common,
> > +	.cm		= &gpu0_clk.mux,
> > +	.delay_us	= 1, /* manual doesn't really say */
> > +	.bypass_index	= 1, /* GPU_CLK1@400MHz */
> > +};
> > +
> > +static struct ccu_pll_nb sun50i_h616_pll_gpu_nb = {
> > +	.common		= &pll_gpu_clk.common,
> > +	.enable		= BIT(29),	/* LOCK_ENABLE */
> > +	.lock		= BIT(28),
> > +};
> > +
> >  static int sun50i_h616_ccu_probe(struct platform_device *pdev)
> >  {
> >  	void __iomem *reg;
> > @@ -1170,6 +1189,14 @@ static int sun50i_h616_ccu_probe(struct platform_device *pdev)
> >  	val |= BIT(0);
> >  	writel(val, reg + SUN50I_H616_PLL_AUDIO_REG);
> >  
> > +	/*
> > +	 * Set the input-divider for the gpu1 clock to 3, to reach a safe 400 MHz.
> > +	 */
> > +	val = readl(reg + SUN50I_H616_GPU_CLK1_REG);
> > +	val &= ~GENMASK(1, 0);
> > +	val |= 2;
> > +	writel(val, reg + SUN50I_H616_GPU_CLK1_REG);
> > +
> >  	/*
> >  	 * First clock parent (osc32K) is unusable for CEC. But since there
> >  	 * is no good way to force parent switch (both run with same frequency),
> > @@ -1190,6 +1217,13 @@ static int sun50i_h616_ccu_probe(struct platform_device *pdev)
> >  	/* Re-lock the CPU PLL after any rate changes */
> >  	ccu_pll_notifier_register(&sun50i_h616_pll_cpu_nb);
> >  
> > +	/* Reparent GPU during GPU PLL rate changes */
> > +	ccu_mux_notifier_register(pll_gpu_clk.common.hw.clk,
> > +				  &sun50i_h616_gpu_nb);
> > +
> > +	/* Re-lock the GPU PLL after any rate changes */
> > +	ccu_pll_notifier_register(&sun50i_h616_pll_gpu_nb);
> > +
> >  	return 0;
> >  }
> >  
> > 
> 
> 
> 
> 
>