Re: [PATCH 3/7] drm/i915: Record the sseu configuration per-context & engine

Chris Wilson
2018-09-05T15:18:12Z

Quoting Tvrtko Ursulin (2018-09-05 15:22:18)
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> We want to expose the ability to reconfigure the slices, subslice and
> eu per context and per engine. To facilitate that, store the current
> configuration on the context for each engine, which is initially set
> to the device default upon creation.
>
> v2: record sseu configuration per context & engine (Chris)
>
> v3: introduce the i915_gem_context_sseu to store powergating
> programming, sseu_dev_info has grown quite a bit (Lionel)
>
> v4: rename i915_gem_sseu into intel_sseu (Chris)
> use to_intel_context() (Chris)
>
> v5: More to_intel_context() (Tvrtko)
> Switch intel_sseu from union to struct (Tvrtko)
> Move context default sseu in existing loop (Chris)
>
> v6: s/intel_sseu_from_device_sseu/intel_device_default_sseu/ (Tvrtko)
>
> Tvrtko Ursulin:
>
> v7:
> * Pass intel_sseu by pointer instead of value to make_rpcs.
> * Rebase for make_rpcs changes.
>
> v8:
> * Rebase for RPCS edit on pin.
>
> v9:
> * Rebase for context image setup changes.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I feel this is substantially different (since I just outlined a v1!) to
merit a

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

and probably deserves a different author. I think Lionel is still the
principle author here, but Tvrtko has done a lot of refactoring and
integrating in the new scheme.

> -static u32 make_rpcs(struct drm_i915_private *dev_priv);
> +static u32 make_rpcs(struct drm_i915_private *dev_priv,
> + struct intel_sseu *ctx_sseu);
>
> static struct intel_context *
> __execlists_context_pin(struct intel_engine_cs *engine,
> @@ -1349,7 +1350,7 @@ __execlists_context_pin(struct intel_engine_cs *engine,
> /* RPCS */
> if (engine->class == RENDER_CLASS) {
> ce->lrc_reg_state[CTX_R_PWR_CLK_STATE + 1] =
> - make_rpcs(engine->i915);
> + make_rpcs(engine->i915, &ce->sseu);

We have different habits here; my vim config just gives this a single
tab indent beyond the incomplete line. (Was going to say it earlier ;)
-Chris