Re: [igt-dev] [RFC v2 2/3] lib: implement new engine discovery interface

[Andi Shyti <andi@etezian.org>]

Hi Tvrtko,

just few things that came to my mind while going through your
review.

> > +	query_engines = malloc(size);
> > +	if (!query_engines)
> > +		return NULL;
> 
> We probably want to igt_assert on this since there is no point going
> further.

isn't igt_assert used for the test outcome? What I mean is that
the test outcome would be to query and set the engines, but if we
fail in malloc we do not necessarily fail the test, but we have a
different kind of system failure.

(this is not important, just for me to understand better :) )

> > +int get_engines(int fd, uint32_t ctx_id)
> 
> setup_ctx_engines?
> 
> Does it need to be exported or it can be static?

How do we reach it from outside if we set it static? This
function is called in the for_each_engine_ctx macro that is used
outside from igt_gt.c

> > +#define for_each_engine_ctx(fd, ctx, e) \
> 
> High level design question: Do we want 'e' to be an integer or a struct
> describing each engine?

Do you mean that you would you prefer iterating with a
'i915_context_param_engines' or a 'intel_execution_engine' struct
instead of an 'e' integer? This way it would also be different
from how the current 'for_each_engine' works.

I could do it in a next patch, so that in this one we keep it as
close as possible to the current way of doing things.

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