[Intel-gfx] [PATCH 08/51] drm/i915: Update alloc_request to return the allocated request

[John Harrison]

On 25/02/2015 21:08, Daniel Vetter wrote:
> On Fri, Feb 13, 2015 at 12:21:29PM +0000, Chris Wilson wrote:
>> On Fri, Feb 13, 2015 at 11:48:17AM +0000, John.C.Harrison at Intel.com wrote:
>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>
>>> The alloc_request() function does not actually return the newly allocated
>>> request. Instead, it must be pulled from ring->outstanding_lazy_request. This
>>> patch fixes this so that code can create a request and start using it knowing
>>> exactly which request it actually owns.
>> Why do we have different functions in the first place?
> There seems to be a bit a layer fumble going on with the lrc alloc request
> also pinning the lrc context. We could pull that out and then share the
> function again since there's indeed no reason no to. At least afaics.
>
> Also we should probably assign the ctx (if there is any) right in the
> request alloc function so that these two bits are always tied together.
> -Daniel

See later patch 'set context in request creation...'. That moves the ctx 
assignment out of _i915_add_request() and into alloc_request() for the 
legacy code path. Thus bringing the legacy and lrc versions back into 
alignment. As for the pinning, I am leaving that exactly as is as I 
don't really know the ins and outs of it. One of the execlist experts 
might be able to comment as to whether that is the right place for it or 
not.

Also, I am currently working on the conversion to struct fence. As part 
of that, the request allocation changes quite a bit. Rather than have 
two clones of the code that have to be independently maintained, I have 
a patch to unify the common portion. We then have 
i915_gem_request_alloc() that all the driver calls instead of the 
indirected function pointer. That then does all the common work and 
chains on to the legacy/execlist specific helper at the end (which 
currently only sets the ringbuffer field for legacy mode and also does 
the LRC pinning for execlist mode).