Re: [Intel-gfx] [PATCH v4 8/9] drm/i915/perf: Add engine class instance parameters to perf

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

On Thu, 09 Mar 2023 14:34:04 -0800, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

> On Wed, Mar 08, 2023 at 10:08:13AM -0800, Dixit, Ashutosh wrote:
> > On Tue, 07 Mar 2023 12:16:10 -0800, Umesh Nerlige Ramappa wrote:
> >>
> >
> >> One or more engines map to a specific OA unit. All reports from these
> >> engines are captured in the OA buffer managed by this OA unit.
> >>
> >> Current i915 OA implementation supports only the OAG unit. OAG primarily
> >> caters to render engine, so i915 OA uses render as the default engine
> >> in the OA implementation. Since there are more OA units on newer
> >> hardware that map to other engines, allow user to pass engine class and
> >> instance to select and program specific OA units.
> >
> > I believe there is another uapi issue to be resolved here: how the engines
> > are associated with OA units, since from the point of view of the uapi
> > there are multiple engines and multiple OA units (even if in the actual
> > implementation at present there is only one OA unit per gt). In the
> > internal tree we had added oa_unit_id to engine_info for this. So if
> > multiple engines had the same oa_unit_id, user could pass class:instance of
> > any of those engines to get oa data from that OA unit (and generally know
> > how engines are hooked up to OA units (the OA unit topology)).
> >
> > So the question is even if we don't implement it as part of this series (or
> > do we have to?) do we at least need to agree to that uapi which will be
> > used to associate OA units with engines?
>
> It did make more sense for xehpsdv and other platforms where we had
> multiple OA units per GT and each GT had render and/or compute engines.  In
> those cases, media and compute/render UMDs may have needed to know that
> topology.
>
> For MTL, I don't think that an end user will benefit from the
> engine<->oa_unit mapping because
>
> (1) media and render are separate GTs
> (2) there is just one OA unit per GT and
> (3) assuming media and render/compute are separate UMDs,

Well our uapi does not expose any of this so we are relying on outside
information here. So I think it is needed even for MTL, but that's a
problem for the person reviewing the uapi. Afaiac we can do it later
if/when there is a specific ask for it, it's ok with me as is.

I will R-b the patch after we fix the engine class/instance pairing.

Thanks.
--
Ashutosh

>
> That's also the reason why the corresponding IGT series just uses a static
> mapping for MTL.
>
> If we come across a case where the UMD needs this info OR we are supporting
> a platform with multiple OA units per tile, we should add the topology.