On Fri, Feb 27, 2015 at 12:34:29PM +0000, John Harrison wrote: > 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@xxxxxxxxx wrote: > >>>From: John Harrison <John.C.Harrison@xxxxxxxxx> > >>> > >>>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). That sounds exactly like what I'd want to see here. So no need for additional frobbing in the mean time imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
