Re: [Intel-gfx] [PATCH 01/19] drm/i915/perf: Fix OA filtering logic for GuC mode

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 21 Sep 2022 20:44:57 -0700, Dixit, Ashutosh wrote:
>
> On Fri, 09 Sep 2022 16:47:36 -0700, Dixit, Ashutosh wrote:
> >
> > On Tue, 23 Aug 2022 13:41:37 -0700, Umesh Nerlige Ramappa wrote:
> > >
> > > +/*
> > > + * For execlist mode of submission, pick an unused context id
> > > + * 0 - (NUM_CONTEXT_TAG -1) are used by other contexts
> > > + * XXX_MAX_CONTEXT_HW_ID is used by idle context
> > > + *
> > > + * For GuC mode of submission read context id from the upper dword of the
> > > + * EXECLIST_STATUS register.
> > > + */
> > > +static int gen12_get_render_context_id(struct i915_perf_stream *stream)
> > > +{
> > > +	u32 ctx_id, mask;
> > > +	int ret;
> > > +
> > > +	if (intel_engine_uses_guc(stream->engine)) {
> > > +		ret = gen12_guc_sw_ctx_id(stream->pinned_ctx, &ctx_id);
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		mask = ((1U << GEN12_GUC_SW_CTX_ID_WIDTH) - 1) <<
> > > +			(GEN12_GUC_SW_CTX_ID_SHIFT - 32);
> > > +	} else if (GRAPHICS_VER_FULL(stream->engine->i915) >= IP_VER(12, 50)) {
> > > +		ctx_id = (XEHP_MAX_CONTEXT_HW_ID - 1) <<
> > > +			(XEHP_SW_CTX_ID_SHIFT - 32);
> > > +
> > > +		mask = ((1U << XEHP_SW_CTX_ID_WIDTH) - 1) <<
> > > +			(XEHP_SW_CTX_ID_SHIFT - 32);
> > > +	} else {
> > > +		ctx_id = (GEN12_MAX_CONTEXT_HW_ID - 1) <<
> > > +			 (GEN11_SW_CTX_ID_SHIFT - 32);
> > > +
> > > +		mask = ((1U << GEN11_SW_CTX_ID_WIDTH) - 1) <<
> > > +			(GEN11_SW_CTX_ID_SHIFT - 32);
> >
> > Previously I missed that these ctx_id's for non-GuC cases are just
> > constants. How does it work in these cases?
>
> For the record, offline reply from Umesh for this question:
>
> Looks like the SW context id is set to a unique value by the KMD for
> execlist mode here - __execlists_schedule_in() as ccid. Later it is written
> to the execlist port here (as lrc.desc) - execlists_submit_ports(). It's
> just a unique value that the kmd determines. For OA we are setting a
> ce->tag when OA use case is active and it used by the
> __execlists_schedule_in().
>
> Related commit from Chris - 2935ed5339c49
>
> I think the reason why OA is setting it is because this value is not
> assigned until __execlists_schedule_in() is called. For OA context, this
> may happen much later. The code that Chris has added, just assigns a value
> in OA and then uses it later in the __execlists_schedule_in() path.

I would still think this should not be a constant value but something which
depends on the context or the context id. Anyway since this is a
pre-existing issue not introducd in this patch, I will disregard this and
continue reviewing this patch.

Thanks.
--
Ashutosh