Re: [PATCH] clk: zynqmp: Work around broken DT GPU node

["Sagar, Vishal" <vishal.sagar@amd.com>]

Hi Marek,

Thanks for sharing this patch.

On 10/31/2024 5:59 PM, Marek Vasut wrote:
> The ZynqMP DT GPU node clock description is wrong and does not represent
> the hardware correctly, it only describes BUS and PP0 clock, while it is
> missing PP1 clock. That means PP1 clock can never be enabled when the GPU
> should be used, which leads to expected GPU hang even with simple basic
> tests like kmscube.

Could you please share how you tested this?
Please share the dt node too.
We will also check at our end and revert for this.

> 
> Since Xilinx does use generated DTs on ZynqMP, the current broken DT
> implementation has to be supported. Add a workaround for this breakage
> into the clock driver, in case of PP0 enablement attempt, enable PP1
> as well and vice versa. This way, the GPU does work and does not hang
> because one of its pixel pipeline clock are not enabled.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Michal Simek <michal.simek@amd.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-clk@vger.kernel.org
> ---
>   drivers/clk/zynqmp/clk-gate-zynqmp.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> index b89e557371984..b013aa33e7abb 100644
> --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> @@ -7,6 +7,7 @@
>    * Gated clock implementation
>    */
>   
> +#include <dt-bindings/clock/xlnx-zynqmp-clk.h>
>   #include <linux/clk-provider.h>
>   #include <linux/slab.h>
>   #include "clk-zynqmp.h"
> @@ -38,7 +39,13 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
>   	u32 clk_id = gate->clk_id;
>   	int ret;
>   
> -	ret = zynqmp_pm_clock_enable(clk_id);
> +	if (clk_id == GPU_PP0_REF || clk_id == GPU_PP1_REF) {
> +		ret = zynqmp_pm_clock_enable(GPU_PP0_REF);
> +		if (!ret)
> +			ret = zynqmp_pm_clock_enable(GPU_PP1_REF);
> +	} else {
> +		ret = zynqmp_pm_clock_enable(clk_id);
> +	}
>   
>   	if (ret)
>   		pr_debug("%s() clock enable failed for %s (id %d), ret = %d\n",
> @@ -58,7 +65,13 @@ static void zynqmp_clk_gate_disable(struct clk_hw *hw)
>   	u32 clk_id = gate->clk_id;
>   	int ret;
>   
> -	ret = zynqmp_pm_clock_disable(clk_id);
> +	if (clk_id == GPU_PP0_REF || clk_id == GPU_PP1_REF) {
> +		ret = zynqmp_pm_clock_disable(GPU_PP1_REF);
> +		if (!ret)
> +			ret = zynqmp_pm_clock_disable(GPU_PP0_REF);
> +	} else {
> +		ret = zynqmp_pm_clock_disable(clk_id);
> +	}
>   
>   	if (ret)
>   		pr_debug("%s() clock disable failed for %s (id %d), ret = %d\n",



Regards
Vishal Sagar