[PATCH 1/6] drm/i915: Unconditionally initialise the interrupt workers

[PATCH 1/6] drm/i915: Unconditionally initialise the interrupt workers

[Chris Wilson]

Rather than duplicate similar code across the IRQ installers, perform
the initialisation of the workers upfront. This will lead to simpler
teardown and quiescent code as we can assume that the workers have
been initialised.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 26172ee..5745b2f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1972,10 +1972,6 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 
 	atomic_set(&dev_priv->irq_received, 0);
 
-	INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
-	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
-	if (IS_GEN6(dev) || IS_IVYBRIDGE(dev))
-		INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
 
 	I915_WRITE(HWSTAM, 0xeffe);
 
@@ -2003,9 +1999,6 @@ static void valleyview_irq_preinstall(struct drm_device *dev)
 
 	atomic_set(&dev_priv->irq_received, 0);
 
-	INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
-	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
-
 	/* VLV magic */
 	I915_WRITE(VLV_IMR, 0);
 	I915_WRITE(RING_IMR(RENDER_RING_BASE), 0);
@@ -2274,9 +2267,6 @@ static void i915_driver_irq_preinstall(struct drm_device * dev)
 
 	atomic_set(&dev_priv->irq_received, 0);
 
-	INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
-	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
-
 	if (I915_HAS_HOTPLUG(dev)) {
 		I915_WRITE(PORT_HOTPLUG_EN, 0);
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2593,6 +2583,12 @@ static void i8xx_irq_uninstall(struct drm_device * dev)
 
 void intel_irq_init(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	INIT_WORK(&dev_priv->hotplug_work, i915_hotplug_work_func);
+	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
+	INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
+
 	dev->driver->get_vblank_counter = i915_get_vblank_counter;
 	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
 	if (IS_G4X(dev) || IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev) ||
-- 
1.7.10

[PATCH 2/6] drm/i915: Remove redundant initialisation of per-ring IRQ waitqueues

[Chris Wilson]

The waitqueues are already initialised during ring initialisation so
kill the redundant and duplicated code to do so in each generations IRQ
installer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |   17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5745b2f..78137df 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2053,12 +2053,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	u32 render_irqs;
 	u32 hotplug_mask;
 
-	DRM_INIT_WAITQUEUE(&dev_priv->ring[RCS].irq_queue);
-	if (HAS_BSD(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[VCS].irq_queue);
-	if (HAS_BLT(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[BCS].irq_queue);
-
 	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
 	dev_priv->irq_mask = ~display_mask;
 
@@ -2128,12 +2122,6 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 	u32 render_irqs;
 	u32 hotplug_mask;
 
-	DRM_INIT_WAITQUEUE(&dev_priv->ring[RCS].irq_queue);
-	if (HAS_BSD(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[VCS].irq_queue);
-	if (HAS_BLT(dev))
-		DRM_INIT_WAITQUEUE(&dev_priv->ring[BCS].irq_queue);
-
 	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
 	dev_priv->irq_mask = ~display_mask;
 
@@ -2184,11 +2172,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~enable_mask;
 
-
-	DRM_INIT_WAITQUEUE(&dev_priv->ring[RCS].irq_queue);
-	DRM_INIT_WAITQUEUE(&dev_priv->ring[VCS].irq_queue);
-	DRM_INIT_WAITQUEUE(&dev_priv->ring[BCS].irq_queue);
-
 	dev_priv->pipestat[0] = 0;
 	dev_priv->pipestat[1] = 0;
 
-- 
1.7.10

[PATCH 3/6] drm/i915: pending_flip_is_done is gen3, name it so

[Chris Wilson]

And remove the cargo-culted copy from the valleyview irq handler.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |    4 ----
 drivers/gpu/drm/i915/i915_drv.h |    2 +-
 drivers/gpu/drm/i915/i915_irq.c |   18 +++++++++---------
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a813f65..1161bf8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1300,10 +1300,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_vga_client;
 
-	/* IIR "flip pending" bit means done if this bit is set */
-	if (IS_GEN3(dev) && (I915_READ(ECOSKPD) & ECO_FLIP_DONE))
-		dev_priv->flip_pending_is_done = true;
-
 	intel_modeset_init(dev);
 
 	ret = i915_load_gem_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c144013..e458b6c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -763,7 +763,7 @@ typedef struct drm_i915_private {
 	struct drm_crtc *plane_to_crtc_mapping[3];
 	struct drm_crtc *pipe_to_crtc_mapping[3];
 	wait_queue_head_t pending_flip_queue;
-	bool flip_pending_is_done;
+	bool gen3_flip_pending_is_done;
 
 	struct intel_pch_pll pch_plls[I915_NUM_PLLS];
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 78137df..d5bc3d7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -541,17 +541,13 @@ static irqreturn_t valleyview_irq_handler(DRM_IRQ_ARGS)
 		if (iir & I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT) {
 			drm_handle_vblank(dev, 0);
 			vblank++;
-			if (!dev_priv->flip_pending_is_done) {
-				intel_finish_page_flip(dev, 0);
-			}
+			intel_finish_page_flip(dev, 0);
 		}
 
 		if (iir & I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT) {
 			drm_handle_vblank(dev, 1);
 			vblank++;
-			if (!dev_priv->flip_pending_is_done) {
-				intel_finish_page_flip(dev, 0);
-			}
+			intel_finish_page_flip(dev, 0);
 		}
 
 		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
@@ -1474,13 +1470,13 @@ static irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
 
 		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT) {
 			intel_prepare_page_flip(dev, 0);
-			if (dev_priv->flip_pending_is_done)
+			if (dev_priv->gen3_flip_pending_is_done)
 				intel_finish_page_flip_plane(dev, 0);
 		}
 
 		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
 			intel_prepare_page_flip(dev, 1);
-			if (dev_priv->flip_pending_is_done)
+			if (dev_priv->gen3_flip_pending_is_done)
 				intel_finish_page_flip_plane(dev, 1);
 		}
 
@@ -1488,7 +1484,7 @@ static irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
 			if (pipe_stats[pipe] & vblank_status &&
 			    drm_handle_vblank(dev, pipe)) {
 				vblank++;
-				if (!dev_priv->flip_pending_is_done) {
+				if (!dev_priv->gen3_flip_pending_is_done) {
 					i915_pageflip_stall_check(dev, pipe);
 					intel_finish_page_flip(dev, pipe);
 				}
@@ -2572,6 +2568,10 @@ void intel_irq_init(struct drm_device *dev)
 	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
 	INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
 
+	/* IIR "flip pending" bit means done if this bit is set */
+	if (IS_GEN3(dev) && (I915_READ(ECOSKPD) & ECO_FLIP_DONE))
+		dev_priv->gen3_flip_pending_is_done = true;
+
 	dev->driver->get_vblank_counter = i915_get_vblank_counter;
 	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
 	if (IS_G4X(dev) || IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev) ||
-- 
1.7.10

[PATCH 4/6] drm/i915: Duplicate and split the gen3/4 irq handler

[Chris Wilson]

In preparation for rewriting the gen3 irq handler.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |  811 ++++++++++++++++++++++++++-------------
 1 file changed, 537 insertions(+), 274 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d5bc3d7..b378555 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1381,144 +1381,6 @@ static void i915_pageflip_stall_check(struct drm_device *dev, int pipe)
 	}
 }
 
-static irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
-{
-	struct drm_device *dev = (struct drm_device *) arg;
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	struct drm_i915_master_private *master_priv;
-	u32 iir, new_iir;
-	u32 pipe_stats[I915_MAX_PIPES];
-	u32 vblank_status;
-	int vblank = 0;
-	unsigned long irqflags;
-	int irq_received;
-	int ret = IRQ_NONE, pipe;
-	bool blc_event = false;
-
-	atomic_inc(&dev_priv->irq_received);
-
-	iir = I915_READ(IIR);
-
-	if (INTEL_INFO(dev)->gen >= 4)
-		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
-	else
-		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
-
-	for (;;) {
-		irq_received = iir != 0;
-
-		/* Can't rely on pipestat interrupt bit in iir as it might
-		 * have been cleared after the pipestat interrupt was received.
-		 * It doesn't set the bit in iir again, but it still produces
-		 * interrupts (for non-MSI).
-		 */
-		spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			i915_handle_error(dev, false);
-
-		for_each_pipe(pipe) {
-			int reg = PIPESTAT(pipe);
-			pipe_stats[pipe] = I915_READ(reg);
-
-			/*
-			 * Clear the PIPE*STAT regs before the IIR
-			 */
-			if (pipe_stats[pipe] & 0x8000ffff) {
-				if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
-					DRM_DEBUG_DRIVER("pipe %c underrun\n",
-							 pipe_name(pipe));
-				I915_WRITE(reg, pipe_stats[pipe]);
-				irq_received = 1;
-			}
-		}
-		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
-
-		if (!irq_received)
-			break;
-
-		ret = IRQ_HANDLED;
-
-		/* Consume port.  Then clear IIR or we'll miss events */
-		if ((I915_HAS_HOTPLUG(dev)) &&
-		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
-			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-
-			DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
-				  hotplug_status);
-			if (hotplug_status & dev_priv->hotplug_supported_mask)
-				queue_work(dev_priv->wq,
-					   &dev_priv->hotplug_work);
-
-			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			I915_READ(PORT_HOTPLUG_STAT);
-		}
-
-		I915_WRITE(IIR, iir);
-		new_iir = I915_READ(IIR); /* Flush posted writes */
-
-		if (dev->primary->master) {
-			master_priv = dev->primary->master->driver_priv;
-			if (master_priv->sarea_priv)
-				master_priv->sarea_priv->last_dispatch =
-					READ_BREADCRUMB(dev_priv);
-		}
-
-		if (iir & I915_USER_INTERRUPT)
-			notify_ring(dev, &dev_priv->ring[RCS]);
-		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 (dev_priv->gen3_flip_pending_is_done)
-				intel_finish_page_flip_plane(dev, 0);
-		}
-
-		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
-			intel_prepare_page_flip(dev, 1);
-			if (dev_priv->gen3_flip_pending_is_done)
-				intel_finish_page_flip_plane(dev, 1);
-		}
-
-		for_each_pipe(pipe) {
-			if (pipe_stats[pipe] & vblank_status &&
-			    drm_handle_vblank(dev, pipe)) {
-				vblank++;
-				if (!dev_priv->gen3_flip_pending_is_done) {
-					i915_pageflip_stall_check(dev, pipe);
-					intel_finish_page_flip(dev, pipe);
-				}
-			}
-
-			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
-				blc_event = true;
-		}
-
-
-		if (blc_event || (iir & I915_ASLE_INTERRUPT))
-			intel_opregion_asle_intr(dev);
-
-		/* With MSI, interrupts are only generated when iir
-		 * transitions from zero to nonzero.  If another bit got
-		 * set while we were handling the existing iir bits, then
-		 * we would never get another interrupt.
-		 *
-		 * This is fine on non-MSI as well, as if we hit this path
-		 * we avoid exiting the interrupt handler only to generate
-		 * another one.
-		 *
-		 * Note that for MSI this could cause a stray interrupt report
-		 * if an interrupt landed in the time between writing IIR and
-		 * the posting read.  This should be rare enough to never
-		 * trigger the 99% of 100,000 interrupts test for disabling
-		 * stray interrupts.
-		 */
-		iir = new_iir;
-	}
-
-	return ret;
-}
-
 static int i915_emit_irq(struct drm_device * dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -2239,106 +2101,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static void i915_driver_irq_preinstall(struct drm_device * dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	int pipe;
-
-	atomic_set(&dev_priv->irq_received, 0);
-
-	if (I915_HAS_HOTPLUG(dev)) {
-		I915_WRITE(PORT_HOTPLUG_EN, 0);
-		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
-	}
-
-	I915_WRITE(HWSTAM, 0xeffe);
-	for_each_pipe(pipe)
-		I915_WRITE(PIPESTAT(pipe), 0);
-	I915_WRITE(IMR, 0xffffffff);
-	I915_WRITE(IER, 0x0);
-	POSTING_READ(IER);
-}
-
-/*
- * Must be called after intel_modeset_init or hotplug interrupts won't be
- * enabled correctly.
- */
-static int i915_driver_irq_postinstall(struct drm_device *dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	u32 enable_mask = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
-	u32 error_mask;
-
-	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
-
-	/* Unmask the interrupts that we always want on. */
-	dev_priv->irq_mask = ~I915_INTERRUPT_ENABLE_FIX;
-
-	dev_priv->pipestat[0] = 0;
-	dev_priv->pipestat[1] = 0;
-
-	if (I915_HAS_HOTPLUG(dev)) {
-		/* Enable in IER... */
-		enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
-		/* and unmask in IMR */
-		dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
-	}
-
-	/*
-	 * Enable some error detection, note the instruction error mask
-	 * bit is reserved, so we leave it masked.
-	 */
-	if (IS_G4X(dev)) {
-		error_mask = ~(GM45_ERROR_PAGE_TABLE |
-			       GM45_ERROR_MEM_PRIV |
-			       GM45_ERROR_CP_PRIV |
-			       I915_ERROR_MEMORY_REFRESH);
-	} else {
-		error_mask = ~(I915_ERROR_PAGE_TABLE |
-			       I915_ERROR_MEMORY_REFRESH);
-	}
-	I915_WRITE(EMR, error_mask);
-
-	I915_WRITE(IMR, dev_priv->irq_mask);
-	I915_WRITE(IER, enable_mask);
-	POSTING_READ(IER);
-
-	if (I915_HAS_HOTPLUG(dev)) {
-		u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
-
-		/* Note HDMI and DP share bits */
-		if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
-			hotplug_en |= HDMIB_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & HDMIC_HOTPLUG_INT_STATUS)
-			hotplug_en |= HDMIC_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & HDMID_HOTPLUG_INT_STATUS)
-			hotplug_en |= HDMID_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS)
-			hotplug_en |= SDVOC_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS)
-			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
-			hotplug_en |= CRT_HOTPLUG_INT_EN;
-
-			/* Programming the CRT detection parameters tends
-			   to generate a spurious hotplug event about three
-			   seconds later.  So just do it once.
-			*/
-			if (IS_G4X(dev))
-				hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
-			hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
-		}
-
-		/* Ignore TV since it's buggy */
-
-		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
-	}
-
-	intel_opregion_enable_asle(dev);
-
-	return 0;
-}
-
 static void valleyview_irq_uninstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -2387,45 +2149,18 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
 }
 
-static void i915_driver_irq_uninstall(struct drm_device * dev)
+static void i8xx_irq_preinstall(struct drm_device * dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int pipe;
 
-	if (!dev_priv)
-		return;
-
-	dev_priv->vblank_pipe = 0;
-
-	if (I915_HAS_HOTPLUG(dev)) {
-		I915_WRITE(PORT_HOTPLUG_EN, 0);
-		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
-	}
+	atomic_set(&dev_priv->irq_received, 0);
 
-	I915_WRITE(HWSTAM, 0xffffffff);
 	for_each_pipe(pipe)
 		I915_WRITE(PIPESTAT(pipe), 0);
-	I915_WRITE(IMR, 0xffffffff);
-	I915_WRITE(IER, 0x0);
-
-	for_each_pipe(pipe)
-		I915_WRITE(PIPESTAT(pipe),
-			   I915_READ(PIPESTAT(pipe)) & 0x8000ffff);
-	I915_WRITE(IIR, I915_READ(IIR));
-}
-
-static void i8xx_irq_preinstall(struct drm_device * dev)
-{
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	int pipe;
-
-	atomic_set(&dev_priv->irq_received, 0);
-
-	for_each_pipe(pipe)
-		I915_WRITE(PIPESTAT(pipe), 0);
-	I915_WRITE16(IMR, 0xffff);
-	I915_WRITE16(IER, 0x0);
-	POSTING_READ16(IER);
+	I915_WRITE16(IMR, 0xffff);
+	I915_WRITE16(IER, 0x0);
+	POSTING_READ16(IER);
 }
 
 static int i8xx_irq_postinstall(struct drm_device *dev)
@@ -2560,6 +2295,529 @@ static void i8xx_irq_uninstall(struct drm_device * dev)
 	I915_WRITE16(IIR, I915_READ16(IIR));
 }
 
+static void i915_irq_preinstall(struct drm_device * dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	int pipe;
+
+	atomic_set(&dev_priv->irq_received, 0);
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		I915_WRITE(PORT_HOTPLUG_EN, 0);
+		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
+	}
+
+	I915_WRITE(HWSTAM, 0xeffe);
+	for_each_pipe(pipe)
+		I915_WRITE(PIPESTAT(pipe), 0);
+	I915_WRITE(IMR, 0xffffffff);
+	I915_WRITE(IER, 0x0);
+	POSTING_READ(IER);
+}
+
+static int i915_irq_postinstall(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	u32 enable_mask = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
+	u32 error_mask;
+
+	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
+
+	/* Unmask the interrupts that we always want on. */
+	dev_priv->irq_mask = ~I915_INTERRUPT_ENABLE_FIX;
+
+	dev_priv->pipestat[0] = 0;
+	dev_priv->pipestat[1] = 0;
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		/* Enable in IER... */
+		enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
+		/* and unmask in IMR */
+		dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
+	}
+
+	/*
+	 * Enable some error detection, note the instruction error mask
+	 * bit is reserved, so we leave it masked.
+	 */
+	if (IS_G4X(dev)) {
+		error_mask = ~(GM45_ERROR_PAGE_TABLE |
+			       GM45_ERROR_MEM_PRIV |
+			       GM45_ERROR_CP_PRIV |
+			       I915_ERROR_MEMORY_REFRESH);
+	} else {
+		error_mask = ~(I915_ERROR_PAGE_TABLE |
+			       I915_ERROR_MEMORY_REFRESH);
+	}
+	I915_WRITE(EMR, error_mask);
+
+	I915_WRITE(IMR, dev_priv->irq_mask);
+	I915_WRITE(IER, enable_mask);
+	POSTING_READ(IER);
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+
+		/* Note HDMI and DP share bits */
+		if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMIB_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & HDMIC_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMIC_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & HDMID_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMID_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS)
+			hotplug_en |= SDVOC_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS)
+			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
+			hotplug_en |= CRT_HOTPLUG_INT_EN;
+
+			/* Programming the CRT detection parameters tends
+			   to generate a spurious hotplug event about three
+			   seconds later.  So just do it once.
+			*/
+			if (IS_G4X(dev))
+				hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
+			hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
+		}
+
+		/* Ignore TV since it's buggy */
+
+		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+	}
+
+	intel_opregion_enable_asle(dev);
+
+	return 0;
+}
+
+static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
+{
+	struct drm_device *dev = (struct drm_device *) arg;
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	struct drm_i915_master_private *master_priv;
+	u32 iir, new_iir;
+	u32 pipe_stats[I915_MAX_PIPES];
+	u32 vblank_status;
+	int vblank = 0;
+	unsigned long irqflags;
+	int irq_received;
+	int ret = IRQ_NONE, pipe;
+	bool blc_event = false;
+
+	atomic_inc(&dev_priv->irq_received);
+
+	iir = I915_READ(IIR);
+
+	if (INTEL_INFO(dev)->gen >= 4)
+		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
+	else
+		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
+
+	for (;;) {
+		irq_received = iir != 0;
+
+		/* Can't rely on pipestat interrupt bit in iir as it might
+		 * have been cleared after the pipestat interrupt was received.
+		 * It doesn't set the bit in iir again, but it still produces
+		 * interrupts (for non-MSI).
+		 */
+		spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
+			i915_handle_error(dev, false);
+
+		for_each_pipe(pipe) {
+			int reg = PIPESTAT(pipe);
+			pipe_stats[pipe] = I915_READ(reg);
+
+			/*
+			 * Clear the PIPE*STAT regs before the IIR
+			 */
+			if (pipe_stats[pipe] & 0x8000ffff) {
+				if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
+					DRM_DEBUG_DRIVER("pipe %c underrun\n",
+							 pipe_name(pipe));
+				I915_WRITE(reg, pipe_stats[pipe]);
+				irq_received = 1;
+			}
+		}
+		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+		if (!irq_received)
+			break;
+
+		ret = IRQ_HANDLED;
+
+		/* Consume port.  Then clear IIR or we'll miss events */
+		if ((I915_HAS_HOTPLUG(dev)) &&
+		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
+			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+
+			DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
+				  hotplug_status);
+			if (hotplug_status & dev_priv->hotplug_supported_mask)
+				queue_work(dev_priv->wq,
+					   &dev_priv->hotplug_work);
+
+			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+			I915_READ(PORT_HOTPLUG_STAT);
+		}
+
+		I915_WRITE(IIR, iir);
+		new_iir = I915_READ(IIR); /* Flush posted writes */
+
+		if (dev->primary->master) {
+			master_priv = dev->primary->master->driver_priv;
+			if (master_priv->sarea_priv)
+				master_priv->sarea_priv->last_dispatch =
+					READ_BREADCRUMB(dev_priv);
+		}
+
+		if (iir & I915_USER_INTERRUPT)
+			notify_ring(dev, &dev_priv->ring[RCS]);
+		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 (dev_priv->gen3_flip_pending_is_done)
+				intel_finish_page_flip_plane(dev, 0);
+		}
+
+		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
+			intel_prepare_page_flip(dev, 1);
+			if (dev_priv->gen3_flip_pending_is_done)
+				intel_finish_page_flip_plane(dev, 1);
+		}
+
+		for_each_pipe(pipe) {
+			if (pipe_stats[pipe] & vblank_status &&
+			    drm_handle_vblank(dev, pipe)) {
+				vblank++;
+				if (!dev_priv->gen3_flip_pending_is_done) {
+					i915_pageflip_stall_check(dev, pipe);
+					intel_finish_page_flip(dev, pipe);
+				}
+			}
+
+			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
+				blc_event = true;
+		}
+
+
+		if (blc_event || (iir & I915_ASLE_INTERRUPT))
+			intel_opregion_asle_intr(dev);
+
+		/* With MSI, interrupts are only generated when iir
+		 * transitions from zero to nonzero.  If another bit got
+		 * set while we were handling the existing iir bits, then
+		 * we would never get another interrupt.
+		 *
+		 * This is fine on non-MSI as well, as if we hit this path
+		 * we avoid exiting the interrupt handler only to generate
+		 * another one.
+		 *
+		 * Note that for MSI this could cause a stray interrupt report
+		 * if an interrupt landed in the time between writing IIR and
+		 * the posting read.  This should be rare enough to never
+		 * trigger the 99% of 100,000 interrupts test for disabling
+		 * stray interrupts.
+		 */
+		iir = new_iir;
+	}
+
+	return ret;
+}
+
+
+static void i915_irq_uninstall(struct drm_device * dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	int pipe;
+
+	if (!dev_priv)
+		return;
+
+	dev_priv->vblank_pipe = 0;
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		I915_WRITE(PORT_HOTPLUG_EN, 0);
+		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
+	}
+
+	I915_WRITE(HWSTAM, 0xffffffff);
+	for_each_pipe(pipe)
+		I915_WRITE(PIPESTAT(pipe), 0);
+	I915_WRITE(IMR, 0xffffffff);
+	I915_WRITE(IER, 0x0);
+
+	for_each_pipe(pipe)
+		I915_WRITE(PIPESTAT(pipe),
+			   I915_READ(PIPESTAT(pipe)) & 0x8000ffff);
+	I915_WRITE(IIR, I915_READ(IIR));
+}
+
+static void i965_irq_preinstall(struct drm_device * dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	int pipe;
+
+	atomic_set(&dev_priv->irq_received, 0);
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		I915_WRITE(PORT_HOTPLUG_EN, 0);
+		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
+	}
+
+	I915_WRITE(HWSTAM, 0xeffe);
+	for_each_pipe(pipe)
+		I915_WRITE(PIPESTAT(pipe), 0);
+	I915_WRITE(IMR, 0xffffffff);
+	I915_WRITE(IER, 0x0);
+	POSTING_READ(IER);
+}
+
+static int i965_irq_postinstall(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	u32 enable_mask = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
+	u32 error_mask;
+
+	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
+
+	/* Unmask the interrupts that we always want on. */
+	dev_priv->irq_mask = ~I915_INTERRUPT_ENABLE_FIX;
+
+	dev_priv->pipestat[0] = 0;
+	dev_priv->pipestat[1] = 0;
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		/* Enable in IER... */
+		enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
+		/* and unmask in IMR */
+		dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
+	}
+
+	/*
+	 * Enable some error detection, note the instruction error mask
+	 * bit is reserved, so we leave it masked.
+	 */
+	if (IS_G4X(dev)) {
+		error_mask = ~(GM45_ERROR_PAGE_TABLE |
+			       GM45_ERROR_MEM_PRIV |
+			       GM45_ERROR_CP_PRIV |
+			       I915_ERROR_MEMORY_REFRESH);
+	} else {
+		error_mask = ~(I915_ERROR_PAGE_TABLE |
+			       I915_ERROR_MEMORY_REFRESH);
+	}
+	I915_WRITE(EMR, error_mask);
+
+	I915_WRITE(IMR, dev_priv->irq_mask);
+	I915_WRITE(IER, enable_mask);
+	POSTING_READ(IER);
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+
+		/* Note HDMI and DP share bits */
+		if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMIB_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & HDMIC_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMIC_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & HDMID_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMID_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS)
+			hotplug_en |= SDVOC_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS)
+			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
+			hotplug_en |= CRT_HOTPLUG_INT_EN;
+
+			/* Programming the CRT detection parameters tends
+			   to generate a spurious hotplug event about three
+			   seconds later.  So just do it once.
+			*/
+			if (IS_G4X(dev))
+				hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
+			hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
+		}
+
+		/* Ignore TV since it's buggy */
+
+		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+	}
+
+	intel_opregion_enable_asle(dev);
+
+	return 0;
+}
+
+static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
+{
+	struct drm_device *dev = (struct drm_device *) arg;
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	struct drm_i915_master_private *master_priv;
+	u32 iir, new_iir;
+	u32 pipe_stats[I915_MAX_PIPES];
+	u32 vblank_status;
+	int vblank = 0;
+	unsigned long irqflags;
+	int irq_received;
+	int ret = IRQ_NONE, pipe;
+	bool blc_event = false;
+
+	atomic_inc(&dev_priv->irq_received);
+
+	iir = I915_READ(IIR);
+
+	if (INTEL_INFO(dev)->gen >= 4)
+		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
+	else
+		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
+
+	for (;;) {
+		irq_received = iir != 0;
+
+		/* Can't rely on pipestat interrupt bit in iir as it might
+		 * have been cleared after the pipestat interrupt was received.
+		 * It doesn't set the bit in iir again, but it still produces
+		 * interrupts (for non-MSI).
+		 */
+		spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
+			i915_handle_error(dev, false);
+
+		for_each_pipe(pipe) {
+			int reg = PIPESTAT(pipe);
+			pipe_stats[pipe] = I915_READ(reg);
+
+			/*
+			 * Clear the PIPE*STAT regs before the IIR
+			 */
+			if (pipe_stats[pipe] & 0x8000ffff) {
+				if (pipe_stats[pipe] & PIPE_FIFO_UNDERRUN_STATUS)
+					DRM_DEBUG_DRIVER("pipe %c underrun\n",
+							 pipe_name(pipe));
+				I915_WRITE(reg, pipe_stats[pipe]);
+				irq_received = 1;
+			}
+		}
+		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+		if (!irq_received)
+			break;
+
+		ret = IRQ_HANDLED;
+
+		/* Consume port.  Then clear IIR or we'll miss events */
+		if ((I915_HAS_HOTPLUG(dev)) &&
+		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
+			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+
+			DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
+				  hotplug_status);
+			if (hotplug_status & dev_priv->hotplug_supported_mask)
+				queue_work(dev_priv->wq,
+					   &dev_priv->hotplug_work);
+
+			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+			I915_READ(PORT_HOTPLUG_STAT);
+		}
+
+		I915_WRITE(IIR, iir);
+		new_iir = I915_READ(IIR); /* Flush posted writes */
+
+		if (dev->primary->master) {
+			master_priv = dev->primary->master->driver_priv;
+			if (master_priv->sarea_priv)
+				master_priv->sarea_priv->last_dispatch =
+					READ_BREADCRUMB(dev_priv);
+		}
+
+		if (iir & I915_USER_INTERRUPT)
+			notify_ring(dev, &dev_priv->ring[RCS]);
+		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 (dev_priv->gen3_flip_pending_is_done)
+				intel_finish_page_flip_plane(dev, 0);
+		}
+
+		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
+			intel_prepare_page_flip(dev, 1);
+			if (dev_priv->gen3_flip_pending_is_done)
+				intel_finish_page_flip_plane(dev, 1);
+		}
+
+		for_each_pipe(pipe) {
+			if (pipe_stats[pipe] & vblank_status &&
+			    drm_handle_vblank(dev, pipe)) {
+				vblank++;
+				if (!dev_priv->gen3_flip_pending_is_done) {
+					i915_pageflip_stall_check(dev, pipe);
+					intel_finish_page_flip(dev, pipe);
+				}
+			}
+
+			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
+				blc_event = true;
+		}
+
+
+		if (blc_event || (iir & I915_ASLE_INTERRUPT))
+			intel_opregion_asle_intr(dev);
+
+		/* With MSI, interrupts are only generated when iir
+		 * transitions from zero to nonzero.  If another bit got
+		 * set while we were handling the existing iir bits, then
+		 * we would never get another interrupt.
+		 *
+		 * This is fine on non-MSI as well, as if we hit this path
+		 * we avoid exiting the interrupt handler only to generate
+		 * another one.
+		 *
+		 * Note that for MSI this could cause a stray interrupt report
+		 * if an interrupt landed in the time between writing IIR and
+		 * the posting read.  This should be rare enough to never
+		 * trigger the 99% of 100,000 interrupts test for disabling
+		 * stray interrupts.
+		 */
+		iir = new_iir;
+	}
+
+	return ret;
+}
+
+static void i965_irq_uninstall(struct drm_device * dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	int pipe;
+
+	if (!dev_priv)
+		return;
+
+	dev_priv->vblank_pipe = 0;
+
+	if (I915_HAS_HOTPLUG(dev)) {
+		I915_WRITE(PORT_HOTPLUG_EN, 0);
+		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
+	}
+
+	I915_WRITE(HWSTAM, 0xffffffff);
+	for_each_pipe(pipe)
+		I915_WRITE(PIPESTAT(pipe), 0);
+	I915_WRITE(IMR, 0xffffffff);
+	I915_WRITE(IER, 0x0);
+
+	for_each_pipe(pipe)
+		I915_WRITE(PIPESTAT(pipe),
+			   I915_READ(PIPESTAT(pipe)) & 0x8000ffff);
+	I915_WRITE(IIR, I915_READ(IIR));
+}
+
 void intel_irq_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2614,11 +2872,16 @@ void intel_irq_init(struct drm_device *dev)
 			dev->driver->irq_postinstall = i8xx_irq_postinstall;
 			dev->driver->irq_handler = i8xx_irq_handler;
 			dev->driver->irq_uninstall = i8xx_irq_uninstall;
+		} else if (INTEL_INFO(dev)->gen == 3) {
+			dev->driver->irq_preinstall = i915_irq_preinstall;
+			dev->driver->irq_postinstall = i915_irq_postinstall;
+			dev->driver->irq_uninstall = i915_irq_uninstall;
+			dev->driver->irq_handler = i915_irq_handler;
 		} else {
-			dev->driver->irq_preinstall = i915_driver_irq_preinstall;
-			dev->driver->irq_postinstall = i915_driver_irq_postinstall;
-			dev->driver->irq_uninstall = i915_driver_irq_uninstall;
-			dev->driver->irq_handler = i915_driver_irq_handler;
+			dev->driver->irq_preinstall = i965_irq_preinstall;
+			dev->driver->irq_postinstall = i965_irq_postinstall;
+			dev->driver->irq_uninstall = i965_irq_uninstall;
+			dev->driver->irq_handler = i965_irq_handler;
 		}
 		dev->driver->enable_vblank = i915_enable_vblank;
 		dev->driver->disable_vblank = i915_disable_vblank;
-- 
1.7.10

[PATCH 5/6] drm/i915: Handle PendingFlip on gen3 robustly

[Chris Wilson]

We appear to allow too many pending pageflips as evidenced by an
apparent pin-leak. So borrow the pageflip completion logic from i8xx for
handling PendingFlip in a robust manner.

References: https://bugzilla.kernel.org/show_bug.cgi?id=41882
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h |    1 -
 drivers/gpu/drm/i915/i915_irq.c |  164 ++++++++++++++-------------------------
 2 files changed, 60 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e458b6c..2e45ba5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -763,7 +763,6 @@ typedef struct drm_i915_private {
 	struct drm_crtc *plane_to_crtc_mapping[3];
 	struct drm_crtc *pipe_to_crtc_mapping[3];
 	wait_queue_head_t pending_flip_queue;
-	bool gen3_flip_pending_is_done;
 
 	struct intel_pch_pll pch_plls[I915_NUM_PLLS];
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b378555..47a540a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2307,7 +2307,7 @@ static void i915_irq_preinstall(struct drm_device * dev)
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 	}
 
-	I915_WRITE(HWSTAM, 0xeffe);
+	I915_WRITE16(HWSTAM, 0xeffe);
 	for_each_pipe(pipe)
 		I915_WRITE(PIPESTAT(pipe), 0);
 	I915_WRITE(IMR, 0xffffffff);
@@ -2318,39 +2318,35 @@ static void i915_irq_preinstall(struct drm_device * dev)
 static int i915_irq_postinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	u32 enable_mask = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
-	u32 error_mask;
+	u32 enable_mask;
 
 	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
 
-	/* Unmask the interrupts that we always want on. */
-	dev_priv->irq_mask = ~I915_INTERRUPT_ENABLE_FIX;
-
 	dev_priv->pipestat[0] = 0;
 	dev_priv->pipestat[1] = 0;
 
+	I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
+
+	/* Unmask the interrupts that we always want on. */
+	dev_priv->irq_mask =
+		~(I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
+		  I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
+		  I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+		  I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT |
+		  I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
+
+	enable_mask =
+		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
+		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
+		I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT |
+		I915_USER_INTERRUPT;
+
 	if (I915_HAS_HOTPLUG(dev)) {
 		/* Enable in IER... */
 		enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
 		/* and unmask in IMR */
 		dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
 	}
-
-	/*
-	 * Enable some error detection, note the instruction error mask
-	 * bit is reserved, so we leave it masked.
-	 */
-	if (IS_G4X(dev)) {
-		error_mask = ~(GM45_ERROR_PAGE_TABLE |
-			       GM45_ERROR_MEM_PRIV |
-			       GM45_ERROR_CP_PRIV |
-			       I915_ERROR_MEMORY_REFRESH);
-	} else {
-		error_mask = ~(I915_ERROR_PAGE_TABLE |
-			       I915_ERROR_MEMORY_REFRESH);
-	}
-	I915_WRITE(EMR, error_mask);
-
 	I915_WRITE(IMR, dev_priv->irq_mask);
 	I915_WRITE(IER, enable_mask);
 	POSTING_READ(IER);
@@ -2371,13 +2367,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
 			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
 		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
 			hotplug_en |= CRT_HOTPLUG_INT_EN;
-
-			/* Programming the CRT detection parameters tends
-			   to generate a spurious hotplug event about three
-			   seconds later.  So just do it once.
-			*/
-			if (IS_G4X(dev))
-				hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
 			hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
 		}
 
@@ -2396,26 +2385,25 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
 	struct drm_device *dev = (struct drm_device *) arg;
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	struct drm_i915_master_private *master_priv;
-	u32 iir, new_iir;
-	u32 pipe_stats[I915_MAX_PIPES];
-	u32 vblank_status;
-	int vblank = 0;
+	u32 iir, new_iir, pipe_stats[I915_MAX_PIPES];
 	unsigned long irqflags;
-	int irq_received;
-	int ret = IRQ_NONE, pipe;
-	bool blc_event = false;
+	u32 flip_mask =
+		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
+		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
+	u32 flip[2] = {
+		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT,
+		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT
+	};
+	int  pipe;
 
 	atomic_inc(&dev_priv->irq_received);
 
 	iir = I915_READ(IIR);
+	if ((iir & ~flip_mask) == 0)
+		return IRQ_NONE;
 
-	if (INTEL_INFO(dev)->gen >= 4)
-		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
-	else
-		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
-
-	for (;;) {
-		irq_received = iir != 0;
+	while (iir & ~flip_mask) {
+		bool blc_event = false;
 
 		/* Can't rely on pipestat interrupt bit in iir as it might
 		 * have been cleared after the pipestat interrupt was received.
@@ -2438,16 +2426,10 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
 					DRM_DEBUG_DRIVER("pipe %c underrun\n",
 							 pipe_name(pipe));
 				I915_WRITE(reg, pipe_stats[pipe]);
-				irq_received = 1;
 			}
 		}
 		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
-		if (!irq_received)
-			break;
-
-		ret = IRQ_HANDLED;
-
 		/* Consume port.  Then clear IIR or we'll miss events */
 		if ((I915_HAS_HOTPLUG(dev)) &&
 		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
@@ -2460,43 +2442,25 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
 					   &dev_priv->hotplug_work);
 
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
-			I915_READ(PORT_HOTPLUG_STAT);
+			POSTING_READ(PORT_HOTPLUG_STAT);
 		}
 
-		I915_WRITE(IIR, iir);
+		I915_WRITE(IIR, iir & ~flip_mask);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
 
-		if (dev->primary->master) {
-			master_priv = dev->primary->master->driver_priv;
-			if (master_priv->sarea_priv)
-				master_priv->sarea_priv->last_dispatch =
-					READ_BREADCRUMB(dev_priv);
-		}
-
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev, &dev_priv->ring[RCS]);
-		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 (dev_priv->gen3_flip_pending_is_done)
-				intel_finish_page_flip_plane(dev, 0);
-		}
-
-		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
-			intel_prepare_page_flip(dev, 1);
-			if (dev_priv->gen3_flip_pending_is_done)
-				intel_finish_page_flip_plane(dev, 1);
-		}
 
 		for_each_pipe(pipe) {
-			if (pipe_stats[pipe] & vblank_status &&
+			int plane = pipe;
+			if (IS_MOBILE(dev))
+				plane = !plane;
+			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
 			    drm_handle_vblank(dev, pipe)) {
-				vblank++;
-				if (!dev_priv->gen3_flip_pending_is_done) {
-					i915_pageflip_stall_check(dev, pipe);
+				if (iir & flip[plane]) {
+					intel_prepare_page_flip(dev, plane);
 					intel_finish_page_flip(dev, pipe);
+					flip_mask &= ~flip[plane];
 				}
 			}
 
@@ -2504,7 +2468,6 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
 				blc_event = true;
 		}
 
-
 		if (blc_event || (iir & I915_ASLE_INTERRUPT))
 			intel_opregion_asle_intr(dev);
 
@@ -2526,18 +2489,21 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
 		iir = new_iir;
 	}
 
-	return ret;
-}
+	if (dev->primary->master) {
+		master_priv = dev->primary->master->driver_priv;
+		if (master_priv->sarea_priv)
+			master_priv->sarea_priv->last_dispatch =
+				READ_BREADCRUMB(dev_priv);
+	}
 
+	return IRQ_HANDLED;
+}
 
 static void i915_irq_uninstall(struct drm_device * dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int pipe;
 
-	if (!dev_priv)
-		return;
-
 	dev_priv->vblank_pipe = 0;
 
 	if (I915_HAS_HOTPLUG(dev)) {
@@ -2545,15 +2511,14 @@ static void i915_irq_uninstall(struct drm_device * dev)
 		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
 	}
 
-	I915_WRITE(HWSTAM, 0xffffffff);
-	for_each_pipe(pipe)
+	I915_WRITE16(HWSTAM, 0xffff);
+	for_each_pipe(pipe) {
+		/* Clear enable bits; then clear status bits */
 		I915_WRITE(PIPESTAT(pipe), 0);
+		I915_WRITE(PIPESTAT(pipe), I915_READ(PIPESTAT(pipe)));
+	}
 	I915_WRITE(IMR, 0xffffffff);
 	I915_WRITE(IER, 0x0);
-
-	for_each_pipe(pipe)
-		I915_WRITE(PIPESTAT(pipe),
-			   I915_READ(PIPESTAT(pipe)) & 0x8000ffff);
 	I915_WRITE(IIR, I915_READ(IIR));
 }
 
@@ -2740,26 +2705,18 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
 		if (iir & I915_BSD_USER_INTERRUPT)
 			notify_ring(dev, &dev_priv->ring[VCS]);
 
-		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT) {
+		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT)
 			intel_prepare_page_flip(dev, 0);
-			if (dev_priv->gen3_flip_pending_is_done)
-				intel_finish_page_flip_plane(dev, 0);
-		}
 
-		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
+		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT)
 			intel_prepare_page_flip(dev, 1);
-			if (dev_priv->gen3_flip_pending_is_done)
-				intel_finish_page_flip_plane(dev, 1);
-		}
 
 		for_each_pipe(pipe) {
 			if (pipe_stats[pipe] & vblank_status &&
 			    drm_handle_vblank(dev, pipe)) {
 				vblank++;
-				if (!dev_priv->gen3_flip_pending_is_done) {
-					i915_pageflip_stall_check(dev, pipe);
-					intel_finish_page_flip(dev, pipe);
-				}
+				i915_pageflip_stall_check(dev, pipe);
+				intel_finish_page_flip(dev, pipe);
 			}
 
 			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
@@ -2826,10 +2783,6 @@ void intel_irq_init(struct drm_device *dev)
 	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
 	INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
 
-	/* IIR "flip pending" bit means done if this bit is set */
-	if (IS_GEN3(dev) && (I915_READ(ECOSKPD) & ECO_FLIP_DONE))
-		dev_priv->gen3_flip_pending_is_done = true;
-
 	dev->driver->get_vblank_counter = i915_get_vblank_counter;
 	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
 	if (IS_G4X(dev) || IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev) ||
@@ -2873,6 +2826,9 @@ void intel_irq_init(struct drm_device *dev)
 			dev->driver->irq_handler = i8xx_irq_handler;
 			dev->driver->irq_uninstall = i8xx_irq_uninstall;
 		} else if (INTEL_INFO(dev)->gen == 3) {
+			/* "flip pending" bit means done if this bit is set */
+			I915_WRITE(ECOSKPD, _MASKED_BIT_DISABLE(ECO_FLIP_DONE));
+
 			dev->driver->irq_preinstall = i915_irq_preinstall;
 			dev->driver->irq_postinstall = i915_irq_postinstall;
 			dev->driver->irq_uninstall = i915_irq_uninstall;
-- 
1.7.10

[PATCH 6/6] drm/i915: Remove gen3 irq code from gen4 irq routine

[Chris Wilson]

---
 drivers/gpu/drm/i915/i915_irq.c |   27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 47a540a..2ff431b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2625,23 +2625,17 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
 	struct drm_i915_master_private *master_priv;
 	u32 iir, new_iir;
 	u32 pipe_stats[I915_MAX_PIPES];
-	u32 vblank_status;
-	int vblank = 0;
 	unsigned long irqflags;
 	int irq_received;
 	int ret = IRQ_NONE, pipe;
-	bool blc_event = false;
 
 	atomic_inc(&dev_priv->irq_received);
 
 	iir = I915_READ(IIR);
 
-	if (INTEL_INFO(dev)->gen >= 4)
-		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
-	else
-		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
-
 	for (;;) {
+		bool blc_event = false;
+
 		irq_received = iir != 0;
 
 		/* Can't rely on pipestat interrupt bit in iir as it might
@@ -2693,13 +2687,6 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
 		I915_WRITE(IIR, iir);
 		new_iir = I915_READ(IIR); /* Flush posted writes */
 
-		if (dev->primary->master) {
-			master_priv = dev->primary->master->driver_priv;
-			if (master_priv->sarea_priv)
-				master_priv->sarea_priv->last_dispatch =
-					READ_BREADCRUMB(dev_priv);
-		}
-
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev, &dev_priv->ring[RCS]);
 		if (iir & I915_BSD_USER_INTERRUPT)
@@ -2712,9 +2699,8 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
 			intel_prepare_page_flip(dev, 1);
 
 		for_each_pipe(pipe) {
-			if (pipe_stats[pipe] & vblank_status &&
+			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
 			    drm_handle_vblank(dev, pipe)) {
-				vblank++;
 				i915_pageflip_stall_check(dev, pipe);
 				intel_finish_page_flip(dev, pipe);
 			}
@@ -2745,6 +2731,13 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
 		iir = new_iir;
 	}
 
+	if (dev->primary->master) {
+		master_priv = dev->primary->master->driver_priv;
+		if (master_priv->sarea_priv)
+			master_priv->sarea_priv->last_dispatch =
+				READ_BREADCRUMB(dev_priv);
+	}
+
 	return ret;
 }
 
-- 
1.7.10

Re: [PATCH 1/6] drm/i915: Unconditionally initialise the interrupt workers

[Jesse Barnes]

On Tue, 24 Apr 2012 18:31:26 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Rather than duplicate similar code across the IRQ installers, perform
> the initialisation of the workers upfront. This will lead to simpler
> teardown and quiescent code as we can assume that the workers have
> been initialised.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Looks nice, and also gives me warm fuzzies about the potential races
between setting up the work queues and interrupts arriving since it
would be all too easy to accidentally call these in postinstall on some
new hw platform.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 2/6] drm/i915: Remove redundant initialisation of per-ring IRQ waitqueues

[Jesse Barnes]

On Tue, 24 Apr 2012 18:31:27 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> The waitqueues are already initialised during ring initialisation so
> kill the redundant and duplicated code to do so in each generations IRQ
> installer.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Yep and that happens before we regsiter the IRQ handler so it looks
good.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 3/6] drm/i915: pending_flip_is_done is gen3, name it so

[Jesse Barnes]

On Tue, 24 Apr 2012 18:31:28 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> And remove the cargo-culted copy from the valleyview irq handler.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Note that VLV flipping needs other work, but it doesn't have this
misfeature so we can drop it.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 5/6] drm/i915: Handle PendingFlip on gen3 robustly

[Jesse Barnes]

On Tue, 24 Apr 2012 18:31:30 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b378555..47a540a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2307,7 +2307,7 @@ static void i915_irq_preinstall(struct drm_device * dev)
>  		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>  	}
>  
> -	I915_WRITE(HWSTAM, 0xeffe);
> +	I915_WRITE16(HWSTAM, 0xeffe);
>  	for_each_pipe(pipe)
>  		I915_WRITE(PIPESTAT(pipe), 0);
>  	I915_WRITE(IMR, 0xffffffff);

Separate patch.

> @@ -2318,39 +2318,35 @@ static void i915_irq_preinstall(struct drm_device * dev)
>  static int i915_irq_postinstall(struct drm_device *dev)
>  {
>  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> -	u32 enable_mask = I915_INTERRUPT_ENABLE_FIX | I915_INTERRUPT_ENABLE_VAR;
> -	u32 error_mask;
> +	u32 enable_mask;
>  
>  	dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B;
>  
> -	/* Unmask the interrupts that we always want on. */
> -	dev_priv->irq_mask = ~I915_INTERRUPT_ENABLE_FIX;

I'd really like to get rid of these defines at the top of i915_irq.c.
Some are unused and the others just make you check for the right bits
everytime your read the code.

But that can be a separate patch.


> -
>  	dev_priv->pipestat[0] = 0;
>  	dev_priv->pipestat[1] = 0;
>  
> +	I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
> +
> +	/* Unmask the interrupts that we always want on. */
> +	dev_priv->irq_mask =
> +		~(I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> +		  I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
> +		  I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
> +		  I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT |
> +		  I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);

ASLE doesn't exist on gen3?  /me pulls out the docs

Gen3 *does* have ASLE, so don't you need to preserve that bit?

> +
> +	enable_mask =
> +		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> +		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
> +		I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT |
> +		I915_USER_INTERRUPT;

Same here.


>  	I915_WRITE(IMR, dev_priv->irq_mask);
>  	I915_WRITE(IER, enable_mask);
>  	POSTING_READ(IER);
> @@ -2371,13 +2367,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
>  			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
>  		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
>  			hotplug_en |= CRT_HOTPLUG_INT_EN;
> -
> -			/* Programming the CRT detection parameters tends
> -			   to generate a spurious hotplug event about three
> -			   seconds later.  So just do it once.
> -			*/
> -			if (IS_G4X(dev))
> -				hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;

This looks like it belongs in a separate "remove gen4 bits from gen3
code" patch.

>  			hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
>  		}
>  
> @@ -2396,26 +2385,25 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
>  	struct drm_device *dev = (struct drm_device *) arg;
>  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>  	struct drm_i915_master_private *master_priv;
> -	u32 iir, new_iir;
> -	u32 pipe_stats[I915_MAX_PIPES];
> -	u32 vblank_status;
> -	int vblank = 0;
> +	u32 iir, new_iir, pipe_stats[I915_MAX_PIPES];
>  	unsigned long irqflags;
> -	int irq_received;
> -	int ret = IRQ_NONE, pipe;
> -	bool blc_event = false;
> +	u32 flip_mask =
> +		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
> +		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
> +	u32 flip[2] = {
> +		I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT,
> +		I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT
> +	};
> +	int  pipe;
>  
>  	atomic_inc(&dev_priv->irq_received);
>  
>  	iir = I915_READ(IIR);
> +	if ((iir & ~flip_mask) == 0)
> +		return IRQ_NONE;

Note that it's possible for a pipestat bit to get stuck causing an
interrupt even though IIR is clear, causing an interrupt storm and
eventual shutdown of the IRQ.

The safest thing to check here would be the IRQ status bit in PCI
config space.  If that indicates an interrupt is active, then it's
definitely ours, even if IIR or pipestat are busted somehow...  But I
haven't tested how slow config space accesses are for us so it may not
be appropriate for the interrupt handler.

>  
> -	if (INTEL_INFO(dev)->gen >= 4)
> -		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
> -	else
> -		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
> -
> -	for (;;) {
> -		irq_received = iir != 0;
> +	while (iir & ~flip_mask) {
> +		bool blc_event = false;
>  
>  		/* Can't rely on pipestat interrupt bit in iir as it might
>  		 * have been cleared after the pipestat interrupt was received.
> @@ -2438,16 +2426,10 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
>  					DRM_DEBUG_DRIVER("pipe %c underrun\n",
>  							 pipe_name(pipe));
>  				I915_WRITE(reg, pipe_stats[pipe]);
> -				irq_received = 1;
>  			}
>  		}
>  		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
> -		if (!irq_received)
> -			break;
> -
> -		ret = IRQ_HANDLED;
> -
>  		/* Consume port.  Then clear IIR or we'll miss events */
>  		if ((I915_HAS_HOTPLUG(dev)) &&
>  		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
> @@ -2460,43 +2442,25 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
>  					   &dev_priv->hotplug_work);
>  
>  			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> -			I915_READ(PORT_HOTPLUG_STAT);
> +			POSTING_READ(PORT_HOTPLUG_STAT);
>  		}
>  
> -		I915_WRITE(IIR, iir);
> +		I915_WRITE(IIR, iir & ~flip_mask);
>  		new_iir = I915_READ(IIR); /* Flush posted writes */
>  
> -		if (dev->primary->master) {
> -			master_priv = dev->primary->master->driver_priv;
> -			if (master_priv->sarea_priv)
> -				master_priv->sarea_priv->last_dispatch =
> -					READ_BREADCRUMB(dev_priv);
> -		}

Separate but worthy patch.  In fact can we get rid of these at all?
Does anyone still pretend that DRI1 works?

> -
>  		if (iir & I915_USER_INTERRUPT)
>  			notify_ring(dev, &dev_priv->ring[RCS]);
> -		if (iir & I915_BSD_USER_INTERRUPT)
> -			notify_ring(dev, &dev_priv->ring[VCS]);

More "remove gen4 from gen3" bits.

> -
> -		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT) {
> -			intel_prepare_page_flip(dev, 0);
> -			if (dev_priv->gen3_flip_pending_is_done)
> -				intel_finish_page_flip_plane(dev, 0);
> -		}
> -
> -		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
> -			intel_prepare_page_flip(dev, 1);
> -			if (dev_priv->gen3_flip_pending_is_done)
> -				intel_finish_page_flip_plane(dev, 1);
> -		}
>  
>  		for_each_pipe(pipe) {
> -			if (pipe_stats[pipe] & vblank_status &&
> +			int plane = pipe;
> +			if (IS_MOBILE(dev))
> +				plane = !plane;

Is this shared with 8xx?  Do we swap every gen3?  Maybe we should just
get rid of the swapping code if FBC is disabled on gen3 by default
anyway...

> +			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
>  			    drm_handle_vblank(dev, pipe)) {
> -				vblank++;
> -				if (!dev_priv->gen3_flip_pending_is_done) {
> -					i915_pageflip_stall_check(dev, pipe);
> +				if (iir & flip[plane]) {
> +					intel_prepare_page_flip(dev, plane);
>  					intel_finish_page_flip(dev, pipe);
> +					flip_mask &= ~flip[plane];
>  				}

IIRC the ECOSKPD bit actually did have meaning on some machines.  For
some reporters, tearing was the result of ignoring the difference, so
we'll need to get lots of QA.

>  			}
>  
> @@ -2504,7 +2468,6 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
>  				blc_event = true;
>  		}
>  
> -
>  		if (blc_event || (iir & I915_ASLE_INTERRUPT))
>  			intel_opregion_asle_intr(dev);

Spurious space removal.

>  
> @@ -2526,18 +2489,21 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
>  		iir = new_iir;
>  	}
>  
> -	return ret;
> -}
> +	if (dev->primary->master) {
> +		master_priv = dev->primary->master->driver_priv;
> +		if (master_priv->sarea_priv)
> +			master_priv->sarea_priv->last_dispatch =
> +				READ_BREADCRUMB(dev_priv);
> +	}
>  
> +	return IRQ_HANDLED;
> +}
>  
>  static void i915_irq_uninstall(struct drm_device * dev)
>  {
>  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>  	int pipe;
>  
> -	if (!dev_priv)
> -		return;
> -
>  	dev_priv->vblank_pipe = 0;
>  
>  	if (I915_HAS_HOTPLUG(dev)) {
> @@ -2545,15 +2511,14 @@ static void i915_irq_uninstall(struct drm_device * dev)
>  		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>  	}
>  
> -	I915_WRITE(HWSTAM, 0xffffffff);
> -	for_each_pipe(pipe)
> +	I915_WRITE16(HWSTAM, 0xffff);
> +	for_each_pipe(pipe) {
> +		/* Clear enable bits; then clear status bits */
>  		I915_WRITE(PIPESTAT(pipe), 0);
> +		I915_WRITE(PIPESTAT(pipe), I915_READ(PIPESTAT(pipe)));
> +	}

A good addition mixed in with the 16 bit change.

>  	I915_WRITE(IMR, 0xffffffff);
>  	I915_WRITE(IER, 0x0);
> -
> -	for_each_pipe(pipe)
> -		I915_WRITE(PIPESTAT(pipe),
> -			   I915_READ(PIPESTAT(pipe)) & 0x8000ffff);
>  	I915_WRITE(IIR, I915_READ(IIR));
>  }
>  
> @@ -2740,26 +2705,18 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
>  		if (iir & I915_BSD_USER_INTERRUPT)
>  			notify_ring(dev, &dev_priv->ring[VCS]);
>  
> -		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT) {
> +		if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT)
>  			intel_prepare_page_flip(dev, 0);
> -			if (dev_priv->gen3_flip_pending_is_done)
> -				intel_finish_page_flip_plane(dev, 0);
> -		}
>  
> -		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
> +		if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT)
>  			intel_prepare_page_flip(dev, 1);
> -			if (dev_priv->gen3_flip_pending_is_done)
> -				intel_finish_page_flip_plane(dev, 1);
> -		}
>  
>  		for_each_pipe(pipe) {
>  			if (pipe_stats[pipe] & vblank_status &&
>  			    drm_handle_vblank(dev, pipe)) {
>  				vblank++;
> -				if (!dev_priv->gen3_flip_pending_is_done) {
> -					i915_pageflip_stall_check(dev, pipe);
> -					intel_finish_page_flip(dev, pipe);
> -				}
> +				i915_pageflip_stall_check(dev, pipe);
> +				intel_finish_page_flip(dev, pipe);
>  			}

These look like "remove gen3 bits from gen4" patch hunks?  If you kept
the field until it was dropped (if we can drop it) you could do it as a
separate patch at the end.

>  
>  			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
> @@ -2826,10 +2783,6 @@ void intel_irq_init(struct drm_device *dev)
>  	INIT_WORK(&dev_priv->error_work, i915_error_work_func);
>  	INIT_WORK(&dev_priv->rps_work, gen6_pm_rps_work);
>  
> -	/* IIR "flip pending" bit means done if this bit is set */
> -	if (IS_GEN3(dev) && (I915_READ(ECOSKPD) & ECO_FLIP_DONE))
> -		dev_priv->gen3_flip_pending_is_done = true;
> -
>  	dev->driver->get_vblank_counter = i915_get_vblank_counter;
>  	dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
>  	if (IS_G4X(dev) || IS_GEN5(dev) || IS_GEN6(dev) || IS_IVYBRIDGE(dev) ||
> @@ -2873,6 +2826,9 @@ void intel_irq_init(struct drm_device *dev)
>  			dev->driver->irq_handler = i8xx_irq_handler;
>  			dev->driver->irq_uninstall = i8xx_irq_uninstall;
>  		} else if (INTEL_INFO(dev)->gen == 3) {
> +			/* "flip pending" bit means done if this bit is set */
> +			I915_WRITE(ECOSKPD, _MASKED_BIT_DISABLE(ECO_FLIP_DONE));
> +

Aha.  I hope this works across platforms.  I don't even know why this
bit exists, so I hope it's safe to change on a given chipset & platform.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 6/6] drm/i915: Remove gen3 irq code from gen4 irq routine

[Jesse Barnes]

On Tue, 24 Apr 2012 18:31:31 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ---
>  drivers/gpu/drm/i915/i915_irq.c |   27 ++++++++++-----------------
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 47a540a..2ff431b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2625,23 +2625,17 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
>  	struct drm_i915_master_private *master_priv;
>  	u32 iir, new_iir;
>  	u32 pipe_stats[I915_MAX_PIPES];
> -	u32 vblank_status;
> -	int vblank = 0;
>  	unsigned long irqflags;
>  	int irq_received;
>  	int ret = IRQ_NONE, pipe;
> -	bool blc_event = false;
>  
>  	atomic_inc(&dev_priv->irq_received);
>  
>  	iir = I915_READ(IIR);
>  
> -	if (INTEL_INFO(dev)->gen >= 4)
> -		vblank_status = PIPE_START_VBLANK_INTERRUPT_STATUS;
> -	else
> -		vblank_status = PIPE_VBLANK_INTERRUPT_STATUS;
> -
>  	for (;;) {
> +		bool blc_event = false;
> +
>  		irq_received = iir != 0;
>  
>  		/* Can't rely on pipestat interrupt bit in iir as it might
> @@ -2693,13 +2687,6 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
>  		I915_WRITE(IIR, iir);
>  		new_iir = I915_READ(IIR); /* Flush posted writes */
>  
> -		if (dev->primary->master) {
> -			master_priv = dev->primary->master->driver_priv;
> -			if (master_priv->sarea_priv)
> -				master_priv->sarea_priv->last_dispatch =
> -					READ_BREADCRUMB(dev_priv);
> -		}
> -
>  		if (iir & I915_USER_INTERRUPT)
>  			notify_ring(dev, &dev_priv->ring[RCS]);
>  		if (iir & I915_BSD_USER_INTERRUPT)
> @@ -2712,9 +2699,8 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
>  			intel_prepare_page_flip(dev, 1);
>  
>  		for_each_pipe(pipe) {
> -			if (pipe_stats[pipe] & vblank_status &&
> +			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
>  			    drm_handle_vblank(dev, pipe)) {
> -				vblank++;
>  				i915_pageflip_stall_check(dev, pipe);
>  				intel_finish_page_flip(dev, pipe);
>  			}
> @@ -2745,6 +2731,13 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
>  		iir = new_iir;
>  	}
>  
> +	if (dev->primary->master) {
> +		master_priv = dev->primary->master->driver_priv;
> +		if (master_priv->sarea_priv)
> +			master_priv->sarea_priv->last_dispatch =
> +				READ_BREADCRUMB(dev_priv);
> +	}
> +
>  	return ret;
>  }
>  

Looks good.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 5/6] drm/i915: Handle PendingFlip on gen3 robustly

[Daniel Vetter]

On Tue, Apr 24, 2012 at 12:50:35PM -0700, Jesse Barnes wrote:
> On Tue, 24 Apr 2012 18:31:30 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > -		if (dev->primary->master) {
> > -			master_priv = dev->primary->master->driver_priv;
> > -			if (master_priv->sarea_priv)
> > -				master_priv->sarea_priv->last_dispatch =
> > -					READ_BREADCRUMB(dev_priv);
> > -		}
> 
> Separate but worthy patch.  In fact can we get rid of these at all?
> Does anyone still pretend that DRI1 works?

We still have to, so it can't go. But if you want some warm fuzzies from
removing dri1 crap, look no further than my patch-bomb - there's a patch
in there which removes this code for gen5+.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 5/6] drm/i915: Handle PendingFlip on gen3 robustly

[Chris Wilson]

On Tue, 24 Apr 2012 12:50:35 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > +			/* "flip pending" bit means done if this bit is set */
> > +			I915_WRITE(ECOSKPD, _MASKED_BIT_DISABLE(ECO_FLIP_DONE));
> > +
> 
> Aha.  I hope this works across platforms.  I don't even know why this
> bit exists, so I hope it's safe to change on a given chipset & platform.

Patch splitting to come. So far I've tested on a 945gm and PineView and
both are happy with the legacy PendingFlip status mode. And Daniel has
tested on a 915g which was demonstrating the same trouble as our dear
bugzilla reporter. One person I'd like to see I can get testing is Simon
Farnsworth if he still has only of his 945s available for testing.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre