Re: [PATCH] drm/imagination: Fix kernel crash when hard resetting the GPU

[Matt Coster <Matt.Coster@imgtec.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3071 bytes --]

On 24/06/2025 15:22, Alessio Belle wrote:
> The GPU hard reset sequence calls pm_runtime_force_suspend() and
> pm_runtime_force_resume(), which according to their documentation should
> only be used during system-wide PM transitions to sleep states.
> 
> The main issue though is that depending on some internal runtime PM
> state as seen by pm_runtime_force_suspend() (whether the usage count is
> <= 1), pm_runtime_force_resume() might not resume the device unless
> needed. If that happens, the runtime PM resume callback
> pvr_power_device_resume() is not called, the GPU clocks are not
> re-enabled, and the kernel crashes on the next attempt to access GPU
> registers as part of the power-on sequence.
> 
> Replace calls to pm_runtime_force_suspend() and
> pm_runtime_force_resume() with direct calls to the driver's runtime PM
> callbacks, pvr_power_device_suspend() and pvr_power_device_resume(),
> to ensure clocks are re-enabled and avoid the kernel crash.
> 
> Fixes: cc1aeedb98ad ("drm/imagination: Implement firmware infrastructure and META FW support")
> 

Nit: no blank line here

> Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks for this fix! I'll push it to drm-misc-fixes on Friday if there
are no objections (with the blank line above removed; no need to re-send
for that), so:

Reviewed-by: Matt Coster <matt.coster@imgtec.com>

Cheers,
Matt

> ---
>  drivers/gpu/drm/imagination/pvr_power.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> index 41f5d89e78b854cf6993838868a4416a220b490a..3e349d039fc0c4176b5c4baf009ffa005a2b28d1 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -386,13 +386,13 @@ pvr_power_reset(struct pvr_device *pvr_dev, bool hard_reset)
>                 if (!err) {
>                         if (hard_reset) {
>                                 pvr_dev->fw_dev.booted = false;
> -                               WARN_ON(pm_runtime_force_suspend(from_pvr_device(pvr_dev)->dev));
> +                               WARN_ON(pvr_power_device_suspend(from_pvr_device(pvr_dev)->dev));
> 
>                                 err = pvr_fw_hard_reset(pvr_dev);
>                                 if (err)
>                                         goto err_device_lost;
> 
> -                               err = pm_runtime_force_resume(from_pvr_device(pvr_dev)->dev);
> +                               err = pvr_power_device_resume(from_pvr_device(pvr_dev)->dev);
>                                 pvr_dev->fw_dev.booted = true;
>                                 if (err)
>                                         goto err_device_lost;
> 
> ---
> base-commit: 1a45ef022f0364186d4fb2f4e5255dcae1ff638a
> change-id: 20250619-fix-kernel-crash-gpu-hard-reset-1ed31981f8cd
> 
> Best regards,
> --
> Alessio Belle <alessio.belle@imgtec.com>
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]