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
