On 04/10/2018 15:32, Joonas Lahtinen wrote:
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?
There is some attractiveness to lose the padding, but I think in general
we trashed it out to be u16:u16. So it is a question of consistency vs
elegance and I give preference to consistency.
Chris, is your recollection also that we said u16:u16 for class:instance
in all uAPI?
+
+ /** 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.
This is to allow some growing space without complicating the userspace
array member start calculation.
If we have to grow struct engine_info itself when adding a new member,
then we have to define the array (via documentation) as potentially not
aligned to sizeof(struct engine_info) as known by userspace.
I don't have such a big aversion to rsvd fields so for me it seems
easier to have some, with the benefit of simpler userspace code.
But if the consensus will be to change it, no problem.
+};
+
+/**
+ * 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.
I need to nuke this comment since from v6 code is not mandating MBZ for
the array.
+ __u32 rsvd[3];
+
Might as well do just __u32 flags, which must be zero?
Could do flags..
I don't think we need to get too excited about adding the reserved
fields in every spot :)
... but I do like my rsvd! :))
Regards,
Tvrtko
Regards, Joonas
+ /** Marker for drm_i915_engine_info structures. */
+ struct drm_i915_engine_info engines[];
+};
+
#if defined(__cplusplus)
}
#endif
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
