[Intel-gfx] [PATCH 15/38] drm/i915: Track active engines within a context

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Fri Mar 1 15:46:31 UTC 2019


On 01/03/2019 14:03, Chris Wilson wrote:
> For use in the next patch, if we track which engines have been used by
> the HW, we can reduce the work required to flush our state off the HW to
> those engines.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c           | 18 +++++----------
>   drivers/gpu/drm/i915/i915_gem_context.c       |  5 +++++
>   drivers/gpu/drm/i915/i915_gem_context.h       |  5 +++++
>   drivers/gpu/drm/i915/intel_lrc.c              | 22 +++++++++----------
>   drivers/gpu/drm/i915/intel_ringbuffer.c       | 14 +++++++-----
>   drivers/gpu/drm/i915/selftests/mock_context.c |  2 ++
>   drivers/gpu/drm/i915/selftests/mock_engine.c  |  6 +++++
>   7 files changed, 43 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 298371aad445..36c63b087ffd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -388,12 +388,9 @@ static void print_context_stats(struct seq_file *m,
>   	struct i915_gem_context *ctx;
>   
>   	list_for_each_entry(ctx, &i915->contexts.list, link) {
> -		struct intel_engine_cs *engine;
> -		enum intel_engine_id id;
> -
> -		for_each_engine(engine, i915, id) {
> -			struct intel_context *ce = to_intel_context(ctx, engine);
> +		struct intel_context *ce;
>   
> +		list_for_each_entry(ce, &ctx->active_engines, active_link) {
>   			if (ce->state)
>   				per_file_stats(0, ce->state->obj, &kstats);
>   			if (ce->ring)
> @@ -1880,9 +1877,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   {
>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>   	struct drm_device *dev = &dev_priv->drm;
> -	struct intel_engine_cs *engine;
>   	struct i915_gem_context *ctx;
> -	enum intel_engine_id id;
>   	int ret;
>   
>   	ret = mutex_lock_interruptible(&dev->struct_mutex);
> @@ -1890,6 +1885,8 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   		return ret;
>   
>   	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> +		struct intel_context *ce;
> +
>   		seq_puts(m, "HW context ");
>   		if (!list_empty(&ctx->hw_id_link))
>   			seq_printf(m, "%x [pin %u]", ctx->hw_id,
> @@ -1912,11 +1909,8 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   		seq_putc(m, ctx->remap_slice ? 'R' : 'r');
>   		seq_putc(m, '\n');
>   
> -		for_each_engine(engine, dev_priv, id) {
> -			struct intel_context *ce =
> -				to_intel_context(ctx, engine);
> -
> -			seq_printf(m, "%s: ", engine->name);
> +		list_for_each_entry(ce, &ctx->active_engines, active_link) {
> +			seq_printf(m, "%s: ", ce->engine->name);
>   			if (ce->state)
>   				describe_obj(m, ce->state->obj);
>   			if (ce->ring)
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 004ffcfb305d..3b5145b30d85 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -224,6 +224,7 @@ 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));
>   
>   	release_hw_id(ctx);
>   	i915_ppgtt_put(ctx->ppgtt);
> @@ -239,6 +240,7 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
>   	put_pid(ctx->pid);
>   
>   	list_del(&ctx->link);
> +	mutex_destroy(&ctx->mutex);
>   
>   	kfree_rcu(ctx, rcu);
>   }
> @@ -351,6 +353,7 @@ intel_context_init(struct intel_context *ce,
>   		   struct intel_engine_cs *engine)
>   {
>   	ce->gem_context = ctx;
> +	ce->engine = engine;
>   
>   	INIT_LIST_HEAD(&ce->signal_link);
>   	INIT_LIST_HEAD(&ce->signals);
> @@ -379,6 +382,8 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>   	list_add_tail(&ctx->link, &dev_priv->contexts.list);
>   	ctx->i915 = dev_priv;
>   	ctx->sched.priority = I915_USER_PRIORITY(I915_PRIORITY_NORMAL);
> +	INIT_LIST_HEAD(&ctx->active_engines);
> +	mutex_init(&ctx->mutex);
>   
>   	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++)
>   		intel_context_init(&ctx->__engine[n], ctx, dev_priv->engine[n]);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index c39dbb32a5c6..df48013b581e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -163,6 +163,9 @@ struct i915_gem_context {
>   	atomic_t hw_id_pin_count;
>   	struct list_head hw_id_link;
>   
> +	struct list_head active_engines;
> +	struct mutex mutex;
> +
>   	/**
>   	 * @user_handle: userspace identifier
>   	 *
> @@ -176,7 +179,9 @@ struct i915_gem_context {
>   	/** engine: per-engine logical HW state */
>   	struct intel_context {
>   		struct i915_gem_context *gem_context;
> +		struct intel_engine_cs *engine;
>   		struct intel_engine_cs *active;
> +		struct list_head active_link;
>   		struct list_head signal_link;
>   		struct list_head signals;
>   		struct i915_vma *state;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 2268860cca44..50a7c87705c8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1276,6 +1276,7 @@ static void execlists_context_unpin(struct intel_context *ce)
>   	i915_gem_object_unpin_map(ce->state->obj);
>   	i915_vma_unpin(ce->state);
>   
> +	list_del(&ce->active_link);
>   	i915_gem_context_put(ce->gem_context);
>   }
>   
> @@ -1360,6 +1361,11 @@ __execlists_context_pin(struct intel_engine_cs *engine,
>   	__execlists_update_reg_state(engine, ce);
>   
>   	ce->state->obj->pin_global++;
> +
> +	mutex_lock(&ctx->mutex);
> +	list_add(&ce->active_link, &ctx->active_engines);
> +	mutex_unlock(&ctx->mutex);
> +
>   	i915_gem_context_get(ctx);
>   	return ce;
>   
> @@ -2880,9 +2886,8 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   
>   void intel_lr_context_resume(struct drm_i915_private *i915)
>   {
> -	struct intel_engine_cs *engine;
>   	struct i915_gem_context *ctx;
> -	enum intel_engine_id id;
> +	struct intel_context *ce;
>   
>   	/*
>   	 * Because we emit WA_TAIL_DWORDS there may be a disparity
> @@ -2896,17 +2901,10 @@ void intel_lr_context_resume(struct drm_i915_private *i915)
>   	 * simplicity, we just zero everything out.
>   	 */
>   	list_for_each_entry(ctx, &i915->contexts.list, link) {
> -		for_each_engine(engine, i915, id) {
> -			struct intel_context *ce =
> -				to_intel_context(ctx, engine);
> -
> -			if (!ce->state)
> -				continue;
> -
> +		list_for_each_entry(ce, &ctx->active_engines, active_link) {
> +			GEM_BUG_ON(!ce->ring);
>   			intel_ring_reset(ce->ring, 0);
> -
> -			if (ce->pin_count) /* otherwise done in context_pin */
> -				__execlists_update_reg_state(engine, ce);
> +			__execlists_update_reg_state(ce->engine, ce);
>   		}
>   	}
>   }
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index e78594ecba6f..764dcc5d5856 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1430,6 +1430,7 @@ static void intel_ring_context_unpin(struct intel_context *ce)
>   	__context_unpin_ppgtt(ce->gem_context);
>   	__context_unpin(ce);
>   
> +	list_del(&ce->active_link);
>   	i915_gem_context_put(ce->gem_context);
>   }
>   
> @@ -1509,6 +1510,10 @@ __ring_context_pin(struct intel_engine_cs *engine,
>   {
>   	int err;
>   
> +	/* One ringbuffer to rule them all */
> +	GEM_BUG_ON(!engine->buffer);
> +	ce->ring = engine->buffer;
> +
>   	if (!ce->state && engine->context_size) {
>   		struct i915_vma *vma;
>   
> @@ -1529,12 +1534,11 @@ __ring_context_pin(struct intel_engine_cs *engine,
>   	if (err)
>   		goto err_unpin;
>   
> -	i915_gem_context_get(ctx);
> -
> -	/* One ringbuffer to rule them all */
> -	GEM_BUG_ON(!engine->buffer);
> -	ce->ring = engine->buffer;
> +	mutex_lock(&ctx->mutex);
> +	list_add(&ce->active_link, &ctx->active_engines);
> +	mutex_unlock(&ctx->mutex);
>   
> +	i915_gem_context_get(ctx);
>   	return ce;
>   
>   err_unpin:
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index b646cdcdd602..353b37b9f78e 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -44,6 +44,8 @@ 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);
> +	INIT_LIST_HEAD(&ctx->active_engines);
> +	mutex_init(&ctx->mutex);
>   
>   	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++)
>   		intel_context_init(&ctx->__engine[n], ctx, i915->engine[n]);
> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
> index c2c954f64226..8032a8a9542f 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
> @@ -125,6 +125,7 @@ static void hw_delay_complete(struct timer_list *t)
>   static void mock_context_unpin(struct intel_context *ce)
>   {
>   	mock_timeline_unpin(ce->ring->timeline);
> +	list_del(&ce->active_link);
>   	i915_gem_context_put(ce->gem_context);
>   }
>   
> @@ -160,6 +161,11 @@ mock_context_pin(struct intel_engine_cs *engine,
>   	mock_timeline_pin(ce->ring->timeline);
>   
>   	ce->ops = &mock_context_ops;
> +
> +	mutex_lock(&ctx->mutex);
> +	list_add(&ce->active_link, &ctx->active_engines);
> +	mutex_unlock(&ctx->mutex);
> +
>   	i915_gem_context_get(ctx);
>   	return ce;
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko


More information about the Intel-gfx mailing list