[Intel-gfx] [PATCH] drm/i915: Reduce register reads around GT interrupts

[Chris Wilson]

The GT subsystem is highly dependent on interrupts for driving the GPU;
while the interrupt is being processed the GPU may idle leading to lower
throughput and higher latency. Since the GT interrupts dominate, push
the incidental display interrupt handling to later.

gem_exec_nop/parallel on ivb [i7-3720QM]:

Before:
  average (individually): 2.278us
  rcs0: 8337674 cycles, 2.399us
  vcs0: 3286990 cycles, 6.085us
  bcs0: 2047917 cycles, 9.766us

After:
  average (individually): 2.118us
  rcs0: 10132462 cycles, 1.974us
  bcs0: 3667130 cycles, 5.454us
  vcs0: 3616618 cycles, 5.530us

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 58 +++++++++++++++++----------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1fa67700d8f4..e156de88bdc6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2099,29 +2099,16 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	void __iomem * const regs = i915->uncore.regs;
-	u32 de_iir, gt_iir, de_ier, sde_ier = 0;
+	u32 de_iir, gt_iir, de_ier;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (unlikely(!intel_irqs_enabled(i915)))
 		return IRQ_NONE;
 
-	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
-	disable_rpm_wakeref_asserts(&i915->runtime_pm);
-
 	/* disable master interrupt before clearing iir  */
 	de_ier = raw_reg_read(regs, DEIER);
 	raw_reg_write(regs, DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
 
-	/* 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
-	 * able to process them after we restore SDEIER (as soon as we restore
-	 * it, we'll get an interrupt if SDEIIR still has something to process
-	 * due to its back queue). */
-	if (!HAS_PCH_NOP(i915)) {
-		sde_ier = raw_reg_read(regs, SDEIER);
-		raw_reg_write(regs, SDEIER, 0);
-	}
-
 	/* Find, clear, then process each source of interrupt */
 
 	gt_iir = raw_reg_read(regs, GTIIR);
@@ -2134,32 +2121,47 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg)
 		ret = IRQ_HANDLED;
 	}
 
+	if (INTEL_GEN(i915) >= 6) {
+		u32 pm_iir = raw_reg_read(regs, GEN6_PMIIR);
+		if (pm_iir) {
+			raw_reg_write(regs, GEN6_PMIIR, pm_iir);
+			gen6_rps_irq_handler(&i915->gt.rps, pm_iir);
+			ret = IRQ_HANDLED;
+		}
+	}
+
 	de_iir = raw_reg_read(regs, DEIIR);
 	if (de_iir) {
+		u32 sde_ier = 0;
+
+		/*
+		 * 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 able to process them after we restore
+		 * SDEIER (as soon as we restore it, we'll get an interrupt if
+		 * SDEIIR still has something to process due to its back queue).
+		 */
+		if (!HAS_PCH_NOP(i915)) {
+			sde_ier = raw_reg_read(regs, SDEIER);
+			raw_reg_write(regs, SDEIER, 0);
+		}
+
+		disable_rpm_wakeref_asserts(&i915->runtime_pm);
+
 		raw_reg_write(regs, DEIIR, de_iir);
 		if (INTEL_GEN(i915) >= 7)
 			ivb_display_irq_handler(i915, de_iir);
 		else
 			ilk_display_irq_handler(i915, de_iir);
+
+		enable_rpm_wakeref_asserts(&i915->runtime_pm);
 		ret = IRQ_HANDLED;
-	}
 
-	if (INTEL_GEN(i915) >= 6) {
-		u32 pm_iir = raw_reg_read(regs, GEN6_PMIIR);
-		if (pm_iir) {
-			raw_reg_write(regs, GEN6_PMIIR, pm_iir);
-			gen6_rps_irq_handler(&i915->gt.rps, pm_iir);
-			ret = IRQ_HANDLED;
-		}
+		if (sde_ier)
+			raw_reg_write(regs, SDEIER, sde_ier);
 	}
 
 	raw_reg_write(regs, DEIER, de_ier);
-	if (sde_ier)
-		raw_reg_write(regs, SDEIER, sde_ier);
-
-	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
-	enable_rpm_wakeref_asserts(&i915->runtime_pm);
-
 	return ret;
 }
 
-- 
2.20.1