On Wed, 13 May 2026 10:06:14 -0700 Chia-I Wu <[email protected]> wrote:
> On Wed, May 13, 2026 at 1:09 AM Boris Brezillon > <[email protected]> wrote: > > > > On Tue, 12 May 2026 12:11:08 -0700 > > Chia-I Wu <[email protected]> wrote: > > > > > On Tue, May 12, 2026 at 4:54 AM Boris Brezillon > > > <[email protected]> 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 <[email protected]> > > > > Reviewed-by: Steven Price <[email protected]> > > > > Signed-off-by: Boris Brezillon <[email protected]> > > > > --- > > > > 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.
