Subject: Re: [RFC PATCH 3/4] drm/etnaviv: Change order of enabling clocks to fix boot on i.MX8MM
On 30.04.20 16:35, Lucas Stach wrote:
Am Donnerstag, den 30.04.2020, 12:46 +0000 schrieb Schrempf Frieder:
From: Frieder Schrempf frieder.schrempf@kontron.de
On some i.MX8MM devices the boot hangs when enabling the GPU clocks. Changing the order of clock initalization to
core -> shader -> bus -> reg
fixes the issue. This is the same order used in the imx platform code of the downstream GPU driver in the NXP kernel [1]. For the sake of consistency we also adjust the order of disabling the clocks to the reverse.
[1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsou
rce.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Ftree%2Fdrivers%2F mx
c%2Fgpu-viv%2Fhal%2Fos%2Flinux%2Fkernel%2Fplatform%2Ffreescale%2Fgc _h
al_kernel_platform_imx.c%3Fh%3Dimx_5.4.3_2.0.0%23n1538&data=02 %7C
01%7Cpeng.fan%40nxp.com%7Cdc7da53f665e4f567e3008d7ed1c27e0%7C6 86ea1d3
bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637238577497969787&sda ta=QRHzu
C6gSKy%2F6y2FTRvlNF5t7DmJIvTgBESYKchI%2FDw%3D&reserved=0
I don't see why the order of the clocks is important. Is this really a GPU issue? As in: does a GPU access hang when enabling the clocks in the wrong order? Or is this a clock driver issue with a clock access hanging due to an upstream clock still being disabled?
Actually you might be right with this being a clock driver issue. The hanging happens while enabling the clocks (unrelated to any GPU register access). The strange thing is that most of the devices we have don't care and work as is and some devices reliably fail each time when enabling the clocks in the "wrong" order.
So I guess this could indeed be some clock being enabled with an upstream PLL not having locked yet or something.
https://patchwork.kernel.org/cover/11433775/
Will this pachset help?
The i.MX8M CCM root mux code in Linux needs a fix.
Regards, Peng.
Regards, Lucas
Signed-off-by: Frieder Schrempf frieder.schrempf@kontron.de
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 42
+++++++++++++--------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 7b138d4dd068..424b2e5951f0 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1487,55 +1487,55 @@ static int etnaviv_gpu_clk_enable(struct
etnaviv_gpu *gpu)
{ int ret;
- if (gpu->clk_reg) {
ret = clk_prepare_enable(gpu->clk_reg);
- if (gpu->clk_core) {
ret = clk_prepare_enable(gpu->clk_core);
- if (gpu->clk_bus) {
ret = clk_prepare_enable(gpu->clk_bus);
- if (gpu->clk_shader) {
ret = clk_prepare_enable(gpu->clk_shader);
goto disable_clk_reg;
goto disable_clk_core;
- if (gpu->clk_core) {
ret = clk_prepare_enable(gpu->clk_core);
- if (gpu->clk_bus) {
ret = clk_prepare_enable(gpu->clk_bus);
goto disable_clk_bus;
goto disable_clk_shader;
- if (gpu->clk_shader) {
ret = clk_prepare_enable(gpu->clk_shader);
- if (gpu->clk_reg) {
ret = clk_prepare_enable(gpu->clk_reg);
goto disable_clk_core;
goto disable_clk_bus;}
return 0;
-disable_clk_core:
- if (gpu->clk_core)
clk_disable_unprepare(gpu->clk_core);-disable_clk_reg:
- if (gpu->clk_reg)
clk_disable_unprepare(gpu->clk_reg);+disable_clk_shader:
- if (gpu->clk_shader)
clk_disable_unprepare(gpu->clk_shader);+disable_clk_core:
if (gpu->clk_core)
clk_disable_unprepare(gpu->clk_core);return ret; }
static int etnaviv_gpu_clk_disable(struct etnaviv_gpu *gpu) {
if (gpu->clk_reg)
clk_disable_unprepare(gpu->clk_reg);if (gpu->clk_bus)
clk_disable_unprepare(gpu->clk_bus);if (gpu->clk_shader) clk_disable_unprepare(gpu->clk_shader); if (gpu->clk_core) clk_disable_unprepare(gpu->clk_core);
if (gpu->clk_bus)
clk_disable_unprepare(gpu->clk_bus);if (gpu->clk_reg)
clk_disable_unprepare(gpu->clk_reg);return 0; }