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

[Umesh Nerlige Ramappa]

On Fri, Jan 31, 2020 at 01:00:54PM +0200, Lionel Landwerlin wrote:
>On 31/01/2020 01:54, Umesh Nerlige Ramappa wrote:
>>On Mon, Jan 27, 2020 at 11:16:32AM +0200, Lionel Landwerlin wrote:
>>
>>[snip]
>>
>>>>>>--- 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).
>>>>
>>>>For gen8-10, I did not see a specific definition for an idle 
>>>>context id.  The from/to idle context switches are indicated by 
>>>>dedicated bits in the CSB instead (from spec).
>>>
>>>
>>>I meant that I remember the periodic OA reports when HW is idle to 
>>>have the contex_id=0xffffffff.
>>
>>For these gens we use 0x1fffff as the context id. Before we return 
>>reports to the user, we are setting context id to 0xffffffff for 
>>invalid and irrelevant contexts.
>>
>>Thanks,
>>Umesh
>
>
>Sorry, I've been a bit out of the loop on OA reports lately.
>
>I just noticed that the context valid bit is not checked on gen12 anymore.
>
>The documentation is really horrible, but BSpec 52198 seems to 
>indicate the bit is still around.

I am looking at the first table describing the report ID in 52198 which 
has TGL next to it. I don't see a definition for this bit.

I will try to ask around to see if that's not the case.
>
>
>Could it be the source of the issue?
You mean - seeing 0xffffffff during idle in periodic reports?

Thanks,
Umesh

>
>
>Thanks for your help :)
>
>
>-Lionel
>
>
>>>
>>>I could remember wrong :/
>>>
>>>
>>>-Lionel
>>>
>>>
>>>>
>>>>Thanks,
>>>>Umesh
>>>>
>>>>>
>>>>>
>>>>>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,
>>>>>
>>>>>
>>>
>