Re: [PATCH v13 06/15] drm/panfrost: Fix PM refcnt and autosuspend issues at device probe/remove

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/panfrost: missing error check on pm_runtime_get_sync() in device removal causes SError
--

commit c158c940eec9a323d868411adbbd13b59c6449b8
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Fix PM refcnt and autosuspend issues at device probe/remove

This commit fixes issues where the panfrost device PM usage count was left
at 0 after probe failure, or increased monotonically after every module
reload. It manually adjusts the PM refcnt at probe and removal, and ensures
autosuspend functions are called properly.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 55fc22e8d4d4e..907d4a14a0b55 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
[ ... ]
> @@ -884,9 +892,12 @@ static void panfrost_remove(struct platform_device *pdev)
>  	drm_dev_unregister(&pfdev->base);
>  
>  	pm_runtime_get_sync(pfdev->base.dev);
> +	pm_runtime_dont_use_autosuspend(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
>  	panfrost_device_fini(pfdev);

[Severity: High]
Does ignoring the return value of pm_runtime_get_sync() in panfrost_remove()
allow the driver to write to unpowered hardware?

During device removal, if the power domain or regulators fail to resume the
device, pm_runtime_get_sync() returns a negative error code. Because the
code proceeds unconditionally to call panfrost_device_fini(), it invokes
panfrost_gpu_fini() and panfrost_perfcnt_fini(), which perform direct MMIO
register writes (such as to SHADER_PWROFF_LO and GPU_PERFCNT_CFG).

Accessing these registers while the hardware is physically powered off can
cause an Oops due to a synchronous external abort on ARM architectures.

Note that even if follow-up patches attempt to address this by wrapping the
call in drm_WARN_ON(), that does not abort or bypass the subsequent hardware
accesses. The system would still crash following the warning.

>  	pm_runtime_set_suspended(pfdev->base.dev);
> +	pm_runtime_put_noidle(pfdev->base.dev);
> +
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-claude-fixes-v13-0-d6a493185860@collabora.com?part=6