[Intel-gfx] [PATCH] drm/i915: avoid processing spurious/shared interrupts in low-power states

[Imre Deak]

On Fri, 2015-02-20 at 19:40 +0000, Chris Wilson wrote:
> 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.

Right. With sync_irq I want to wait for any running handler on another
CPU to finish. Subsequent ones due to any shared/spurious interrupts
will also not run since the barrier guarantees that they see already the
updated value and do an early return. Will add a comment about this.

> 
> > +	if (!enabled)
> > +		synchronize_irq(dev_priv->dev->irq);
> > +}
>