Re: [PATCH] drm/i915/perf: Fix OA context id overlap with idle context id

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>]

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,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx