On Tue, Aug 11, 2026 at 04:08:32PM +0200, Nicolas Frattaroli wrote:
> panthor_gpu_flush_caches() and panthor_gpu_soft_reset() acquire their
> reqs_lock spinlock with the IRQ-disabling variants of the spinlocking
> functions. This isn't necessary, as the lock is never taken from an
> atomic context, as Panthor uses threaded interrupt handlers. The result
> of this overly strict locking is that IRQs may be disabled more
> frequently and for longer than they should be, resulting in increased
> system latency.
>
> Switch the locking to use non-IRQ-disabling scoped_guard statements for
> locking. The wait_event_timeout read of pending_reqs outside of the
> spinlock is fine as wait_event_timeout is a memory barrier according to
> the Linux Memory Model.
>
> 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 | 66
> ++++++++++++++++-------------------
> 1 file changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c
> b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 68e2dd2527df..cb5319d1c5de 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -330,37 +330,32 @@ 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);
> - trace_gpu_cache_flush_start(ptdev->base.dev, l2, lsc, other);
> - 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) {
> - trace_gpu_cache_flush_end(ptdev->base.dev, l2, lsc, other);
> - return ret;
> + scoped_guard(spinlock, &ptdev->gpu->reqs_lock) {
> + trace_gpu_cache_flush_start(ptdev->base.dev, l2, lsc, other);
> + 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 {
> + trace_gpu_cache_flush_end(ptdev->base.dev, l2, lsc,
> other);
> + return -EIO;
> + }
> }
>
> if (!wait_event_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);
> - 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);
> + scoped_guard(spinlock, &ptdev->gpu->reqs_lock) {
> + 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;
> + }
> }
>
> trace_gpu_cache_flush_end(ptdev->base.dev, l2, lsc, other);
> @@ -383,27 +378,26 @@ 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);
> - 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);
> + scoped_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,
> !(ptdev->gpu->pending_reqs &
> GPU_IRQ_RESET_COMPLETED),
> 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);
> + scoped_guard(spinlock, &ptdev->gpu->reqs_lock) {
> + 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;
> + }
> }
>
> if (timedout) {
>
> --
> 2.55.0
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
