[igt-dev] [PATCH i-g-t 18/93] tests/i915/gem_exec_fence: Move the engine data into inter_engine_context

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Mon Jun 14 05:34:22 UTC 2021


On Fri, Jun 11, 2021 at 02:28:06PM -0700, Dixit, Ashutosh wrote:
> On Wed, 09 Jun 2021 08:02:04 -0700, Jason Ekstrand wrote:
> > On Wed, Jun 9, 2021 at 2:18 AM Zbigniew Kempczyński
> > <zbigniew.kempczynski at intel.com> wrote:
> > > On Tue, Jun 08, 2021 at 11:30:04PM -0500, Jason Ekstrand wrote:
> > > > -static void setup_timeline_chain_engines(struct inter_engine_context *context, int fd, struct intel_engine_data *engines)
> > > > +static void setup_timeline_chain_engines(struct inter_engine_context *context, int fd)
> > > >  {
> > > >       memset(context, 0, sizeof(*context));
> > > >
> > > >       context->fd = fd;
> > > > -     context->engines = engines;
> > > > +     context->engines = intel_init_engine_list(fd, 0);
> > >
> > > Original code verifies there's nengines > 0. I would keep this igt_require().
> >
> > I'm not seeing that in the diff or the code.  Mind pointing me at it?
> > In any case, I'm fine with the igt_require(); it should always be true
> > anyway.
> 
> Note that original code has "igt_require(engines.nengines > 1)" not >
> 0. Still always true but probably there just to show the intent of the
> test.

Yes, I mean igt_require(engines.nengines > 1).

Issue comes because I'm using side-by-side code compare so I compared
all commits applied to gem_exec_fence.c. If there's no option to have
less than two engines we could just remove that check.

--
Zbigniew 
 
> >
> > > With that minor nit:
> > >
> > > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> >
> > Thanks!


More information about the igt-dev mailing list