[PATCH 11/12] drm/i915: Reduce context HW ID lifetime
Chris Wilson
chris at chris-wilson.co.uk
Fri Jan 12 20:40:47 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).
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
drivers/gpu/drm/i915/i915_gem_context.c | 117 +++++++++++++++++---------
drivers/gpu/drm/i915/i915_gem_context.h | 4 +
drivers/gpu/drm/i915/i915_perf.c | 52 +++++-------
drivers/gpu/drm/i915/i915_trace.h | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 6 ++
drivers/gpu/drm/i915/selftests/mock_context.c | 5 +-
7 files changed, 112 insertions(+), 78 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index dfa65d8d6d36..6263aeb25d07 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1887,7 +1887,9 @@ 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 (ctx->hw_id_pin_count)
+ seq_printf(m, "%x ", ctx->hw_id);
if (ctx->pid) {
struct task_struct *task;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 648e7536ff51..7f407a676069 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -141,7 +141,9 @@ 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 (ctx->hw_id_pin_count)
+ ida_simple_remove(&ctx->i915->contexts.hw_ida, ctx->hw_id);
+
kfree_rcu(ctx, rcu);
}
@@ -208,28 +210,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;
-
- ret = ida_simple_get(&dev_priv->contexts.hw_ida,
- 0, MAX_CONTEXT_HW_ID, 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_gem_retire_requests(dev_priv);
- ret = ida_simple_get(&dev_priv->contexts.hw_ida,
- 0, MAX_CONTEXT_HW_ID, 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)
{
@@ -265,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;
@@ -418,15 +392,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_get_hw_id(ctx);
+ if (err) {
+ destroy_kernel_context(&ctx);
+ return ERR_PTR(err);
+ }
+
i915_gem_context_clear_bannable(ctx);
ctx->priority = prio;
ctx->ring_size = PAGE_SIZE;
@@ -436,19 +430,6 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
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);
-}
-
int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
{
struct i915_gem_context *ctx;
@@ -476,6 +457,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->hw_id_pin_count);
dev_priv->kernel_context = ctx;
/* highest priority; preempting task */
@@ -869,6 +851,57 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
return ret;
}
+static int assign_hw_id(struct drm_i915_private *i915, unsigned *out)
+{
+ int ret;
+
+ ret = ida_simple_get(&i915->contexts.hw_ida,
+ 0, MAX_CONTEXT_HW_ID, 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.
+ */
+ ret = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
+ if (ret)
+ return ret;
+
+ ret = ida_simple_get(&i915->contexts.hw_ida,
+ 0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ }
+
+ *out = ret;
+ return 0;
+}
+
+int i915_gem_context_get_hw_id(struct i915_gem_context *ctx)
+{
+ int err;
+
+ lockdep_assert_held(&ctx->i915->drm.struct_mutex);
+
+ GEM_BUG_ON(ctx->hw_id_pin_count == ~0u);
+ if (!ctx->hw_id_pin_count++) {
+ err = assign_hw_id(ctx->i915, &ctx->hw_id);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+void i915_gem_context_put_hw_id(struct i915_gem_context *ctx)
+{
+ lockdep_assert_held(&ctx->i915->drm.struct_mutex);
+
+ GEM_BUG_ON(ctx->hw_id_pin_count == 0u);
+ if (!--ctx->hw_id_pin_count)
+ ida_simple_remove(&ctx->i915->contexts.hw_ida, ctx->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 4bfb72f8e1cb..63eacd9b2627 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -125,6 +125,7 @@ struct i915_gem_context {
* id for the lifetime of the context.
*/
unsigned int hw_id;
+ unsigned int hw_id_pin_count;
/**
* @user_handle: userspace identifier
@@ -254,6 +255,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_get_hw_id(struct i915_gem_context *ctx);
+void i915_gem_context_put_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 f0cfdece14ae..26b6d3707543 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1215,30 +1215,23 @@ 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_ring *ring;
+ int ret;
+
+ ret = i915_mutex_lock_interruptible(&dev_priv->drm);
+ if (ret)
+ return ret;
+
+ ring = engine->context_pin(engine, stream->ctx);
+ if (IS_ERR(ring)) {
+ ret = PTR_ERR(ring);
+ goto unlock;
+ }
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_ring *ring;
- int 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...
- */
- ring = engine->context_pin(engine, stream->ctx);
- mutex_unlock(&dev_priv->drm.struct_mutex);
- if (IS_ERR(ring))
- return PTR_ERR(ring);
-
-
/*
* Explicitly track the ID (instead of calling
* i915_ggtt_offset() on the fly) considering the difference
@@ -1248,7 +1241,9 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
i915_ggtt_offset(stream->ctx->engine[engine->id].state);
}
- return 0;
+unlock:
+ mutex_unlock(&dev_priv->drm.struct_mutex);
+ return ret;
}
/**
@@ -1261,19 +1256,14 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
static void oa_put_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];
- if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
- dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID;
- } else {
- struct intel_engine_cs *engine = dev_priv->engine[RCS];
-
- mutex_lock(&dev_priv->drm.struct_mutex);
+ mutex_lock(&dev_priv->drm.struct_mutex);
- dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID;
- engine->context_unpin(engine, stream->ctx);
+ dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID;
+ engine->context_unpin(engine, stream->ctx);
- mutex_unlock(&dev_priv->drm.struct_mutex);
- }
+ mutex_unlock(&dev_priv->drm.struct_mutex);
}
static void
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index e1169c02eb2b..0581180effee 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -950,7 +950,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->hw_id_pin_count ? 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 ff25f209d0a5..118f767b3d21 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1149,6 +1149,10 @@ execlists_context_pin(struct intel_engine_cs *engine,
if (ret)
goto unpin_map;
+ ret = i915_gem_context_get_hw_id(ctx);
+ if (ret)
+ goto unpin_ring;
+
intel_lr_context_descriptor_update(ctx, engine);
ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
@@ -1160,6 +1164,8 @@ execlists_context_pin(struct intel_engine_cs *engine,
out:
return ce->ring;
+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 bbf80d42e793..bfe4efb2b1d3 100644
--- a/drivers/gpu/drm/i915/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/selftests/mock_context.c
@@ -43,11 +43,9 @@ mock_context(struct drm_i915_private *i915,
INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
INIT_LIST_HEAD(&ctx->handles_list);
- ret = ida_simple_get(&i915->contexts.hw_ida,
- 0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
+ ret = i915_gem_context_get_hw_id(ctx);
if (ret < 0)
goto err_handles;
- ctx->hw_id = ret;
if (name) {
ctx->name = kstrdup(name, GFP_KERNEL);
@@ -66,6 +64,7 @@ mock_context(struct drm_i915_private *i915,
return NULL;
err_put:
+ i915_gem_context_put_hw_id(ctx);
i915_gem_context_set_closed(ctx);
i915_gem_context_put(ctx);
return NULL;
--
2.15.1
More information about the Intel-gfx-trybot
mailing list