[Intel-gfx] [PATCH 08/21] drm/i915/guc: Make use of the SW counter field in the context descriptor
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Thu Aug 30 00:08:01 UTC 2018
On 29/08/2018 20:16, Michal Wajdeczko wrote:
> The new context descriptor format contains two assignable fields:
> the SW Context ID (technically 11 bits, but practically limited to 2032
> entries due to some being reserved for future use by the GuC) and the
> SW Counter (6 bits).
>
> We don't want to limit ourselves too much in the maximum number of
> concurrent contexts we want to allow, so ideally we want to employ
> every possible bit available. Unfortunately, a further limitation in
> the interface with the GuC means the combination of SW Context ID +
> SW Counter has to be unique within the same engine class (as we use
> the SW Context ID to index in the GuC stage descriptor pool, and the
> Engine Class + SW Counter to index in the 2-dimensional lrc array).
> This essentially means we need to somehow encode the engine instance.
>
> Since the BSpec allows 6 bits for engine instance, we use the whole
> SW counter for this task. If the limitation of 2032 maximum simultaneous
> contexts is too restrictive, we can always squeeze things a bit more
> (3 extras bits for hw_id, 3 bits for instance) and things will still
> work (Gen11 does not instance more than 8 engines of any class).
>
> Another alternative would be to generate the hw_id per HW context
> instead of per GEM context, but that has other problems (e.g. maximum
> number of user-created contexts would be variable, no relationship
> between a GuC principal descriptor and the proxy descriptor it uses, ...)
>
> Bspec: 12254
>
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 15 +++++++++++----
> drivers/gpu/drm/i915/i915_gem_context.c | 5 ++++-
> drivers/gpu/drm/i915/i915_gem_context.h | 2 ++
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_lrc.c | 12 +++++++++---
> 5 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e5b9d3c..34f5495 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1866,14 +1866,21 @@ struct drm_i915_private {
> struct llist_head free_list;
> struct work_struct free_work;
>
> - /* The hw wants to have a stable context identifier for the
> + /*
> + * The HW wants to have a stable context identifier for the
> * lifetime of the context (for OA, PASID, faults, etc).
> * This is limited in execlists to 21 bits.
> + * In enhanced execlist (GEN11+) this is limited to 11 bits
> + * (the SW Context ID field) but GuC limits it a bit further
> + * (11 bits - 16) due to some entries being reserved for future
> + * use (so the firmware only supports a GuC stage descriptor
> + * pool of 2032 entries).
> */
> struct ida hw_ida;
> -#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
> -#define MAX_GUC_CONTEXT_HW_ID (1 << 20) /* exclusive */
> -#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
> +#define MAX_CONTEXT_HW_ID (1 << 21) /* exclusive */
> +#define MAX_GUC_CONTEXT_HW_ID (1 << 20) /* exclusive */
> +#define GEN11_MAX_CONTEXT_HW_ID (1 << 11) /* exclusive */
> +#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC (GEN11_MAX_CONTEXT_HW_ID - 16)
> } contexts;
>
> u32 fdi_rx_config;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index f15a039..e3b500c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -209,7 +209,10 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
> unsigned int max;
>
> if (INTEL_GEN(dev_priv) >= 11) {
> - max = GEN11_MAX_CONTEXT_HW_ID;
> + if (USES_GUC_SUBMISSION(dev_priv))
> + max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
> + else
> + max = GEN11_MAX_CONTEXT_HW_ID;
> } else {
> /*
> * When using GuC in proxy submission, GuC consumes the
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 851dad6..4b87f5d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -154,6 +154,8 @@ struct i915_gem_context {
> struct intel_ring *ring;
> u32 *lrc_reg_state;
> u64 lrc_desc;
> + u32 sw_context_id;
> + u32 sw_counter;
> int pin_count;
>
> const struct intel_context_ops *ops;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f232178..ea65d7b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3900,6 +3900,8 @@ enum {
> #define GEN8_CTX_ID_WIDTH 21
> #define GEN11_SW_CTX_ID_SHIFT 37
> #define GEN11_SW_CTX_ID_WIDTH 11
> +#define GEN11_SW_COUNTER_SHIFT 55
> +#define GEN11_SW_COUNTER_WIDTH 6
> #define GEN11_ENGINE_CLASS_SHIFT 61
> #define GEN11_ENGINE_CLASS_WIDTH 3
> #define GEN11_ENGINE_INSTANCE_SHIFT 48
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f4b9972..3001a14 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -240,14 +240,15 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
> * anything below.
> */
> if (INTEL_GEN(ctx->i915) >= 11) {
Hey Michal,
There is a comment just above the if () about updating the i915_perf.c
(oa_get_render_ctx_id) when descriptor is updated.
Otherwise this will break some part of the observability feature.
You can verify this with the IGT tests/perf
--run-subtest=gen8-unprivileged-single-ctx-counters
Thanks a lot,
-
Lionel
> - GEM_BUG_ON(ctx->hw_id >= BIT(GEN11_SW_CTX_ID_WIDTH));
> - desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
> + GEM_BUG_ON(ce->sw_context_id >= BIT(GEN11_SW_CTX_ID_WIDTH));
> + desc |= (u64)ce->sw_context_id << GEN11_SW_CTX_ID_SHIFT;
> /* bits 37-47 */
>
> desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
> /* bits 48-53 */
>
> - /* TODO: decide what to do with SW counter (bits 55-60) */
> + desc |= (u64)ce->sw_counter << GEN11_SW_COUNTER_SHIFT;
> + /* bits 55-60 */
>
> /*
> * Although GuC will never see this upper part as it fills
> @@ -2771,6 +2772,11 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> ce->ring = ring;
> ce->state = vma;
>
> + if (INTEL_GEN(ctx->i915) >= 11) {
> + ce->sw_context_id = ctx->hw_id;
> + ce->sw_counter = engine->instance;
> + }
> +
> return 0;
>
> error_ring_free:
More information about the Intel-gfx
mailing list