Re: [PATCH v2 04/11] drm/panthor: Extend the IRQ logic to allow fast/hard IRQ handlers

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

On Wed, 13 May 2026 10:06:14 -0700
Chia-I Wu <olvaffe@gmail.com> wrote:

> On Wed, May 13, 2026 at 1:09 AM Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > On Tue, 12 May 2026 12:11:08 -0700
> > Chia-I Wu <olvaffe@gmail.com> wrote:
> >  
> > > On Tue, May 12, 2026 at 4:54 AM Boris Brezillon
> > > <boris.brezillon@collabora.com> wrote:  
> > > >
> > > > All drivers except panthor signal their fences from their interrupt
> > > > handler to minimize latency. We could do the same from the threaded
> > > > handler, but the latency is still quite high in that case, so let's
> > > > allow components to choose the context they want their IRQ handler
> > > > to run in by exposing support for custom hard handlers.
> > > >
> > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > Reviewed-by: Steven Price <steven.price@arm.com>
> > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > > > ---
> > > >  drivers/gpu/drm/panthor/panthor_device.h | 11 ++++++++---
> > > >  drivers/gpu/drm/panthor/panthor_fw.c     |  1 +
> > > >  drivers/gpu/drm/panthor/panthor_gpu.c    |  1 +
> > > >  drivers/gpu/drm/panthor/panthor_mmu.c    |  1 +
> > > >  drivers/gpu/drm/panthor/panthor_pwr.c    |  1 +
> > > >  5 files changed, 12 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> > > > index 393fcda73d88..1aaf06df875b 100644
> > > > --- a/drivers/gpu/drm/panthor/panthor_device.h
> > > > +++ b/drivers/gpu/drm/panthor/panthor_device.h
> > > > @@ -672,6 +672,7 @@ static inline void panthor_irq_disable_events(struct panthor_irq *pirq, u32 mask
> > > >  static inline int
> > > >  panthor_irq_request(struct panthor_device *ptdev, struct panthor_irq *pirq,
> > > >                     int irq, u32 mask, void __iomem *iomem, const char *name,
> > > > +                   irqreturn_t (*raw_handler)(int, void *data),
> > > >                     irqreturn_t (*threaded_handler)(int, void *data))
> > > >  {
> > > >         const char *full_name;
> > > > @@ -687,9 +688,13 @@ panthor_irq_request(struct panthor_device *ptdev, struct panthor_irq *pirq,
> > > >                 return -ENOMEM;
> > > >
> > > >         panthor_irq_resume(pirq);
> > > > -       return devm_request_threaded_irq(ptdev->base.dev, irq,
> > > > -                                        panthor_irq_default_raw_handler,
> > > > -                                        threaded_handler,
> > > > +
> > > > +       if (!threaded_handler) {
> > > > +               return devm_request_irq(ptdev->base.dev, irq, raw_handler,
> > > > +                                       IRQF_SHARED, full_name, pirq);
> > > > +       }  
> > > devm_request_irq expands to devm_request_threaded_irq plus
> > > IRQF_COND_ONESHOT. This appears redundant.  
> >
> > I considered going for devm_request_threaded_irq(COND_ONESHOT), but I
> > thought it was easier to reason about with a regular devm_request_irq()
> > and an extra conditional since request_irq() is what people tend
> > to use when they just have a hard handler (see [1], there's just one
> > driver using it, and it's not even needed, because it's calling
> > devm_request_irq() which adds this flag already)  
> It is unclear to me why the current version wants IRQF_COND_ONESHOT in
> one case but not in another. Can't we call devm_request_threaded_irq
> without IRQF_COND_ONESHOT for both cases?

Hm, I thought this had to do with the automatic hard -> threaded
downgrade happening when RT is enabled, but I fail to see why it
matters, since all irqaction end up with IRQF_ONESHOT in that case
anyway. Honestly, I'm tempted to stay on the safe side, and have
devm_request_irq() called when we just have a hard handler, because I'm
sure there's a reason for this COND_ONESHOT flag.