[PATCH] drm/v3d: Add clock handling

[Maíra Canal]

Hi Stefan,

Thanks for your patch!

On 01/02/25 09:50, Stefan Wahren wrote:
> Since the initial commit 57692c94dcbe ("drm/v3d: Introduce a new DRM driver
> for Broadcom V3D V3.x+") the struct v3d_dev reserved a pointer for
> an optional V3D clock. But there wasn't any code, which fetched it.
> So add the missing clock handling before accessing any V3D registers.

Actually, I believe we should remove `v3d->clk`. In the past, we used
`v3d->clk` for PM management, but we removed PM management a while ago
(it was somewhat broken).

I believe the best move would be to remove `v3d->clk`. If we decide to
use the clock at some point, we can reintroduce the variable to the
struct. Right now, it doesn't make sense to add clock handling if we
don't use it.

Let me know if you have any use case for this variable. If you have,
then we can add clock handling.

Best Regards,
- Maíra

> 
> Signed-off-by: Stefan Wahren <wahrenst at gmx.net>
> ---
>   drivers/gpu/drm/v3d/v3d_drv.c | 25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
> index 930737a9347b..852015214e97 100644
> --- a/drivers/gpu/drm/v3d/v3d_drv.c
> +++ b/drivers/gpu/drm/v3d/v3d_drv.c
> @@ -295,11 +295,21 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
> 
> +	v3d->clk = devm_clk_get_optional(dev, NULL);
> +	if (IS_ERR(v3d->clk))
> +		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
> +
> +	ret = clk_prepare_enable(v3d->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Couldn't enable the V3D clock\n");
> +		return ret;
> +	}
> +
>   	mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
>   	mask = DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_PA_WIDTH));
>   	ret = dma_set_mask_and_coherent(dev, mask);
>   	if (ret)
> -		return ret;
> +		goto clk_disable;
> 
>   	v3d->va_width = 30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_VA_WIDTH);
> 
> @@ -319,28 +329,29 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
>   		ret = PTR_ERR(v3d->reset);
> 
>   		if (ret == -EPROBE_DEFER)
> -			return ret;
> +			goto clk_disable;
> 
>   		v3d->reset = NULL;
>   		ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
>   		if (ret) {
>   			dev_err(dev,
>   				"Failed to get reset control or bridge regs\n");
> -			return ret;
> +			goto clk_disable;
>   		}
>   	}
> 
>   	if (v3d->ver < 41) {
>   		ret = map_regs(v3d, &v3d->gca_regs, "gca");
>   		if (ret)
> -			return ret;
> +			goto clk_disable;
>   	}
> 
>   	v3d->mmu_scratch = dma_alloc_wc(dev, 4096, &v3d->mmu_scratch_paddr,
>   					GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
>   	if (!v3d->mmu_scratch) {
>   		dev_err(dev, "Failed to allocate MMU scratch page\n");
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto clk_disable;
>   	}
> 
>   	ret = v3d_gem_init(drm);
> @@ -369,6 +380,8 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
>   	v3d_gem_destroy(drm);
>   dma_free:
>   	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
> +clk_disable:
> +	clk_disable_unprepare(v3d->clk);
>   	return ret;
>   }
> 
> @@ -386,6 +399,8 @@ static void v3d_platform_drm_remove(struct platform_device *pdev)
> 
>   	dma_free_wc(v3d->drm.dev, 4096, v3d->mmu_scratch,
>   		    v3d->mmu_scratch_paddr);
> +
> +	clk_disable_unprepare(v3d->clk);
>   }
> 
>   static struct platform_driver v3d_platform_driver = {
> --
> 2.34.1
>