On Thu, Jun 25, 2026 at 02:40:31PM +0200, Boris Brezillon wrote:
> In theory, our hardirq handler can be called while the device (and
> thus the panthor_irq) is suspended, because the IRQ line is shared.
> In practice though, in all the designs we've seen, the line is only
> shared within the GPU, and because sub-component suspend state is
> consistent (all-suspended or all-resumed), we shouldn't end up with
> an interrupt triggered while we're suspended.
>
> Fix the problem anyway, if nothing else, for our sanity.
>
> Fixes: 0b2d86670a84 ("drm/panthor: Rework panthor_irq::suspended into
> panthor_irq::state")
> Reported-by: [email protected]
> Closes:
> https://sashiko.dev/#/patchset/[email protected]?part=1
> Signed-off-by: Boris Brezillon <[email protected]>
> ---
> drivers/gpu/drm/panthor/panthor_device.h | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h
> b/drivers/gpu/drm/panthor/panthor_device.h
> index 35679bfa1f3a..a39386bd6382 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -512,9 +512,6 @@ static irqreturn_t panthor_ ## __name ##
> _irq_raw_handler(int irq, void *data)
> struct panthor_irq *pirq = data;
> \
> enum panthor_irq_state old_state;
> \
>
> \
> - if (!gpu_read(pirq->iomem, INT_STAT))
> \
> - return IRQ_NONE;
> \
> -
> \
> guard(spinlock_irqsave)(&pirq->mask_lock);
> \
> old_state = atomic_cmpxchg(&pirq->state,
> \
> PANTHOR_IRQ_STATE_ACTIVE,
> \
> @@ -522,6 +519,13 @@ static irqreturn_t panthor_ ## __name ##
> _irq_raw_handler(int irq, void *data)
> if (old_state != PANTHOR_IRQ_STATE_ACTIVE)
> \
> return IRQ_NONE;
> \
>
> \
> + if (!gpu_read(pirq->iomem, INT_STAT)) {
> \
> + atomic_cmpxchg(&pirq->state,
> \
> + PANTHOR_IRQ_STATE_PROCESSING,
> \
> + PANTHOR_IRQ_STATE_ACTIVE);
> \
> + return IRQ_NONE;
> \
> + }
> \
Hmm,
I get it that you're trying to revert the effect of the previous
atomic_cmpxchg() here but it feels
like a better option would be to not do the swap at all if the state is not
ACTIVE.
Best regards,
Liviu
> +
> \
> gpu_write(pirq->iomem, INT_MASK, 0);
> \
> return IRQ_WAKE_THREAD;
> \
> }
> \
>
> --
> 2.54.0
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
