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 <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> 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 <maarten.lankhorst@xxxxxxxxxxxxxxx>
> @@ -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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
