Re: [PATCH 08/59] drm/i915: Set context in request from creation even in legacy mode

[Tomas Elf <tomas.elf@intel.com>]

On 19/03/2015 12:30, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> In execlist mode, the context object pointer is written in to the request
> structure (and reference counted) at the point of request creation. In legacy
> mode, this only happens inside i915_add_request().
>
> This patch updates the legacy code path to match the execlist version. This
> allows all the intermediate code between request creation and request submission
> to get at the context object given only a request structure. Thus negating the
> need to pass context pointers here, there and everywhere.
>
> v2: Moved the context reference so it does not need to be undone if the
> get_seqno() fails.
>
> v3: Fixed execlist mode always hitting a warning about invalid last_contexts
> (which don't exist in execlist mode).
>
> v4: Updated for new i915_gem_request_alloc() scheme.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c  |   14 +++++---------
>   drivers/gpu/drm/i915/intel_lrc.c |   11 ++++-------
>   drivers/gpu/drm/i915/intel_lrc.h |    3 +--
>   3 files changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index cdf1c9d..f35ac7f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2393,14 +2393,7 @@ void __i915_add_request(struct intel_engine_cs *ring,
>   	 */
>   	request->batch_obj = obj;
>
> -	if (!i915.enable_execlists) {
> -		/* Hold a reference to the current context so that we can inspect
> -		 * it later in case a hangcheck error event fires.
> -		 */
> -		request->ctx = ring->last_context;
> -		if (request->ctx)
> -			i915_gem_context_reference(request->ctx);
> -	}
> +	WARN_ON(!i915.enable_execlists && (request->ctx != ring->last_context));
>
>   	request->emitted_jiffies = jiffies;
>   	list_add_tail(&request->list, &ring->request_list);
> @@ -2545,12 +2538,15 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring,
>   	kref_init(&request->ref);
>   	request->ring = ring;
>   	request->uniq = dev_private->request_uniq++;
> +	request->ctx  = ctx;
> +	i915_gem_context_reference(request->ctx);
>
>   	if (i915.enable_execlists)
> -		ret = intel_logical_ring_alloc_request_extras(request, ctx);
> +		ret = intel_logical_ring_alloc_request_extras(request);
>   	else
>   		ret = intel_ring_alloc_request_extras(request);
>   	if (ret) {
> +		i915_gem_context_unreference(request->ctx);
>   		kfree(request);
>   		return ret;
>   	}
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 8d48761..a0ce65b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -611,20 +611,17 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
>   	return logical_ring_invalidate_all_caches(ringbuf, ctx);
>   }
>
> -int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request,
> -					    struct intel_context *ctx)
> +int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>   {
>   	int ret;
>
> -	if (ctx != request->ring->default_context) {
> -		ret = intel_lr_context_pin(request->ring, ctx);
> +	if (request->ctx != request->ring->default_context) {
> +		ret = intel_lr_context_pin(request->ring, request->ctx);
>   		if (ret)
>   			return ret;
>   	}
>
> -	request->ringbuf = ctx->engine[request->ring->id].ringbuf;
> -	request->ctx     = ctx;
> -	i915_gem_context_reference(request->ctx);
> +	request->ringbuf = request->ctx->engine[request->ring->id].ringbuf;
>
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 04d3a6d..4148de0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -36,8 +36,7 @@
>   #define RING_CONTEXT_STATUS_PTR(ring)	((ring)->mmio_base+0x3a0)
>
>   /* Logical Rings */
> -int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request,
> -					    struct intel_context *ctx);
> +int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
>   void intel_logical_ring_stop(struct intel_engine_cs *ring);
>   void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
>   int intel_logical_rings_init(struct drm_device *dev);
>

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

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