Re: [PATCH 30/46] drm/i915: Track active engines within a context

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 06/02/2019 13: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@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem_context.c       | 3 +++
>   drivers/gpu/drm/i915/i915_gem_context.h       | 4 ++++
>   drivers/gpu/drm/i915/intel_lrc.c              | 2 ++
>   drivers/gpu/drm/i915/intel_ringbuffer.c       | 2 ++
>   drivers/gpu/drm/i915/selftests/mock_context.c | 1 +
>   drivers/gpu/drm/i915/selftests/mock_engine.c  | 2 ++
>   6 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 582a7015e6a4..91037ca96be1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -210,6 +210,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);
> @@ -337,6 +338,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);
> @@ -364,6 +366,7 @@ __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);
>   
>   	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 651f2e4badb6..ab89c7501408 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -161,6 +161,8 @@ struct i915_gem_context {
>   	atomic_t hw_id_pin_count;
>   	struct list_head hw_id_link;
>   
> +	struct list_head active_engines;
> +
>   	/**
>   	 * @user_handle: userspace identifier
>   	 *
> @@ -174,7 +176,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 2424eb2b1fc6..b3555b1b0e07 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);
>   }
>   
> @@ -1361,6 +1362,7 @@ __execlists_context_pin(struct intel_engine_cs *engine,
>   
>   	ce->state->obj->pin_global++;
>   	i915_gem_context_get(ctx);
> +	list_add(&ce->active_link, &ctx->active_engines);

Why is it called active_engines if it lists active_contexts? :)

>   	return ce;
>   
>   unpin_ring:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 91c49f644898..4557f715663d 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);
>   }
>   
> @@ -1530,6 +1531,7 @@ __ring_context_pin(struct intel_engine_cs *engine,
>   		goto err_unpin;
>   
>   	i915_gem_context_get(ctx);
> +	list_add(&ce->active_link, &ctx->active_engines);
>   
>   	/* One ringbuffer to rule them all */
>   	GEM_BUG_ON(!engine->buffer);
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index b646cdcdd602..1b5073a362eb 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -44,6 +44,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);
> +	INIT_LIST_HEAD(&ctx->active_engines);
>   
>   	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..b8c6769571c4 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);
>   }
>   
> @@ -161,6 +162,7 @@ mock_context_pin(struct intel_engine_cs *engine,
>   
>   	ce->ops = &mock_context_ops;
>   	i915_gem_context_get(ctx);
> +	list_add(&ce->active_link, &ctx->active_engines);
>   	return ce;
>   
>   err:
> 

No other complaints! :)

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx