On Thu, Jul 30, 2026 at 01:45:15PM +0200, Nicolas Frattaroli wrote:
> panthor_gpu_flush_caches() and panthor_gpu_soft_reset() would read (and
> even reset) the contents of the pending_reqs register outside of holding
> the reqs_lock. Additionally, when it did hold the lock, it did so with
> the irqsave/irqrestore variants, even though the spinlock was never
> acquired in an atomic context, just the threaded handler.
>
> Use the new wait_event_lock_timeout() macro to check pending_reqs under
> the lock, and only do so without disabling interrupts.
>
> Fixes: 5cd894e258c4 ("drm/panthor: Add the GPU logical block")
> Signed-off-by: Nicolas Frattaroli <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]>
Best regards,
Liviu
> ---
> drivers/gpu/drm/panthor/panthor_gpu.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c
> b/drivers/gpu/drm/panthor/panthor_gpu.c
> index c013d6bf9a59..f015bde80abf 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -330,35 +330,34 @@ int panthor_gpu_flush_caches(struct panthor_device
> *ptdev,
> u32 l2, u32 lsc, u32 other)
> {
> struct panthor_gpu *gpu = ptdev->gpu;
> - unsigned long flags;
> int ret = 0;
>
> /* Serialize cache flush operations. */
> guard(mutex)(&ptdev->gpu->cache_flush_lock);
>
> - spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> + spin_lock(&ptdev->gpu->reqs_lock);
> if (!(ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED)) {
> ptdev->gpu->pending_reqs |= GPU_IRQ_CLEAN_CACHES_COMPLETED;
> gpu_write(gpu->iomem, GPU_CMD, GPU_FLUSH_CACHES(l2, lsc,
> other));
> } else {
> ret = -EIO;
> }
> - spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
>
> - if (ret)
> + if (ret) {
> + spin_unlock(&ptdev->gpu->reqs_lock);
> return ret;
> + }
>
> - if (!wait_event_timeout(ptdev->gpu->reqs_acked,
> + if (!wait_event_lock_timeout(ptdev->gpu->reqs_acked,
> !(ptdev->gpu->pending_reqs &
> GPU_IRQ_CLEAN_CACHES_COMPLETED),
> - msecs_to_jiffies(100))) {
> - spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> + ptdev->gpu->reqs_lock, msecs_to_jiffies(100))) {
> if ((ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED)
> != 0 &&
> !(gpu_read(gpu->irq.iomem, INT_RAWSTAT) &
> GPU_IRQ_CLEAN_CACHES_COMPLETED))
> ret = -ETIMEDOUT;
> else
> ptdev->gpu->pending_reqs &=
> ~GPU_IRQ_CLEAN_CACHES_COMPLETED;
> - spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
> }
> + spin_unlock(&ptdev->gpu->reqs_lock);
>
> if (ret) {
> panthor_device_schedule_reset(ptdev);
> @@ -378,27 +377,25 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
> {
> struct panthor_gpu *gpu = ptdev->gpu;
> bool timedout = false;
> - unsigned long flags;
>
> - spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> + guard(spinlock)(&ptdev->gpu->reqs_lock);
> +
> if (!drm_WARN_ON(&ptdev->base,
> ptdev->gpu->pending_reqs & GPU_IRQ_RESET_COMPLETED)) {
> ptdev->gpu->pending_reqs |= GPU_IRQ_RESET_COMPLETED;
> gpu_write(gpu->irq.iomem, INT_CLEAR, GPU_IRQ_RESET_COMPLETED);
> gpu_write(gpu->iomem, GPU_CMD, GPU_SOFT_RESET);
> }
> - spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
>
> - if (!wait_event_timeout(ptdev->gpu->reqs_acked,
> + if (!wait_event_lock_timeout(ptdev->gpu->reqs_acked,
> !(ptdev->gpu->pending_reqs &
> GPU_IRQ_RESET_COMPLETED),
> + ptdev->gpu->reqs_lock,
> msecs_to_jiffies(100))) {
> - spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> if ((ptdev->gpu->pending_reqs & GPU_IRQ_RESET_COMPLETED) != 0 &&
> !(gpu_read(gpu->irq.iomem, INT_RAWSTAT) &
> GPU_IRQ_RESET_COMPLETED))
> timedout = true;
> else
> ptdev->gpu->pending_reqs &= ~GPU_IRQ_RESET_COMPLETED;
> - spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
> }
>
> if (timedout) {
>
> --
> 2.55.0
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
