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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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

?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx