Re: [igt-dev] [PATCH v13 6/9] lib/i915: add gem_engine_topology library

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

> > +{
> > +	const char *class_names[] = { "rcs", "bcs", "vcs", "vecs" };
> > +	char *__name;
> > +
> > +	/* if we don't recognise the class, then we mark it as "unk" */
> > +	if (name) {
> > +		e2->name = name;
> > +	} else {
> > +		if (class >= ARRAY_SIZE(class_names))
> > +			igt_assert(asprintf(&__name, "unk-%d:%d",
> > +					    class, instance) > 0);
> > +		else
> > +			igt_assert(asprintf(&__name, "%s%d",
> > +					    class_names[class], instance) > 0);
> > +
> > +		e2->name = __name;
> 
> We discussed on the IRC how names can be borrowed from the static engine
> table, which we have to keep maintaining due CI limitations anyway. That way
> all leaks are solved.
> 
> So you would just replace the above with a name lookup against the static
> table and use that pointer.

Yes, this is the tax we need to pay for having counts name inside
intel_execution_engine2.

I can take the pointer from our intel_exectuion_engines2 array,
and then we wouldn't have "unk<class>:<instance>".

As I said the best thing here would be to remove the "const" and
be able to use all the string functions.

> > +	} else if (cparam.size == sizeof(struct i915_context_param_engines)) {
> 
> This should be "== 0" since Chris has special plans for the other.
> 
> > +		/* else if context doesn't have mapped engines */
> > +		query_engine_list(&engine_data);
> > +		map_engine_context(&engine_data, &cparam);
> > +
> > +	} else {
> > +		/* context has a list of mapped engines */
> > +
> > +		uint8_t nengines = (cparam.size -
> > +				sizeof(struct i915_context_param_engines)) /
> > +				sizeof(cengine->class_instance[0]);
> > +
> > +		for (i = 0; i < nengines - 1; i++)
> 
> nengines - 1 is not right I think, just nengines.
> 
> > +			dup_engine(&engine_data.engines[i], NULL,
> > +				   cengine->class_instance[i].engine_class,
> > +				   cengine->class_instance[i].engine_instance,
> > +				   i + 1);
> 
> Just "i" since index zero is not special any longer.

I was running on a messed up kernel that and was receiving weird
values :/

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