Commit 2dd2a883 authored by Imre Deak's avatar Imre Deak Committed by Jani Nikula

drm/i915: avoid processing spurious/shared interrupts in low-power states

Atm, it's possible that the interrupt handler is called when the device
is in D3 or some other low-power state. It can be due to another device
that is still in D0 state and shares the interrupt line with i915, or on
some platforms there could be spurious interrupts even without sharing
the interrupt line. The latter case was reported by Klaus Ethgen using a
Lenovo x61p machine (gen 4). He noticed this issue via a system
suspend/resume hang and bisected it to the following commit:

commit e11aa362
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Wed Jun 18 09:52:55 2014 -0700

    drm/i915: use runtime irq suspend/resume in freeze/thaw

This is a problem, since in low-power states IIR will always read
0xffffffff resulting in an endless IRQ servicing loop.

Fix this by handling interrupts only when the driver explicitly enables
them and so it's guaranteed that the interrupt registers return a valid
value.

Note that this issue existed even before the above commit, since during
runtime suspend/resume we never unregistered the handler.

v2:
- clarify the purpose of smp_mb() vs. synchronize_irq() in the
  code comment (Chris)

v3:
- no need for an explicit smp_mb(), we can assume that synchronize_irq()
  and the mmio read/writes in the install hooks provide for this (Daniel)
- remove code comment as the remaining synchronize_irq() is self
  explanatory (Daniel)

v4:
- drm_irq_uninstall() implies synchronize_irq(), so no need to call it
  explicitly (Daniel)

Reference: https://lkml.org/lkml/2015/2/11/205Reported-and-bisected-by: default avatarKlaus Ethgen <Klaus@Ethgen.ch>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 6c31a614
...@@ -1892,6 +1892,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) ...@@ -1892,6 +1892,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
u32 iir, gt_iir, pm_iir; u32 iir, gt_iir, pm_iir;
irqreturn_t ret = IRQ_NONE; irqreturn_t ret = IRQ_NONE;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
while (true) { while (true) {
/* Find, clear, then process each source of interrupt */ /* Find, clear, then process each source of interrupt */
...@@ -1936,6 +1939,9 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg) ...@@ -1936,6 +1939,9 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
u32 master_ctl, iir; u32 master_ctl, iir;
irqreturn_t ret = IRQ_NONE; irqreturn_t ret = IRQ_NONE;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
for (;;) { for (;;) {
master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL; master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL;
iir = I915_READ(VLV_IIR); iir = I915_READ(VLV_IIR);
...@@ -2208,6 +2214,9 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) ...@@ -2208,6 +2214,9 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
u32 de_iir, gt_iir, de_ier, sde_ier = 0; u32 de_iir, gt_iir, de_ier, sde_ier = 0;
irqreturn_t ret = IRQ_NONE; irqreturn_t ret = IRQ_NONE;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
/* We get interrupts on unclaimed registers, so check for this before we /* We get interrupts on unclaimed registers, so check for this before we
* do any I915_{READ,WRITE}. */ * do any I915_{READ,WRITE}. */
intel_uncore_check_errors(dev); intel_uncore_check_errors(dev);
...@@ -2279,6 +2288,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg) ...@@ -2279,6 +2288,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
enum pipe pipe; enum pipe pipe;
u32 aux_mask = GEN8_AUX_CHANNEL_A; u32 aux_mask = GEN8_AUX_CHANNEL_A;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
if (IS_GEN9(dev)) if (IS_GEN9(dev))
aux_mask |= GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C | aux_mask |= GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C |
GEN9_AUX_CHANNEL_D; GEN9_AUX_CHANNEL_D;
...@@ -3771,6 +3783,9 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) ...@@ -3771,6 +3783,9 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT; I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
iir = I915_READ16(IIR); iir = I915_READ16(IIR);
if (iir == 0) if (iir == 0)
return IRQ_NONE; return IRQ_NONE;
...@@ -3951,6 +3966,9 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) ...@@ -3951,6 +3966,9 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT; I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
int pipe, ret = IRQ_NONE; int pipe, ret = IRQ_NONE;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
iir = I915_READ(IIR); iir = I915_READ(IIR);
do { do {
bool irq_received = (iir & ~flip_mask) != 0; bool irq_received = (iir & ~flip_mask) != 0;
...@@ -4171,6 +4189,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) ...@@ -4171,6 +4189,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT; I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
iir = I915_READ(IIR); iir = I915_READ(IIR);
for (;;) { for (;;) {
...@@ -4520,6 +4541,7 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv) ...@@ -4520,6 +4541,7 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
{ {
dev_priv->dev->driver->irq_uninstall(dev_priv->dev); dev_priv->dev->driver->irq_uninstall(dev_priv->dev);
dev_priv->pm.irqs_enabled = false; dev_priv->pm.irqs_enabled = false;
synchronize_irq(dev_priv->dev->irq);
} }
/** /**
......
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