[Intel-gfx] [PATCH v7] drm/i915: Engine discovery query

[Joonas Lahtinen]

Some comments below, mostly related to trying to keep the uapi header
nice and tidy.

Quoting Tvrtko Ursulin (2018-10-04 14:32:48)
> @@ -1747,6 +1748,52 @@ struct drm_i915_query_topology_info {
>         __u8 data[];
>  };
>  
> +/**
> + * struct drm_i915_engine_info
> + *
> + * Describes one engine known to the driver, whether or not it is an user-
> + * accessible or hardware only engine, and what are it's capabilities where
> + * applicable.
> + */
> +struct drm_i915_engine_info {
> +       /** Engine class as in enum drm_i915_gem_engine_class. */
> +       __u16 class;
> +
> +       /** Engine instance number. */
> +       __u16 instance;
> +
> +       /** Reserved field must be cleared to zero. */
> +       __u32 rsvd0;

u32 class, u32 instance just to put the padding to good use?

> +
> +       /** Engine flags. */
> +       __u64 flags;
> +
> +       /** Capabilities of this engine. */
> +       __u64 capabilities;
> +#define I915_VCS_CLASS_CAPABILITY_HEVC (1 << 0)
> +#define I915_VCS_CLASS_CAPABILITY_SFC  (1 << 1)
> +
> +       /** Reserved fields must be cleared to zero. */
> +       __u64 rsvd1[4];

Why this at the end of the struct? We have flags which we should be able
to use for new versions.

> +};
> +
> +/**
> + * struct drm_i915_query_engine_info
> + *
> + * Engine info query enumerates all engines known to the driver by filling in
> + * an array of struct drm_i915_engine_info structures.
> + */
> +struct drm_i915_query_engine_info {
> +       /** Number of struct drm_i915_engine_info structs following. */
> +       __u32 num_engines;
> +
> +       /** MBZ */

Just copy the non-abbreviated comment from other reserved fields.

> +       __u32 rsvd[3];
> +

Might as well do just __u32 flags, which must be zero?

I don't think we need to get too excited about adding the reserved
fields in every spot :)

Regards, Joonas

> +       /** Marker for drm_i915_engine_info structures. */
> +       struct drm_i915_engine_info engines[];
> +};
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> -- 
> 2.17.1
>