[Intel-gfx] [PATCH 14/15] drm/i915: Track frontbuffer invalidation/flushing

[Chris Wilson]

On Tue, Jun 17, 2014 at 11:33:41AM +0200, Daniel Vetter wrote:
> On Tue, Jun 17, 2014 at 09:10:14AM +0100, Chris Wilson wrote:
> > On Tue, Jun 17, 2014 at 09:52:17AM +0200, Daniel Vetter wrote:
> > > On Tue, Jun 17, 2014 at 08:00:05AM +0100, Chris Wilson wrote:
> > > > On Mon, Jun 16, 2014 at 07:51:34PM +0200, Daniel Vetter wrote:
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > index b8359f4a6dc4..dfdbf2a02844 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -2755,6 +2755,8 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> > > > >  
> > > > >  	dev_priv->display.update_primary_plane(crtc, fb, x, y);
> > > > >  
> > > > > +	intel_frontbuffer_flush(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
> > > > 
> > > > Conceptually all of these are just intel_fb_flip_complete. It may be
> > > > easier in your tracking scheme to have:
> > > > 
> > > > intel_fb_flip() { intel_fb_flip_prepare(); intel_fb_fip_complete(); }
> > > 
> > > Yeah, this is just the direct flush for synchronous updates. Implementing
> > > that as a prepare+complete is imo too confusing - the entire point of
> > > prepare+complete is to catch an intermediate invalidates and not complete
> > > the flush and so only really required for async flips. Hence why I prefer
> > > to only do the full prepare+complete dance where needed. Otherwise there
> > > is nothing special about flips.
> > 
> > I just prefer to have the like operations having like names. The key
> > point here is that we just want to treat the set-plane operations as a
> > synchronous flip, which is distinct to the invalidate/flush operations on
> > an active frontbuffer.
> 
> Well the main thing is the setplane is a flush operation. Flip is also a
> flush, but an asynchronous one. Maybe s/flip/flush/ instead to make it
> clear that the special part isn't that it's a flip, but that it's
> two-stage?

Yes, having s/flip/flush/ is preferrable from a consistency pov. I am
just not yet sold on that setplane is a intel_fb_flush. I think the busy
state tracking on the new fb is distinct from the state tracking on the
old fb, and we are marking the transition from one fb to the other. I
would rather keep the invalidate/flush as boundaries on the current fb,
with the distinction made for change of fb.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre