[Intel-gfx] [PATCH] drm-intel: disable irq before reading iir for MSI. Use right vblank signal.

Keith Packard keithp at keithp.com
Sun Sep 14 03:46:27 CEST 2008


Reading any interrupt registers before masking out interrupts causes all
kinds of MSI interrupt failure. New order is:

	mask out interrupts
	read iir
	read pipe registers (if interrupt signalled)
	write pipe registers
	write iir
	mask in interrupts
	posting read

Also, this patch changes the vblank code to use the vblank signal available
in the pipe status registers instead of the simple vblank interrupt. On 965,
the simple vblank interrupt comes too late and can cause visible tearing.

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 drivers/gpu/drm/i915/i915_irq.c |  138 ++++++++++++++++++++++++++------------
 1 files changed, 94 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d47805a..742297c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -35,11 +35,19 @@
 
 /** These are the interrupts used by the driver */
 #define I915_INTERRUPT_ENABLE_MASK (I915_USER_INTERRUPT |		\
-				    I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT | \
-				    I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT | \
 				    I915_ASLE_INTERRUPT |		\
+				    I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | \
 				    I915_DISPLAY_PIPE_B_EVENT_INTERRUPT)
 
+#define I915_PIPE_VBLANK_STATUS	(PIPE_START_VBLANK_INTERRUPT_STATUS |\
+				 PIPE_VBLANK_INTERRUPT_STATUS)
+
+#define I915_PIPE_VBLANK_ENABLE	(PIPE_START_VBLANK_INTERRUPT_ENABLE |\
+				 PIPE_VBLANK_INTERRUPT_ENABLE)
+
+#define DRM_I915_VBLANK_PIPE_ALL	(DRM_I915_VBLANK_PIPE_A | \
+					 DRM_I915_VBLANK_PIPE_B)
+
 void
 i915_enable_irq(drm_i915_private_t *dev_priv, u32 mask)
 {
@@ -271,14 +279,13 @@ 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;
-	u32 pipea_stats, pipeb_stats;
+	u32 pipea_stats = 0, pipeb_stats = 0;
 	u32 iir;
-
-	pipea_stats = I915_READ(PIPEASTAT);
-	pipeb_stats = I915_READ(PIPEBSTAT);
+	int vblank_pipe = dev_priv->vblank_pipe;
 
 	if (dev->pdev->msi_enabled)
 		I915_WRITE(IMR, ~0);
+    
 	iir = I915_READ(IIR);
 
 	DRM_DEBUG("iir=%08x\n", iir);
@@ -291,37 +298,49 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS)
 		return IRQ_NONE;
 	}
 
-	I915_WRITE(PIPEASTAT, pipea_stats);
-	I915_WRITE(PIPEBSTAT, pipeb_stats);
+	if (iir & I915_DISPLAY_PIPE_A_EVENT_INTERRUPT) {
+		pipea_stats = I915_READ(PIPEASTAT);
+		I915_WRITE(PIPEASTAT, pipea_stats);
+	}
+    
+	if (iir & I915_DISPLAY_PIPE_B_EVENT_INTERRUPT) {
+		pipeb_stats = I915_READ(PIPEBSTAT);
+		/* The vblank interrupt gets enabled even if we didn't ask for
+		   it, so make sure it's shut down again */
+		if ((vblank_pipe & DRM_I915_VBLANK_PIPE_B) == 0)
+			pipeb_stats &= ~(I915_VBLANK_INTERRUPT_ENABLE);
+		I915_WRITE(PIPEBSTAT, pipeb_stats);
+	}
 
 	I915_WRITE(IIR, iir);
 	if (dev->pdev->msi_enabled)
 		I915_WRITE(IMR, dev_priv->irq_mask_reg);
 	(void) I915_READ(IIR); /* Flush posted writes */
 
+	/* Ok, interrupt registers dealt with, now lets see
+	 * what we've got
+	 */
+
 	if (dev_priv->sarea_priv)
-		dev_priv->sarea_priv->last_dispatch =
-			READ_BREADCRUMB(dev_priv);
+		dev_priv->sarea_priv->last_dispatch = READ_BREADCRUMB(dev_priv);
 
 	if (iir & I915_USER_INTERRUPT) {
 		dev_priv->mm.irq_gem_seqno = i915_get_gem_seqno(dev);
 		DRM_WAKEUP(&dev_priv->irq_queue);
 	}
 
-	if (iir & (I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT |
-		   I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT)) {
-		int vblank_pipe = dev_priv->vblank_pipe;
-
-		if ((vblank_pipe &
-		     (DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B))
-		    == (DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B)) {
-			if (iir & I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT)
+	if ((pipeb_stats|pipea_stats) & I915_PIPE_VBLANK_STATUS) {
+	
+		/* When tracking both pipes, split the counts */
+		if ((vblank_pipe & DRM_I915_VBLANK_PIPE_ALL) ==
+		    DRM_I915_VBLANK_PIPE_ALL) {
+			if (pipea_stats & I915_PIPE_VBLANK_STATUS)
 				atomic_inc(&dev->vbl_received);
-			if (iir & I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT)
+			if (pipeb_stats & I915_PIPE_VBLANK_STATUS)
 				atomic_inc(&dev->vbl_received2);
-		} else if (((iir & I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT) &&
+		} else if (((pipea_stats & I915_PIPE_VBLANK_STATUS) &&
 			    (vblank_pipe & DRM_I915_VBLANK_PIPE_A)) ||
-			   ((iir & I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT) &&
+			   ((pipeb_stats & I915_PIPE_VBLANK_STATUS) &&
 			    (vblank_pipe & DRM_I915_VBLANK_PIPE_B)))
 			atomic_inc(&dev->vbl_received);
 
@@ -500,6 +519,48 @@ int i915_irq_wait(struct drm_device *dev, void *data,
 	return i915_wait_irq(dev, irqwait->irq_seq);
 }
 
+static void i915_vblank_configure(struct drm_device *dev, int pipe)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	u32 enable_a_mask = 0, disable_a_mask = 0;
+	u32 enable_b_mask = 0, disable_b_mask = 0;
+	u32 mask;
+	u32 pipe_a_stat, pipe_b_stat;
+
+	/*
+	 * For newer chips, use START_VBLANK_INTERRUPT_ENABLE
+	 * as the VBLANK_INTERRUPT_ENABLE signal occurs much
+	 * later
+	 */
+	if (IS_I965G(dev))
+		mask = PIPE_START_VBLANK_INTERRUPT_ENABLE;
+	else
+		mask = PIPE_VBLANK_INTERRUPT_ENABLE;
+	
+	if (pipe & DRM_I915_VBLANK_PIPE_A)
+		enable_a_mask |= mask;
+	else
+		disable_a_mask |= mask;
+
+	if (pipe & DRM_I915_VBLANK_PIPE_B)
+		enable_b_mask |= mask;
+	else
+		disable_b_mask |= mask;
+
+	pipe_a_stat = I915_READ (PIPEASTAT);
+	pipe_a_stat |= enable_a_mask;
+	pipe_a_stat &= ~disable_a_mask;
+	I915_WRITE (PIPEASTAT, pipe_a_stat);
+	
+	pipe_b_stat = I915_READ (PIPEBSTAT);
+	pipe_b_stat |= enable_b_mask;
+	pipe_b_stat &= ~disable_b_mask;
+	I915_WRITE (PIPEBSTAT, pipe_b_stat);
+
+	/* post writes */
+	(void) I915_READ(PIPEBSTAT);
+}
+
 /* Set the vblank monitor pipe
  */
 int i915_vblank_pipe_set(struct drm_device *dev, void *data,
@@ -507,7 +568,6 @@ int i915_vblank_pipe_set(struct drm_device *dev, void *data,
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	drm_i915_vblank_pipe_t *pipe = data;
-	u32 enable_mask = 0, disable_mask = 0;
 
 	if (!dev_priv) {
 		DRM_ERROR("called with no initialization\n");
@@ -519,18 +579,10 @@ int i915_vblank_pipe_set(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	if (pipe->pipe & DRM_I915_VBLANK_PIPE_A)
-		enable_mask |= I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
-	else
-		disable_mask |= I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
-
-	if (pipe->pipe & DRM_I915_VBLANK_PIPE_B)
-		enable_mask |= I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
-	else
-		disable_mask |= I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
+	i915_vblank_configure(dev, pipe->pipe);
 
-	i915_enable_irq(dev_priv, enable_mask);
-	i915_disable_irq(dev_priv, disable_mask);
+	i915_enable_irq(dev_priv, I915_DISPLAY_PIPE_A_EVENT_INTERRUPT);
+	i915_enable_irq(dev_priv, I915_DISPLAY_PIPE_B_EVENT_INTERRUPT);
 
 	dev_priv->vblank_pipe = pipe->pipe;
 
@@ -542,18 +594,19 @@ int i915_vblank_pipe_get(struct drm_device *dev, void *data,
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	drm_i915_vblank_pipe_t *pipe = data;
-	u16 flag;
+	u32 pipe_a_stat, pipe_b_stat;
 
 	if (!dev_priv) {
 		DRM_ERROR("called with no initialization\n");
 		return -EINVAL;
 	}
 
-	flag = I915_READ(IMR);
+	pipe_a_stat = I915_READ(PIPEASTAT);
+	pipe_b_stat = I915_READ(PIPEBSTAT);
 	pipe->pipe = 0;
-	if (flag & I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT)
+	if (pipe_a_stat & I915_PIPE_VBLANK_ENABLE)
 		pipe->pipe |= DRM_I915_VBLANK_PIPE_A;
-	if (flag & I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT)
+	if (pipe_b_stat & I915_PIPE_VBLANK_ENABLE)
 		pipe->pipe |= DRM_I915_VBLANK_PIPE_B;
 
 	return 0;
@@ -671,7 +724,7 @@ void i915_driver_irq_preinstall(struct drm_device * dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 
-	I915_WRITE(HWSTAM, 0xfffe);
+	I915_WRITE(HWSTAM, 0xeffe);
 	I915_WRITE(IMR, 0x0);
 	I915_WRITE(IER, 0x0);
 }
@@ -688,18 +741,15 @@ void i915_driver_irq_postinstall(struct drm_device * dev)
 		dev_priv->vblank_pipe = DRM_I915_VBLANK_PIPE_A;
 
 	/* Set initial unmasked IRQs to just the selected vblank pipes. */
-	dev_priv->irq_mask_reg = ~0;
-	if (dev_priv->vblank_pipe & DRM_I915_VBLANK_PIPE_A)
-		dev_priv->irq_mask_reg &= ~I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
-	if (dev_priv->vblank_pipe & DRM_I915_VBLANK_PIPE_B)
-		dev_priv->irq_mask_reg &= ~I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
 
-	dev_priv->irq_mask_reg &= I915_INTERRUPT_ENABLE_MASK;
+	dev_priv->irq_mask_reg = I915_INTERRUPT_ENABLE_MASK;
 
 	I915_WRITE(IMR, dev_priv->irq_mask_reg);
 	I915_WRITE(IER, I915_INTERRUPT_ENABLE_MASK);
 	(void) I915_READ(IER);
 
+	i915_vblank_configure(dev, dev_priv->vblank_pipe);
+
 	opregion_enable_asle(dev);
 
 	DRM_INIT_WAITQUEUE(&dev_priv->irq_queue);
-- 
1.5.6.5
-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20080913/e8186654/attachment.sig>


More information about the Intel-gfx mailing list