[PATCH] drm/v3d: Add clock handling

[Maíra Canal]

Hi Stefan,

On 10/02/25 10:27, Stefan Wahren wrote:
> 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 see your point now, thanks for taking your time to send this patch!

Applied to misc/kernel.git (drm-misc-next) [1]

[1] 
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/4dd40b5f9c3d89b67af0dbe059cf4a51aac6bf06

Best Regards,
- Maíra

>>
>> 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