Re: [igt-dev] [PATCH v19 5/6] lib: igt_dummyload: use for_each_context_engine()

Andi Shyti
2019-04-08T16:45:51Z

Hi Tvrtko,

> > - engines[nengine++] = opts->engine;
> > + struct intel_execution_engine2 *e;
> > + int class;
> > +
> > + if (!gem_ctx_get_engine(fd, opts->engine, opts->ctx, e)) {
> > + class = e->class;
> > + } else {
> > + gem_require_ring(fd, opts->engine);
> > + class = gem_eb_to_class(opts->engine);
> > + }
> > +
> > + if (opts->flags & IGT_SPIN_POLL_RUN)
> > + igt_require(gem_class_can_store_dword(fd, class));
> > +
> > + flags[nengine++] = opts->engine;
> > }
>
> Patch looks good but I'm afraid I made an oversight earlier in review when I
> asked you to move this else block in this function. I forgot that the
> differentiation between igt_spin_batch_new and __igt_spin_batch_new was that
> the latter should not do any execbuf calls apart from the spinner itself.
> Since gem_require_ring does one it could be a problem.
>
> Would you mind moving just this else block back into the main constructor?

No problem, I'll move it back.

> With that you can have:
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks!
Andi