[Intel-gfx] [PATCH 32/38] drm/i915: Defer request emission

[Chris Wilson]

On Mon, Sep 26, 2016 at 12:06:19PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-09-26 at 10:04 +0100, Chris Wilson wrote:
> > On Mon, Sep 26, 2016 at 11:53:05AM +0300, Joonas Lahtinen wrote:
> > > 
> > > On ti, 2016-09-20 at 09:30 +0100, Chris Wilson wrote:
> > > > 
> > > > Move the actual emission of the request (the closing breadcrumb) from
> > > > i915_add_request() to the submit callback. (It can be moved later when
> > > > required.) This allows us to defer the allocation of the global_seqno
> > > > from request construction to actual submission, allowing us to emit the
> > > > requests out of order (wrt to the order of their construction, they
> > > > still will only be executed one all of their dependencies are resolved
> > > > including that all earlier requests on their timeline have been
> > > > submitted.) We have to specialise how we then emit the request in order
> > > > to write into the preallocated space, rather than at the tail of the
> > > > ringbuffer (which will have been advanced by the addition of new
> > > > requests).
> > > 
> > > No changelog, so assuming *out++ style change is the only one.
> > 
> > Yeah, it was only stylistic changes, there should have been no
> > functional changes. I went with *out++ after confirming Tvrtko's report
> > that gcc is now smart enough to emit the same code as out[0..N] using
> > *out++.
> 
> It's a nice to have the changelog for the reviewer no matter if it was
> stylistic or not :P
> 
> Anyway, does it somehow get worse if "out" is embedded in the struct
> and not passed alongside it?

I'm not a fan. We have the start of the breadcrumb already, and the
current position of the emitter is not interesting after the event, only
through the function flow. That should be more consistent once we start
only refering to a local pointer for request emission rather than
intel_ring_emit(). (Who knows maybe we won't always be emitting the
requests into the ring in future...) I think storing request->ring_out (or
whatever) just leaves a dangling pointer behind.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre