Re: [igt-dev] [RFC v2 05/43] lib/i915: Helper func for engine class to exec_flag

Andi Shyti
2019-06-25T12:03:42Z

On Fri, Jun 21, 2019 at 03:33:07PM +0530, Ramalingam C wrote:
> Helper function is defined to get the execbuf flag for a engine of a
> class mapped for a ctx.
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
> lib/i915/gem_engine_topology.c | 22 ++++++++++++++++++++++
> lib/i915/gem_engine_topology.h | 2 ++
> 2 files changed, 24 insertions(+)
>
> diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
> index fdd1b951672b..c64f7a4d05bb 100644
> --- a/lib/i915/gem_engine_topology.c
> +++ b/lib/i915/gem_engine_topology.c
> @@ -289,3 +289,25 @@ bool gem_has_engine_topology(int fd)
>
> return !__gem_context_get_param(fd, &param);
> }
> +
> +/* Return -EINVAL when the Engine with mentioned class is not found */
> +int gem_engine_class_to_execbuf_flags(int fd, uint32_t ctx_id, int class)
> +{
> + const struct intel_execution_engine2 *e;
> + DEFINE_CONTEXT_ENGINES_PARAM(engines, param, ctx_id, GEM_MAX_ENGINES);
> + int i;

Here Chris would complain that variable declaration should be
declared in length order.

> + if (gem_topology_get_param(fd, &param)) {
> + __for_each_static_engine(e) {
> + if (e->class == class)
> + return e->flags;
> + }
> + return -EINVAL;
> + }

Here the case where length is '0' is missing.

> + for (i = 0; i < param.size; i++)
> + if (engines.engines[i].engine_class == class)
> + return i;
> +
> + return -EINVAL;
> +}
> diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
> index 2415fd1e379b..aa934622cab0 100644
> --- a/lib/i915/gem_engine_topology.h
> +++ b/lib/i915/gem_engine_topology.h
> @@ -53,6 +53,8 @@ int gem_context_lookup_engine(int fd, uint64_t engine, uint32_t ctx_id,
>
> void gem_context_set_all_engines(int fd, uint32_t ctx);
>
> +int gem_engine_class_to_execbuf_flags(int fd, uint32_t ctx_id, int class);
> +
> #define __for_each_static_engine(e__) \
> for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)

oh... looks familiar :) perhaps you can put my patch (or ours,
they are basically the same) in this series and add sob by you,
as well... there would be three sob this way (tvrtko, me, you).

Andi