On Tue, 24 Feb 2015, Imre Deak <imre.d...@intel.com> wrote:
> 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 e11aa362308f5de467ce355a2a2471321b15a35c
> Author: Jesse Barnes <jbar...@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/205
> Reported-and-bisected-by: Klaus Ethgen <kl...@ethgen.ch>
> Signed-off-by: Imre Deak <imre.d...@intel.com>
> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Pushed to drm-intel-fixes, cc: stable, thanks for the patch and review.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/i915_irq.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9073119..1561248 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1889,6 +1889,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void
> *arg)
> u32 iir, gt_iir, pm_iir;
> irqreturn_t ret = IRQ_NONE;
>
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> while (true) {
> /* Find, clear, then process each source of interrupt */
>
> @@ -1933,6 +1936,9 @@ static irqreturn_t cherryview_irq_handler(int irq, void
> *arg)
> u32 master_ctl, iir;
> irqreturn_t ret = IRQ_NONE;
>
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> for (;;) {
> master_ctl = I915_READ(GEN8_MASTER_IRQ) &
> ~GEN8_MASTER_IRQ_CONTROL;
> iir = I915_READ(VLV_IIR);
> @@ -2205,6 +2211,9 @@ static irqreturn_t ironlake_irq_handler(int irq, void
> *arg)
> u32 de_iir, gt_iir, de_ier, sde_ier = 0;
> 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
> * do any I915_{READ,WRITE}. */
> intel_uncore_check_errors(dev);
> @@ -2276,6 +2285,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> enum pipe pipe;
> u32 aux_mask = GEN8_AUX_CHANNEL_A;
>
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> if (IS_GEN9(dev))
> aux_mask |= GEN9_AUX_CHANNEL_B | GEN9_AUX_CHANNEL_C |
> GEN9_AUX_CHANNEL_D;
> @@ -3768,6 +3780,9 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
> I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
> I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
>
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> iir = I915_READ16(IIR);
> if (iir == 0)
> return IRQ_NONE;
> @@ -3948,6 +3963,9 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
> I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
> int pipe, ret = IRQ_NONE;
>
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> iir = I915_READ(IIR);
> do {
> bool irq_received = (iir & ~flip_mask) != 0;
> @@ -4168,6 +4186,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
> I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT |
> I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
>
> + if (!intel_irqs_enabled(dev_priv))
> + return IRQ_NONE;
> +
> iir = I915_READ(IIR);
>
> for (;;) {
> @@ -4517,6 +4538,7 @@ void intel_runtime_pm_disable_interrupts(struct
> drm_i915_private *dev_priv)
> {
> dev_priv->dev->driver->irq_uninstall(dev_priv->dev);
> dev_priv->pm.irqs_enabled = false;
> + synchronize_irq(dev_priv->dev->irq);
> }
>
> /**
> --
> 2.1.0
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
