Re: [igt-dev] [PATCH i-g-t 09/93] lib: Add an intel_ctx wrapper struct and helpers (v4)

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 08 Jun 2021 21:29:55 -0700, Jason Ekstrand wrote:
> diff --git a/lib/intel_ctx.h b/lib/intel_ctx.h
> new file mode 100644
> index 00000000..79bc1154
> --- /dev/null
> +++ b/lib/intel_ctx.h
> @@ -0,0 +1,75 @@

/snip/

> +/**
> + * intel_ctx_cfg_t:
> + * @flags: Context create flags
> + * @vm: VM to inherit or 0 for using a per-context VM
> + * @num_engines: Number of client-specified engines or 0 for legacy mode
> + * @engines: Client-specified engines
> + *
> + * Represents the full configuration of an intel_ctx.
> + */
> +typedef struct intel_ctx_cfg {
> +	uint32_t flags;
> +	uint32_t vm;
> +	unsigned int num_engines;
> +	struct i915_engine_class_instance engines[GEM_MAX_ENGINES];
> +} intel_ctx_cfg_t;
> +
> +intel_ctx_cfg_t intel_ctx_cfg_for_engine(unsigned int class, unsigned int inst);
> +intel_ctx_cfg_t intel_ctx_cfg_all_physical(int fd);
> +
> +/**
> + * intel_ctx_t:
> + * @id: the context id/handle
> + * @cfg: the config used to create this context
> + *
> + * Represents the full configuration of an intel_ctx.
> + */
> +typedef struct intel_ctx {
> +	uint32_t id;
> +	intel_ctx_cfg_t cfg;
> +} intel_ctx_t;

Just a comment about cosmetics. I am ok either way but I believe in the
linux kernel (and in IGT which follows the kernel coding style) such
typedefs are supposedly only to be used for "opaque" types and are
generally avoided. E.g.

https://yarchive.net/comp/linux/typedefs.html

There are not very many instances of such typedef's in IGT, certainly not
those which are this widely used.

I copying Petri about this to see what he prefers, whether we should keep
these typedefs or replace intel_ctx_cfg_t and intel_ctx_t with 'struct
intel_ctx_cfg' and 'struct intel_ctx'.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev