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

[Jesse Barnes]

On Thu, 01 Jul 2010 16:09:00 -0700
Eric Anholt <eric at anholt.net> wrote:

> On Mon, 3 May 2010 12:40:30 -0700, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > 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.
> 
> status update: taking this one off my todo list, since it needs to be
> updated for ironlake

Yes, just ignore this one and the gen3 one.  The don't seem to be
required at least on my 945 for stable page flipping.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center