| Message ID | 1315106655-26782-1-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 3 Sep 2011 20:24:15 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > I couldn't reproduce this one, but... Interrupt mask state is lost if > three interrupts occur before the workqueue has run. > > Should be straight forward to reproduce even without SMP. I'm pretty > sure Dan Vetter was trying to explain this to me, and I couldn't get it. > My solution I think is different than his though. This logic is now duplicated in ivybridge_irq_handler(). This simply fits the scenario Daniel described, whilst also fitting in with our understanding of IMR, IER and IIR. (A big assumption ;-) Reported-by: Soeren Sonnenburg <sonne@debian.org> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Hi all, Let's have some new ideas for races around the rps code. Not yet tested (I can't hit the WARN anyway in my testing), I've just wanted to get this out for discussion. Yours, Daniel Daniel Vetter (3): drm/i915: close PM interrupt masking races in the irq handler drm/i915: close PM interrupt masking races in the rps work func drm/i915: close rps work vs. rps disable races drivers/gpu/drm/i915/i915_irq.c | 8 +++++--- drivers/gpu/drm/i915/intel_display.c | 8 +++++++- 2 files changed, 12 insertions(+), 4 deletions(-)
On Sun, 04 Sep 2011 10:03:21 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Sat, 3 Sep 2011 20:24:15 -0700, Ben Widawsky <ben@bwidawsk.net> > wrote: > > I couldn't reproduce this one, but... Interrupt mask state is lost > > if three interrupts occur before the workqueue has run. > > > > Should be straight forward to reproduce even without SMP. I'm pretty > > sure Dan Vetter was trying to explain this to me, and I couldn't > > get it. My solution I think is different than his though. > > This logic is now duplicated in ivybridge_irq_handler(). This simply > fits the scenario Daniel described, whilst also fitting in with our > understanding of IMR, IER and IIR. (A big assumption ;-) > > Reported-by: Soeren Sonnenburg <sonne@debian.org> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris > I completely screwed this one up because I was tired. Chris sent me this exact patch, which I told him was wrong, and then proceeded to rewrite the same thing on my own. Chris, I suggest you resubmit what you sent me privately with my reviewed by, and please accept my apology. I am nak'ing my patch. Ben
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9cbb0cd..55518e3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -649,8 +649,8 @@ 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); spin_unlock_irqrestore(&dev_priv->rps_lock, flags); queue_work(dev_priv->wq, &dev_priv->rps_work); }
I couldn't reproduce this one, but... Interrupt mask state is lost if three interrupts occur before the workqueue has run. Should be straight forward to reproduce even without SMP. I'm pretty sure Dan Vetter was trying to explain this to me, and I couldn't get it. My solution I think is different than his though. Here is a sample failure: <pm irq 1<<0 occurs> local iir = 1 spin lock mask_pm_irq(1 << 0); dev_priv->iir = 1; spin_unlock clear_pm_irq(1 << 0); <pm irq 1 << 1 occurs> local iir = 2 spin lock mask_pm_irq(1 << 1); // here is the bug dev_priv->iir = 3; clear_pm_irq(1 << 1); <pm irq 1 << 0 occurs again> local iir = 1 spin lock WARN!!! Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_irq.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)