diff mbox

[RFC] drm/i915: Skip BSW display interrupt checks if we have a GT interrupt

Message ID 20170120101509.6886-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Jan. 20, 2017, 10:15 a.m. UTC 
GT interrupts are very, very frequent as they are used for submitting
every request to the hardware (thanks be to execlists). Given their
prevalence and the comparity rarity of display interrupts, if we do
receive an IRQ and we process a GT interrupt skip the *unconditional*
checking of the display pipes.

This gives a 20% improvement in *walltime* of GEM execution tests on my
Braswell nuc.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

Comments

Chris Wilson Jan. 20, 2017, 1:13 p.m. UTC | #1 
On Fri, Jan 20, 2017 at 10:15:09AM +0000, Chris Wilson wrote:
> GT interrupts are very, very frequent as they are used for submitting
> every request to the hardware (thanks be to execlists). Given their
> prevalence and the comparity rarity of display interrupts, if we do
> receive an IRQ and we process a GT interrupt skip the *unconditional*
> checking of the display pipes.
> 
> This gives a 20% improvement in *walltime* of GEM execution tests on my
> Braswell nuc.

Ignoring the fact that this patch missed an interrupt and so is fubar,
10% of that walltime improvement actually came from latency reductions
elsewhere, I compared against the wrong baseline.
-Chris
Ville Syrjälä Jan. 20, 2017, 2:21 p.m. UTC | #2 
On Fri, Jan 20, 2017 at 01:13:48PM +0000, Chris Wilson wrote:
> On Fri, Jan 20, 2017 at 10:15:09AM +0000, Chris Wilson wrote:
> > GT interrupts are very, very frequent as they are used for submitting
> > every request to the hardware (thanks be to execlists). Given their
> > prevalence and the comparity rarity of display interrupts, if we do
> > receive an IRQ and we process a GT interrupt skip the *unconditional*
> > checking of the display pipes.
> > 
> > This gives a 20% improvement in *walltime* of GEM execution tests on my
> > Braswell nuc.
> 
> Ignoring the fact that this patch missed an interrupt and so is fubar,
> 10% of that walltime improvement actually came from latency reductions
> elsewhere, I compared against the wrong baseline.

	+ if (iir & (PIPE_EVENT_A | ...))
		valleyview_pipestar_irq_ack(...);

should be the simple way to avoid looking for display interrupts when
processing just GT interrupts. But it would mean losing underrun
detection when only GT interrupts are occurring. But perhaps it'si
still a worthwile tradeoff to make?
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7aaa0121c2e9..071e0fa21ca0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1743,16 +1743,17 @@  static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
-static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
+static bool valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 					u32 iir, u32 pipe_stats[I915_MAX_PIPES])
 {
+	bool active = false;
 	int pipe;
 
 	spin_lock(&dev_priv->irq_lock);
 
 	if (!dev_priv->display_irqs_enabled) {
 		spin_unlock(&dev_priv->irq_lock);
-		return;
+		return false;
 	}
 
 	for_each_pipe(dev_priv, pipe) {
@@ -1797,8 +1798,12 @@  static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 		if (pipe_stats[pipe] & (PIPE_FIFO_UNDERRUN_STATUS |
 					PIPESTAT_INT_STATUS_MASK))
 			I915_WRITE(reg, pipe_stats[pipe]);
+
+		active = true;
 	}
 	spin_unlock(&dev_priv->irq_lock);
+
+	return active;
 }
 
 static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
@@ -1966,12 +1971,19 @@  static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		u32 gt_iir[4] = {};
 		u32 pipe_stats[I915_MAX_PIPES] = {};
 		u32 hotplug_status = 0;
+		bool any_pipe_stats = false;
 		u32 ier = 0;
 
 		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
-		iir = I915_READ(VLV_IIR);
+		I915_WRITE(GEN8_MASTER_IRQ, 0);
+
+		if (gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir)) {
+			ret = IRQ_HANDLED;
+			goto skip_display;
+		}
 
-		if (master_ctl == 0 && iir == 0)
+		iir = I915_READ(VLV_IIR);
+		if (iir == 0)
 			break;
 
 		ret = IRQ_HANDLED;
@@ -1989,18 +2001,17 @@  static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		 * don't end up clearing all the VLV_IIR and GEN8_MASTER_IRQ_CONTROL
 		 * bits this time around.
 		 */
-		I915_WRITE(GEN8_MASTER_IRQ, 0);
 		ier = I915_READ(VLV_IER);
 		I915_WRITE(VLV_IER, 0);
 
-		gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
 
 		if (iir & I915_DISPLAY_PORT_INTERRUPT)
 			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
-		valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+		any_pipe_stats =
+			valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		/*
 		 * VLV_IIR is single buffered, and reflects the level
@@ -2010,6 +2021,8 @@  static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 			I915_WRITE(VLV_IIR, iir);
 
 		I915_WRITE(VLV_IER, ier);
+
+skip_display:
 		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 		POSTING_READ(GEN8_MASTER_IRQ);
 
@@ -2018,7 +2031,8 @@  static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
 
-		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
+		if (any_pipe_stats)
+			valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
 	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);