From 4bc9d4301573403c578e545b34dceac61891f39c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 27 Jun 2013 13:44:58 +0200 Subject: [PATCH] drm/i915: fix locking around ironlake_enable|disable_display_irq 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. v2: ivb_can_enable_err_int also needs to be protected by the spinlock. To ensure this won't happen in the future again also sprinkle a spinlock assert in there. v3: Kill the 2nd call to ivb_can_enable_err_int I've accidentally left behind, spotted by Paulo. Cc: Paulo Zanoni Reviewed-by: Paulo Zanoni Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d6bd0d7a748..61cd87999df 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -86,6 +86,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); @@ -96,6 +98,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); @@ -109,6 +113,8 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) struct intel_crtc *crtc; enum pipe pipe; + assert_spin_locked(&dev_priv->irq_lock); + for_each_pipe(pipe) { crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); @@ -1217,8 +1223,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) { @@ -1271,8 +1280,12 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) ret = IRQ_HANDLED; } - if (IS_HASWELL(dev) && ivb_can_enable_err_int(dev)) - ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB); + if (IS_HASWELL(dev)) { + spin_lock(&dev_priv->irq_lock); + if (ivb_can_enable_err_int(dev)) + ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB); + spin_unlock(&dev_priv->irq_lock); + } I915_WRITE(DEIER, de_ier); POSTING_READ(DEIER); @@ -2697,6 +2710,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 | @@ -2735,7 +2750,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; -- 2.46.0