[Intel-gfx] [PATCH v2] drm/i915/icl: GSE interrupt moves from DE_MISC to GU_MISC

[Dhinakaran Pandiyan]

On Fri, 2018-05-25 at 20:56 +0100, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-05-25 20:43:13)
> > 
> > The Graphics System Event(GSE) interrupt bit has a new location in
> > the
> > GU_MISC_INTERRUPT_{IIR, ISR, IMR, IER} registers. Since GSE was the
> > only
> > DE_MISC interrupt that was enabled, with this change we don't
> > enable/handle
> > any of DE_MISC interrupts for gen11. Credits to Paulo for pointing
> > out
> > the register change.
> > 
> > v2: from DK
> > raw_reg_[read/write], branch prediction hint and drop platform
> > check (Mika)
> > 
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > [Paulo: bikesheds and rebases]
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 31
> > ++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_reg.h |  7 +++++++
> >  2 files changed, 37 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 2fd92a886789..cdbc23b21df6 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2943,6 +2943,26 @@ gen11_gt_irq_handler(struct drm_i915_private
> > * const i915,
> >         spin_unlock(&i915->irq_lock);
> >  }
> >  
> > +static void
> > +gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv,
> > +                         const u32 master_ctl)
> > +{
> > +       void __iomem * const regs = dev_priv->regs;
> > +       u32 iir;
> > +
> > +       if (!(master_ctl & GEN11_GU_MISC_IRQ))
> > +               return;
> > +
> > +       iir = raw_reg_read(regs, GEN11_GU_MISC_IIR);
> > +       if (likely(iir)) {
> > +               raw_reg_write(regs, GEN11_GU_MISC_IIR, iir);
> > +               if (iir & GEN11_GU_MISC_GSE)
> > +                       intel_opregion_asle_intr(dev_priv);
> > +               else
> > +                       DRM_ERROR("Unexpected GU Misc interrupt
> > 0x%08x\n", iir);
> You should be re-enabling the master interrupt *before* doing any
> work.
> No?
intel_opregion_asle_intr() doesn't do much other than scheduling work
and this is similar to how interrupts are handled for other platforms.

Are you suggesting we optimize our interrupt handling logic to read all
the required IIR's first, re-enable the master interrupt and then call
the specific handlers based on the set IIR's? This would be a
widespread change.

> 
> Keeping the master interrupt disabled stops all other CPUs from
> processing our interrupts; e.g. basically stopping us feeding the GPU
> with work while we wait for you.
> -Chris