[Intel-gfx] [PATCH 5/6] drm/i915: Expose RPCS (SSEU) configuration to userspace (Gen11 only)

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-01-24 11:42:02)
> +static int gen8_emit_rpcs_config(struct i915_request *rq,
> +                                struct intel_context *ce,
> +                                struct intel_sseu sseu)
> +{
> +       u64 offset;
> +       u32 *cs;
> +
> +       cs = intel_ring_begin(rq, 4);
> +       if (IS_ERR(cs))
> +               return PTR_ERR(cs);
> +
> +       offset = ce->state->node.start +

Noted that we can do

u32 offset = i915_ggtt_offset(ce->state);

and chain all the asserts that this is indeed GGTT.

> +               LRC_STATE_PN * PAGE_SIZE +
> +               (CTX_R_PWR_CLK_STATE + 1) * 4;
> +
> +       *cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
> +       *cs++ = lower_32_bits(offset);
> +       *cs++ = upper_32_bits(offset);
> +       *cs++ = gen8_make_rpcs(rq->i915, &sseu);
> +
> +       intel_ring_advance(rq, cs);
> +
> +       return 0;
> +}
> +
> +static int
> +gen8_modify_rpcs_gpu(struct intel_context *ce,
> +                    struct intel_engine_cs *engine,
> +                    struct intel_sseu sseu)
> +{
> +       struct drm_i915_private *i915 = engine->i915;
> +       struct i915_request *rq, *prev;
> +       intel_wakeref_t wakeref;
> +       int ret;
> +
> +       GEM_BUG_ON(!ce->pin_count);
> +
> +       lockdep_assert_held(&i915->drm.struct_mutex);
> +
> +       /* Submitting requests etc needs the hw awake. */
> +       wakeref = intel_runtime_pm_get(i915);
> +
> +       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_sw_fence_await_sw_fence_gfp(&rq->submit,
> +                                                      &prev->submit,
> +                                                      I915_FENCE_GFP);

Looking back at this, this function is a bit on the lowlevel side for
us. You have to look carefully at the construction to be sure that both
rq and prev are on the same engine (which isn't available from the
immediate function parameters)

I suggest we use the more friendly
	ret = i915_request_await_dma_fence(rq, &prev->fence);
	if (ret)
		goto out_add;
The downside is that we add the timeline sync points, which we don't
ever expect to benefit from (except maybe on errors).

> +               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);

I've made this return an error, beware. :)

> +
> +       /*
> +        * 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);
> +
> +out_add:
> +       i915_request_add(rq);
> +out_put:
> +       intel_runtime_pm_put(i915, wakeref);
> +
> +       return ret;
> +}