Re: [PATCH] drm/xe/oa: Enable Xe2+ PES disaggregation

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

On Tue, 10 Sep 2024 17:57:59 -0700, Lucas De Marchi wrote:
>
> On Tue, Sep 10, 2024 at 05:40:02PM GMT, Ashutosh Dixit wrote:
> > On Tue, 10 Sep 2024 10:04:32 -0700, Lucas De Marchi wrote:
> >>
> >
> > Hi Lucas,
> >
> > We had a whole email thread to figure this out :/
> >
> >> On Mon, Sep 09, 2024 at 09:59:33AM GMT, Ashutosh Dixit wrote:
> >> > Xe2+ PES disaggregation for OAG needs to be enabled to obtain disaggregated
> >> > metrics when disaggregated data is needed. There is no uapi impact of this
> >>
> >> it looks like the *control* for disaggregated data is only available on
> >> xe2
> >
> > Correct, that is why the check below (also this is available only for OAG,
> > not say for OAM):
> >
> >	if (GRAPHICS_VER(stream->oa->xe) >= 20 &&
> >	    stream->hwe->oa_unit->type == DRM_XE_OA_UNIT_TYPE_OAG)
> >
> >> and we always use disaggregated, right?
> >
> > No we don't, please see below.
> >
> >>
> >> > change.
> >> >
> >> > v2: Minor change to commit message
> >> >
> >> > Bspec: 61101
> >>
> >> "Note: After enabling desired disaggregation mode/s using this control
> >> field, PES MODE_SELECT field is used to configure appropriate
> >> disaggregation mode of corresponding PEC."
> >>
> >> Apparently MODE_SELECT == Aggregated is the hw default for xe2. Don't
> >> we need to program anything else?
> >
> > So the code in this patch only enables disaggregation, this additional step
> > (PES_MODE_SELECT) is still needed to actually use disaggregation. However
> > this second step is done via OA configurations which come in from userspace
> > (using add_config observation stream op). So userspace can request either
> > aggregated or disaggregated data depending on the OA config it programs.
> >
> > The goal of this patch is only to enable disaggregation so userspace can
> > use it if they want, otherwise they can't, they can only use
> > aggregated.
>
> can you add a paragraph to the commit message stating something like
> above?  Because it was not clear it was intentional to enable it in the
> control and not in the PES_MODE_SELECT.

I have changed the commit message to the following:

"
Enable Xe2+ PES disaggregation (for OAG) to retrieve disaggregated metrics
when disaggregated data is needed. Userspace can select whether to receive
aggregated or disaggregated metrics via the particular OA configuration it
uses (programmed via DRM_XE_OBSERVATION_OP_ADD_CONFIG).
"

I think this provides sufficient high level information, not bringing in
stuff like PES_MODE_SELECT. Or should I mention PES_MODE_SELECT too?
Thanks.



>
> thanks
> Lucas De Marchi