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

[Chris Wilson]

On Mon, Dec 21, 2015 at 04:04:42PM +0000, 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. For
> such requests, we associate them with the default context for the engine
> that the request will be submitted to.
> 
> This patch provides a shorthand for doing such request allocations and
> changes all such calls to use the new function.
> 
> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/i915_gem.c      | 36 ++++++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_display.c |  6 ++++--
>  drivers/gpu/drm/i915/intel_overlay.c | 24 ++++++++++++------------
>  4 files changed, 46 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 666d07c..4955db9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2270,6 +2270,8 @@ struct drm_i915_gem_request {
>  int i915_gem_request_alloc(struct intel_engine_cs *ring,
>  			   struct intel_context *ctx,
>  			   struct drm_i915_gem_request **req_out);
> +struct drm_i915_gem_request * __must_check
> +	i915_gem_request_alloc_anon(struct intel_engine_cs *ring);
>  void i915_gem_request_cancel(struct drm_i915_gem_request *req);
>  void i915_gem_request_free(struct kref *req_ref);
>  int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index be1f984..9f9c0c0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2751,6 +2751,21 @@ err:
>  	return ret;
>  }
>  
> +/*
> + * Allocate a request associated with the default context for the given
> + * ring. This can be used where the driver needs a request for internal
> + * purposes not directly related to a user batch submission.
> + */
> +struct drm_i915_gem_request *
> +i915_gem_request_alloc_anon(struct intel_engine_cs *ring)
> +{

As demonstrated, no. Contexts need to be considered properly first.

> +	struct drm_i915_gem_request *req;
> +	int err;
> +
> +	err = i915_gem_request_alloc(ring, ring->default_context, &req);
> +	return err ? ERR_PTR(err) : req;
> +}
> +
>  void i915_gem_request_cancel(struct drm_i915_gem_request *req)
>  {
>  	intel_ring_reserved_space_cancel(req->ringbuf);
> @@ -3168,9 +3183,13 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj,
>  			return 0;
>  
>  		if (*to_req == NULL) {
> -			ret = i915_gem_request_alloc(to, to->default_context, to_req);
> -			if (ret)
> -				return ret;
> +			struct drm_i915_gem_request *req;
> +
> +			req = i915_gem_request_alloc_anon(to);

Wrong context. Please see patches to fix this mess.

> +			if (IS_ERR(req))
> +				return PTR_ERR(req);
> +
> +			*to_req = req;
>  		}
>  
>  		trace_i915_gem_ring_sync_to(*to_req, from, from_req);
> @@ -3370,9 +3389,9 @@ int i915_gpu_idle(struct drm_device *dev)
>  		if (!i915.enable_execlists) {
>  			struct drm_i915_gem_request *req;
>  
> -			ret = i915_gem_request_alloc(ring, ring->default_context, &req);
> -			if (ret)
> -				return ret;
> +			req = i915_gem_request_alloc_anon(ring);
> +			if (IS_ERR(req))
> +				return PTR_ERR(req);
>  
>  			ret = i915_switch_context(req);
>  			if (ret) {
> @@ -4867,8 +4886,9 @@ i915_gem_init_hw(struct drm_device *dev)
>  	for_each_ring(ring, dev_priv, i) {
>  		struct drm_i915_gem_request *req;
>  
> -		ret = i915_gem_request_alloc(ring, ring->default_context, &req);
> -		if (ret) {
> +		req = i915_gem_request_alloc_anon(ring);
> +		if (IS_ERR(req)) {
> +			ret = PTR_ERR(req);
>  			i915_gem_cleanup_ringbuffer(dev);
>  			goto out;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index abd2d29..5716f4a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11662,9 +11662,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  					obj->last_write_req);
>  	} else {
>  		if (!request) {
> -			ret = i915_gem_request_alloc(ring, ring->default_context, &request);
> -			if (ret)

Wrong context.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre