We switched to a tree of per-engine HW context to accommodate the
introduction of virtual engines. However, we plan to also support
multiple instances of the same engine within the GEM context, defeating
our use of the engine as a key to looking up the HW context. Just
allocate a logical per-engine instance and always use an index into the
ctx->engines[]. Later on, this ctx->engines[] may be replaced by a user
specified map.
v2: Add for_each_gem_engine() helper to iterator within the engines lock
v3: intel_context_create_request() helper
v4: s/unsigned long/unsigned int/ 4 billion engines is quite enough.
v5: Push iterator locking to caller
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_context.c | 112 ++++--------------
drivers/gpu/drm/i915/gt/intel_context.h | 27 +----
drivers/gpu/drm/i915/gt/intel_context_types.h | 2 -
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/gt/mock_engine.c | 3 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 24 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 97 +++++++++++++--
drivers/gpu/drm/i915/i915_gem_context.h | 58 +++++++++
drivers/gpu/drm/i915/i915_gem_context_types.h | 32 ++++-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 70 +++++------
drivers/gpu/drm/i915/i915_perf.c | 77 ++++++------
drivers/gpu/drm/i915/i915_request.c | 15 +--
drivers/gpu/drm/i915/intel_guc_submission.c | 22 ++--
.../gpu/drm/i915/selftests/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/selftests/mock_context.c | 14 ++-
16 files changed, 319 insertions(+), 240 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index 15ac99c5dd4a..5e506e648454 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -17,7 +17,7 @@ static struct i915_global_context {
struct kmem_cache *slab_ce;
} global;
-struct intel_context *intel_context_alloc(void)
+static struct intel_context *intel_context_alloc(void)
{
return kmem_cache_zalloc(global.slab_ce, GFP_KERNEL);
}
@@ -28,104 +28,17 @@ void intel_context_free(struct intel_context *ce)
}
struct intel_context *
-intel_context_lookup(struct i915_gem_context *ctx,
+intel_context_create(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
- struct intel_context *ce = NULL;
- struct rb_node *p;
-
- spin_lock(&ctx->hw_contexts_lock);
- p = ctx->hw_contexts.rb_node;
- while (p) {
- struct intel_context *this =
- rb_entry(p, struct intel_context, node);
-
- if (this->engine == engine) {
- GEM_BUG_ON(this->gem_context != ctx);
- ce = this;
- break;
- }
-
- if (this->engine < engine)
- p = p->rb_right;
- else
- p = p->rb_left;
- }
- spin_unlock(&ctx->hw_contexts_lock);
-
- return ce;
-}
-
-struct intel_context *
-__intel_context_insert(struct i915_gem_context *ctx,
- struct intel_engine_cs *engine,
- struct intel_context *ce)
-{
- struct rb_node **p, *parent;
- int err = 0;
-
- spin_lock(&ctx->hw_contexts_lock);
-
- parent = NULL;
- p = &ctx->hw_contexts.rb_node;
- while (*p) {
- struct intel_context *this;
-
- parent = *p;
- this = rb_entry(parent, struct intel_context, node);
-
- if (this->engine == engine) {
- err = -EEXIST;
- ce = this;
- break;
- }
-
- if (this->engine < engine)
- p = &parent->rb_right;
- else
- p = &parent->rb_left;
- }
- if (!err) {
- rb_link_node(&ce->node, parent, p);
- rb_insert_color(&ce->node, &ctx->hw_contexts);
- }
-
- spin_unlock(&ctx->hw_contexts_lock);
-
- return ce;
-}
-
-void __intel_context_remove(struct intel_context *ce)
-{
- struct i915_gem_context *ctx = ce->gem_context;
-
- spin_lock(&ctx->hw_contexts_lock);
- rb_erase(&ce->node, &ctx->hw_contexts);
- spin_unlock(&ctx->hw_contexts_lock);
-}
-
-struct intel_context *
-intel_context_instance(struct i915_gem_context *ctx,
- struct intel_engine_cs *engine)
-{
- struct intel_context *ce, *pos;
-
- ce = intel_context_lookup(ctx, engine);
- if (likely(ce))
- return intel_context_get(ce);
+ struct intel_context *ce;
ce = intel_context_alloc();
if (!ce)
return ERR_PTR(-ENOMEM);
intel_context_init(ce, ctx, engine);
-
- pos = __intel_context_insert(ctx, engine, ce);
- if (unlikely(pos != ce)) /* Beaten! Use their HW context instead */
- intel_context_free(ce);
-
- GEM_BUG_ON(intel_context_lookup(ctx, engine) != pos);
- return intel_context_get(pos);
+ return ce;
}
int __intel_context_do_pin(struct intel_context *ce)
@@ -204,6 +117,8 @@ intel_context_init(struct intel_context *ce,
struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
+ GEM_BUG_ON(!engine->cops);
+
kref_init(&ce->ref);
ce->gem_context = ctx;
@@ -254,3 +169,18 @@ void intel_context_exit_engine(struct
intel_context *ce)
{
intel_engine_pm_put(ce->engine);
}
+
+struct i915_request *intel_context_create_request(struct
intel_context *ce)
+{
+ struct i915_request *rq;
+ int err;
+
+ err = intel_context_pin(ce);
+ if (unlikely(err))
+ return ERR_PTR(err);
+
+ rq = i915_request_create(ce);
+ intel_context_unpin(ce);
+
+ return rq;
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h
b/drivers/gpu/drm/i915/gt/intel_context.h
index b746add6b71d..63392c88cd98 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -12,24 +12,16 @@
#include "intel_context_types.h"
#include "intel_engine_types.h"
-struct intel_context *intel_context_alloc(void);
-void intel_context_free(struct intel_context *ce);
-
void intel_context_init(struct intel_context *ce,
struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
-/**
- * intel_context_lookup - Find the matching HW context for this (ctx,
engine)
- * @ctx - the parent GEM context
- * @engine - the target HW engine
- *
- * May return NULL if the HW context hasn't been instantiated (i.e.
unused).
- */
struct intel_context *
-intel_context_lookup(struct i915_gem_context *ctx,
+intel_context_create(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
+void intel_context_free(struct intel_context *ce);
+
/**
* intel_context_lock_pinned - Stablises the 'pinned' status of the
HW context
* @ce - the context
@@ -71,17 +63,6 @@ static inline void
intel_context_unlock_pinned(struct intel_context *ce)
mutex_unlock(&ce->pin_mutex);
}
-struct intel_context *
-__intel_context_insert(struct i915_gem_context *ctx,
- struct intel_engine_cs *engine,
- struct intel_context *ce);
-void
-__intel_context_remove(struct intel_context *ce);
-
-struct intel_context *
-intel_context_instance(struct i915_gem_context *ctx,
- struct intel_engine_cs *engine);
-
int __intel_context_do_pin(struct intel_context *ce);
static inline int intel_context_pin(struct intel_context *ce)
@@ -144,4 +125,6 @@ static inline void
intel_context_timeline_unlock(struct intel_context *ce)
mutex_unlock(&ce->ring->timeline->mutex);
}
+struct i915_request *intel_context_create_request(struct
intel_context *ce);
+
#endif /* __INTEL_CONTEXT_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index f02d27734e3b..3579c2708321 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -10,7 +10,6 @@
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/mutex.h>
-#include <linux/rbtree.h>
#include <linux/types.h>
#include "i915_active_types.h"
@@ -61,7 +60,6 @@ struct intel_context {
struct i915_active_request active_tracker;
const struct intel_context_ops *ops;
- struct rb_node node;
/** sseu: Control eu/slice partitioning */
struct intel_sseu sseu;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6cb90137b2fa..2a1c438bf0ad 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -716,7 +716,7 @@ static int pin_context(struct i915_gem_context *ctx,
struct intel_context *ce;
int err;
- ce = intel_context_instance(ctx, engine);
+ ce = i915_gem_context_get_engine(ctx, engine->id);
if (IS_ERR(ce))
return PTR_ERR(ce);
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c
b/drivers/gpu/drm/i915/gt/mock_engine.c
index 85cdbfe1d989..2941916b37bf 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -23,6 +23,7 @@
*/
#include "i915_drv.h"
+#include "i915_gem_context.h"
#include "intel_context.h"
#include "intel_engine_pm.h"
@@ -286,7 +287,7 @@ int mock_engine_init(struct intel_engine_cs *engine)
i915_timeline_set_subclass(&engine->timeline, TIMELINE_ENGINE);
engine->kernel_context =
- intel_context_instance(i915->kernel_context, engine);
+ i915_gem_context_get_engine(i915->kernel_context, engine->id);
if (IS_ERR(engine->kernel_context))
goto err_timeline;
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 606fc2713240..8b6574e1b495 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -1188,7 +1188,7 @@ int intel_vgpu_setup_submission(struct
intel_vgpu *vgpu)
INIT_LIST_HEAD(&s->workload_q_head[i]);
s->shadow[i] = ERR_PTR(-EINVAL);
- ce = intel_context_instance(ctx, engine);
+ ce = i915_gem_context_get_engine(ctx, i);
if (IS_ERR(ce)) {
ret = PTR_ERR(ce);
goto out_shadow_ctx;
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 50266e87c225..e2d89c64ab52 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4312,8 +4312,9 @@ int i915_gem_init_hw(struct drm_i915_private
*dev_priv)
static int __intel_engines_record_defaults(struct drm_i915_private
*i915)
{
- struct i915_gem_context *ctx;
struct intel_engine_cs *engine;
+ struct i915_gem_context *ctx;
+ struct i915_gem_engines *e;
enum intel_engine_id id;
int err = 0;
@@ -4330,18 +4331,21 @@ static int
__intel_engines_record_defaults(struct drm_i915_private *i915)
if (IS_ERR(ctx))
return PTR_ERR(ctx);
+ e = i915_gem_context_lock_engines(ctx);
+
for_each_engine(engine, i915, id) {
+ struct intel_context *ce = e->engines[id];
struct i915_request *rq;
- rq = i915_request_alloc(engine, ctx);
+ rq = intel_context_create_request(ce);
if (IS_ERR(rq)) {
err = PTR_ERR(rq);
- goto out_ctx;
+ goto err_active;
}
err = 0;
- if (engine->init_context)
- err = engine->init_context(rq);
+ if (rq->engine->init_context)
+ err = rq->engine->init_context(rq);
i915_request_add(rq);
if (err)
@@ -4355,15 +4359,10 @@ static int
__intel_engines_record_defaults(struct drm_i915_private *i915)
}
for_each_engine(engine, i915, id) {
- struct intel_context *ce;
- struct i915_vma *state;
+ struct intel_context *ce = e->engines[id];
+ struct i915_vma *state = ce->state;
void *vaddr;
- ce = intel_context_lookup(ctx, engine);
- if (!ce)
- continue;
-
- state = ce->state;
if (!state)
continue;
@@ -4419,6 +4418,7 @@ static int
__intel_engines_record_defaults(struct drm_i915_private *i915)
}
out_ctx:
+ i915_gem_context_unlock_engines(ctx);
i915_gem_context_set_closed(ctx);
i915_gem_context_put(ctx);
return err;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 1e1770047cc2..1a1373f08fa9 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -150,7 +150,7 @@ lookup_user_engine(struct i915_gem_context *ctx,
u16 class, u16 instance)
if (!engine)
return ERR_PTR(-EINVAL);
- return intel_context_instance(ctx, engine);
+ return i915_gem_context_get_engine(ctx, engine->id);
}
static inline int new_hw_id(struct drm_i915_private *i915, gfp_t gfp)
@@ -242,10 +242,51 @@ static void release_hw_id(struct
i915_gem_context *ctx)
mutex_unlock(&i915->contexts.mutex);
}
-static void i915_gem_context_free(struct i915_gem_context *ctx)
+static void __free_engines(struct i915_gem_engines *e, unsigned int
count)
{
- struct intel_context *it, *n;
+ while (count--) {
+ if (!e->engines[count])
+ continue;
+
+ intel_context_put(e->engines[count]);
+ }
+ kfree(e);
+}
+
+static void free_engines(struct i915_gem_engines *e)
+{
+ __free_engines(e, e->num_engines);
+}
+
+static struct i915_gem_engines *default_engines(struct
i915_gem_context *ctx)
+{
+ struct intel_engine_cs *engine;
+ struct i915_gem_engines *e;
+ enum intel_engine_id id;
+
+ e = kzalloc(struct_size(e, engines, I915_NUM_ENGINES), GFP_KERNEL);
+ if (!e)
+ return ERR_PTR(-ENOMEM);
+
+ e->i915 = ctx->i915;
+ for_each_engine(engine, ctx->i915, id) {
+ struct intel_context *ce;
+
+ ce = intel_context_create(ctx, engine);
+ if (IS_ERR(ce)) {
+ __free_engines(e, id);
+ return ERR_CAST(ce);
+ }
+ e->engines[id] = ce;
+ }
+ e->num_engines = id;
+
+ return e;
+}
+
+static void i915_gem_context_free(struct i915_gem_context *ctx)
+{
lockdep_assert_held(&ctx->i915->drm.struct_mutex);
GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
GEM_BUG_ON(!list_empty(&ctx->active_engines));
@@ -253,8 +294,8 @@ static void i915_gem_context_free(struct
i915_gem_context *ctx)
release_hw_id(ctx);
i915_ppgtt_put(ctx->ppgtt);
- rbtree_postorder_for_each_entry_safe(it, n, &ctx->hw_contexts, node)
- intel_context_put(it);
+ free_engines(rcu_access_pointer(ctx->engines));
+ mutex_destroy(&ctx->engines_mutex);
if (ctx->timeline)
i915_timeline_put(ctx->timeline);
@@ -363,6 +404,8 @@ static struct i915_gem_context *
__create_context(struct drm_i915_private *dev_priv)
{
struct i915_gem_context *ctx;
+ struct i915_gem_engines *e;
+ int err;
int i;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -376,8 +419,13 @@ __create_context(struct drm_i915_private *dev_priv)
INIT_LIST_HEAD(&ctx->active_engines);
mutex_init(&ctx->mutex);
- ctx->hw_contexts = RB_ROOT;
- spin_lock_init(&ctx->hw_contexts_lock);
+ mutex_init(&ctx->engines_mutex);
+ e = default_engines(ctx);
+ if (IS_ERR(e)) {
+ err = PTR_ERR(e);
+ goto err_free;
+ }
+ RCU_INIT_POINTER(ctx->engines, e);
INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
INIT_LIST_HEAD(&ctx->handles_list);
@@ -399,6 +447,10 @@ __create_context(struct drm_i915_private *dev_priv)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
return ctx;
+
+err_free:
+ kfree(ctx);
+ return ERR_PTR(err);
}
static struct i915_hw_ppgtt *
@@ -862,7 +914,7 @@ static int context_barrier_task(struct
i915_gem_context *ctx,
{
struct drm_i915_private *i915 = ctx->i915;
struct context_barrier_task *cb;
- struct intel_context *ce, *next;
+ struct i915_gem_engines_iter it;
int err = 0;
lockdep_assert_held(&i915->drm.struct_mutex);
@@ -875,20 +927,23 @@ static int context_barrier_task(struct
i915_gem_context *ctx,
i915_active_init(i915, &cb->base, cb_retire);
i915_active_acquire(&cb->base);
- rbtree_postorder_for_each_entry_safe(ce, next, &ctx->hw_contexts,
node) {
- struct intel_engine_cs *engine = ce->engine;
+ for_each_gem_engine(it, i915_gem_context_lock_engines(ctx)) {
+ struct intel_context *ce = it.context;
struct i915_request *rq;
- if (!(engine->mask & engines))
+ if (!(ce->engine->mask & engines))
+ continue;
+
+ if (!intel_context_is_pinned(ce))
continue;
if (I915_SELFTEST_ONLY(context_barrier_inject_fault &
- engine->mask)) {
+ ce->engine->mask)) {
err = -ENXIO;
break;
}
- rq = i915_request_alloc(engine, ctx);
+ rq = intel_context_create_request(ce);
if (IS_ERR(rq)) {
err = PTR_ERR(rq);
break;
@@ -904,6 +959,7 @@ static int context_barrier_task(struct
i915_gem_context *ctx,
if (err)
break;
}
+ i915_gem_context_unlock_engines(ctx);
cb->task = err ? NULL : task; /* caller needs to unwind instead */
cb->data = data;
@@ -1739,6 +1795,21 @@ int __i915_gem_context_pin_hw_id(struct
i915_gem_context *ctx)
return err;
}
+/* GEM context-engines iterator: for_each_gem_engine() */
+bool i915_gem_engines_iter_next(struct i915_gem_engines_iter *it)
+{
+ GEM_BUG_ON(!it->engines);
+
+ do {
+ if (it->idx >= it->engines->num_engines)
+ return false;
+
+ it->context = it->engines->engines[it->idx++];
+ } while (!it->context);
+
+ return true;
+}
+
#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 5a8e080499fb..2657a2ce82da 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -176,6 +176,64 @@ static inline void i915_gem_context_put(struct
i915_gem_context *ctx)
kref_put(&ctx->ref, i915_gem_context_release);
}
+static inline struct i915_gem_engines *
+i915_gem_context_engines(struct i915_gem_context *ctx)
+{
+ return rcu_dereference_protected(ctx->engines,
+ lockdep_is_held(&ctx->engines_mutex));
+}
+
+static inline struct i915_gem_engines *
+i915_gem_context_lock_engines(struct i915_gem_context *ctx)
+ __acquires(&ctx->engines_mutex)
+{
+ mutex_lock(&ctx->engines_mutex);
+ return i915_gem_context_engines(ctx);
+}
+
+static inline void
+i915_gem_context_unlock_engines(struct i915_gem_context *ctx)
+ __releases(&ctx->engines_mutex)
+{
+ mutex_unlock(&ctx->engines_mutex);
+}
+
+static inline struct intel_context *
+i915_gem_context_lookup_engine(struct i915_gem_context *ctx, unsigned
int idx)
+{
+ return i915_gem_context_engines(ctx)->engines[idx];
+}
+
+static inline struct intel_context *
+i915_gem_context_get_engine(struct i915_gem_context *ctx, unsigned
int idx)
+{
+ struct intel_context *ce = ERR_PTR(-EINVAL);
+
+ rcu_read_lock(); {
+ struct i915_gem_engines *e = rcu_dereference(ctx->engines);
+ if (likely(idx < e->num_engines && e->engines[idx]))
+ ce = intel_context_get(e->engines[idx]);
+ } rcu_read_unlock();
+
+ return ce;
+}
+
+static inline void i915_gem_engines_iter_init(struct
i915_gem_engines_iter *it,
+ struct i915_gem_engines *engines)
+{
+ GEM_BUG_ON(!engines);
+
+ it->idx = 0;
+ it->context = NULL;
+ it->engines = engines;
+}
+
+bool i915_gem_engines_iter_next(struct i915_gem_engines_iter *it);
+
+#define for_each_gem_engine(it, engines) \
+ for (i915_gem_engines_iter_init(&(it), (engines)); \
+ i915_gem_engines_iter_next(&(it));)
+
struct i915_lut_handle *i915_lut_handle_alloc(void);
void i915_lut_handle_free(struct i915_lut_handle *lut);
diff --git a/drivers/gpu/drm/i915/i915_gem_context_types.h
b/drivers/gpu/drm/i915/i915_gem_context_types.h
index d282a6ab3b9f..c74f0545375f 100644
--- a/drivers/gpu/drm/i915/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/i915_gem_context_types.h
@@ -29,6 +29,19 @@ struct i915_hw_ppgtt;
struct i915_timeline;
struct intel_ring;
+struct i915_gem_engines {
+ struct rcu_work rcu;
+ struct drm_i915_private *i915;
+ unsigned int num_engines;
+ struct intel_context *engines[];
+};
+
+struct i915_gem_engines_iter {
+ struct intel_context *context;
+ struct i915_gem_engines *engines;
+ unsigned int idx;
+};
+
/**
* struct i915_gem_context - client state
*
@@ -42,6 +55,21 @@ struct i915_gem_context {
/** file_priv: owning file descriptor */
struct drm_i915_file_private *file_priv;
+ /**
+ * @engines: User defined engines for this context
+ *
+ * NULL means to use legacy definitions (including random meaning of
+ * I915_EXEC_BSD with I915_EXEC_BSD_SELECTOR overrides).
