[Intel-gfx] [PATCH 01/24] drm/i915: fix locking around ironlake_enable|disable_display_irq

[Daniel Vetter]

The haswell unclaimed register handling code forgot to take the
spinlock. Since this is in the context of the non-rentrant interupt
handler and we only have one interrupt handler it is sufficient to
just grab the spinlock - we do not need to exclude any other
interrupts from running on the same cpu.

To prevent such gaffles in the future sprinkle assert_spin_locked over
these functions. Unfornately this requires us to hold the spinlock in
the ironlake postinstall hook where it is not strictly required:
Currently that is run in single-threaded context and with userspace
exlcuded from running concurrent ioctls. Add a comment explaining
this.

Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c482e8a..567945f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -95,6 +95,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev);
 static void
 ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
 {
+	assert_spin_locked(&dev_priv->irq_lock);
+
 	if ((dev_priv->irq_mask & mask) != 0) {
 		dev_priv->irq_mask &= ~mask;
 		I915_WRITE(DEIMR, dev_priv->irq_mask);
@@ -105,6 +107,8 @@ ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
 static void
 ironlake_disable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
 {
+	assert_spin_locked(&dev_priv->irq_lock);
+
 	if ((dev_priv->irq_mask & mask) != mask) {
 		dev_priv->irq_mask |= mask;
 		I915_WRITE(DEIMR, dev_priv->irq_mask);
@@ -1218,8 +1222,11 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	/* On Haswell, also mask ERR_INT because we don't want to risk
 	 * generating "unclaimed register" interrupts from inside the interrupt
 	 * handler. */
-	if (IS_HASWELL(dev))
+	if (IS_HASWELL(dev)) {
+		spin_lock(&dev_priv->irq_lock);
 		ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
+		spin_unlock(&dev_priv->irq_lock);
+	}
 
 	gt_iir = I915_READ(GTIIR);
 	if (gt_iir) {
@@ -1272,8 +1279,11 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 		ret = IRQ_HANDLED;
 	}
 
-	if (IS_HASWELL(dev) && ivb_can_enable_err_int(dev))
+	if (IS_HASWELL(dev) && ivb_can_enable_err_int(dev)) {
+		spin_lock(&dev_priv->irq_lock);
 		ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB);
+		spin_unlock(&dev_priv->irq_lock);
+	}
 
 	I915_WRITE(DEIER, de_ier);
 	POSTING_READ(DEIER);
@@ -2633,6 +2643,8 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
+	unsigned long irqflags;
+
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	/* enable kind of interrupts always enabled */
 	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
@@ -2671,7 +2683,13 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 		/* Clear & enable PCU event interrupts */
 		I915_WRITE(DEIIR, DE_PCU_EVENT);
 		I915_WRITE(DEIER, I915_READ(DEIER) | DE_PCU_EVENT);
+
+		/* spinlocking not required here for correctness since interrupt
+		 * setup is guaranteed to run in single-threaded context. But we
+		 * need it to make the assert_spin_locked happy. */
+		spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 		ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT);
+		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 	}
 
 	return 0;
-- 
1.8.1.4