| Message ID | 20190301140404.26690-15-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/38] drm/i915/execlists: Suppress redundant preemption | expand |
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@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@intel.com> Regards, Tvrtko
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;
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_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(-)