[PATCH 3/3] drm/i915: Skip pipestats for GT operations in chv/vlv irq handler

Chris Wilson chris at chris-wilson.co.uk
Wed Sep 13 17:57:55 UTC 2017


When handling context-switch interrupts, we are very latency sensitive;
every unnecessary mmio (uncached) read contributes toward a large
decrease in request throughput. An example is gem_exec_whisper, which
ping-pongs between the engines, where avoiding the pipe underflow
checking reduces the runtime of the test from 29s to 22s. On the other
hand, we are now not checking for pipe underflows at 100KHz, more or
less reducing it to a check every pageflip (60Hz) or modeset at worst.

add/remove: 0/0 grow/shrink: 1/2 up/down: 24/-40 (-16)
function                                     old     new   delta
valleyview_pipestat_irq_ack                  259     283     +24
valleyview_irq_handler                       521     517      -4
cherryview_irq_handler                       457     421     -36

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 123 ++++++++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e1d503b7352e..345d73bd4039 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1703,16 +1703,17 @@ static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 	}
 }
 
-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 handled = 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) {
@@ -1744,8 +1745,10 @@ static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
 		if (iir & iir_bit)
 			mask |= dev_priv->pipestat_irq_mask[pipe];
 
-		if (!mask)
+		if (!mask) {
+			pipe_stats[pipe] = 0;
 			continue;
+		}
 
 		reg = PIPESTAT(pipe);
 		mask |= PIPESTAT_INT_ENABLE_MASK;
@@ -1757,8 +1760,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]);
+
+		handled = true;
 	}
 	spin_unlock(&dev_priv->irq_lock);
+
+	return handled;
 }
 
 static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
@@ -1836,8 +1843,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 	do {
 		u32 iir, gt_iir, pm_iir;
-		u32 pipe_stats[I915_MAX_PIPES] = {};
+		u32 pipe_stats[I915_MAX_PIPES];
 		u32 hotplug_status = 0;
+		bool has_pipe_stats;
 		u32 ier = 0;
 
 		gt_iir = I915_READ(GTIIR);
@@ -1876,7 +1884,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 		/* Call regardless, as some status bits might not be
 		 * signalled in iir */
-		valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+		has_pipe_stats =
+			valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
 		if (iir & (I915_LPE_PIPE_A_INTERRUPT |
 			   I915_LPE_PIPE_B_INTERRUPT))
@@ -1901,7 +1910,8 @@ static irqreturn_t valleyview_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 (has_pipe_stats)
+			valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
 	} while (0);
 
 	enable_rpm_wakeref_asserts(dev_priv);
@@ -1914,27 +1924,14 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	irqreturn_t ret = IRQ_NONE;
+	u32 master_ctl;
 
 	if (!intel_irqs_enabled(dev_priv))
 		return IRQ_NONE;
 
-	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
-	disable_rpm_wakeref_asserts(dev_priv);
-
+	master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
 	do {
-		u32 master_ctl, iir;
-		u32 gt_iir[4] = {};
-		u32 pipe_stats[I915_MAX_PIPES] = {};
-		u32 hotplug_status = 0;
-		u32 ier = 0;
-
-		master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
-		iir = I915_READ(VLV_IIR);
-
-		if (master_ctl == 0 && iir == 0)
-			break;
-
-		ret = IRQ_HANDLED;
+		u32 iir;
 
 		/*
 		 * Theory on interrupt generation, based on empirical evidence:
@@ -1949,44 +1946,70 @@ 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);
+		if (master_ctl & ~GEN8_MASTER_IRQ_CONTROL) {
+			u32 gt_iir[4];
 
-		gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
+			I915_WRITE_FW(GEN8_MASTER_IRQ, 0);
 
-		if (iir & I915_DISPLAY_PORT_INTERRUPT)
-			hotplug_status = i9xx_hpd_irq_ack(dev_priv);
+			gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
 
-		/* Call regardless, as some status bits might not be
-		 * signalled in iir */
-		valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+			I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+			ret = IRQ_HANDLED;
 
-		if (iir & (I915_LPE_PIPE_A_INTERRUPT |
-			   I915_LPE_PIPE_B_INTERRUPT |
-			   I915_LPE_PIPE_C_INTERRUPT))
-			intel_lpe_audio_irq_handler(dev_priv);
+			gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir);
+			master_ctl = 0;
+		}
 
-		/*
-		 * VLV_IIR is single buffered, and reflects the level
-		 * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last.
-		 */
-		if (iir)
-			I915_WRITE(VLV_IIR, iir);
+		iir = I915_READ_FW(VLV_IIR);
+		if (iir) {
+			u32 pipe_stats[I915_MAX_PIPES];
+			u32 hotplug_status = 0;
+			bool has_pipe_stats = false;
+			u32 ier;
 
-		I915_WRITE(VLV_IER, ier);
-		I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
-		POSTING_READ(GEN8_MASTER_IRQ);
+			/*
+			 * IRQs are synced during runtime_suspend,
+			 * we don't require a wakeref
+			 */
+			disable_rpm_wakeref_asserts(dev_priv);
 
-		gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir);
 
-		if (hotplug_status)
-			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
+			ier = I915_READ_FW(VLV_IER);
+			I915_WRITE_FW(VLV_IER, 0);
 
-		valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
-	} while (0);
+			if (iir & I915_DISPLAY_PORT_INTERRUPT)
+				hotplug_status = i9xx_hpd_irq_ack(dev_priv);
 
-	enable_rpm_wakeref_asserts(dev_priv);
+			if (iir & (I915_LPE_PIPE_A_INTERRUPT |
+				   I915_LPE_PIPE_B_INTERRUPT |
+				   I915_LPE_PIPE_C_INTERRUPT))
+				intel_lpe_audio_irq_handler(dev_priv);
+
+			/*
+			 * Call regardless, as some status bits might not be
+			 * signalled in iir.
+			 */
+			has_pipe_stats =
+				valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats);
+
+			/*
+			 * VLV_IIR is single buffered, and reflects the level
+			 * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last.
+			 */
+			I915_WRITE_FW(VLV_IIR, iir);
+			I915_WRITE_FW(VLV_IER, ier);
+			ret = IRQ_HANDLED;
+
+			if (hotplug_status)
+				i9xx_hpd_irq_handler(dev_priv, hotplug_status);
+
+			if (has_pipe_stats)
+				valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
+
+			enable_rpm_wakeref_asserts(dev_priv);
+			master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
+		}
+	} while (master_ctl & ~GEN8_MASTER_IRQ_CONTROL);
 
 	return ret;
 }
-- 
2.14.1



More information about the Intel-gfx-trybot mailing list