Fixes: 2935ed5339c4 ("drm/i915: Remove logical HW ID") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuoppala@linux.intel.com Cc: stable@vger.kernel.org # 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; } 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); 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)
Chris Wilson chris@chris-wilson.co.uk writes:
Fixes: 2935ed5339c4 ("drm/i915: Remove logical HW ID") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuoppala@linux.intel.com Cc: stable@vger.kernel.org # 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));
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*/ stream->specific_ctx_id = (GEN12_MAX_CONTEXT_HW_ID - 1) << (GEN11_SW_CTX_ID_SHIFT - 32);
- GEN12_MAX_CONTEXT_HW_ID (0x7ff) is used by idle context
BUILD_BUG_ON((GEN12_MAX_CONTEXT_HW_ID - 1) < NUM_CONTEXT_TAG);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
Quoting Mika Kuoppala (2020-04-27 18:28:12)
Chris Wilson chris@chris-wilson.co.uk writes:
Fixes: 2935ed5339c4 ("drm/i915: Remove logical HW ID") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuoppala@linux.intel.com Cc: stable@vger.kernel.org # 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?
It's a single special case atm.
It's a problem for tomorrow, but it'll probably mean a small ida if we have multiple users who must dictate the CCID. Pita.
} 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.
We control the contents, though. Oops, but what I did forget was the shift. -Chris
Fixes: 2935ed5339c4 ("drm/i915: Remove logical HW ID") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuoppala@linux.intel.com Cc: stable@vger.kernel.org # v5.5+ --- drivers/gpu/drm/i915/gt/intel_engine_types.h | 3 +-- drivers/gpu/drm/i915/gt/intel_lrc.c | 25 ++++++++++++++------ drivers/gpu/drm/i915/i915_perf.c | 3 +-- drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +- 4 files changed, 21 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..3a9d3ebf3e5b 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1404,13 +1404,17 @@ __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; } 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); + + GEM_BUG_ON(tag >= BITS_PER_LONG); + 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 +1456,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, + unsigned int ccid) { struct intel_context * const ce = rq->context;
@@ -1470,6 +1475,10 @@ __execlists_schedule_out(struct i915_request *rq, i915_request_completed(rq)) intel_engine_add_retire(engine, ce->timeline);
+ ccid >>= (GEN11_SW_CTX_ID_SHIFT - 32); + if (ccid < BITS_PER_TYPE(engine->context_tag)) + set_bit(ccid - 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 +1504,17 @@ execlists_schedule_out(struct i915_request *rq) { struct intel_context * const ce = rq->context; struct intel_engine_cs *cur, *old; + unsigned int ccid;
trace_i915_request_out(rq);
+ ccid = upper_32_bits(rq->context->lrc_desc); 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, ccid);
i915_request_put(rq); } @@ -4002,7 +4013,7 @@ static void enable_execlists(struct intel_engine_cs *engine)
enable_error_interrupt(engine);
- engine->context_tag = 0; + engine->context_tag = GENMASK(BITS_PER_LONG - 2, 0); }
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)
Hi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag fixing commit: 2935ed5339c4 ("drm/i915: Remove logical HW ID").
The bot has tested the following trees: v5.6.7.
v5.6.7: Failed to apply! Possible dependencies: 03d0ed8a8e93 ("drm/i915: Skip capturing errors from internal contexts") 1883a0a4658e ("drm/i915: Track hw reported context runtime") 6f280b133dc2 ("drm/i915/perf: Fix OA context id overlap with idle context id") 70a76a9b8e9d ("drm/i915/gt: Hook up CS_MASTER_ERROR_INTERRUPT") ff3d4ff6c9e6 ("drm/i915/gt: Tidy repetition in declaring gen8+ interrupts")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
linux-stable-mirror@lists.linaro.org
-
Chris Wilson -
Mika Kuoppala -
Sasha Levin