Re: [PATCH 6/9] drm/i915: Move context initialisation to first-use

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

On 19/04/16 07:49, Chris Wilson wrote:
> Instead of allocating a new request when allocating a context, use the
> request that initiated the allocation to emit the context
> initialisation.

A few words on pros and cons?

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h  |  1 +
>   drivers/gpu/drm/i915/intel_lrc.c | 39 ++++++++++++++++++---------------------
>   2 files changed, 19 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 77becfd5a09d..b65e3b16e1b0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -870,6 +870,7 @@ struct intel_context {
>   		struct i915_vma *lrc_vma;
>   		u64 lrc_desc;
>   		uint32_t *lrc_reg_state;
> +		bool initialised;
>   	} engine[I915_NUM_ENGINES];
>
>   	struct list_head link;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 79ac6f06a502..c104015813d3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -669,9 +669,10 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
>
>   int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>   {
> -	int ret = 0;
> +	struct intel_engine_cs *engine = request->engine;
> +	int ret;
>
> -	request->ringbuf = request->ctx->engine[request->engine->id].ringbuf;
> +	request->ringbuf = request->ctx->engine[engine->id].ringbuf;
>
>   	if (i915.enable_guc_submission) {
>   		/*
> @@ -686,7 +687,20 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>   			return ret;
>   	}
>
> -	return intel_lr_context_pin(request->ctx, request->engine);
> +	ret = intel_lr_context_pin(request->ctx, engine);
> +	if (ret)
> +		return ret;
> +
> +	if (!request->ctx->engine[engine->id].initialised) {
> +		ret = engine->init_context(request);
> +		if (ret) {
> +			intel_lr_context_unpin(request->ctx, engine);
> +			return ret;
> +		}
> +		request->ctx->engine[engine->id].initialised = true;
> +	}
> +
> +	return 0;
>   }
>
>   static int logical_ring_wait_for_space(struct drm_i915_gem_request *req,
> @@ -2576,25 +2590,8 @@ int intel_lr_context_deferred_alloc(struct intel_context *ctx,
>
>   	ctx->engine[engine->id].ringbuf = ringbuf;
>   	ctx->engine[engine->id].state = ctx_obj;
> +	ctx->engine[engine->id].initialised = engine->init_context == NULL;
>
> -	if (ctx != ctx->i915->kernel_context && engine->init_context) {
> -		struct drm_i915_gem_request *req;
> -
> -		req = i915_gem_request_alloc(engine, ctx);
> -		if (IS_ERR(req)) {
> -			ret = PTR_ERR(req);
> -			DRM_ERROR("ring create req: %d\n", ret);
> -			goto error_ringbuf;
> -		}
> -
> -		ret = engine->init_context(req);
> -		i915_add_request_no_flush(req);
> -		if (ret) {
> -			DRM_ERROR("ring init context: %d\n",
> -				ret);
> -			goto error_ringbuf;
> -		}
> -	}
>   	return 0;
>
>   error_ringbuf:
>

Otherwise looks correct. Just missing the motivation explained.

Regards,

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