[Intel-gfx] [PATCH v4 1/3] drm/i915: simplify allocation of driver-internal requests

[Chris Wilson]

On Wed, Jan 20, 2016 at 09:56:10AM +0000, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 19/01/16 19:02, Dave Gordon wrote:
> >There are a number of places where the driver needs a request, but isn't
> >working on behalf of any specific user or in a specific context. At
> >present, we associate them with the per-engine default context. A future
> >patch will abolish those per-engine context pointers; but we can already
> >eliminate a lot of the references to them, just by making the allocator
> >allow NULL as a shorthand for "an appropriate context for this ring",
> >which will mean that the callers don't need to know anything about how
> >the "appropriate context" is found (e.g. per-ring vs per-device, etc).
> >
> >So this patch renames the existing i915_gem_request_alloc(), and makes
> >it local (static inline), and replaces it with a wrapper that provides
> >a default if the context is NULL, and also has a nicer calling
> >convention (doesn't require a pointer to an output parameter). Then we
> >change all callers to use the new convention:
> >OLD:
> >	err = i915_gem_request_alloc(ring, user_ctx, &req);
> >	if (err) ...
> >NEW:
> >	req = i915_gem_request_alloc(ring, user_ctx);
> >	if (IS_ERR(req)) ...
> >OLD:
> >	err = i915_gem_request_alloc(ring, ring->default_context, &req);
> >	if (err) ...
> >NEW:
> >	req = i915_gem_request_alloc(ring, NULL);
> >	if (IS_ERR(req)) ...
> >
> >v4:	Rebased
> 
> Wasn't following the discussion in detail but there was always big
> resistance towards API which takes NULLs inferring some default
> state. At least in some of my past work that was an repeated
> objection. Maybe a way around it would be to have two functions,
> like:
> 
> i915_gem_request_alloc(ring); // default context
> i915_gem_request_alloc_ctx(ring, ctx); // user context

There is only one piece of code where we even want to use a default
kernel context, so it makes no sense from that point of view.
Imho, knowing the context is paramount to achieving context separation
in GEM - which we are far from achieving yet.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre