Re: [PATCH v3 13/21] drm/i915: Replace the pinned context address with its unique ID

[Dave Gordon <david.s.gordon@intel.com>]

On 24/04/16 19:10, Chris Wilson wrote:
> Rather than reuse the current location of the context in the global GTT
> for its hardware identifier, use the context's unique ID assigned to it
> for its whole lifetime.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Hmmm I wonder whether this will work with the GuC?
IIRC the GuC firmware requires the LRCA to be used as the CTX ID :(
Or at least it used to, maybe that has changed now.

.Dave.

> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++-------
>   drivers/gpu/drm/i915/intel_lrc.c    | 36 ++++++------------------------------
>   drivers/gpu/drm/i915/intel_lrc.h    |  3 ---
>   3 files changed, 11 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3bd2f89933ff..be2a4a0fae13 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2043,15 +2043,13 @@ static void i915_dump_lrc_obj(struct seq_file *m,
>   	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
>   	unsigned long ggtt_offset = 0;
>
> +	seq_printf(m, "CONTEXT: %s %u\n", engine->name, ctx->hw_id);
> +
>   	if (ctx_obj == NULL) {
> -		seq_printf(m, "Context on %s with no gem object\n",
> -			   engine->name);
> +		seq_puts(m, "\tNot allocated\n");
>   		return;
>   	}
>
> -	seq_printf(m, "CONTEXT: %s %u\n", engine->name,
> -		   intel_execlists_ctx_id(ctx, engine));
> -
>   	if (!i915_gem_obj_ggtt_bound(ctx_obj))
>   		seq_puts(m, "\tNot bound in GGTT\n");
>   	else
> @@ -2170,8 +2168,8 @@ static int i915_execlists(struct seq_file *m, void *data)
>
>   		seq_printf(m, "\t%d requests in queue\n", count);
>   		if (head_req) {
> -			seq_printf(m, "\tHead request id: %u\n",
> -				   intel_execlists_ctx_id(head_req->ctx, engine));
> +			seq_printf(m, "\tHead request context: %u\n",
> +				   head_req->ctx->hw_id);
>   			seq_printf(m, "\tHead request tail: %u\n",
>   				   head_req->tail);
>   		}
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6179b591ee84..2ed7363f76ea 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -314,14 +314,12 @@ static void
>   intel_lr_context_descriptor_update(struct intel_context *ctx,
>   				   struct intel_engine_cs *engine)
>   {
> -	uint64_t lrca, desc;
> +	u64 desc;
>
> -	lrca = ctx->engine[engine->id].lrc_vma->node.start +
> -	       LRC_PPHWSP_PN * PAGE_SIZE;
> -
> -	desc = engine->ctx_desc_template;			   /* bits  0-11 */
> -	desc |= lrca;					   /* bits 12-31 */
> -	desc |= (lrca >> PAGE_SHIFT) << GEN8_CTX_ID_SHIFT; /* bits 32-51 */
> +	desc = engine->ctx_desc_template; /* bits  0-11 */
> +	desc |= ctx->engine[engine->id].lrc_vma->node.start +
> +	       LRC_PPHWSP_PN * PAGE_SIZE; /* bits 12-31 */
> +	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-51 */
>
>   	ctx->engine[engine->id].lrc_desc = desc;
>   }
> @@ -332,28 +330,6 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
>   	return ctx->engine[engine->id].lrc_desc;
>   }
>
> -/**
> - * intel_execlists_ctx_id() - get the Execlists Context ID
> - * @ctx: Context to get the ID for
> - * @ring: Engine to get the ID for
> - *
> - * Do not confuse with ctx->id! Unfortunately we have a name overload
> - * here: the old context ID we pass to userspace as a handler so that
> - * they can refer to a context, and the new context ID we pass to the
> - * ELSP so that the GPU can inform us of the context status via
> - * interrupts.
> - *
> - * The context ID is a portion of the context descriptor, so we can
> - * just extract the required part from the cached descriptor.
> - *
> - * Return: 20-bits globally unique context ID.
> - */
> -u32 intel_execlists_ctx_id(struct intel_context *ctx,
> -			   struct intel_engine_cs *engine)
> -{
> -	return intel_lr_context_descriptor(ctx, engine) >> GEN8_CTX_ID_SHIFT;
> -}
> -
>   static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
>   				 struct drm_i915_gem_request *rq1)
>   {
> @@ -499,7 +475,7 @@ execlists_check_remove_request(struct intel_engine_cs *engine, u32 request_id)
>   	if (!head_req)
>   		return 0;
>
> -	if (unlikely(intel_execlists_ctx_id(head_req->ctx, engine) != request_id))
> +	if (unlikely(head_req->ctx->hw_id != request_id))
>   		return 0;
>
>   	WARN(head_req->elsp_submitted == 0, "Never submitted head request\n");
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 461f1ef9b5c1..b17ab79333aa 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -114,9 +114,6 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
>   uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
>   				     struct intel_engine_cs *engine);
>
> -u32 intel_execlists_ctx_id(struct intel_context *ctx,
> -			   struct intel_engine_cs *engine);
> -
>   /* Execlists */
>   int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists);
>   struct i915_execbuffer_params;
>

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