[Intel-gfx] [PATCH] drm/i915: Some polish for the new pipestat_irq_handler

[Daniel Vetter]

Just a bit of polish which I hope will help me with massaging some
internal patches to use Imre's reworked pipestat handling:
- Don't check for underrun reporting or enable pipestat interrupts
  twice.
- Frob the comments a bit.
- Do the iir PIPE_EVENT to pipe mapping explicitly with a switch. We
  only have one place which does this, so better to make it explicit.

v2: Ville noticed that I've broken the logic a bit with trying to
avoid checking whether we're interested in a given pipe twice. push
the PIPESTAT read down after we've computed the mask of interesting
bits first to avoid that duplication properly.

Cc: Imre Deak <imre.deak at intel.com>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_reg.h |  4 ----
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 386a640b7c92..a45ed67407bd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1559,25 +1559,39 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 	spin_lock(&dev_priv->irq_lock);
 	for_each_pipe(pipe) {
 		int reg;
-		u32 mask;
-
-		if (!dev_priv->pipestat_irq_mask[pipe] &&
-		    !__cpu_fifo_underrun_reporting_enabled(dev, pipe))
-			continue;
-
-		reg = PIPESTAT(pipe);
-		pipe_stats[pipe] = I915_READ(reg);
+		u32 mask, iir_bit;
 
 		/*
-		 * Clear the PIPE*STAT regs before the IIR
+		 * PIPESTAT bits get signalled even when the interrupt is
+		 * disabled with the mask bits, and some of the status bits do
+		 * not generate interrupts at all (like the underrun bit). Hence
+		 * we need to be careful that we only handle what we want to
+		 * handle.
 		 */
 		mask = PIPESTAT_INT_ENABLE_MASK;
 		if (__cpu_fifo_underrun_reporting_enabled(dev, pipe))
 			mask |= PIPE_FIFO_UNDERRUN_STATUS;
-		if (iir & I915_DISPLAY_PIPE_EVENT_INTERRUPT(pipe))
+
+		switch (pipe) {
+		case PIPE_A:
+			iir_bit = I915_DISPLAY_PIPE_A_EVENT_INTERRUPT;
+			break;
+		case PIPE_B:
+			iir_bit = I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
+			break;
+		}
+		if (iir & iir_bit)
 			mask |= dev_priv->pipestat_irq_mask[pipe];
-		pipe_stats[pipe] &= mask;
 
+		if (!mask)
+			continue;
+
+		reg = PIPESTAT(pipe);
+		pipe_stats[pipe] = I915_READ(reg) & 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]);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 645221270c34..8344541bbb93 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -997,10 +997,6 @@
 #define I915_DISPLAY_PIPE_A_EVENT_INTERRUPT		(1<<6)
 #define I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT		(1<<5)
 #define I915_DISPLAY_PIPE_B_EVENT_INTERRUPT		(1<<4)
-#define I915_DISPLAY_PIPE_EVENT_INTERRUPT(pipe)				\
-	((pipe) == PIPE_A ? I915_DISPLAY_PIPE_A_EVENT_INTERRUPT :	\
-	 I915_DISPLAY_PIPE_B_EVENT_INTERRUPT)
-
 #define I915_DEBUG_INTERRUPT				(1<<2)
 #define I915_USER_INTERRUPT				(1<<1)
 #define I915_ASLE_INTERRUPT				(1<<0)
-- 
1.8.1.4