Re: [PATCH 4/5] drm/i915: Expose RPCS (SSEU) configuration to userspace (Gen11 only)

Chris Wilson
2019-01-30T09:46:20Z

Quoting Tvrtko Ursulin (2019-01-30 09:34:15)
> + rq = i915_request_alloc(engine, i915->kernel_context);
> + if (IS_ERR(rq)) {
> + ret = PTR_ERR(rq);
> + goto out_put;
> + }
> +
> + /* Queue this switch after all other activity by this context. */
> + prev = i915_gem_active_raw(&ce->ring->timeline->last_request,
> + &i915->drm.struct_mutex);
> + if (prev && !i915_request_completed(prev)) {
> + ret = i915_request_await_dma_fence(rq, &prev->fence);
> + if (ret < 0)
> + goto out_add;
> + }
> +
> + ret = gen8_emit_rpcs_config(rq, ce, sseu);
> + if (ret)
> + goto out_add;
> +
> + /* Order all following requests to be after. */
> + i915_timeline_set_barrier(ce->ring->timeline, rq);

You are making me regret not adding __must_check everywhere.

If you set the barrier before the emit_rpcs_config, then as we maintain
barrier ordering, it will be ok if we have to not emit the config
change.

> +
> + /*
> + * Guarantee context image and the timeline remains pinned until the
> + * modifying request is retired by setting the ce activity tracker.
> + *
> + * But we only need to take one pin on the account of it. Or in other
> + * words transfer the pinned ce object to tracked active request.
> + */
> + if (!i915_gem_active_isset(&ce->active_tracker))
> + __intel_context_pin(ce);
> + i915_gem_active_set(&ce->active_tracker, rq);

Hello ce->active_tracker, I have great plans for you.
-Chris