Quoting Tvrtko Ursulin (2022-03-14 17:35:17) > > On 12/03/2022 04:16, Matt Atwood wrote: > > On Thu, Mar 10, 2022 at 12:26:12PM +0000, Tvrtko Ursulin wrote: > >> > >> On 10/03/2022 05:18, Matt Atwood wrote: > >>> Newer platforms have DSS that aren't necessarily available for both > >>> geometry and compute, two queries will need to exist. This introduces > >>> the first, when passing a valid engine class and engine instance in the > >>> flags returns a topology describing geometry. > >>> > >>> v2: fix white space errors > >>> > >>> Cc: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > >>> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > >>> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > >>> UMD (mesa): https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14143 > >>> Signed-off-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx> <SNIP> > >>> @@ -2714,6 +2715,9 @@ struct drm_i915_query_item { > >>> * - DRM_I915_QUERY_PERF_CONFIG_LIST > >>> * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID > >>> * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID > >>> + * > >>> + * When query_id == DRM_I915_QUERY_GEOMETRY_SUBSLICES must have bits 0:7 set > >>> + * as a valid engine class, and bits 8:15 must have a valid engine instance. > >> > >> Alternatively, all other uapi uses struct i915_engine_class_instance to > >> address engines which uses u16:u16. > >> > >> How ugly it is to stuff a struct into u32 flags is the question... But you > >> could at least use u16:u16 for consistency. Unless you wanted to leave some > >> bits free for the future? > > Originally when I wrote this I was wanting to leave space in case it was > > ever needed. I'm not particularly for or against keeping the space now. > > Yes, shrug... Neither I can't guess if we are ever likely to hit a > problem by having fewer bits for class:instance here compared to other > uapi, or if stuffing struct i915_engine_class_instance into flags would > just be too ugly. I mean there is option to define a new struct and not > use flags at all but that's probably to complicated for what it is. > > Anyone else with an opinion? Consistency or should be fine even like it is? Stuffing a full i915_engine_class_instance was definitely intended when putting it into the flags was suggested. If that is hit with a complication, the next proposed alternative was a new struct. That's why the query interface was made easily extensible, after all... Regards, Joonas