Op 11-03-2020 om 13:49 schreef Chris Wilson: > When applying the context-barrier, we only care about the current > engines, as the next set of engines will be naturally after the barrier. > So we can skip holding the ctx->engines_mutex while constructing the > request by taking a sneaky reference to the i915_gem_engines instead. > > Signed-off-by: Chris Wilson <[email protected]> > Cc: Maarten Lankhorst <[email protected]> > --- > drivers/gpu/drm/i915/gem/i915_gem_context.c | 89 ++++++++++++++------- > 1 file changed, 58 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c > b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index 50ecc0b2b235..e2357099a9ed 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -261,6 +261,34 @@ static void free_engines_rcu(struct rcu_head *rcu) > free_engines(engines); > } > > +static int engines_notify(struct i915_sw_fence *fence, > + enum i915_sw_fence_notify state) > +{ > + struct i915_gem_engines *engines = > + container_of(fence, typeof(*engines), fence); > + > + switch (state) { > + case FENCE_COMPLETE: > + if (!list_empty(&engines->link)) { > + struct i915_gem_context *ctx = engines->ctx; > + unsigned long flags; > + > + spin_lock_irqsave(&ctx->stale.lock, flags); > + list_del(&engines->link); > + spin_unlock_irqrestore(&ctx->stale.lock, flags); > + } > + i915_gem_context_put(engines->ctx); > + break; > + > + case FENCE_FREE: > + init_rcu_head(&engines->rcu); > + call_rcu(&engines->rcu, free_engines_rcu); > + break; > + } > + > + return NOTIFY_DONE; > +} > + > static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx) > { > const struct intel_gt *gt = &ctx->i915->gt; > @@ -272,6 +300,8 @@ static struct i915_gem_engines *default_engines(struct > i915_gem_context *ctx) > if (!e) > return ERR_PTR(-ENOMEM); > > + i915_sw_fence_init(&e->fence, engines_notify); > + > for_each_engine(engine, gt, id) { > struct intel_context *ce; > > @@ -519,41 +549,12 @@ static void kill_context(struct i915_gem_context *ctx) > kill_stale_engines(ctx); > } > > -static int engines_notify(struct i915_sw_fence *fence, > - enum i915_sw_fence_notify state) > -{ > - struct i915_gem_engines *engines = > - container_of(fence, typeof(*engines), fence); > - > - switch (state) { > - case FENCE_COMPLETE: > - if (!list_empty(&engines->link)) { > - struct i915_gem_context *ctx = engines->ctx; > - unsigned long flags; > - > - spin_lock_irqsave(&ctx->stale.lock, flags); > - list_del(&engines->link); > - spin_unlock_irqrestore(&ctx->stale.lock, flags); > - } > - i915_gem_context_put(engines->ctx); > - break; > - > - case FENCE_FREE: > - init_rcu_head(&engines->rcu); > - call_rcu(&engines->rcu, free_engines_rcu); > - break; > - } > - > - return NOTIFY_DONE; > -} > - > static void engines_idle_release(struct i915_gem_context *ctx, > struct i915_gem_engines *engines) > { > struct i915_gem_engines_iter it; > struct intel_context *ce; > > - i915_sw_fence_init(&engines->fence, engines_notify); > INIT_LIST_HEAD(&engines->link); > > engines->ctx = i915_gem_context_get(ctx); > @@ -1079,6 +1080,30 @@ static void cb_retire(struct i915_active *base) > kfree(cb); > } > > +static inline struct i915_gem_engines * > +__context_engines_await(const struct i915_gem_context *ctx) > +{ > + struct i915_gem_engines *engines; > + > + rcu_read_lock(); > + do { > + engines = rcu_dereference(ctx->engines); > + if (!engines) > + break; > + > + if (!i915_sw_fence_await(&engines->fence)) > + continue; > + > + if (engines == rcu_access_pointer(ctx->engines)) > + break; > + > + i915_sw_fence_complete(&engines->fence); > + } while(1); > + rcu_read_unlock(); > + > + return engines; > +} > + > I915_SELFTEST_DECLARE(static intel_engine_mask_t > context_barrier_inject_fault); > static int context_barrier_task(struct i915_gem_context *ctx, > intel_engine_mask_t engines, > @@ -1089,6 +1114,7 @@ static int context_barrier_task(struct i915_gem_context > *ctx, > { > struct context_barrier_task *cb; > struct i915_gem_engines_iter it; > + struct i915_gem_engines *e; > struct intel_context *ce; > int err = 0; > > @@ -1105,7 +1131,8 @@ static int context_barrier_task(struct i915_gem_context > *ctx, > return err; > } > > - for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { > + e = __context_engines_await(ctx); > + for_each_gem_engine(ce, e, it) { > struct i915_request *rq; > > if (I915_SELFTEST_ONLY(context_barrier_inject_fault &
This doesn't need RCU, but it will work anyway. :) Reviewed-by: Maarten Lankhorst <[email protected]> > @@ -1136,7 +1163,7 @@ static int context_barrier_task(struct i915_gem_context > *ctx, > if (err) > break; > } > - i915_gem_context_unlock_engines(ctx); > + i915_sw_fence_complete(&e->fence); > > cb->task = err ? NULL : task; /* caller needs to unwind instead */ > cb->data = data; _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
