[igt-dev] [PATCH i-g-t] i915/perf: Make __perf_open() and friends public

[Dixit, Ashutosh]

On Tue, 07 Feb 2023 12:04:17 -0800, Janusz Krzysztofik wrote:
>
> Hi Umesh,
>
> On Tuesday, 7 February 2023 20:33:50 CET Umesh Nerlige Ramappa wrote:
> > On Tue, Feb 07, 2023 at 11:25:00AM -0800, Umesh Nerlige Ramappa wrote:
> > >I wouldn't do this. Please keep the changes local to the specific test
> > >that you implemented in your first rev. While it is a good idea to
> > >have the some of the perf capabilities in the library, this is way too
> > >much churn to implement a specific test for the original failure.
> > >Unless multiple IGT subsytems area already dependent on perf APIs to
> > >implement multiple tests, let's not do this.
> > >
> >
> > Also note that the perf library implemented in IGT is not entirely used
> > by IGT tests alone. The library is also linked to GPUvis software. Only
> > a few pieces of reusable code in the perf library is used by IGT tests.
>
> Do you think that my changes will break other users?  How?
>
> Also, it looks like there are somehow conflicting expectations from different
> reviewers.  Ashutosh wanted the new subtest to be implemented outside of i915/
> perf test.  That's why I proposed to extend the library with open/close and
> related helpers, just to avoid code duplication, and I'm about to resend it in
> series with the new subtest implemented inside gem_ctx_exec.  Now, after I
> submitted this patch for initial review, you say that a specific test is not
> the way to go.  What are you afraid of?
>
> Whose expectations should I try to satisfy in order to have a subtest accepted
> and merged?  Or should I just give up and duplicate the code from i915/perf in
> another test?  Or maybe you can have a look at the whole series before you
> decide?

Hi Janusz,

I agree with Umesh. Given that here perf is just being used as a 'dummy
workload' let's just duplicate the minimal code required for perf
open/close wherever we are adding the new test. This will keep the real
perf functionality undisturbed for reasons Umesh cited.

Thanks.
--
Ashutosh