Re: [PATCH 3/3] drm/i915: Update primary planes after a GPU reset

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Feb 18, 2013 at 10:47:37AM +0000, Chris Wilson wrote:
> On Mon, Feb 18, 2013 at 12:15:34PM +0200, Ville Syrjälä wrote:
> > On Fri, Feb 15, 2013 at 11:47:52PM +0000, Chris Wilson wrote:
> > > On Fri, Feb 15, 2013 at 06:56:03PM +0200, Ville Syrjälä wrote:
> > > > On Fri, Feb 15, 2013 at 03:28:33PM +0000, Chris Wilson wrote:
> > > > > On Fri, Feb 15, 2013 at 05:07:46PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > 
> > > > > > GPU reset will drop all flips that are still in the ring. So after the
> > > > > > reset, call update_plane() for all CRTCs to make sure the primary
> > > > > > planes are scanning out from the correct buffer.
> > > > > > 
> > > > > > The base address update will also generate a FLIP_DONE interrupt, which
> > > > > > will complete any pending flips. That means user space will get its
> > > > > > page flip events and won't get stuck waiting for them.
> > > > > 
> > > > > Not for all generations.
> > > > 
> > > > Hmm OK those seem to be the ones with the pending flip status bit
> > > > (Gen4 and older?).
> > > 
> > > Yes. Also notably the ones unlikely to survive the GPU reset :)
> > > 
> > > > But can someone explain why on those platforms we also check the
> > > > vblank interrupt status bit before handling the page flip interrupt?
> > > 
> > > For those generations, we are meant to detect the transition of pending
> > > flip status from 1 -> 0. That transition of course doesn't generate an
> > > interrupt (rather it stops generating one), so the nearest I could come
> > > up with was in anticipating the vblank after we saw the pending flip
> > > status was close enough. In the case of a pending vblank raising an
> > > interrupt just as the pending flip status is asserted, shouldn't MSI
> > > prevent the pending flip from being asserted as we process the IIR for
> > > the vblank. Of course not all of those chipsets even have MSI. I'm not
> > > even sure if we can close that race.
> > 
> > This is what I *think* gen2-gen4 Bspec are telling me:
> > 
> > 1. CS executes MI_DISPLAY_FLIP
> >    -> ISR:flip bit 0 -> 1
> > 2. Flip completes
> >    -> ISR:flip bit 1 -> 0
> >    -> IIR:flip bit 0 -> 1
> >    -> interrupt generated
> 
> Ah, no. The IIR mirrors the ISR and it really does continually generate
> the PendingFlip interrupt on gen2/3.

Continually? Oh that's nasty, but makes sense if it's just a level
interrupt.

> It wasn't until midway through the gen3 cycle, that they introduced the
> behaviour you describe (which is what the "pending flip is actually flip
> done" bit in ECOSKPD is about). But since we already have to try
> and handle gen2/3, I didn't think it was worth a second gen3 code path
> with the revised behaviour.

Ok. I see we never actually enable the pending flip interrupt on gen2/3
due to this. The same race exists there as well then. I can try to make
a patch for that too.

And we turn off the bit in ECOSKPD on gen3 to make sure we always behave
the same way. Makes sense.

> Can you send that as a patch for gen4? As I have a few people hitting
> pageflip system hangs, but only on gen4 afaict.

I must admit that I'm confused now. Which model does Gen4 actually
follow, pending flip or flip done?

-- 
Ville Syrjälä
Intel OTC