[PATCH 02/76] drm/i915: Reduce context HW ID lifetime

Chris Wilson chris at chris-wilson.co.uk
Sat Jun 2 09:37:05 UTC 2018


Future gen reduce the number of bits we will have available to
differentiate between contexts, so reduce the lifetime of the ID
assignment from that of the context to its current active cycle (i.e.
only while it is pinned for use by the HW, will it have a constant ID).
This means that instead of a max of 2k allocated contexts (worst case
before fun with bit twiddling), we instead have a limit of 2k in flight
contexts (minus a few that have been pinned by the kernel or by perf).

We cannot reduce the scope of an HW-ID to an engine (allowing the same
gem_context to have different ids on each engine) as in the future we
will need to preassign an id before we know which engine the
context is being executed on.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Cc: Oscar Mateo <oscar.mateo at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Mika Kuoppala <mika.kuoppala at intel.com>
Cc: Michel Thierry <michel.thierry at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c           |   5 +-
 drivers/gpu/drm/i915/i915_drv.h               |   1 +
 drivers/gpu/drm/i915/i915_gem_context.c       | 201 +++++++++++++-----
 drivers/gpu/drm/i915/i915_gem_context.h       |   5 +
 drivers/gpu/drm/i915/i915_perf.c              |  35 ++-
 drivers/gpu/drm/i915/i915_request.c           |   6 +-
 drivers/gpu/drm/i915/i915_request.h           |   2 +-
 drivers/gpu/drm/i915/i915_trace.h             |   2 +-
 drivers/gpu/drm/i915/intel_lrc.c              |   8 +
 drivers/gpu/drm/i915/selftests/mock_context.c |  11 +-
 10 files changed, 190 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 15e86d34a81c..ccecaf96cc91 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1955,7 +1955,10 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		return ret;
 
 	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
-		seq_printf(m, "HW context %u ", ctx->hw_id);
+		seq_puts(m, "HW context ");
+		if (!list_empty(&ctx->hw_id_link))
+			seq_printf(m, "%x [pin %u]",
+				   ctx->hw_id, ctx->pin_hw_id);
 		if (ctx->pid) {
 			struct task_struct *task;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 38157df6ff5c..21729d648269 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1842,6 +1842,7 @@ struct drm_i915_private {
 		struct ida hw_ida;
 #define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
 #define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
+		struct list_head hw_id_list;
 	} contexts;
 
 	u32 fdi_rx_config;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 94e4db1870aa..aa7ed5a9cb7b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -136,11 +136,15 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
 
 	list_del(&ctx->link);
 
-	ida_simple_remove(&ctx->i915->contexts.hw_ida, ctx->hw_id);
+	if (!list_empty(&ctx->hw_id_link)) {
+		ida_simple_remove(&ctx->i915->contexts.hw_ida, ctx->hw_id);
+		list_del(&ctx->hw_id_link);
+	}
+
 	kfree_rcu(ctx, rcu);
 }
 
-static void contexts_free(struct drm_i915_private *i915)
+static bool contexts_free(struct drm_i915_private *i915)
 {
 	struct llist_node *freed = llist_del_all(&i915->contexts.free_list);
 	struct i915_gem_context *ctx, *cn;
@@ -149,6 +153,8 @@ static void contexts_free(struct drm_i915_private *i915)
 
 	llist_for_each_entry_safe(ctx, cn, freed, free_link)
 		i915_gem_context_free(ctx);
+
+	return freed;
 }
 
 static void contexts_free_first(struct drm_i915_private *i915)
@@ -203,34 +209,6 @@ static void context_close(struct i915_gem_context *ctx)
 	i915_gem_context_put(ctx);
 }
 
-static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
-{
-	int ret;
-	unsigned int max;
-
-	if (INTEL_GEN(dev_priv) >= 11)
-		max = GEN11_MAX_CONTEXT_HW_ID;
-	else
-		max = MAX_CONTEXT_HW_ID;
-
-	ret = ida_simple_get(&dev_priv->contexts.hw_ida,
-			     0, max, GFP_KERNEL);
-	if (ret < 0) {
-		/* Contexts are only released when no longer active.
-		 * Flush any pending retires to hopefully release some
-		 * stale contexts and try again.
-		 */
-		i915_retire_requests(dev_priv);
-		ret = ida_simple_get(&dev_priv->contexts.hw_ida,
-				     0, max, GFP_KERNEL);
-		if (ret < 0)
-			return ret;
-	}
-
-	*out = ret;
-	return 0;
-}
-
 static u32 default_desc_template(const struct drm_i915_private *i915,
 				 const struct i915_hw_ppgtt *ppgtt)
 {
@@ -267,12 +245,6 @@ __create_hw_context(struct drm_i915_private *dev_priv,
 	if (ctx == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	ret = assign_hw_id(dev_priv, &ctx->hw_id);
-	if (ret) {
-		kfree(ctx);
-		return ERR_PTR(ret);
-	}
-
 	kref_init(&ctx->ref);
 	list_add_tail(&ctx->link, &dev_priv->contexts.list);
 	ctx->i915 = dev_priv;
@@ -286,6 +258,7 @@ __create_hw_context(struct drm_i915_private *dev_priv,
 
 	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
 	INIT_LIST_HEAD(&ctx->handles_list);
+	INIT_LIST_HEAD(&ctx->hw_id_link);
 
 	/* Default context will never have a file_priv */
 	ret = DEFAULT_CONTEXT_HANDLE;
@@ -422,15 +395,35 @@ i915_gem_context_create_gvt(struct drm_device *dev)
 	return ctx;
 }
 
+static void
+destroy_kernel_context(struct i915_gem_context **ctxp)
+{
+	struct i915_gem_context *ctx;
+
+	/* Keep the context ref so that we can free it immediately ourselves */
+	ctx = i915_gem_context_get(fetch_and_zero(ctxp));
+	GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
+
+	context_close(ctx);
+	i915_gem_context_free(ctx);
+}
+
 struct i915_gem_context *
 i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
 {
 	struct i915_gem_context *ctx;
+	int err;
 
 	ctx = i915_gem_create_context(i915, NULL);
 	if (IS_ERR(ctx))
 		return ctx;
 
+	err = i915_gem_context_pin_hw_id(ctx);
+	if (err) {
+		destroy_kernel_context(&ctx);
+		return ERR_PTR(err);
+	}
+
 	i915_gem_context_clear_bannable(ctx);
 	ctx->sched.priority = prio;
 	ctx->ring_size = PAGE_SIZE;
@@ -440,17 +433,18 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
 	return ctx;
 }
 
-static void
-destroy_kernel_context(struct i915_gem_context **ctxp)
+static void init_contexts(struct drm_i915_private *i915)
 {
-	struct i915_gem_context *ctx;
+	INIT_LIST_HEAD(&i915->contexts.list);
 
-	/* Keep the context ref so that we can free it immediately ourselves */
-	ctx = i915_gem_context_get(fetch_and_zero(ctxp));
-	GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
+	/* Using the simple ida interface, the max is limited by sizeof(int) */
+	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
+	BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > INT_MAX);
+	ida_init(&i915->contexts.hw_ida);
+	INIT_LIST_HEAD(&i915->contexts.hw_id_list);
 
-	context_close(ctx);
-	i915_gem_context_free(ctx);
+	INIT_WORK(&i915->contexts.free_work, contexts_free_worker);
+	init_llist_head(&i915->contexts.free_list);
 }
 
 static bool needs_preempt_context(struct drm_i915_private *i915)
@@ -471,14 +465,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	INIT_LIST_HEAD(&dev_priv->contexts.list);
-	INIT_WORK(&dev_priv->contexts.free_work, contexts_free_worker);
-	init_llist_head(&dev_priv->contexts.free_list);
-
-	/* Using the simple ida interface, the max is limited by sizeof(int) */
-	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
-	BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > INT_MAX);
-	ida_init(&dev_priv->contexts.hw_ida);
+	init_contexts(dev_priv);
 
 	/* lowest priority; idle task */
 	ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
@@ -491,6 +478,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 	 * all user contexts will have non-zero hw_id.
 	 */
 	GEM_BUG_ON(ctx->hw_id);
+	GEM_BUG_ON(!ctx->pin_hw_id);
 	dev_priv->kernel_context = ctx;
 
 	/* highest priority; preempting task */
@@ -528,6 +516,7 @@ void i915_gem_contexts_fini(struct drm_i915_private *i915)
 	destroy_kernel_context(&i915->kernel_context);
 
 	/* Must free all deferred contexts (via flush_workqueue) first */
+	GEM_BUG_ON(!list_empty(&i915->contexts.hw_id_list));
 	ida_destroy(&i915->contexts.hw_ida);
 }
 
@@ -940,6 +929,114 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
 	return ret;
 }
 
+static inline int new_hw_id(struct drm_i915_private *i915)
+{
+	unsigned int max;
+
+	if (INTEL_GEN(i915) >= 11)
+		max = GEN11_MAX_CONTEXT_HW_ID;
+	else
+		max = MAX_CONTEXT_HW_ID;
+
+	return ida_simple_get(&i915->contexts.hw_ida, 0, max, GFP_KERNEL);
+}
+
+static int steal_hw_id(struct drm_i915_private *i915)
+{
+	struct i915_gem_context *ctx, *cn;
+	LIST_HEAD(pinned);
+	int id = 0;
+
+	list_for_each_entry_safe(ctx, cn,
+				 &i915->contexts.hw_id_list, hw_id_link) {
+		if (ctx->pin_hw_id) {
+			list_move_tail(&ctx->hw_id_link, &pinned);
+			continue;
+		}
+
+		GEM_BUG_ON(!ctx->hw_id);
+		list_del_init(&ctx->hw_id_link);
+		id = ctx->hw_id;
+		break;
+	}
+
+	list_splice_tail(&pinned, &i915->contexts.hw_id_list);
+	return id;
+}
+
+static int assign_hw_id(struct drm_i915_private *i915, unsigned int *out)
+{
+	int ret;
+
+	ret = new_hw_id(i915);
+	if (unlikely(ret < 0)) {
+		ret = steal_hw_id(i915);
+		if (ret)
+			goto out;
+
+		/*
+		 * Contexts are only released when no longer active.
+		 * Flush any pending retires to hopefully release some
+		 * stale contexts and try again.
+		 */
+		if (i915_retire_requests(i915)) {
+			ret = steal_hw_id(i915);
+			if (ret)
+				goto out;
+
+			ret = i915_gem_wait_for_idle(i915,
+						     I915_WAIT_INTERRUPTIBLE |
+						     I915_WAIT_LOCKED);
+			if (ret)
+				return ret;
+
+			ret = steal_hw_id(i915);
+			if (ret)
+				goto out;
+		}
+
+		/* One last attempt, to determine the errno */
+		ret = -ENOSPC;
+		if (contexts_free(i915)) {
+			ret = new_hw_id(i915);
+			GEM_BUG_ON(ret == 0);
+		}
+		if (ret < 0)
+			return ret;
+	}
+
+out:
+	*out = ret;
+	return 0;
+}
+
+int i915_gem_context_pin_hw_id(struct i915_gem_context *ctx)
+{
+	int err;
+
+	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
+
+	GEM_BUG_ON(ctx->pin_hw_id == ~0u);
+	if (!ctx->pin_hw_id++ && list_empty(&ctx->hw_id_link)) {
+		err = assign_hw_id(ctx->i915, &ctx->hw_id);
+		if (err)
+			return err;
+
+		list_add_tail(&ctx->hw_id_link,
+			      &ctx->i915->contexts.hw_id_list);
+	}
+
+	return 0;
+}
+
+void i915_gem_context_unpin_hw_id(struct i915_gem_context *ctx)
+{
+	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
+
+	GEM_BUG_ON(ctx->pin_hw_id == 0u);
+	--ctx->pin_hw_id;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_context.c"
 #include "selftests/i915_gem_context.c"
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index b116e4942c10..1e41d983bdd7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -136,6 +136,8 @@ struct i915_gem_context {
 	 * id for the lifetime of the context.
 	 */
 	unsigned int hw_id;
+	unsigned int pin_hw_id;
+	struct list_head hw_id_link;
 
 	/**
 	 * @user_handle: userspace identifier
@@ -257,6 +259,9 @@ static inline void i915_gem_context_set_force_single_submission(struct i915_gem_
 	__set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ctx->flags);
 }
 
+int i915_gem_context_pin_hw_id(struct i915_gem_context *ctx);
+void i915_gem_context_unpin_hw_id(struct i915_gem_context *ctx);
+
 static inline bool i915_gem_context_is_default(const struct i915_gem_context *c)
 {
 	return c->user_handle == DEFAULT_CONTEXT_HANDLE;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 4f0eb84b3c00..f449e6a31f75 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1216,31 +1216,24 @@ static int i915_oa_read(struct i915_perf_stream *stream,
 static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
+	struct intel_engine_cs *engine = dev_priv->engine[RCS];
+	struct intel_context *ce;
+	int ret;
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
-		dev_priv->perf.oa.specific_ctx_id = stream->ctx->hw_id;
-	} else {
-		struct intel_engine_cs *engine = dev_priv->engine[RCS];
-		struct intel_context *ce;
-		int ret;
-
-		ret = i915_mutex_lock_interruptible(&dev_priv->drm);
-		if (ret)
-			return ret;
+	ret = i915_mutex_lock_interruptible(&dev_priv->drm);
+	if (ret)
+		return ret;
 
-		/*
-		 * As the ID is the gtt offset of the context's vma we
-		 * pin the vma to ensure the ID remains fixed.
-		 *
-		 * NB: implied RCS engine...
-		 */
-		ce = intel_context_pin(stream->ctx, engine);
-		mutex_unlock(&dev_priv->drm.struct_mutex);
-		if (IS_ERR(ce))
-			return PTR_ERR(ce);
+	ce = intel_context_pin(stream->ctx, engine);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+	if (IS_ERR(ce))
+		return PTR_ERR(ce);
 
-		dev_priv->perf.oa.pinned_ctx = ce;
+	dev_priv->perf.oa.pinned_ctx = ce;
 
+	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
+		dev_priv->perf.oa.specific_ctx_id = stream->ctx->hw_id;
+	} else {
 		/*
 		 * Explicitly track the ID (instead of calling
 		 * i915_ggtt_offset() on the fly) considering the difference
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index f187250e60c6..dd8b1a273e8f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1419,17 +1419,19 @@ static void ring_retire_requests(struct intel_ring *ring)
 	}
 }
 
-void i915_retire_requests(struct drm_i915_private *i915)
+bool i915_retire_requests(struct drm_i915_private *i915)
 {
 	struct intel_ring *ring, *tmp;
 
 	lockdep_assert_held(&i915->drm.struct_mutex);
 
 	if (!i915->gt.active_requests)
-		return;
+		return false;
 
 	list_for_each_entry_safe(ring, tmp, &i915->gt.active_rings, active_link)
 		ring_retire_requests(ring);
+
+	return true;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 491ff81d0fea..479c6afac002 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -318,7 +318,7 @@ static inline bool i915_sched_node_signaled(const struct i915_sched_node *node)
 	return i915_request_completed(rq);
 }
 
-void i915_retire_requests(struct drm_i915_private *i915);
+bool i915_retire_requests(struct drm_i915_private *i915);
 
 /*
  * We treat requests as fences. This is not be to confused with our
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 5d4f78765083..883c90711a4c 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -935,7 +935,7 @@ DECLARE_EVENT_CLASS(i915_context,
 	TP_fast_assign(
 			__entry->dev = ctx->i915->drm.primary->index;
 			__entry->ctx = ctx;
-			__entry->hw_id = ctx->hw_id;
+			__entry->hw_id = ctx->pin_hw_id ? ctx->hw_id : ~0u;
 			__entry->vm = ctx->ppgtt ? &ctx->ppgtt->base : NULL;
 	),
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 285640f116e2..94bd224bf245 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1344,6 +1344,8 @@ static void execlists_context_destroy(struct intel_context *ce)
 
 static void execlists_context_unpin(struct intel_context *ce)
 {
+	i915_gem_context_unpin_hw_id(ce->gem_context);
+
 	intel_ring_unpin(ce->ring);
 
 	ce->state->obj->pin_global--;
@@ -1403,6 +1405,10 @@ __execlists_context_pin(struct intel_engine_cs *engine,
 	if (ret)
 		goto unpin_map;
 
+	ret = i915_gem_context_pin_hw_id(ctx);
+	if (ret)
+		goto unpin_ring;
+
 	intel_lr_context_descriptor_update(ctx, engine, ce);
 
 	ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
@@ -1414,6 +1420,8 @@ __execlists_context_pin(struct intel_engine_cs *engine,
 	i915_gem_context_get(ctx);
 	return ce;
 
+unpin_ring:
+	intel_ring_unpin(ce->ring);
 unpin_map:
 	i915_gem_object_unpin_map(ce->state->obj);
 unpin_vma:
diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
index 8904f1ce64e3..d937bdff26f9 100644
--- a/drivers/gpu/drm/i915/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/selftests/mock_context.c
@@ -43,6 +43,7 @@ mock_context(struct drm_i915_private *i915,
 
 	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
 	INIT_LIST_HEAD(&ctx->handles_list);
+	INIT_LIST_HEAD(&ctx->hw_id_link);
 
 	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) {
 		struct intel_context *ce = &ctx->__engine[n];
@@ -50,11 +51,9 @@ mock_context(struct drm_i915_private *i915,
 		ce->gem_context = ctx;
 	}
 
-	ret = ida_simple_get(&i915->contexts.hw_ida,
-			     0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
+	ret = i915_gem_context_pin_hw_id(ctx);
 	if (ret < 0)
 		goto err_handles;
-	ctx->hw_id = ret;
 
 	if (name) {
 		ctx->name = kstrdup(name, GFP_KERNEL);
@@ -85,11 +84,7 @@ void mock_context_close(struct i915_gem_context *ctx)
 
 void mock_init_contexts(struct drm_i915_private *i915)
 {
-	INIT_LIST_HEAD(&i915->contexts.list);
-	ida_init(&i915->contexts.hw_ida);
-
-	INIT_WORK(&i915->contexts.free_work, contexts_free_worker);
-	init_llist_head(&i915->contexts.free_list);
+	init_contexts(i915);
 }
 
 struct i915_gem_context *
-- 
2.17.1



More information about the Intel-gfx-trybot mailing list