[Intel-gfx] [PATCH 09/53] drm/i915: Add request to execbuf params and add explicit cleanup

[Tomas Elf]

On 19/02/2015 17:17, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
>
> Rather than just having a local request variable in the execbuff code, the
> request pointer is now stored in the execbuff params structure. Also added
> explicit cleanup of the request (plus wiping the OLR to match) in the error
> case. This means that the execbuff code is no longer dependent upon the OLR
> keeping track of the request so as to not leak it when things do go wrong. Note
> that in the success case, the i915_add_request() at the end of the submission
> function will tidy up the request and clear the OLR.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h            |    1 +
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |   13 +++++++++++--
>   2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 90223f208..678b190 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1653,6 +1653,7 @@ struct i915_execbuffer_params {
>   	struct intel_engine_cs          *ring;
>   	struct drm_i915_gem_object      *batch_obj;
>   	struct intel_context            *ctx;
> +	struct drm_i915_gem_request     *request;
>   };
>
>   struct drm_i915_private {
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 37dcc6f..10462f6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1353,7 +1353,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>   	struct i915_address_space *vm;
>   	struct i915_execbuffer_params params_master; /* XXX: will be removed later */
>   	struct i915_execbuffer_params *params = &params_master;
> -	struct drm_i915_gem_request *request;
>   	const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
>   	u32 dispatch_flags;
>   	int ret;
> @@ -1532,7 +1531,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>   		params->batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm);
>
>   	/* Allocate a request for this batch buffer nice and early. */
> -	ret = dev_priv->gt.alloc_request(ring, ctx, &request);
> +	ret = dev_priv->gt.alloc_request(ring, ctx, &params->request);
>   	if (ret)
>   		goto err;
>
> @@ -1565,6 +1564,16 @@ err:
>   	i915_gem_context_unreference(ctx);
>   	eb_destroy(eb);
>
> +	/*
> +	 * If the request was created but not successfully submitted then it
> +	 * must be freed again. If it was submitted then it is being tracked
> +	 * on the active request list and no clean up is required here.
> +	 */
> +	if (ret && params->request) {
> +		i915_gem_request_unreference(params->request);
> +		ring->outstanding_lazy_request = NULL;
> +	}
> +
>   	mutex_unlock(&dev->struct_mutex);
>
>   pre_mutex_err:
>

Reviewed-by: Tomas Elf <tomas.elf at intel.com>

Thanks,
Tomas