| Message ID | 20180604214022.22853-1-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/06/18 22:40, Michel Thierry wrote: > The upper 32 bits of the lrc_desc (bits 52-32 to be precise) are the > context hw id in GEN8-10, so use them and have one less thing to > maintain in the unlikely case we change the descriptor sw fields. > > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_perf.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index a6c8d61add0c..36b6d64d6018 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1279,7 +1279,8 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) > i915->perf.oa.specific_ctx_id_mask = > (1U << (GEN8_CTX_ID_WIDTH - 1)) - 1; > } else { > - i915->perf.oa.specific_ctx_id = stream->ctx->hw_id; > + i915->perf.oa.specific_ctx_id = > + upper_32_bits(ce->lrc_desc); > i915->perf.oa.specific_ctx_id_mask = > (1U << GEN8_CTX_ID_WIDTH) - 1; > } I would do this : i915->perf.oa.specific_ctx_id_mask = (1U << GEN8_CTX_ID_WIDTH) - 1; i915->perf.oa.specific_ctx_id = upper_32_bits(ce->lrc_desc) & i915->perf.oa.specific_ctx_id_mask; Same for Gen11. I'm concerned otherwise we might get incorrect comparison in the gen8_append_oa_reports on the "reserved" bits. Thanks, - Lionel
On 6/4/2018 4:11 PM, Lionel Landwerlin wrote: > On 04/06/18 22:40, Michel Thierry wrote: >> The upper 32 bits of the lrc_desc (bits 52-32 to be precise) are the >> context hw id in GEN8-10, so use them and have one less thing to >> maintain in the unlikely case we change the descriptor sw fields. >> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> --- >> drivers/gpu/drm/i915/i915_perf.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_perf.c >> b/drivers/gpu/drm/i915/i915_perf.c >> index a6c8d61add0c..36b6d64d6018 100644 >> --- a/drivers/gpu/drm/i915/i915_perf.c >> +++ b/drivers/gpu/drm/i915/i915_perf.c >> @@ -1279,7 +1279,8 @@ static int oa_get_render_ctx_id(struct >> i915_perf_stream *stream) >> i915->perf.oa.specific_ctx_id_mask = >> (1U << (GEN8_CTX_ID_WIDTH - 1)) - 1; >> } else { >> - i915->perf.oa.specific_ctx_id = stream->ctx->hw_id; >> + i915->perf.oa.specific_ctx_id = >> + upper_32_bits(ce->lrc_desc); >> i915->perf.oa.specific_ctx_id_mask = >> (1U << GEN8_CTX_ID_WIDTH) - 1; >> } > > I would do this : > > i915->perf.oa.specific_ctx_id_mask = (1U << GEN8_CTX_ID_WIDTH) - 1; > i915->perf.oa.specific_ctx_id = upper_32_bits(ce->lrc_desc) & > i915->perf.oa.specific_ctx_id_mask; > > Same for Gen11. > I'm concerned otherwise we might get incorrect comparison in the > gen8_append_oa_reports on the "reserved" bits. > For some reason I thought specific_ctx_id_mask was already being applied to this ctx_id... but no, you're right, otherwise the oa.specific_ctx_id == ctx_id check in gen8_append_oa_reports may fail. > Thanks, > > - > Lionel >
On 05/06/18 00:18, Michel Thierry wrote: > On 6/4/2018 4:11 PM, Lionel Landwerlin wrote: >> On 04/06/18 22:40, Michel Thierry wrote: >>> The upper 32 bits of the lrc_desc (bits 52-32 to be precise) are the >>> context hw id in GEN8-10, so use them and have one less thing to >>> maintain in the unlikely case we change the descriptor sw fields. >>> >>> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> --- >>> drivers/gpu/drm/i915/i915_perf.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_perf.c >>> b/drivers/gpu/drm/i915/i915_perf.c >>> index a6c8d61add0c..36b6d64d6018 100644 >>> --- a/drivers/gpu/drm/i915/i915_perf.c >>> +++ b/drivers/gpu/drm/i915/i915_perf.c >>> @@ -1279,7 +1279,8 @@ static int oa_get_render_ctx_id(struct >>> i915_perf_stream *stream) >>> i915->perf.oa.specific_ctx_id_mask = >>> (1U << (GEN8_CTX_ID_WIDTH - 1)) - 1; >>> } else { >>> - i915->perf.oa.specific_ctx_id = stream->ctx->hw_id; >>> + i915->perf.oa.specific_ctx_id = >>> + upper_32_bits(ce->lrc_desc); >>> i915->perf.oa.specific_ctx_id_mask = >>> (1U << GEN8_CTX_ID_WIDTH) - 1; >>> } >> >> I would do this : >> >> i915->perf.oa.specific_ctx_id_mask = (1U << GEN8_CTX_ID_WIDTH) - 1; >> i915->perf.oa.specific_ctx_id = upper_32_bits(ce->lrc_desc) & >> i915->perf.oa.specific_ctx_id_mask; >> >> Same for Gen11. >> I'm concerned otherwise we might get incorrect comparison in the >> gen8_append_oa_reports on the "reserved" bits. >> > > For some reason I thought specific_ctx_id_mask was already being > applied to this ctx_id... but no, you're right, otherwise the > oa.specific_ctx_id == ctx_id check in gen8_append_oa_reports may fail. Yeah, only applied on the OA report side :( > >> Thanks, >> >> - >> Lionel >> >
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index a6c8d61add0c..36b6d64d6018 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1279,7 +1279,8 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) i915->perf.oa.specific_ctx_id_mask = (1U << (GEN8_CTX_ID_WIDTH - 1)) - 1; } else { - i915->perf.oa.specific_ctx_id = stream->ctx->hw_id; + i915->perf.oa.specific_ctx_id = + upper_32_bits(ce->lrc_desc); i915->perf.oa.specific_ctx_id_mask = (1U << GEN8_CTX_ID_WIDTH) - 1; }
The upper 32 bits of the lrc_desc (bits 52-32 to be precise) are the context hw id in GEN8-10, so use them and have one less thing to maintain in the unlikely case we change the descriptor sw fields. Signed-off-by: Michel Thierry <michel.thierry@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_perf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)