[Intel-gfx] [PATCH] drm/i915: Delegate our irq handler to a thread

[Chris Wilson]

Quoting Chris Wilson (2019-09-26 16:40:34)
> Quoting Sebastian Andrzej Siewior (2019-09-26 16:32:52)
> > On 2019-09-26 16:24:59 [+0100], Chris Wilson wrote:
> > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > > > index bc83f094065a..f3df7714a3f3 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > > @@ -4491,8 +4491,8 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
> > > > >  
> > > > >       intel_irq_reset(dev_priv);
> > > > >  
> > > > > -     ret = request_irq(irq, intel_irq_handler(dev_priv),
> > > > > -                       IRQF_SHARED, DRIVER_NAME, dev_priv);
> > > > > +     ret = request_threaded_irq(irq, NULL, intel_irq_handler(dev_priv),
> > > > > +                                IRQF_SHARED, DRIVER_NAME, dev_priv);
> > > > 
> > > > I think you should add IRQF_ONESHOT. Otherwise a LEVEL interrupt will
> > > > keep interrupting the CPU and you never manage to switch to the thread.
> > > 
> > > The interrupts only keep coming if we feed the GPU, and we only feed the
> > > GPU if we service the interrupt. That should provide a natural
> > > quiescence :)
> > 
> > In IRQ-context your primary handler gets invoked which wakes the thread
> > (what ever intel_irq_handler() returns). Then you leave the IRQ context
> > and should switch to your IRQ-handler. This will never happen because
> > the IRQ line remains asserted and CPU ends up in the primary handler
> > again.
> > An EDGE typed IRQ wouldn't notice a difference. But a LINE typed IRQ
> > will remain asserted until the hardware de-asserts the interrupt again.
> > Since you never reach your thread handler, I don't see how this can
> > happen.
> 
> It's all edge interrupts -- although for gen2/3 my memory is hazy. But
> the GPU (circa gen6) can generate more than enough interrupts to saturate
> a CPU.

So everything older than gen5 has MSI disabled it appears and needs
ONESHOT.
-Chris