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

[Andi Shyti <andi@etezian.org>]

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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev