Re: [PATCH i-g-t 1/2] lib/xe/oa: Add observation type input to intel_xe_perf_ioctl and intel_xe_perf_ioctl_err

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

On Fri, 31 Jan 2025 12:36:45 -0800, Dixit, Ashutosh wrote:
>
> On Fri, 31 Jan 2025 12:34:31 -0800, Dixit, Ashutosh wrote:
> >
> > On Tue, 31 Dec 2024 01:46:28 -0800, Harish Chegondi wrote:
> > >
> > > In order to reuse intel_xe_perf_ioctl() and intel_xe_perf_ioctl_err()
> > > functions for EU stall sampling, add observation type as an input
> > > parameter to these functions and set the input observation type to
> > > observation_type in struct drm_xe_observation_param.
> > >
> > > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com>
> > > ---
> > >  lib/xe/xe_oa.c                   | 18 +++++++---
> > >  lib/xe/xe_oa.h                   | 11 ++++--
> > >  tests/intel/xe_oa.c              | 57 +++++++++++++++++++++-----------
> > >  tools/xe-perf/xe_perf_configs.c  |  2 +-
> > >  tools/xe-perf/xe_perf_recorder.c |  3 +-
> >
> > Why would EU stall want to use functions from lib/xe/xe_oa.c, which is a
> > library specific to OA. Why don't you just copy these functions into
> > tests/intel/xe_eu_stall.c stall, rename them and use them?
> >
> > The functions are in the library for OA because of use by other OA/perf
> > tools, they are not meant for use by EU stall.
> >
> > So this is a NAK on this patch.
>
> Also I don't want to create any cross dependencies between OA and EU stall
> (in case there are changes in the future). It is better to keep them
> completely separate.

And also I forgot there are external tools such as Gpuvis which depened on
these library functions, so this is breaking that too:

https://github.com/mikesart/gpuvis/commit/7daaa26eb02a96b1c2802421858e1fb32e568984