Re: [PATCH 1/2] drm/i915: fix race when clearing RPS IIR bits

Chris Wilson
2015-03-24T09:24:17Z

On Tue, Mar 24, 2015 at 10:14:03AM +0100, Daniel Vetter wrote:
> On Mon, Mar 23, 2015 at 09:10:15PM +0000, Chris Wilson wrote:
> > On Mon, Mar 23, 2015 at 07:11:34PM +0200, Imre Deak wrote:
> > > When disabling RPS interrupts there is a race where we disable RPS
> > > inerrupts while the interrupt handler is running and the handler has
> > > already latched the pending RPS interrupt from the master IIR register.
> > > Afterwards the disabling path clears the PM IIR bits, making the state
> > > of pending interrupts inconsistent from the interrupt handler's point of
> > > view. This triggers the following warning: "The master control interrupt
> > > lied (PM)!".
> > >
> > > To fix this make sure that any running interrupt handler (which may
> > > have already latched the master IIR) finishes before clearing the IIR
> > > bits.
> > >
> >
> > Isn't this overkill for what is just a bogus WARN? If the WARN is a
> > logical consequence of the code, let's just remove the WARN.
> >
> > Or iow can you not find a cheaper way to fix this?
>
> We only run this on suspend/resume afaik, overhead should be acceptable.
> And we've had that much overhead before we've done all the runtime pm
> unification, it's still less synchronization than disabling interrupts
> completely.

Hmm, I thought this was in conjunction with RPS pm masking (i.e. fired
everytime we no longer expect to receive RPS interrupts). If it is only
the infrequent, then yeah I can't complain too much - I still think it
is slightly fishy, but I can accept that it is just a quirk of the
buffering the interrupt does.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre