| Message ID | 20190206130356.18771-31-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/46] drm/i915: Hack and slash, throttle execbuffer hogs | expand |
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
Quoting Tvrtko Ursulin (2019-02-11 19:11:08) > > On 06/02/2019 13:03, Chris Wilson wrote: > > @@ -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? :) active_context_engines doesn't quite have the same ring to it. It's the list of engines on which this GEM context has been pinned, which is my approximation for active and is the list I need to flush if we change logical state. -Chris
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); 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:
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(+)