Re: [PATCH v2 09/11] drm/panthor: Process FW events in IRQ context

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue, 12 May 2026 15:09:47 -0700
Chia-I Wu <olvaffe@gmail.com> wrote:

> On Tue, May 12, 2026 at 3:05 PM Chia-I Wu <olvaffe@gmail.com> wrote:
> >
> > On Tue, May 12, 2026 at 4:54 AM Boris Brezillon
> > <boris.brezillon@collabora.com> wrote:  
> > >
> > > Now that everything is set to allow processing FW events in IRQ context,
> > > go for it. This should reduce the dma_fence signaling latency.
> > >
> > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > > ---
> > >  drivers/gpu/drm/panthor/panthor_fw.c | 27 +++++++++++++++++++++++----
> > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> > > index f5e0ceca4130..8cfebf180de7 100644
> > > --- a/drivers/gpu/drm/panthor/panthor_fw.c
> > > +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> > > @@ -1087,9 +1087,29 @@ static void panthor_job_irq_handler(struct panthor_irq *pirq, u32 status)
> > >         }
> > >  }
> > >
> > > -static irqreturn_t panthor_job_irq_threaded_handler(int irq, void *data)
> > > +static irqreturn_t panthor_job_irq_raw_handler(int irq, void *data)
> > >  {
> > > -       return panthor_irq_default_threaded_handler(data, panthor_job_irq_handler);
> > > +       struct panthor_irq *pirq = data;
> > > +
> > > +       if (!gpu_read(pirq->iomem, INT_STAT))
> > > +               return IRQ_NONE;
> > > +
> > > +       scoped_guard(spinlock_irqsave, &pirq->mask_lock) {
> > > +               if (pirq->state != PANTHOR_IRQ_STATE_ACTIVE)
> > > +                       return IRQ_NONE;
> > > +
> > > +               pirq->state = PANTHOR_IRQ_STATE_PROCESSING;
> > > +       }
> > > +
> > > +       /* We can use INT_STAT here, because we didn't mask the IRQs. */
> > > +       panthor_job_irq_handler(pirq, gpu_read(pirq->iomem, INT_STAT));  
> > We should loop here until INT_STAT is cleared.  
> Perhaps not. This is hardirq.

Yep, the absence of loop is intentional here. We process it one at a
time, and give other interrupt handlers a chance to do their stuff
before we get called again.

> 
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> > > +
> > > +       scoped_guard(spinlock_irqsave, &pirq->mask_lock) {
> > > +               if (pirq->state == PANTHOR_IRQ_STATE_PROCESSING)
> > > +                       pirq->state = PANTHOR_IRQ_STATE_ACTIVE;
> > > +       }
> > > +
> > > +       return IRQ_HANDLED;
> > >  }
> > >
> > >  static int panthor_fw_start(struct panthor_device *ptdev)
> > > @@ -1489,8 +1509,7 @@ int panthor_fw_init(struct panthor_device *ptdev)
> > >
> > >         ret = panthor_irq_request(ptdev, &fw->irq, irq, 0,
> > >                                   ptdev->iomem + JOB_INT_BASE, "job",
> > > -                                 panthor_irq_default_raw_handler,
> > > -                                 panthor_job_irq_threaded_handler);
> > > +                                 panthor_job_irq_raw_handler, NULL);
> > >         if (ret) {
> > >                 drm_err(&ptdev->base, "failed to request job irq");
> > >                 return ret;
> > >
> > > --
> > > 2.54.0
> > >