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

Jason Ekstrand jason at jlekstrand.net
Mon Jun 14 15:04:26 UTC 2021


On Mon, Jun 14, 2021 at 12:34 AM Zbigniew Kempczyński
<zbigniew.kempczynski at intel.com> wrote:
>
> 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.

Ok, I found it.  It's actually all in the previous patch.  I'll send a
v3 of that one for you to look at.

--Jason

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


More information about the igt-dev mailing list