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

[Andi Shyti <andi.shyti@intel.com>]

Hi Antonio,

> > A new function gem_init_engine_list() is addedthat is called
> 
> nit: missing white space ^^^^^^^^^^^^^^^^^^^^^

Thanks :)

> > +	igt_assert(!igt_ioctl(fd,
> > +			      DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, &ctx_param));
> 
> It would be good to have a wrapper for assigning the context to an engine
> instance and then another wrapper (or maybe a macro) for assigning the
> context to all engines, so not to hide the intent of the IOCLT underneath. I
> assume tests will want to have contexts assigned only to some engines and
> not others.

yes, that's the same thing Chris pointed out, I'll put everything
into a macro.

> > +	igt_assert((intel_execution_engines2 =
> > +		    calloc(64, sizeof(*intel_execution_engines2))));
> 
> If you require !ret, I think you should require ^. Also, don't you want to
> request 'query_engine->num_engines' elements?

Yes, the design of this changed form the previous iterations and
this is a leftover. It can, indeed, be safely num_engines. Thanks.

Thank you,
Andi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev