diff mbox

[1/3] drm/i915: close PM interrupt masking races in the irq handler

Message ID 1315150502-12537-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Sept. 4, 2011, 3:35 p.m. UTC
Quoting Chris Wilson's more concise description:

"Ah I think I see the problem. As you point out we only mask the
current interrupt received, so that if we have a task pending (and so
IMR != 0) we actually unmask the pending interrupt and so could
receive it again before the tasklet is finally kicked off by the
grumpy scheduler."

So we need the hw to issue PM interrupts A, B, A while the scheduler
is hating us and refuses to run the rps work item. On receiving PM
interrupt A we hit the
WARN because

dev_priv->pm_iir == PM_A | PM_B

Also add a posting read as suggested by Chris to ensure proper
ordering of the writes to PMIMR and PMIIR. Just in case somebody
weakens write ordering.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Comments

Ben Widawsky Sept. 4, 2011, 5:09 p.m. UTC | #1
On Sun,  4 Sep 2011 17:35:00 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Quoting Chris Wilson's more concise description:
> 
> "Ah I think I see the problem. As you point out we only mask the
> current interrupt received, so that if we have a task pending (and so
> IMR != 0) we actually unmask the pending interrupt and so could
> receive it again before the tasklet is finally kicked off by the
> grumpy scheduler."
> 
> So we need the hw to issue PM interrupts A, B, A while the scheduler
> is hating us and refuses to run the rps work item. On receiving PM
> interrupt A we hit the
> WARN because
> 
> dev_priv->pm_iir == PM_A | PM_B
> 
> Also add a posting read as suggested by Chris to ensure proper
> ordering of the writes to PMIMR and PMIIR. Just in case somebody
> weakens write ordering.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9cbb0cd..2fdd9f9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -536,8 +536,9 @@  static irqreturn_t ivybridge_irq_handler(DRM_IRQ_ARGS)
 		unsigned long flags;
 		spin_lock_irqsave(&dev_priv->rps_lock, flags);
 		WARN(dev_priv->pm_iir & pm_iir, "Missed a PM interrupt\n");
-		I915_WRITE(GEN6_PMIMR, pm_iir);
 		dev_priv->pm_iir |= pm_iir;
+		I915_WRITE(GEN6_PMIMR, dev_priv->pm_iir);
+		POSTING_READ(GEN6_PMIMR);
 		spin_unlock_irqrestore(&dev_priv->rps_lock, flags);
 		queue_work(dev_priv->wq, &dev_priv->rps_work);
 	}
@@ -649,8 +650,9 @@  static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS)
 		unsigned long flags;
 		spin_lock_irqsave(&dev_priv->rps_lock, flags);
 		WARN(dev_priv->pm_iir & pm_iir, "Missed a PM interrupt\n");
-		I915_WRITE(GEN6_PMIMR, pm_iir);
 		dev_priv->pm_iir |= pm_iir;
+		I915_WRITE(GEN6_PMIMR, dev_priv->pm_iir);
+		POSTING_READ(GEN6_PMIMR);
 		spin_unlock_irqrestore(&dev_priv->rps_lock, flags);
 		queue_work(dev_priv->wq, &dev_priv->rps_work);
 	}