On Sun, Sep 04, 2011 at 05:35:02PM +0200, Daniel Vetter wrote: > The rps disabling code wasn't properly cancelling outstanding work > items. Also add a comment that explains why we're not racing with > the work item, that could again unmask interrupts. > > This also fixes a bug on module unload because nothing was properly > syncing up with that work item, possibly leading to it being run after > freeing dev_priv or removing the module code. > > Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 56a8554..ccd4600 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7483,10 +7483,16 @@ void gen6_disable_rps(struct drm_device *dev) > > I915_WRITE(GEN6_RPNSWREQ, 1 << 31); > I915_WRITE(GEN6_PMINTRMSK, 0xffffffff); > - I915_WRITE(GEN6_PMIER, 0); > + /* Complete PM interrupt masking here doesn't race with the rps work > + * item again unmasking PM interrupts because that is using PMIMR > + * (logically sitting atop of PMINTRMSK) to mask interrupts. */ > + cancel_work_sync(&dev_priv->rps_work); > > + /* Clear PMIMR and dev_priv->pm_iir in case outstanding work gets > + * cancelled before having run. */
This comment is wrong, you are clearing PMIER, not IMR. IMR gets cleared in enable() iirc > spin_lock_irq(&dev_priv->rps_lock); > dev_priv->pm_iir = 0; > + I915_WRITE(GEN6_PMIER, 0); > spin_unlock_irq(&dev_priv->rps_lock); > > I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR)); > -- > 1.7.6 > I'm not sure this actually fixes a problem. The existing code: 1. disables all interrupts (no more can occur). 2. sets pm_iir to 0 safe in rps lock <workqueues can run at this point, but IMR has no effect with IER = 0> I think you should do the cancel work sync somewhere in the code before module unload (to be correct). I just don't think this fixes a race. Ben _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx