[Intel-gfx] [PATCH] drm/i915: don't queue flips during a flip pending event

[Jesse Barnes]

On Thu, 29 Apr 2010 15:52:36 -0700
Eric Anholt <eric at anholt.net> wrote:
> > +	if (intel_crtc->plane)
> > +		flip_mask =
> > I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
> > +	else
> > +		flip_mask =
> > I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT; +
> > +	/* Wait for any previous flip to finish */
> > +	while (I915_READ(ISR) & flip_mask)
> > +		;
> > +
> 
> I'm confused about how this patch works.  It looks like there's a race
> where the ISR bit could get set at this point after the loop checking
> it, and you would lose.  The whole thing seems very dubious -- why
> does the state of the ISR at the time you queue a flip matter?  I
> could imagine there being an issue with the state of the ISR at the
> point that a flip packet is processed, but this doesn't handle that
> (seems like only throttling flips to avoid outstanding flips would)

[We talked about this in person; this is for posterity]

We have mutexes and spinlocks in place to make sure only one flip is
queued at once, so checking the ISR should be safe here.

The reason we need to check is that hw could have queued a previous
flip, and we may even have completed the first half of it (due to the
flip pending interrupt).  But until the flip actually completes at
vblank time, queuing another one will be "undefined" according to the
docs.  Turns out in this case "undefined" means hangs and other bad
behavior.

That said, I think this needs an Ironlake check too, since the bits
have moved recently.

Jesse