[Intel-gfx] [PATCH] drm/i915/perf: Fix OA context id overlap with idle context id

[Lionel Landwerlin]

On 25/01/2020 03:37, Lionel Landwerlin wrote:
> On 24/01/2020 03:37, Umesh Nerlige Ramappa wrote:
>> Engine context pinned in perf OA was set to same context id as
>> the idle context. Set the context id to an unused value.
>>
>> Clear the sw context id field in lrc descriptor before ORing with
>> ce->tag (Chris)
>>
>> Fixes: https://gitlab.freedesktop.org/drm/intel/issues/756

I think the Fixes: tag is to point to another commit. And I think you 
want to point to the commit that you're fixing.

Not sure what you should use gitlab MRs.

-Lionel

>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
>>   drivers/gpu/drm/i915/i915_perf.c    | 9 +++++++--
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
>> b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> index a13a8c4b65ab..cf6c43bd540a 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> @@ -1211,12 +1211,12 @@ __execlists_schedule_in(struct i915_request *rq)
>>       if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
>>           execlists_check_context(ce, engine);
>>   +    ce->lrc_desc &= ~GENMASK_ULL(47, 37);
>>       if (ce->tag) {
>>           /* Use a fixed tag for OA and friends */
>>           ce->lrc_desc |= (u64)ce->tag << 32;
>>       } else {
>>           /* We don't need a strict matching tag, just different 
>> values */
>> -        ce->lrc_desc &= ~GENMASK_ULL(47, 37);
> I guess you can remove the line just above.
>>           ce->lrc_desc |=
>>               (u64)(++engine->context_tag % NUM_CONTEXT_TAG) <<
>>               GEN11_SW_CTX_ID_SHIFT;
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>> b/drivers/gpu/drm/i915/i915_perf.c
>> index 3c4647054557..5bd878c64504 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1323,7 +1323,12 @@ static int oa_get_render_ctx_id(struct 
>> i915_perf_stream *stream)
>>       case 12: {
>>           stream->specific_ctx_id_mask =
>>               ((1U << GEN11_SW_CTX_ID_WIDTH) - 1) << 
>> (GEN11_SW_CTX_ID_SHIFT - 32);
>> -        stream->specific_ctx_id = stream->specific_ctx_id_mask;
>> +        /* Pick an unused context id
>> +         * 0 - (NUM_CONTEXT_TAG - 1) 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);
>
>
> Arg yeah, we can't use an id that has all bits to 1 because that 
> matches the idle value in the OA reports :/
>
> This also affects gen8-10 cases (afaik).
>
>
> Thanks for spotting this!
>
>
> -Lionel
>
>
>>           break;
>>       }
>>   @@ -1331,7 +1336,7 @@ static int oa_get_render_ctx_id(struct 
>> i915_perf_stream *stream)
>>           MISSING_CASE(INTEL_GEN(ce->engine->i915));
>>       }
>>   -    ce->tag = stream->specific_ctx_id_mask;
>> +    ce->tag = stream->specific_ctx_id;
>>         DRM_DEBUG_DRIVER("filtering on ctx_id=0x%x ctx_id_mask=0x%x\n",
>>                stream->specific_ctx_id,
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx