[RFC] drm/i915: Fix races in gen4 page flip interrupt handling

Commit Message

Ville Syrjälä Feb. 18, 2013, 3:16 p.m. UTC

From: Ville Syrjälä <[e-mail]>

Use the gen3 logic for handling page flip interrupts on gen4.

Unfortuantely this kills the stall_check since that looks like it can
easily trigger too early. With the current logic the stall check would
kick in on the first vblank after the flip has been submitted to the
ring. If the CS takes longer than that to process the commands in the
ring, the stall check will cause the page flip to be complete too
early. That doesn't sound like a very good idea. Something better
should be deviced if we still need the stall check.

Signed-off-by: Ville Syrjälä <[e-mail]>
---
 drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

Comments

Chris Wilson Feb. 19, 2013, 10:38 a.m. UTC | #1

Also,

References: https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/1116587
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2aea737..1347940 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2684,6 +2684,13 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 	unsigned long irqflags;
 	int irq_received;
 	int ret = IRQ_NONE, pipe;
+	u32 flip[2] = {
+		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT,
+		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT
+	};
+	u32 flip_mask =
+		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
 
 	atomic_inc(&dev_priv->irq_received);
 
@@ -2692,7 +2699,7 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 	for (;;) {
 		bool blc_event = false;
 
-		irq_received = iir != 0;
+		irq_received = (iir & ~flip_mask) != 0;
 
 		/* Can't rely on pipestat interrupt bit in iir as it might
 		 * have been cleared after the pipestat interrupt was received.
@@ -2739,7 +2746,7 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
 
-		I915_WRITE(IIR, iir);
+		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
 
 		if (iir & I915_USER_INTERRUPT)
@@ -2747,17 +2754,17 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 		if (iir & I915_BSD_USER_INTERRUPT)
 			notify_ring(dev, &dev_priv->ring[VCS]);
 
-		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT)
-			intel_prepare_page_flip(dev, 0);
-
-		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT)
-			intel_prepare_page_flip(dev, 1);
-
 		for_each_pipe(pipe) {
 			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
 			    drm_handle_vblank(dev, pipe)) {
-				i915_pageflip_stall_check(dev, pipe);
-				intel_finish_page_flip(dev, pipe);
+				if (iir & flip[pipe]) {
+					intel_prepare_page_flip(dev, pipe);
+
+					if ((I915_READ(ISR) & flip[pipe]) == 0) {
+						intel_finish_page_flip(dev, pipe);
+						flip_mask &= ~flip[pipe];
+					}
+				}
 			}
 
 			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)