* [PATCH 1/2] drm/i915/execlists: Avoid reusing the same logical CC_ID
@ 2020-04-27 17:05 ` Chris Wilson
0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, Mika Kuoppala, stable
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)
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Intel-gfx] [PATCH 2/2] drm/i915/execlists: Verify we don't submit two identical CCIDs
2020-04-27 17:05 ` Chris Wilson
(?)
@ 2020-04-27 17:05 ` Chris Wilson
2020-04-27 17:31 ` Mika Kuoppala
-1 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson
Check that we do not submit two contexts into ELSP with the same CCID
[upper portion of the descriptor].
References: https://gitlab.freedesktop.org/drm/intel/-/issues/1793
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index d68a04f2a9d5..f8a8cd72f227 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1621,6 +1621,7 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
struct i915_request * const *port, *rq;
struct intel_context *ce = NULL;
bool sentinel = false;
+ u32 ccid = -1;
trace_ports(execlists, msg, execlists->pending);
@@ -1654,6 +1655,14 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
}
ce = rq->context;
+ if (ccid == upper_32_bits(ce->lrc_desc)) {
+ GEM_TRACE_ERR("Dup ccid:%x context:%llx in pending[%zd]\n",
+ ccid, ce->timeline->fence_context,
+ port - execlists->pending);
+ return false;
+ }
+ ccid = upper_32_bits(ce->lrc_desc);
+
/*
* Sentinels are supposed to be lonely so they flush the
* current exection off the HW. Check that they are the
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH 2/2] drm/i915/execlists: Verify we don't submit two identical CCIDs
2020-04-27 17:05 ` [Intel-gfx] [PATCH 2/2] drm/i915/execlists: Verify we don't submit two identical CCIDs Chris Wilson
@ 2020-04-27 17:31 ` Mika Kuoppala
2020-04-27 17:43 ` Chris Wilson
0 siblings, 1 reply; 14+ messages in thread
From: Mika Kuoppala @ 2020-04-27 17:31 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Chris Wilson
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Check that we do not submit two contexts into ELSP with the same CCID
> [upper portion of the descriptor].
>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/1793
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index d68a04f2a9d5..f8a8cd72f227 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1621,6 +1621,7 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
> struct i915_request * const *port, *rq;
> struct intel_context *ce = NULL;
> bool sentinel = false;
> + u32 ccid = -1;
>
> trace_ports(execlists, msg, execlists->pending);
>
> @@ -1654,6 +1655,14 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
> }
> ce = rq->context;
>
> + if (ccid == upper_32_bits(ce->lrc_desc)) {
> + GEM_TRACE_ERR("Dup ccid:%x context:%llx in pending[%zd]\n",
> + ccid, ce->timeline->fence_context,
> + port - execlists->pending);
The trace was lost, atleast from me, on the previous logs I looked
and thus the value. trace buffer overflowed? But if it
was reader error, then perhaps putting this explicitly in dmesg
is not necessary.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> + return false;
> + }
> + ccid = upper_32_bits(ce->lrc_desc);
> +
> /*
> * Sentinels are supposed to be lonely so they flush the
> * current exection off the HW. Check that they are the
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH 2/2] drm/i915/execlists: Verify we don't submit two identical CCIDs
2020-04-27 17:31 ` Mika Kuoppala
@ 2020-04-27 17:43 ` Chris Wilson
0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:43 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2020-04-27 18:31:37)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Check that we do not submit two contexts into ELSP with the same CCID
> > [upper portion of the descriptor].
> >
> > References: https://gitlab.freedesktop.org/drm/intel/-/issues/1793
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index d68a04f2a9d5..f8a8cd72f227 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -1621,6 +1621,7 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
> > struct i915_request * const *port, *rq;
> > struct intel_context *ce = NULL;
> > bool sentinel = false;
> > + u32 ccid = -1;
> >
> > trace_ports(execlists, msg, execlists->pending);
> >
> > @@ -1654,6 +1655,14 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
> > }
> > ce = rq->context;
> >
> > + if (ccid == upper_32_bits(ce->lrc_desc)) {
> > + GEM_TRACE_ERR("Dup ccid:%x context:%llx in pending[%zd]\n",
> > + ccid, ce->timeline->fence_context,
> > + port - execlists->pending);
>
> The trace was lost, atleast from me, on the previous logs I looked
> and thus the value. trace buffer overflowed? But if it
> was reader error, then perhaps putting this explicitly in dmesg
> is not necessary.
The trick is to look at the pstores. Or to reproduce it locally where
you can remotely capture the full trace.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915/execlists: Avoid reusing the same logical CC_ID
2020-04-27 17:05 ` Chris Wilson
@ 2020-04-27 17:28 ` Mika Kuoppala
-1 siblings, 0 replies; 14+ messages in thread
From: Mika Kuoppala @ 2020-04-27 17:28 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: stable, Chris Wilson
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?
> } 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] drm/i915/execlists: Avoid reusing the same logical CC_ID
@ 2020-04-27 17:28 ` Mika Kuoppala
0 siblings, 0 replies; 14+ messages in thread
From: Mika Kuoppala @ 2020-04-27 17:28 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Chris Wilson, stable
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?
> } 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH 1/2] drm/i915/execlists: Avoid reusing the same logical CC_ID
2020-04-27 17:28 ` Mika Kuoppala
@ 2020-04-27 17:36 ` Chris Wilson
-1 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:36 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: stable
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH 1/2] drm/i915/execlists: Avoid reusing the same logical CC_ID
@ 2020-04-27 17:36 ` Chris Wilson
0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:36 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/execlists: Avoid reusing the same logical CC_ID
2020-04-27 17:05 ` Chris Wilson
@ 2020-04-27 17:41 ` Chris Wilson
-1 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:41 UTC (permalink / raw)
To: intel-gfx; +Cc: stable, Chris Wilson
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)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH] drm/i915/execlists: Avoid reusing the same logical CC_ID
@ 2020-04-27 17:41 ` Chris Wilson
0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2020-04-27 17:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, Mika Kuoppala, stable
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)
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915/execlists: Avoid reusing the same logical CC_ID
2020-04-27 17:41 ` Chris Wilson
@ 2020-05-01 2:55 ` Sasha Levin
-1 siblings, 0 replies; 14+ messages in thread
From: Sasha Levin @ 2020-05-01 2:55 UTC (permalink / raw)
To: Sasha Levin, Chris Wilson, intel-gfx; +Cc: stable, Chris Wilson
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?
--
Thanks
Sasha
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] drm/i915/execlists: Avoid reusing the same logical CC_ID
@ 2020-05-01 2:55 ` Sasha Levin
0 siblings, 0 replies; 14+ messages in thread
From: Sasha Levin @ 2020-05-01 2:55 UTC (permalink / raw)
To: Sasha Levin, Chris Wilson, intel-gfx; +Cc: Chris Wilson, Mika Kuoppala, stable
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?
--
Thanks
Sasha
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with drm/i915/execlists: Avoid reusing the same logical CC_ID (rev2)
2020-04-27 17:05 ` Chris Wilson
` (3 preceding siblings ...)
(?)
@ 2020-04-27 20:25 ` Patchwork
-1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2020-04-27 20:25 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with drm/i915/execlists: Avoid reusing the same logical CC_ID (rev2)
URL : https://patchwork.freedesktop.org/series/76563/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8376 -> Patchwork_17484
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_17484 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_17484, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_17484:
### IGT changes ###
#### Possible regressions ####
* igt@gem_close_race@basic-process:
- fi-icl-y: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-icl-y/igt@gem_close_race@basic-process.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-icl-y/igt@gem_close_race@basic-process.html
* igt@gem_close_race@basic-threads:
- fi-icl-u2: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-icl-u2/igt@gem_close_race@basic-threads.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-icl-u2/igt@gem_close_race@basic-threads.html
- fi-icl-guc: [PASS][5] -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-icl-guc/igt@gem_close_race@basic-threads.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-icl-guc/igt@gem_close_race@basic-threads.html
* igt@gem_exec_gttfill@basic:
- fi-kbl-x1275: [PASS][7] -> [INCOMPLETE][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-kbl-x1275/igt@gem_exec_gttfill@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-kbl-x1275/igt@gem_exec_gttfill@basic.html
- fi-tgl-y: [PASS][9] -> [INCOMPLETE][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-tgl-y/igt@gem_exec_gttfill@basic.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-tgl-y/igt@gem_exec_gttfill@basic.html
- fi-cfl-8109u: [PASS][11] -> [INCOMPLETE][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-cfl-8109u/igt@gem_exec_gttfill@basic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-cfl-8109u/igt@gem_exec_gttfill@basic.html
- fi-bdw-5557u: [PASS][13] -> [INCOMPLETE][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-bdw-5557u/igt@gem_exec_gttfill@basic.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-bdw-5557u/igt@gem_exec_gttfill@basic.html
- fi-skl-guc: [PASS][15] -> [INCOMPLETE][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-skl-guc/igt@gem_exec_gttfill@basic.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-skl-guc/igt@gem_exec_gttfill@basic.html
- fi-kbl-7500u: [PASS][17] -> [INCOMPLETE][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-kbl-7500u/igt@gem_exec_gttfill@basic.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-kbl-7500u/igt@gem_exec_gttfill@basic.html
- fi-kbl-guc: [PASS][19] -> [INCOMPLETE][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-kbl-guc/igt@gem_exec_gttfill@basic.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-kbl-guc/igt@gem_exec_gttfill@basic.html
- fi-cfl-8700k: [PASS][21] -> [INCOMPLETE][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-cfl-8700k/igt@gem_exec_gttfill@basic.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-cfl-8700k/igt@gem_exec_gttfill@basic.html
- fi-whl-u: [PASS][23] -> [INCOMPLETE][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-whl-u/igt@gem_exec_gttfill@basic.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-whl-u/igt@gem_exec_gttfill@basic.html
- fi-skl-6600u: [PASS][25] -> [INCOMPLETE][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-skl-6600u/igt@gem_exec_gttfill@basic.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-skl-6600u/igt@gem_exec_gttfill@basic.html
- fi-skl-lmem: [PASS][27] -> [INCOMPLETE][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-skl-lmem/igt@gem_exec_gttfill@basic.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-skl-lmem/igt@gem_exec_gttfill@basic.html
- fi-cfl-guc: [PASS][29] -> [INCOMPLETE][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-cfl-guc/igt@gem_exec_gttfill@basic.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-cfl-guc/igt@gem_exec_gttfill@basic.html
- fi-skl-6700k2: [PASS][31] -> [INCOMPLETE][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-skl-6700k2/igt@gem_exec_gttfill@basic.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-skl-6700k2/igt@gem_exec_gttfill@basic.html
* igt@gem_sync@basic-each:
- fi-bsw-nick: [PASS][33] -> [INCOMPLETE][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-bsw-nick/igt@gem_sync@basic-each.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-bsw-nick/igt@gem_sync@basic-each.html
- fi-kbl-soraka: [PASS][35] -> [INCOMPLETE][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-kbl-soraka/igt@gem_sync@basic-each.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-kbl-soraka/igt@gem_sync@basic-each.html
- fi-apl-guc: [PASS][37] -> [INCOMPLETE][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-apl-guc/igt@gem_sync@basic-each.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-apl-guc/igt@gem_sync@basic-each.html
- fi-bxt-dsi: [PASS][39] -> [INCOMPLETE][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-bxt-dsi/igt@gem_sync@basic-each.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-bxt-dsi/igt@gem_sync@basic-each.html
* igt@i915_selftest@live@gt_contexts:
- fi-bsw-kefka: [PASS][41] -> [INCOMPLETE][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-bsw-kefka/igt@i915_selftest@live@gt_contexts.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-bsw-kefka/igt@i915_selftest@live@gt_contexts.html
- fi-cml-s: [PASS][43] -> [INCOMPLETE][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-cml-s/igt@i915_selftest@live@gt_contexts.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-cml-s/igt@i915_selftest@live@gt_contexts.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@gem_close_race@basic-process:
- {fi-ehl-1}: [PASS][45] -> [INCOMPLETE][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-ehl-1/igt@gem_close_race@basic-process.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-ehl-1/igt@gem_close_race@basic-process.html
* igt@gem_close_race@basic-threads:
- {fi-tgl-u}: [PASS][47] -> [INCOMPLETE][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-tgl-u/igt@gem_close_race@basic-threads.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-tgl-u/igt@gem_close_race@basic-threads.html
* {igt@gem_exec_parallel@engines@contexts}:
- {fi-tgl-dsi}: [PASS][49] -> [INCOMPLETE][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-tgl-dsi/igt@gem_exec_parallel@engines@contexts.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-tgl-dsi/igt@gem_exec_parallel@engines@contexts.html
* igt@gem_sync@basic-each:
- {fi-kbl-7560u}: NOTRUN -> [INCOMPLETE][51]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-kbl-7560u/igt@gem_sync@basic-each.html
Known issues
------------
Here are the changes found in Patchwork_17484 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@blt:
- fi-bsw-n3050: [PASS][52] -> [INCOMPLETE][53] ([i915#392])
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-bsw-n3050/igt@i915_selftest@live@blt.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-bsw-n3050/igt@i915_selftest@live@blt.html
* igt@i915_selftest@live@gem_contexts:
- fi-glk-dsi: [PASS][54] -> [INCOMPLETE][55] ([i915#1591] / [i915#58] / [k.org#198133])
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-glk-dsi/igt@i915_selftest@live@gem_contexts.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-glk-dsi/igt@i915_selftest@live@gem_contexts.html
* igt@i915_selftest@live@hangcheck:
- fi-kbl-8809g: [PASS][56] -> [INCOMPLETE][57] ([fdo#108744])
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8376/fi-kbl-8809g/igt@i915_selftest@live@hangcheck.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/fi-kbl-8809g/igt@i915_selftest@live@hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[i915#1591]: https://gitlab.freedesktop.org/drm/intel/issues/1591
[i915#392]: https://gitlab.freedesktop.org/drm/intel/issues/392
[i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (49 -> 44)
------------------------------
Missing (5): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_8376 -> Patchwork_17484
CI-20190529: 20190529
CI_DRM_8376: 3c32812d682625cce30878fa3139b58a014304c4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5612: c8dc1fd926a550308b971ca7d83fe0a927a38152 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_17484: f8ef49c73a6ed9cf7d1e70b8de63700c0cdd4496 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
f8ef49c73a6e drm/i915/execlists: Verify we don't submit two identical CCIDs
7285bba3d1a5 drm/i915/execlists: Avoid reusing the same logical CC_ID
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17484/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread