[Intel-gfx] [PATCH v11] drm/i915: Engine discovery query
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Wed May 1 15:51:28 UTC 2019
On 01/05/2019 12:55, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-01 12:45:36)
>>
>> On 01/05/2019 12:10, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-05-01 11:52:28)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>
>>>> Engine discovery query allows userspace to enumerate engines, probe their
>>>> configuration features, all without needing to maintain the internal PCI
>>>> ID based database.
>>>>
>>>> A new query for the generic i915 query ioctl is added named
>>>> DRM_I915_QUERY_ENGINE_INFO, together with accompanying structure
>>>> drm_i915_query_engine_info. The address of latter should be passed to the
>>>> kernel in the query.data_ptr field, and should be large enough for the
>>>> kernel to fill out all known engines as struct drm_i915_engine_info
>>>> elements trailing the query.
>>>>
>>>> As with other queries, setting the item query length to zero allows
>>>> userspace to query minimum required buffer size.
>>>>
>>>> Enumerated engines have common type mask which can be used to query all
>>>> hardware engines, versus engines userspace can submit to using the execbuf
>>>> uAPI.
>>>>
>>>> Engines also have capabilities which are per engine class namespace of
>>>> bits describing features not present on all engine instances.
>>>>
>>>> v2:
>>>> * Fixed HEVC assignment.
>>>> * Reorder some fields, rename type to flags, increase width. (Lionel)
>>>> * No need to allocate temporary storage if we do it engine by engine.
>>>> (Lionel)
>>>>
>>>> v3:
>>>> * Describe engine flags and mark mbz fields. (Lionel)
>>>> * HEVC only applies to VCS.
>>>>
>>>> v4:
>>>> * Squash SFC flag into main patch.
>>>> * Tidy some comments.
>>>>
>>>> v5:
>>>> * Add uabi_ prefix to engine capabilities. (Chris Wilson)
>>>> * Report exact size of engine info array. (Chris Wilson)
>>>> * Drop the engine flags. (Joonas Lahtinen)
>>>> * Added some more reserved fields.
>>>> * Move flags after class/instance.
>>>>
>>>> v6:
>>>> * Do not check engine info array was zeroed by userspace but zero the
>>>> unused fields for them instead.
>>>>
>>>> v7:
>>>> * Simplify length calculation loop. (Lionel Landwerlin)
>>>>
>>>> v8:
>>>> * Remove MBZ comments where not applicable.
>>>> * Rename ABI flags to match engine class define naming.
>>>> * Rename SFC ABI flag to reflect it applies to VCS and VECS.
>>>> * SFC is wired to even _logical_ engine instances.
>>>> * SFC applies to VCS and VECS.
>>>> * HEVC is present on all instances on Gen11. (Tony)
>>>> * Simplify length calculation even more. (Chris Wilson)
>>>> * Move info_ptr assigment closer to loop for clarity. (Chris Wilson)
>>>> * Use vdbox_sfc_access from runtime info.
>>>> * Rebase for RUNTIME_INFO.
>>>> * Refactor for lower indentation.
>>>> * Rename uAPI class/instance to engine_class/instance to avoid C++
>>>> keyword.
>>>>
>>>> v9:
>>>> * Rebase for s/num_rings/num_engines/ in RUNTIME_INFO.
>>>>
>>>> v10:
>>>> * Use new copy_query_item.
>>>>
>>>> v11:
>>>> * Consolidate with struct i915_engine_class_instnace.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>> Cc: Jon Bloomfield <jon.bloomfield at intel.com>
>>>> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>>> Cc: Tony Ye <tony.ye at intel.com>
>>>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com> # v7
>>>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> # v7
>>>> ---
>>>> +/**
>>>> + * struct drm_i915_engine_info
>>>> + *
>>>> + * Describes one engine and it's capabilities as known to the driver.
>>>> + */
>>>> +struct drm_i915_engine_info {
>>>> + /** Engine class and instance. */
>>>> + struct i915_engine_class_instance engine;
>>>> +
>>>> + /** Reserved field. */
>>>> + __u32 rsvd0;
>>>> +
>>>> + /** Engine flags. */
>>>> + __u64 flags;
>>>
>>> Do you think we could do something like
>>> BUILD_BUG_ON(!IS_ALIGNED(offsetof(*info, flags), sizeof(info->flags));
>>>
>>> Will that work, and worthwhile? Maybe work into a
>>>
>>> BUILD_BUG_ON(check_user_alignment(info, flags));
>>
>> Hmm.. probably manual check for no holes _and_ alignment is good enough
>> for uAPI since once it's in it's in. Will triple-check.
>
> Yeah, we actually need something more like
> offsetofend(previous_field) == offsetof(next_field)
>
> BUILD_BUG_ON(check_user_struct(info, previous_field, next_field)) ?
How would you logistically do it? List all struct members for each uapi
struct you want to check?
Maybe a variadic macro like:
CHECK_USER_STRUCT_FUNCTION(type, member0, ... memberN);
Which expands to a dedicated function to check this type, using
va_start/va_end to iterate all members checking for holes. So somewhere
in code we would also need:
CHECK_USER_STRUCT(type);
Which would call the function. But thats not build time.. Could be under
debug and selftests I guess. Could even be IGT in this case.
But I am not to keen in listing each struct member with a
prev/next_field BUILD_BUG_ON.
Perhaps IGT is indeed a better place to start testing for this. Since we
anyway require each new uAPI to have good IGT coverage.
Regards,
Tvrtko
More information about the Intel-gfx
mailing list