[igt-dev] [RFC PATCH v6 2/4] lib: implement new engine discovery interface

Chris Wilson chris at chris-wilson.co.uk
Fri Feb 8 13:03:08 UTC 2019


Quoting Andi Shyti (2019-02-08 12:55:42)
> Hi Chris,
> 
> Thanks for the review!
> 
> I think fixed everything, just a few questions on the following
> 
> > > @@ -1459,6 +1460,8 @@ void igt_require_gem(int fd)
> > >         err = 0;
> > >         if (ioctl(fd, DRM_IOCTL_I915_GEM_THROTTLE))
> > >                 err = -errno;
> > > +       else
> > > +               gem_init_engine_list(fd);
> > 
> > igt_require_gem() maybe called multiple times per subtest.
> 
> I'm actually checking that with:
> 
>         if (intel_execution_engines2)
>                 return;
> 
> inside the gem_init_engine_list(). Shouldn't that be enough to
> prevent multiple calls?

Still not keen on having it here. It doesn't smell right; the purpose of
igt_require_gem() is to check the gpu is functional. Having a
side-effect of initialising an obscure struct used by a few tests?

> > Just gem_require_engine_list() ? Gives a good indicator which tests are
> > converted and which not, and a reminder that we still need to test
> > legacy ABIs in conjunction with the new era.
> 
> Can do that, but I thought that in the new era, when your and
> Tvrtko's patch will get in, we will convert everything to the new
> ABI's and this would remain an extra step (which BTW, wouldn't
> prevent multiple calls either, as above).

NO! The ABI doesn't simply no longer exist because we wish it so. The
tests for the old ABI will have to remain for as long as it is
accessible; which means that we need both sets of ABI for anything that
looks suspiciously like an ABI test.

It only bits and pieces that merely want to run over a set of HW that we
can simplify.
-Chris


More information about the igt-dev mailing list