Re: [PATCH v10 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: PM domain race in remove path due to early pm_runtime_set_suspended()
--

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

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

This commit adjusts the PM reference count at device probe and removal
time to ensure the device goes to sleep when idle and doesn't leak
references on unload. It also reorders pm_runtime_set_suspended() to
occur before panfrost_device_fini() in the teardown paths so resource
unwinding happens in the opposite order of initialization.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 55fc22e8d4d4e..02ddd391231c0 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
[ ... ]
> @@ -866,13 +871,16 @@ static int panfrost_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto err_out1;
>  
> +	pm_runtime_put_autosuspend(pfdev->base.dev);
>  
>  	return 0;
>  
>  err_out1:
> +	pm_runtime_dont_use_autosuspend(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
> -	panfrost_device_fini(pfdev);
> +	pm_runtime_put_noidle(pfdev->base.dev);
>  	pm_runtime_set_suspended(pfdev->base.dev);
> +	panfrost_device_fini(pfdev);

[Severity: High]
Does moving pm_runtime_set_suspended() before panfrost_device_fini() in
panfrost_probe() create a race condition with the PM workqueue?

When pm_runtime_set_suspended() is called, it triggers rpm_put_suppliers()
which drops the active reference for the attached secondary PM domains
(shader/tiler) and queues an asynchronous suspend via pm_request_idle().

Concurrently, panfrost_device_fini() proceeds to execute
panfrost_gpu_fini(), which writes to GPU MMIO (e.g., SHADER_PWROFF_LO) and
polls for power transitions.

Could the PM workqueue power off the domains asynchronously during this
teardown, leading to hardware lockups or fatal bus errors if MMIO accesses
are made to unpowered domains?

>  err_out0:
>  	return err;
>  }
> @@ -884,9 +892,11 @@ 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_put_noidle(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
> -	panfrost_device_fini(pfdev);
>  	pm_runtime_set_suspended(pfdev->base.dev);
> +	panfrost_device_fini(pfdev);

[Severity: High]
Can this race condition also happen in the panfrost_remove() path?

Similar to the probe error path, setting the status to RPM_SUSPENDED
before panfrost_device_fini() might prematurely power off the secondary PM
domains while hardware teardown is still accessing them.

>  }
>  
>  static ssize_t profiling_show(struct device *dev,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-claude-fixes-v10-0-755929b3cc19@collabora.com?part=6