Re: [PATCH v7 10/17] drm/panfrost: Add warning messages to fatal error conditions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Spurious WARN_ON triggered during system suspend when GPU jobs are active.

Pre-existing issues:
- [High] Unconditional hardware access after `pm_runtime_get_sync()` failure causes kernel panic.
--

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

drm/panfrost: Add warning messages to fatal error conditions

This commit adds drm_WARN_ON macros to panfrost_device_fini and
panfrost_device_runtime_suspend to warn users when PM reference acquisition
fails or when inflight jobs are reported during the PM suspend path.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 70c8109ea6987..d8acae9b8cfac 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -367,7 +368,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  
>  void panfrost_device_fini(struct panfrost_device *pfdev)
>  {
> -	pm_runtime_get_sync(pfdev->base.dev);
> +	drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0);
>  	pm_runtime_dont_use_autosuspend(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
>  	pm_runtime_put_noidle(pfdev->base.dev);

[Severity: High]
This isn't a bug introduced by this patch, but while we are adding a warning
here, does it make sense to continue execution if pm_runtime_get_sync fails?

If the device fails to power on due to a regulator or clock failure, the
warning is logged but the function continues and calls:

panfrost_device_fini()
    panfrost_jm_stop_sched_jobs()
    panfrost_device_disable_hw()

Both of these subsequent functions write to GPU registers. Accessing hardware
registers of an unclocked or unpowered device on ARM SoCs triggers a Data
Abort exception, leading to a hard kernel panic.

Should we abort the teardown or skip the hardware disable steps if the PM
resume fails?

[ ... ]

> @@ -517,7 +518,7 @@ static int panfrost_device_runtime_suspend(struct device *dev)
>  {
>  	struct panfrost_device *pfdev = dev_get_drvdata(dev);
>  
> -	if (!panfrost_jm_is_idle(pfdev))
> +	if (drm_WARN_ON(&pfdev->base, !panfrost_jm_is_idle(pfdev)))
>  		return -EBUSY;
>  
>  	panfrost_device_disable_hw(pfdev);

[Severity: Medium]
Will this trigger a spurious warning stack trace if the system is suspended
while a GPU workload is still executing?

When a user suspends the system with an active GPU workload, the kernel
freezes userspace tasks but jobs already submitted to the hardware can
continue running. During the system suspend flow, pm_runtime_force_suspend()
intentionally bypasses usage counters and forces a call directly to
panfrost_device_runtime_suspend().

Since the job might still be executing on the hardware, panfrost_jm_is_idle()
can legitimately return false here. Returning -EBUSY to abort the suspend is
the expected behavior, but the new warning assumes this state is always a bug
and will print a stack trace.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-claude-fixes-v7-0-72a13b2c125d@collabora.com?part=10