On Wed, Apr 13, 2016 at 09:53:38PM +0100, Chris Wilson wrote:
> On Wed, Apr 13, 2016 at 09:19:51PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > On VLV/CHV the master interrupt enable bit only affects GT/PM
> > interrupts. Display interrupts are not affected by the master
> > irq control.
> >
> > Also it seems that the CPU interrupt will only be generated when
> > the combined result of all GT/PM/display interrupts has a 0->1
> > edge. We already use the master interrupt enable bit to make sure
> > GT/PM interrupt can generate such an edge if we don't end up clearing
> > all IIR bits. We must do the same for display interrupts, and for
> > that we can simply clear out VLV_IER, and restore after we've acked
> > all the interrupts we are about to process.
> >
> > So with both master interrupt enable and VLV_IER cleared out, we will
> > guarantee that there will be a 0->1 edge if any IIR bits are still set
> > at the end, and thus another CPU interrupt will be generated.
> >
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after one pass")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++-
> > 1 file changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 626775039919..46be03c616f4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> > disable_rpm_wakeref_asserts(dev_priv);
> >
> > while (true) {
> > - /* Find, clear, then process each source of interrupt */
> > + u32 ier = 0;
> >
> > gt_iir = I915_READ(GTIIR);
> > pm_iir = I915_READ(GEN6_PMIIR);
> > @@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> >
> > ret = IRQ_HANDLED;
> >
> > + /*
> > + * Theory on interrupt generation, based on empirical evidence:
> > + *
> > + * x = ((VLV_IIR & VLV_IER) ||
> > + * (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) &&
> > + * (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE)));
> > + *
> > + * A CPU interrupt will only be raised when 'x' has a 0->1 edge.
> > + * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to
> > + * guarantee the CPU interrupt will be raised again even if we
> > + * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR
> > + * bits this time around.
>
> Following this logic, we want to enable MASTER_IER before VLV_IER such
> that we get an immediate irq if there is a residual VLV_IIR.
The order between master irq enable and VLV_IIR shouldn't matter. They
are totally independent of each other. Master irq enable is for GT,
VLV_IER is for display. We just have to make sure both of them are
going to be zero simultaneously, which will guarantee that the CPU
interrupt generation logic will see x==0 at that point.
>
> > + */
> > I915_WRITE(VLV_MASTER_IER, 0);
> > + ier = I915_READ(VLV_IER);
>
> +wishlist: dev_priv->irq_enabled to save adding another mmio read.
I suppose such a thing could be added.
>
> > + I915_WRITE(VLV_IER, 0);
> >
> > if (gt_iir)
> > I915_WRITE(GTIIR, gt_iir);
> > @@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> > if (iir)
> > I915_WRITE(VLV_IIR, iir);
> >
> > + I915_WRITE(VLV_IER, ier);
> > I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
> > POSTING_READ(VLV_MASTER_IER);
> > }
> >
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
