diff mbox

[03/11] drm/i915: return ctx->global_id from intel_execlists_ctx_id()

Message ID 1455600439-18480-4-git-send-email-sourab.gupta@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sourab.gupta@intel.com Feb. 16, 2016, 5:27 a.m. UTC 
From: Robert Bragg <robert@sixbynine.org>

The newly added intel_context::global_id is suitable (a globally unique
20 bit ID) for giving to the hardware as a unique context identifier.

Compared to using the pinned address of a logical ring context these IDs
are constant for the lifetime of a context whereas a context could be
repinned at different addresses during its lifetime.

Having a stable ID is useful when we need to buffer information
associated with a context based on this ID so the association can't be
lost. For example the OA unit writes out counter reports to a circular
buffer tagged with this ID and we want to be able to accurately filter
reports for a specific context, ideally without the added complexity of
tracking context re-pinning while the OA buffer may contain reports with
older IDs.

Cc: Sourab Gupta <sourab.gupta@intel.com>
Signed-off-by: Robert Bragg <robert@sixbynine.org>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  7 ++++---
 drivers/gpu/drm/i915/intel_lrc.c    | 22 ++++++++++------------
 drivers/gpu/drm/i915/intel_lrc.h    |  2 +-
 3 files changed, 15 insertions(+), 16 deletions(-)

Comments

Dave Gordon Feb. 16, 2016, 9:34 a.m. UTC | #1 
On 16/02/16 05:27, sourab.gupta@intel.com wrote:
> From: Robert Bragg <robert@sixbynine.org>
>
> The newly added intel_context::global_id is suitable (a globally unique
> 20 bit ID) for giving to the hardware as a unique context identifier.
>
> Compared to using the pinned address of a logical ring context these IDs
> are constant for the lifetime of a context whereas a context could be
> repinned at different addresses during its lifetime.
>
> Having a stable ID is useful when we need to buffer information
> associated with a context based on this ID so the association can't be
> lost. For example the OA unit writes out counter reports to a circular
> buffer tagged with this ID and we want to be able to accurately filter
> reports for a specific context, ideally without the added complexity of
> tracking context re-pinning while the OA buffer may contain reports with
> older IDs.

Unfortunately, I suspect that the GuC firmware has conflated the context 
ID (which ought to be a purely software-defined tag) with the (hardware) 
GTT page address of the LRC. So this will probably break GuC submission.

But it looks like a good idea otherwise; maybe we can get the GuC 
firmware updated so it doesn't make this assumption ...

.Dave.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8aab974..ff4a6fe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1970,6 +1970,7 @@  static int i915_context_status(struct seq_file *m, void *unused)
 
 static void i915_dump_lrc_obj(struct seq_file *m,
 			      struct intel_engine_cs *ring,
+			      struct intel_context *ctx,
 			      struct drm_i915_gem_object *ctx_obj)
 {
 	struct page *page;
@@ -1984,7 +1985,7 @@  static void i915_dump_lrc_obj(struct seq_file *m,
 	}
 
 	seq_printf(m, "CONTEXT: %s %u\n", ring->name,
-		   intel_execlists_ctx_id(ctx_obj));
+		   intel_execlists_ctx_id(ctx));
 
 	if (!i915_gem_obj_ggtt_bound(ctx_obj))
 		seq_puts(m, "\tNot bound in GGTT\n");
@@ -2033,7 +2034,7 @@  static int i915_dump_lrc(struct seq_file *m, void *unused)
 	list_for_each_entry(ctx, &dev_priv->context_list, link) {
 		for_each_ring(ring, dev_priv, i) {
 			if (ring->default_context != ctx)
-				i915_dump_lrc_obj(m, ring,
+				i915_dump_lrc_obj(m, ring, ctx,
 						  ctx->engine[i].state);
 		}
 	}
@@ -2112,7 +2113,7 @@  static int i915_execlists(struct seq_file *m, void *data)
 
 			ctx_obj = head_req->ctx->engine[ring_id].state;
 			seq_printf(m, "\tHead request id: %u\n",
-				   intel_execlists_ctx_id(ctx_obj));
+				   intel_execlists_ctx_id(head_req->ctx));
 			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 40bda8d..4789555 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -260,7 +260,7 @@  int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
 
 /**
  * intel_execlists_ctx_id() - get the Execlists Context ID
- * @ctx_obj: Logical Ring Context backing object.
+ * @ctx: LR context
  *
  * 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
@@ -269,15 +269,15 @@  int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
  * interrupts.
  *
  * Return: 20-bits globally unique context ID.
+ *
+ * Further the ID given to HW can now be relied on to be constant for
+ * the lifetime of the context, unlike previously when we used an
+ * associated logical ring context address (which could be repinned at
+ * a different address).
  */
-u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
+u32 intel_execlists_ctx_id(struct intel_context *ctx)
 {
-	u32 lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
-			LRC_PPHWSP_PN * PAGE_SIZE;
-
-	/* LRCA is required to be 4K aligned so the more significant 20 bits
-	 * are globally unique */
-	return lrca >> 12;
+	return ctx->global_id;
 }
 
 static bool disable_lite_restore_wa(struct intel_engine_cs *ring)
@@ -305,7 +305,7 @@  uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
 		desc |= GEN8_CTX_L3LLC_COHERENT;
 	desc |= GEN8_CTX_PRIVILEGE;
 	desc |= lrca;
-	desc |= (u64)intel_execlists_ctx_id(ctx_obj) << GEN8_CTX_ID_SHIFT;
+	desc |= (u64)intel_execlists_ctx_id(ctx) << GEN8_CTX_ID_SHIFT;
 
 	/* TODO: WaDisableLiteRestore when we start using semaphore
 	 * signalling between Command Streamers */
@@ -475,9 +475,7 @@  static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 					    execlist_link);
 
 	if (head_req != NULL) {
-		struct drm_i915_gem_object *ctx_obj =
-				head_req->ctx->engine[ring->id].state;
-		if (intel_execlists_ctx_id(ctx_obj) == request_id) {
+		if (intel_execlists_ctx_id(head_req->ctx) == request_id) {
 			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 4e60d54..1b08cd2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -93,7 +93,7 @@  struct i915_execbuffer_params;
 int intel_execlists_submission(struct i915_execbuffer_params *params,
 			       struct drm_i915_gem_execbuffer2 *args,
 			       struct list_head *vmas);
-u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
+u32 intel_execlists_ctx_id(struct intel_context *ctx);
 
 void intel_lrc_irq_handler(struct intel_engine_cs *ring);
 void intel_execlists_retire_requests(struct intel_engine_cs *ring);