diff mbox

[1/6] drm/i915: assert_spin_locked for pipestat interrupt enable/disable

Message ID 1372348335-6824-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter June 27, 2013, 3:52 p.m. UTC 
Just to keep the paranoia equal also sprinkle locking asserts over the
pipestat interrupt enable/disable functions.

Again this results in false positives in the interrupt setup. Add
bogo-locking for these and a big comment explaining why it's there and
that it's indeed unnecessary.

v2: Fix up the spelling fail Paulo spotted in comments.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Daniel Vetter July 4, 2013, 7:22 p.m. UTC | #1 
On Thu, Jun 27, 2013 at 05:52:10PM +0200, Daniel Vetter wrote:
> Just to keep the paranoia equal also sprinkle locking asserts over the
> pipestat interrupt enable/disable functions.
> 
> Again this results in false positives in the interrupt setup. Add
> bogo-locking for these and a big comment explaining why it's there and
> that it's indeed unnecessary.
> 
> v2: Fix up the spelling fail Paulo spotted in comments.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Oops, somehow I've missed this patch here. It's now merged into dinq, too.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 465588f..e7c879b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -322,6 +322,8 @@ i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
>  	u32 reg = PIPESTAT(pipe);
>  	u32 pipestat = I915_READ(reg) & 0x7fff0000;
>  
> +	assert_spin_locked(&dev_priv->irq_lock);
> +
>  	if ((pipestat & mask) == mask)
>  		return;
>  
> @@ -337,6 +339,8 @@ i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
>  	u32 reg = PIPESTAT(pipe);
>  	u32 pipestat = I915_READ(reg) & 0x7fff0000;
>  
> +	assert_spin_locked(&dev_priv->irq_lock);
> +
>  	if ((pipestat & mask) == 0)
>  		return;
>  
> @@ -2826,6 +2830,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>  	u32 gt_irqs;
>  	u32 enable_mask;
>  	u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
> +	unsigned long irqflags;
>  
>  	enable_mask = I915_DISPLAY_PORT_INTERRUPT;
>  	enable_mask |= I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> @@ -2851,9 +2856,13 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>  	I915_WRITE(PIPESTAT(1), 0xffff);
>  	POSTING_READ(VLV_IER);
>  
> +	/* Interrupt setup is already guaranteed to be single-threaded, this is
> +	 * just to make the assert_spin_locked check happy. */
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	i915_enable_pipestat(dev_priv, 0, pipestat_enable);
>  	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
>  	i915_enable_pipestat(dev_priv, 1, pipestat_enable);
> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
>  	I915_WRITE(VLV_IIR, 0xffffffff);
>  	I915_WRITE(VLV_IIR, 0xffffffff);
> @@ -3335,6 +3344,7 @@ 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;
>  	u32 error_mask;
> +	unsigned long irqflags;
>  
>  	/* Unmask the interrupts that we always want on. */
>  	dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT |
> @@ -3353,7 +3363,11 @@ static int i965_irq_postinstall(struct drm_device *dev)
>  	if (IS_G4X(dev))
>  		enable_mask |= I915_BSD_USER_INTERRUPT;
>  
> +	/* Interrupt setup is already guaranteed to be single-threaded, this is
> +	 * just to make the assert_spin_locked check happy. */
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
>  	/*
>  	 * Enable some error detection, note the instruction error mask
> -- 
> 1.8.1.4
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 465588f..e7c879b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -322,6 +322,8 @@  i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
 	u32 reg = PIPESTAT(pipe);
 	u32 pipestat = I915_READ(reg) & 0x7fff0000;
 
+	assert_spin_locked(&dev_priv->irq_lock);
+
 	if ((pipestat & mask) == mask)
 		return;
 
@@ -337,6 +339,8 @@  i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
 	u32 reg = PIPESTAT(pipe);
 	u32 pipestat = I915_READ(reg) & 0x7fff0000;
 
+	assert_spin_locked(&dev_priv->irq_lock);
+
 	if ((pipestat & mask) == 0)
 		return;
 
@@ -2826,6 +2830,7 @@  static int valleyview_irq_postinstall(struct drm_device *dev)
 	u32 gt_irqs;
 	u32 enable_mask;
 	u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
+	unsigned long irqflags;
 
 	enable_mask = I915_DISPLAY_PORT_INTERRUPT;
 	enable_mask |= I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
@@ -2851,9 +2856,13 @@  static int valleyview_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(PIPESTAT(1), 0xffff);
 	POSTING_READ(VLV_IER);
 
+	/* Interrupt setup is already guaranteed to be single-threaded, this is
+	 * just to make the assert_spin_locked check happy. */
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	i915_enable_pipestat(dev_priv, 0, pipestat_enable);
 	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
 	i915_enable_pipestat(dev_priv, 1, pipestat_enable);
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	I915_WRITE(VLV_IIR, 0xffffffff);
 	I915_WRITE(VLV_IIR, 0xffffffff);
@@ -3335,6 +3344,7 @@  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;
 	u32 error_mask;
+	unsigned long irqflags;
 
 	/* Unmask the interrupts that we always want on. */
 	dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT |
@@ -3353,7 +3363,11 @@  static int i965_irq_postinstall(struct drm_device *dev)
 	if (IS_G4X(dev))
 		enable_mask |= I915_BSD_USER_INTERRUPT;
 
+	/* Interrupt setup is already guaranteed to be single-threaded, this is
+	 * just to make the assert_spin_locked check happy. */
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	/*
 	 * Enable some error detection, note the instruction error mask