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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
