[Intel-gfx] [PATCH] drm/i915: Clear PIPE.STAT before IIR on VLV/CHV

[Chris Wilson]

The PIPE.STAT register contains some interrupt status bits per pipe, and
if assert cause the corresponding bit in the IIR to be asserted (thus
raising an interrupt). When handling an interrupt, we should clear the
PIPE.STAT generator first before clearing the IIR so that we do not miss
events or cause spurious work.

This ordering was broken by

commit 27b6c122512ca30399bb1b39cc42eda83901f304
Author: Oscar Mateo <oscar.mateo at intel.com>
Date:   Mon Jun 16 16:11:00 2014 +0100

    drm/i915/chv: Ack interrupts before handling them (CHV)

commit 3ff60f89bc4836583f5bd195062f16c563bd97aa
Author: Oscar Mateo <oscar.mateo at intel.com>
Date:   Mon Jun 16 16:10:58 2014 +0100

    drm/i915/vlv: Ack interrupts before handling them (VLV)

in attempting to reduce the amount of work done between receiving an
interrupt and acknowledging it. In light of this motivation, split the
pipestat interrupt handler into two, one to acknowledge and clear the
interrupt and a later pass to process it.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Oscar Mateo <oscar.mateo at intel.com>
Cc: Bob Beckett <robert.beckett at intel.com>
Cc: Imre Deak <imre.deak at intel.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 55 +++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 66064511cffb..92bdfe6f91d8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1457,24 +1457,21 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 	}
 }
 
-static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
+static inline bool
+intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
 {
-	if (!drm_handle_vblank(dev, pipe))
-		return false;
-
-	return true;
+	return drm_handle_vblank(dev, pipe);
 }
 
-static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
+static void valleyview_pipestat_irq_get(struct drm_i915_private *dev_priv,
+					u32 iir, u32 *pipe_stats)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 pipe_stats[I915_MAX_PIPES] = { };
 	int pipe;
 
 	spin_lock(&dev_priv->irq_lock);
 	for_each_pipe(dev_priv, pipe) {
+		u32 mask, val, iir_bit = 0;
 		int reg;
-		u32 mask, iir_bit = 0;
 
 		/*
 		 * PIPESTAT bits get signalled even when the interrupt is
@@ -1486,6 +1483,7 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 
 		/* fifo underruns are filterered in the underrun handler. */
 		mask = PIPE_FIFO_UNDERRUN_STATUS;
+		mask |= PIPESTAT_INT_ENABLE_MASK;
 
 		switch (pipe) {
 		case PIPE_A:
@@ -1501,21 +1499,25 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		if (iir & iir_bit)
 			mask |= dev_priv->pipestat_irq_mask[pipe];
 
-		if (!mask)
-			continue;
-
 		reg = PIPESTAT(pipe);
-		mask |= PIPESTAT_INT_ENABLE_MASK;
-		pipe_stats[pipe] = I915_READ(reg) & mask;
+		val = I915_READ(reg);
+		pipe_stats[pipe] |= val & mask;
 
 		/*
 		 * Clear the PIPE*STAT regs before the IIR
 		 */
-		if (pipe_stats[pipe] & (PIPE_FIFO_UNDERRUN_STATUS |
-					PIPESTAT_INT_STATUS_MASK))
-			I915_WRITE(reg, pipe_stats[pipe]);
+		if (val & (PIPE_FIFO_UNDERRUN_STATUS |
+			   PIPESTAT_INT_STATUS_MASK))
+			I915_WRITE(reg, val);
 	}
 	spin_unlock(&dev_priv->irq_lock);
+}
+
+static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
+					    u32 *pipe_stats)
+{
+	struct drm_device *dev = dev_priv->dev;
+	int pipe;
 
 	for_each_pipe(dev_priv, pipe) {
 		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
@@ -1578,6 +1580,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 pipe_stats[I915_MAX_PIPES] = {};
 	u32 iir, gt_iir, pm_iir;
 	irqreturn_t ret = IRQ_NONE;
 
@@ -1600,6 +1603,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			/* Consume port before clearing IIR or we'll miss events */
 			if (iir & I915_DISPLAY_PORT_INTERRUPT)
 				i9xx_hpd_irq_handler(dev);
+			valleyview_pipestat_irq_get(dev_priv, iir, pipe_stats);
 			I915_WRITE(VLV_IIR, iir);
 		}
 
@@ -1612,12 +1616,13 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 			snb_gt_irq_handler(dev, dev_priv, gt_iir);
 		if (pm_iir)
 			gen6_rps_irq_handler(dev_priv, pm_iir);
-		/* Call regardless, as some status bits might not be
-		 * signalled in iir */
-		valleyview_pipestat_irq_handler(dev, iir);
 	}
 
 out:
+
+	/* Call regardless, as some status bits might not be
+	 * signalled in iir */
+	valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
 	return ret;
 }
 
@@ -1625,6 +1630,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 pipe_stats[I915_MAX_PIPES] = {};
 	u32 master_ctl, iir;
 	irqreturn_t ret = IRQ_NONE;
 
@@ -1648,18 +1654,19 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 			/* Consume port before clearing IIR or we'll miss events */
 			if (iir & I915_DISPLAY_PORT_INTERRUPT)
 				i9xx_hpd_irq_handler(dev);
+			valleyview_pipestat_irq_get(dev_priv, iir, pipe_stats);
 			I915_WRITE(VLV_IIR, iir);
 		}
 
 		gen8_gt_irq_handler(dev_priv, master_ctl);
 
-		/* Call regardless, as some status bits might not be
-		 * signalled in iir */
-		valleyview_pipestat_irq_handler(dev, iir);
-
 		I915_WRITE_FW(GEN8_MASTER_IRQ, DE_MASTER_IRQ_CONTROL);
 	}
 
+	/* Call regardless, as some status bits might not be
+	 * signalled in iir */
+	valleyview_pipestat_irq_handler(dev_priv, pipe_stats);
+
 	return ret;
 }
 
-- 
2.5.0