[PATCH] drm/v3d: Add clock handling

[Stefan Wahren]

Hi Maíra,

Am 10.02.25 um 13:23 schrieb 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 disagree. Clock handling and power management are not the same things.
Btw the brokeness partly based on the missing clock handling, but this
not my point here. The DT binding of the Broadcom V3D GPU describe an
optional clock, so the Linux kernel should ensure that before accessing
any V3D registers, the relevant clock must be enabled. Please compare
with VC4, which does the same thing.

At the end we had just luck because the GPU firmware enabled the clock
at boot?
>
> 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.
clk_prepare_enable() / clk_disable_unprepare() is actually using the
clock. There is no need to set some kind of rate, in case you expecting
this.

Best regards