Message-ID: <sashiko-outbox-207361@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH v2 23/28] drm/panthor: Synchronize HW component PM
 transitions
To: "Karunika Choo" <karunika.choo@arm.com>, dri-devel@lists.freedesktop.org
In-Reply-To: <20261110153907.3241978-24-karunika.choo@arm.com>
References: <20261110153907.3241978-1-karunika.choo@arm.com>
 <20261110153907.3241978-24-karunika.choo@arm.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 10 Nov 2026 16:23:39 +0000
Precedence: list
Reply-To: sashiko-reviews@lists.linux.dev
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/panthor: missing slow-reset fallback in
  panthor_device_resume_hw_components()
--

commit b392fdf733ca625b6c9ab7dbb3e11caa5a1d1b14
Author: Karunika Choo <karunika.choo@arm.com>

drm/panthor: Synchronize HW component PM transitions

This commit splits hardware component suspend and resume into
panthor_device_suspend_hw_components() and
panthor_device_resume_hw_components(), serializing transitions with
ptdev->pm.hw_component_lock and ptdev->pm.hw_component_state.

> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/p=
anthor/panthor_device.c
> index 98c0dd3ce7bcb..2ccd8ad6318d1 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
[ ... ]
> @@ -484,21 +491,55 @@ int panthor_device_mmap_io(struct panthor_device *p=
tdev, struct vm_area_struct *
>  	return 0;
>  }
> =20
> -static int panthor_device_resume_hw_components(struct panthor_device *pt=
dev)
> +int panthor_device_suspend_hw_components(struct panthor_device *ptdev)
> +{
> +	guard(mutex)(&ptdev->pm.hw_component_lock);
> +
> +	if (atomic_read(&ptdev->pm.hw_component_state) =3D=3D
> +	    PANTHOR_DEVICE_PM_STATE_SUSPENDED)
> +		return 0;
> +
> +	if (!pm_runtime_suspended(ptdev->base.dev)) {
> +		panthor_fw_suspend(ptdev);
> +		panthor_mmu_suspend(ptdev);
> +		panthor_gpu_suspend(ptdev);
> +		panthor_pwr_suspend(ptdev);
> +	}
> +
> +	atomic_set(&ptdev->pm.hw_component_state,
> +		   PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> +
> +	return 0;
> +}
> +
> +int panthor_device_resume_hw_components(struct panthor_device *ptdev)
>  {
>  	int ret;
> =20
> +	guard(mutex)(&ptdev->pm.hw_component_lock);
> +
> +	if (atomic_read(&ptdev->pm.hw_component_state) =3D=3D
> +	    PANTHOR_DEVICE_PM_STATE_ACTIVE)
> +		return 0;
> +
>  	panthor_pwr_resume(ptdev);
>  	panthor_gpu_resume(ptdev);
>  	panthor_mmu_resume(ptdev);
> =20
>  	ret =3D panthor_fw_resume(ptdev);
> -	if (!ret)
> +	if (!ret) {
> +		atomic_set(&ptdev->pm.hw_component_state,
> +			   PANTHOR_DEVICE_PM_STATE_ACTIVE);
>  		return 0;
> +	}
> =20
>  	panthor_mmu_suspend(ptdev);
>  	panthor_gpu_suspend(ptdev);
>  	panthor_pwr_suspend(ptdev);
> +
> +	atomic_set(&ptdev->pm.hw_component_state,
> +		   PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> +
>  	return ret;
>  }

[Severity: Medium]
Should the ptdev->reset.pending check and the fast-to-slow reset retry live
inside panthor_device_resume_hw_components() under
ptdev->pm.hw_component_lock rather than only in panthor_device_resume()?

drivers/gpu/drm/panthor/panthor_device.c:panthor_device_resume() {
    ...
		if (atomic_read(&ptdev->reset.pending)) {
			ptdev->reset.fast =3D false;
			atomic_set(&ptdev->reset.pending, 0);
		}

		ret =3D panthor_device_resume_hw_components(ptdev);
		if (ret && ptdev->reset.fast) {
			drm_err(&ptdev->base, "Fast reset failed, trying a slow reset");
			ptdev->reset.fast =3D false;
			ret =3D panthor_device_resume_hw_components(ptdev);
		}
    ...
}

When access-window arbitration later calls
panthor_device_resume_hw_components() while runtime PM remains active,
panthor_device_resume() is not run.  A pending reset in ptdev->reset.pending
is then ignored, and a failed fast reset does not fall back to a slow reset
by reloading firmware sections, causing component resume to fail.  In
panthor_device_resume(), ptdev->reset.fast is also modified outside
ptdev->pm.hw_component_lock and the lock is dropped between the fast-reset
attempt and the slow-reset retry.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261110153907.3241=
978-1-karunika.choo@arm.com?part=3D23