Re: [igt-dev] [PATCH] lib/i915: gem_engine_topology: get eb flags from engine's coordiantes

Andi Shyti
2019-06-20T08:10:59Z

Hi Tvrtko,

> > + struct intel_execution_engine2 *e;
> > +
> > + for_each_context_engine(fd, ctx_id, e)
> > + if (class == e->class && instance == e->instance)
> > + return e->flags;
>
> And most important difference, I wouldn't configure the context from this helper. Instead I think all we need is:
>
> int gem_context_get_eb_flags(int fd, uint32_t ctx, struct i915_engine_class_instance ci)
> {
> DEFINE_CONTEXT_ENGINES_PARAM(engines, param, ctx, GEM_MAX_ENGINES);
> int ret
>
> ret = gem_topology_get_param(fd, &param);
> if (ret) {
> const struct intel_execution_engine2 *e;
>
> /* Legacy kernels. */
> __for_each_static_engine(e) {
> if (e->class == ci.engine_class &&
> e->instance == ci.engine_instance)
> return e->flags;
> }
>
> return -EINVAL;
> }
>
> /* Engine map with no engines. */
> if (!param.size)
> return -EINVAL;

this means that tests have responsibility to alway create a
context and do the mapping before (we haven't always assumed it
in other tests).

> /* Engine map lookup. */
> for (unsigned int i = 0; i < param.size; i++) {
> if (engines.engines[i].engine_class == ci.engine_class &&
> engines.engines[i].engine_instance == ci.engine_instance)
> return i;
> }
>
> return -EINVAL;
> }

This version, a the end, becomes a slim version of
'intel_init_engine_list', with the only difference that it
wouldn't do any mapping (which I agree would be too much but we
did it in other tests), that's why, at the end I preferred the
short version I sent.

OK, we can definitely do it this way, too

Andi