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.
Yeah, seems fine by me:
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> enable_rpm_wakeref_asserts(dev_priv);
> --
> 2.18.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
