On 22/05/16 14:02, Chris Wilson wrote:
struct intel_context contains two substructs, one for the legacy RCS and
one for every execlists engine. Since legacy RCS is a subset of the
execlists engine support, just combine the two substructs.
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++-------------
drivers/gpu/drm/i915/i915_drv.h | 7 ---
drivers/gpu/drm/i915/i915_gem_context.c | 75 +++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_lrc.c | 26 ------------
drivers/gpu/drm/i915/intel_lrc.h | 1 -
5 files changed, 55 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 945fe4710b37..30cb26fe2fa9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -199,13 +199,6 @@ describe_obj(struct seq_file *m, struct
drm_i915_gem_object *obj)
seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
}
-static void describe_ctx(struct seq_file *m, struct i915_gem_context *ctx)
-{
- seq_putc(m, ctx->legacy_hw_ctx.initialized ? 'I' : 'i');
- seq_putc(m, ctx->remap_slice ? 'R' : 'r');
- seq_putc(m, ' ');
-}
-
static int i915_gem_object_list_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
@@ -2001,7 +1994,6 @@ static int i915_context_status(struct seq_file *m, void
*unused)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
struct i915_gem_context *ctx;
- enum intel_engine_id id;
int ret;
ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -2009,10 +2001,6 @@ static int i915_context_status(struct seq_file *m, void
*unused)
return ret;
list_for_each_entry(ctx, &dev_priv->context_list, link) {
- if (!i915.enable_execlists &&
- ctx->legacy_hw_ctx.rcs_state == NULL)
- continue;
-
seq_printf(m, "HW context %u ", ctx->hw_id);
if (IS_ERR(ctx->file_priv)) {
seq_puts(m, "(deleted) ");
@@ -2029,25 +2017,18 @@ static int i915_context_status(struct seq_file *m, void
*unused)
} else
seq_puts(m, "(kernel) ");
- describe_ctx(m, ctx);
+ seq_putc(m, ctx->remap_slice ? 'R' : 'r');
+ seq_putc(m, '\n');
- if (i915.enable_execlists) {
+ for_each_engine(engine, dev_priv) {
+ struct intel_context *ce = &ctx->engine[engine->id];
+ seq_printf(m, "%s: ", engine->name);
+ seq_putc(m, ce->initialised ? 'I' : 'i');
+ if (ce->state)
+ describe_obj(m, ce->state);
+ if (ce->ringbuf)
+ describe_ctx_ringbuf(m, ce->ringbuf);
seq_putc(m, '\n');
- for_each_engine_id(engine, dev_priv, id) {
- struct drm_i915_gem_object *ctx_obj =
- ctx->engine[id].state;
- struct intel_ringbuffer *ringbuf =
- ctx->engine[id].ringbuf;
-
- seq_printf(m, "%s: ", engine->name);
- if (ctx_obj)
- describe_obj(m, ctx_obj);
- if (ringbuf)
- describe_ctx_ringbuf(m, ringbuf);
- seq_putc(m, '\n');
- }
- } else {
- describe_obj(m, ctx->legacy_hw_ctx.rcs_state);
}
seq_putc(m, '\n');
@@ -2062,10 +2043,10 @@ static void i915_dump_lrc_obj(struct seq_file *m,
struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
struct page *page;
uint32_t *reg_state;
int j;
- 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);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d3d2538d17e8..259a0ee7a601 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -862,13 +862,6 @@ struct i915_gem_context {
/* Unique identifier for this context, used by the hw for tracking */
unsigned hw_id;
- /* Legacy ring buffer submission */
- struct {
- struct drm_i915_gem_object *rcs_state;
- bool initialized;
- } legacy_hw_ctx;
-
- /* Execlists */
struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 51e83a79ab36..9847235997b9 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -152,13 +152,11 @@ static void i915_gem_context_clean(struct
i915_gem_context *ctx)
void i915_gem_context_free(struct kref *ctx_ref)
{
struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
+ int i;
lockdep_assert_held(&ctx->i915->dev->struct_mutex);
trace_i915_context_free(ctx);
- if (i915.enable_execlists)
- intel_lr_context_free(ctx);
-
/*
* This context is going away and we need to remove all VMAs still
* around. This is to handle imported shared objects for which
@@ -168,8 +166,19 @@ void i915_gem_context_free(struct kref *ctx_ref)
i915_ppgtt_put(ctx->ppgtt);
- if (ctx->legacy_hw_ctx.rcs_state)
- drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
+ for (i = 0; i < I915_NUM_ENGINES; i++) {
+ struct intel_context *ce = &ctx->engine[i];
+
+ if (!ce->state)
+ continue;
+
+ WARN_ON(ce->pin_count);
+ if (ce->ringbuf)
+ intel_ringbuffer_free(ce->ringbuf);
+
+ drm_gem_object_unreference(&ce->state->base);
+ }
+
list_del(&ctx->link);
ida_simple_remove(&ctx->i915->context_hw_ida, ctx->hw_id);
@@ -266,7 +275,7 @@ __create_hw_context(struct drm_device *dev,
ret = PTR_ERR(obj);
goto err_out;
}
- ctx->legacy_hw_ctx.rcs_state = obj;
+ ctx->engine[RCS].state = obj;
}
/* Default context will never have a file_priv */
@@ -335,8 +344,11 @@ static void i915_gem_context_unpin(struct i915_gem_context
*ctx,
if (i915.enable_execlists) {
intel_lr_context_unpin(ctx, engine);
} else {
- if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
-
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ struct intel_context *ce = &ctx->engine[engine->id];
+
+ if (ce->state)
+ i915_gem_object_ggtt_unpin(ce->state);
+
i915_gem_context_put(ctx);
}
}
@@ -400,7 +412,7 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
- if (ctx->legacy_hw_ctx.rcs_state) {
+ if (ctx->engine[RCS].state) {
Maybe now put a comment saying this is for legacy now, to make the
asymmetry in ctx pinning paths between the two stick out more.
int ret;
/* We may need to do things with the shrinker which
@@ -410,7 +422,7 @@ int i915_gem_context_init(struct drm_device *dev)
* be available. To avoid this we always pin the default
* context.
*/
- ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+ ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
get_context_alignment(dev_priv), 0);
if (ret) {
DRM_ERROR("Failed to pinned default global context (error
%d)\n",
@@ -435,15 +447,17 @@ void i915_gem_context_lost(struct drm_i915_private
*dev_priv)
lockdep_assert_held(&dev_priv->dev->struct_mutex);
for_each_engine(engine, dev_priv) {
- if (engine->last_context == NULL)
- continue;
+ if (engine->last_context) {
+ i915_gem_context_unpin(engine->last_context, engine);
+ engine->last_context = NULL;
+ }
- i915_gem_context_unpin(engine->last_context, engine);
- engine->last_context = NULL;
+ /* Force the GPU state to be reinitialised on enabling */
+ dev_priv->kernel_context->engine[engine->id].initialised =
+ engine->init_context == NULL;
}
/* Force the GPU state to be reinitialised on enabling */
- dev_priv->kernel_context->legacy_hw_ctx.initialized = false;
dev_priv->kernel_context->remap_slice = ALL_L3_SLICES(dev_priv);
}
@@ -454,8 +468,8 @@ void i915_gem_context_fini(struct drm_device *dev)
lockdep_assert_held(&dev->struct_mutex);
- if (dctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+ if (!i915.enable_execlists && dctx->engine[RCS].state)
+ i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
i915_gem_context_put(dctx);
dev_priv->kernel_context = NULL;
@@ -562,7 +576,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32
hw_flags)
intel_ring_emit(engine, MI_NOOP);
intel_ring_emit(engine, MI_SET_CONTEXT);
intel_ring_emit(engine,
-
i915_gem_obj_ggtt_offset(req->ctx->legacy_hw_ctx.rcs_state) |
+ i915_gem_obj_ggtt_offset(req->ctx->engine[RCS].state) |
flags);
/*
* w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
@@ -639,7 +653,7 @@ static inline bool skip_rcs_switch(struct i915_hw_ppgtt
*ppgtt,
if (to->remap_slice)
return false;
- if (!to->legacy_hw_ctx.initialized)
+ if (!to->engine[RCS].initialised)
return false;
if (ppgtt && (intel_engine_flag(engine) & ppgtt->pd_dirty_rings))
@@ -704,7 +718,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
return 0;
/* Trying to pin first makes error handling easier. */
- ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
+ ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
get_context_alignment(engine->i915),
0);
if (ret)
@@ -727,7 +741,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
*
* XXX: We need a real interface to do this instead of trickery.
*/
- ret = i915_gem_object_set_to_gtt_domain(to->legacy_hw_ctx.rcs_state,
false);
+ ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state, false);
if (ret)
goto unpin_out;
@@ -742,7 +756,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
goto unpin_out;
}
- if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+ if (!to->engine[RCS].initialised || i915_gem_context_is_default(to))
/* NB: If we inhibit the restore, the context is not allowed to
* die because future work may end up depending on valid address
* space. This means we must enforce that a page table load
@@ -766,8 +780,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
* MI_SET_CONTEXT instead of when the next seqno has completed.
*/
if (from != NULL) {
- from->legacy_hw_ctx.rcs_state->base.read_domains =
I915_GEM_DOMAIN_INSTRUCTION;
-
i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->legacy_hw_ctx.rcs_state),
req);
+ from->engine[RCS].state->base.read_domains =
I915_GEM_DOMAIN_INSTRUCTION;
+
i915_vma_move_to_active(i915_gem_obj_to_ggtt(from->engine[RCS].state), req);
/* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
* whole damn pipeline, we don't need to explicitly mark the
* object dirty. The only exception is that the context must be
@@ -775,10 +789,10 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
* able to defer doing this until we know the object would be
* swapped, but there is no way to do that yet.
*/
- from->legacy_hw_ctx.rcs_state->dirty = 1;
+ from->engine[RCS].state->dirty = 1;
/* obj is kept alive until the next request by its active ref */
- i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+ i915_gem_object_ggtt_unpin(from->engine[RCS].state);
i915_gem_context_put(from);
}
engine->last_context = i915_gem_context_get(to);
@@ -812,19 +826,19 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
to->remap_slice &= ~(1<<i);
}
- if (!to->legacy_hw_ctx.initialized) {
+ if (!to->engine[RCS].initialised) {
if (engine->init_context) {
ret = engine->init_context(req);
if (ret)
return ret;
}
- to->legacy_hw_ctx.initialized = true;
+ to->engine[RCS].initialised = true;
}
return 0;
unpin_out:
- i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
+ i915_gem_object_ggtt_unpin(to->engine[RCS].state);
return ret;
}
@@ -848,8 +862,7 @@ int i915_switch_context(struct drm_i915_gem_request *req)
WARN_ON(i915.enable_execlists);
lockdep_assert_held(&req->i915->dev->struct_mutex);
- if (engine->id != RCS ||
- req->ctx->legacy_hw_ctx.rcs_state == NULL) {
+ if (!req->ctx->engine[engine->id].state) {
struct i915_gem_context *to = req->ctx;
struct i915_hw_ppgtt *ppgtt =
to->ppgtt ?: req->i915->mm.aliasing_ppgtt;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4b7c9680b097..558f069cd6d8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2413,31 +2413,6 @@ populate_lr_context(struct i915_gem_context *ctx,
}
/**
- * intel_lr_context_free() - free the LRC specific bits of a context
- * @ctx: the LR context to free.
- *
- * The real context freeing is done in i915_gem_context_free: this only
- * takes care of the bits that are LRC related: the per-engine backing
- * objects and the logical ringbuffer.
- */
-void intel_lr_context_free(struct i915_gem_context *ctx)
-{
- int i;
-
- for (i = I915_NUM_ENGINES; --i >= 0; ) {
- struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
- struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
-
- if (!ctx_obj)
- continue;
-
- WARN_ON(ctx->engine[i].pin_count);
- intel_ringbuffer_free(ringbuf);
- drm_gem_object_unreference(&ctx_obj->base);
- }
-}
-
-/**
* intel_lr_context_size() - return the size of the context for an engine
* @ring: which engine to find the context size for
*
@@ -2497,7 +2472,6 @@ static int execlists_context_deferred_alloc(struct
i915_gem_context *ctx,
struct intel_ringbuffer *ringbuf;
int ret;
- WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
WARN_ON(ce->state);
context_size = round_up(intel_lr_context_size(engine), 4096);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index eb2e1d157fed..a8db42a9c50f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -101,7 +101,6 @@ static inline void intel_logical_ring_emit_reg(struct
intel_ringbuffer *ringbuf,
struct i915_gem_context;
-void intel_lr_context_free(struct i915_gem_context *ctx);
uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
void intel_lr_context_unpin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
Looks good to me.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
