Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Fixes: 2935ed5339c4 ("drm/i915: Remove logical HW ID")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.5+
> ---
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 3 +--
> drivers/gpu/drm/i915/gt/intel_lrc.c | 23 ++++++++++++++------
> drivers/gpu/drm/i915/i915_perf.c | 3 +--
> drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +-
> 4 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index bf395227c99f..a9fc3fbbe482 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -304,8 +304,7 @@ struct intel_engine_cs {
> u32 context_size;
> u32 mmio_base;
>
> - unsigned int context_tag;
> -#define NUM_CONTEXT_TAG roundup_pow_of_two(2 * EXECLIST_MAX_PORTS)
> + unsigned long context_tag;
>
> struct rb_node uabi_node;
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 93a1b73ad96b..d68a04f2a9d5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1404,13 +1404,16 @@ __execlists_schedule_in(struct i915_request *rq)
> ce->lrc_desc &= ~GENMASK_ULL(47, 37);
> if (ce->tag) {
> /* Use a fixed tag for OA and friends */
> + GEM_BUG_ON(ce->tag <= BITS_PER_TYPE(engine->context_tag));
> ce->lrc_desc |= (u64)ce->tag << 32;
I see danger here to completely trash the upper part our our lrc_desc.
Is the ce->tag validated or should we add more enforcement in here?
> } else {
> /* We don't need a strict matching tag, just different values */
> - ce->lrc_desc |=
> - (u64)(++engine->context_tag % NUM_CONTEXT_TAG) <<
> - GEN11_SW_CTX_ID_SHIFT;
> - BUILD_BUG_ON(NUM_CONTEXT_TAG > GEN12_MAX_CONTEXT_HW_ID);
> + unsigned int tag = ffs(engine->context_tag);
> +
> + clear_bit(tag - 1, &engine->context_tag);
> + ce->lrc_desc |= (u64)tag << GEN11_SW_CTX_ID_SHIFT;
> +
> + BUILD_BUG_ON(BITS_PER_TYPE(engine->context_tag) > GEN12_MAX_CONTEXT_HW_ID);
> }
>
> __intel_gt_pm_get(engine->gt);
> @@ -1452,7 +1455,8 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
>
> static inline void
> __execlists_schedule_out(struct i915_request *rq,
> - struct intel_engine_cs * const engine)
> + struct intel_engine_cs * const engine,
> + int tag)
> {
> struct intel_context * const ce = rq->context;
>
> @@ -1470,6 +1474,9 @@ __execlists_schedule_out(struct i915_request *rq,
> i915_request_completed(rq))
> intel_engine_add_retire(engine, ce->timeline);
>
> + if (tag <= BITS_PER_TYPE(engine->context_tag))
> + set_bit(tag - 1, &engine->context_tag);
> +
> intel_context_update_runtime(ce);
> intel_engine_context_out(engine);
> execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
> @@ -1495,15 +1502,17 @@ execlists_schedule_out(struct i915_request *rq)
> {
> struct intel_context * const ce = rq->context;
> struct intel_engine_cs *cur, *old;
> + int tag;
>
> trace_i915_request_out(rq);
>
> + tag = upper_32_bits(rq->context->lrc_desc);
There is more in the upper part than just a tag (sw field).
So we need to only set/get a particular masked field.
-Mika
> old = READ_ONCE(ce->inflight);
> do
> cur = ptr_unmask_bits(old, 2) ? ptr_dec(old) : NULL;
> while (!try_cmpxchg(&ce->inflight, &old, cur));
> if (!cur)
> - __execlists_schedule_out(rq, old);
> + __execlists_schedule_out(rq, old, tag);
>
> i915_request_put(rq);
> }
> @@ -4002,7 +4011,7 @@ static void enable_execlists(struct intel_engine_cs *engine)
>
> enable_error_interrupt(engine);
>
> - engine->context_tag = 0;
> + engine->context_tag = -1u;
> }
>
> static bool unexpected_starting_state(struct intel_engine_cs *engine)
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index dec1b33e4da8..1863a5c4778d 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1281,11 +1281,10 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream)
> ((1U << GEN11_SW_CTX_ID_WIDTH) - 1) << (GEN11_SW_CTX_ID_SHIFT - 32);
> /*
> * Pick an unused context id
> - * 0 - (NUM_CONTEXT_TAG - 1) are used by other contexts
> + * 0 - BITS_PER_LONG are used by other contexts
> * GEN12_MAX_CONTEXT_HW_ID (0x7ff) is used by idle context
> */
> stream->specific_ctx_id = (GEN12_MAX_CONTEXT_HW_ID - 1) << (GEN11_SW_CTX_ID_SHIFT - 32);
> - BUILD_BUG_ON((GEN12_MAX_CONTEXT_HW_ID - 1) < NUM_CONTEXT_TAG);
> break;
> }
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
> index 58b5f40a07dd..af89c7fc8f59 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_vma.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
> @@ -173,7 +173,7 @@ static int igt_vma_create(void *arg)
> }
>
> nc = 0;
> - for_each_prime_number(num_ctx, 2 * NUM_CONTEXT_TAG) {
> + for_each_prime_number(num_ctx, 2 * BITS_PER_LONG) {
> for (; nc < num_ctx; nc++) {
> ctx = mock_context(i915, "mock");
> if (!ctx)
> --
> 2.20.1
