On Fri, Feb 20, 2015 at 09:09:02PM +0200, Imre Deak wrote: > +static void intel_irq_set_state(struct drm_i915_private *dev_priv, > + bool enabled) > +{ > + dev_priv->pm.irqs_enabled = enabled; > + /* > + * Before we unmask the interrupt or synchronize against it, make sure > + * that a corresponding interrupt handler running on another CPU sees > + * the updated irqs_enabled value. > + */ > + smp_mb(); I would like a comment here to say something like: before powering down the hardware make sure that we have no further access via pending interrupt handlers. At the moment, I read the block comment above and thought you were trying to imply something about the serialisation from synchronize_irq() - but the only reason I can see you want sync_irq here is to be sure that once the irq is disabled it is not going to run again. > + if (!enabled) > + synchronize_irq(dev_priv->dev->irq); > +} -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx