Commit 4bc9d430 authored by Daniel Vetter's avatar Daniel Vetter

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 <przanoni@gmail.com>
Reviewed-by: default avatarPaulo Zanoni <przanoni@gmail.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent a0de80a0
...@@ -86,6 +86,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev); ...@@ -86,6 +86,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev);
static void static void
ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask) 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) { if ((dev_priv->irq_mask & mask) != 0) {
dev_priv->irq_mask &= ~mask; dev_priv->irq_mask &= ~mask;
I915_WRITE(DEIMR, dev_priv->irq_mask); I915_WRITE(DEIMR, dev_priv->irq_mask);
...@@ -96,6 +98,8 @@ ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask) ...@@ -96,6 +98,8 @@ ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
static void static void
ironlake_disable_display_irq(drm_i915_private_t *dev_priv, u32 mask) 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) { if ((dev_priv->irq_mask & mask) != mask) {
dev_priv->irq_mask |= mask; dev_priv->irq_mask |= mask;
I915_WRITE(DEIMR, dev_priv->irq_mask); I915_WRITE(DEIMR, dev_priv->irq_mask);
...@@ -109,6 +113,8 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) ...@@ -109,6 +113,8 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
struct intel_crtc *crtc; struct intel_crtc *crtc;
enum pipe pipe; enum pipe pipe;
assert_spin_locked(&dev_priv->irq_lock);
for_each_pipe(pipe) { for_each_pipe(pipe) {
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[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) ...@@ -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 /* On Haswell, also mask ERR_INT because we don't want to risk
* generating "unclaimed register" interrupts from inside the interrupt * generating "unclaimed register" interrupts from inside the interrupt
* handler. */ * 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); ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
spin_unlock(&dev_priv->irq_lock);
}
gt_iir = I915_READ(GTIIR); gt_iir = I915_READ(GTIIR);
if (gt_iir) { if (gt_iir) {
...@@ -1271,8 +1280,12 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg) ...@@ -1271,8 +1280,12 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
ret = IRQ_HANDLED; ret = IRQ_HANDLED;
} }
if (IS_HASWELL(dev) && ivb_can_enable_err_int(dev)) if (IS_HASWELL(dev)) {
ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB); 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); I915_WRITE(DEIER, de_ier);
POSTING_READ(DEIER); POSTING_READ(DEIER);
...@@ -2697,6 +2710,8 @@ static void ibx_irq_postinstall(struct drm_device *dev) ...@@ -2697,6 +2710,8 @@ static void ibx_irq_postinstall(struct drm_device *dev)
static int ironlake_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; drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
/* enable kind of interrupts always enabled */ /* enable kind of interrupts always enabled */
u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT | 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) ...@@ -2735,7 +2750,13 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
/* Clear & enable PCU event interrupts */ /* Clear & enable PCU event interrupts */
I915_WRITE(DEIIR, DE_PCU_EVENT); I915_WRITE(DEIIR, DE_PCU_EVENT);
I915_WRITE(DEIER, I915_READ(DEIER) | 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); ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT);
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
} }
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment