clk: sunxi-ng: h616: Reparent GPU clock during frequency changes
Subsystems (2)
PATCH v2 1/1 clk: sunxi-ng: h616: Reparent GPU clock during frequency changes
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.
--- drivers/clk/sunxi-ng/ccu-sun50i-h616.c | 38 ++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +36 -2
@@ -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); -static SUNXI_CCU_M_WITH_GATE(gpu1_clk, "gpu1", "pll-periph0-2x", 0x674, + +/* +* 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. +*/ + val =readl (reg +SUN50I_H616_GPU_CLK1_REG ); + val &= ~GENMASK (1 ,0 ); + val |=BIT (1 ); +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 ; }
Signature · 1 line
2.48.1
On Wed, 12 Feb 2025 18:36:39 +0100 Philippe Simons <[email protected]> wrote:
Hi,
Quoted 21 lines
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 <[email protected]>--- drivers/clk/sunxi-ng/ccu-sun50i-h616.c | 38 ++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)drivers/clk/sunxi-ng/ccu-sun50i-h616.c +0
-0@@ -328,10 +328,16 @@ static SUNXI_CCU_M_WITH_MUX_GATE(gpu0_clk, "gpu0", gpu0_parents, 0x670, 24, 1, /* mux */
drivers/clk/sunxi-ng/ccu-sun50i-h616.c static SUNXI_CCU_M_WITH_MUX_GATE(gpu0_clk, "gpu0", gpu0_parents, 0x670, · line 337
Quoted 10 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +7
-1BIT (31 ),/* gate */ CLK_SET_RATE_PARENT); -static SUNXI_CCU_M_WITH_GATE(gpu1_clk, "gpu1", "pll-periph0-2x", 0x674, + +/* +* 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,
There seems to be a stray space there at the beginning of those two lines.
Quoted 8 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +1
-10 ,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);
drivers/clk/sunxi-ng/ccu-sun50i-h616.c static struct ccu_pll_nb sun50i_h616_pll_cpu_nb = { · line 1132
Quoted 7 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +4
-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 */
Indentation is off here. Please align to the last line here below.
Quoted 13 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +9
-0+ .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;
drivers/clk/sunxi-ng/ccu-sun50i-h616.c static int sun50i_h616_ccu_probe(struct platform_device *pdev) · line 1193
Quoted 5 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +2
-0@@ -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.
Can you extend the comment, like:
... clock to 3, to reach a safe 400 MHz.
drivers/clk/sunxi-ng/ccu-sun50i-h616.c static int sun50i_h616_ccu_probe(struct platform_device *pdev) · line 1197
Quoted 4 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +4
-0+*/ + val =readl (reg +SUN50I_H616_GPU_CLK1_REG ); + val &= ~GENMASK (1 ,0 ); + val |=BIT (1 );
Can you replace the BIT(1) with a "2" here? At the moment it looks like it's a single flag to be set, whereas we really just want to write the value 2 (3 - 1) into that field.
The rest looks alright, so with those smaller things fixed:
Cheers,
Andre
Quoted 19 lines
drivers/clk/sunxi-ng/ccu-sun50i-h616.c +9
-0+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 ; }