Quoting Ville Syrjälä (2018-06-28 14:06:40)
> On Thu, Jun 28, 2018 at 01:33:43PM +0100, Chris Wilson wrote:
> > We do not need to do a posting read of our uncached mmio write to
> > re-enable the master interrupt lines after handling an interrupt, so
> > don't. This saves us a slow UC read before we can process the interrupt,
> > most noticeable in execlists where any stalls imposes extra latency on
> > GPU command execution.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 9 +--------
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 7a7c4a2bd778..e83fcedcbf1d 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2168,7 +2168,6 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> >
> > I915_WRITE(VLV_IER, ier);
> > I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
> > - POSTING_READ(VLV_MASTER_IER);
> >
> > if (gt_iir)
> > snb_gt_irq_handler(dev_priv, gt_iir);
> > @@ -2253,7 +2252,6 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
> >
> > I915_WRITE(VLV_IER, ier);
> > I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> > - POSTING_READ(GEN8_MASTER_IRQ);
> >
> > gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir);
> >
> > @@ -2622,7 +2620,6 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
> > /* disable master interrupt before clearing iir */
> > de_ier = I915_READ(DEIER);
> > I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
> > - POSTING_READ(DEIER);
> >
> > /* Disable south interrupts. We'll only write to SDEIIR once, so further
> > * interrupts will will be stored on its back queue, and then we'll be
> > @@ -2632,7 +2629,6 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
> > if (!HAS_PCH_NOP(dev_priv)) {
> > sde_ier = I915_READ(SDEIER);
> > I915_WRITE(SDEIER, 0);
> > - POSTING_READ(SDEIER);
> > }
> >
> > /* Find, clear, then process each source of interrupt */
> > @@ -2667,11 +2663,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
> > }
> >
> > I915_WRITE(DEIER, de_ier);
> > - POSTING_READ(DEIER);
> > - if (!HAS_PCH_NOP(dev_priv)) {
> > + if (!HAS_PCH_NOP(dev_priv))
> > I915_WRITE(SDEIER, sde_ier);
> > - POSTING_READ(SDEIER);
> > - }
>
> Not 100% sure about the SDEIER thing. I believe it goes over the link
> to the PCH so I suppose it might be posted. No idea how it would get
> ordered wrt. subsequent accesses to CPU side registers. Hmm. Oh we
> actually restore master first, then SDEIER. But I suppose the order
> doesn't actually matter. As long as SDEIER is 0 at some point we should
> get a new edge in the end if SDEIIR still has something asserted.
I have vague memories of you saying "oh master is done before sdeier",
that's odd (or that might have been me)!
> Yeah, seems fine by me:
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Ta,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
