Re: [PATCH v11] drm/i915: Engine discovery query

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 01/05/2019 12:10, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-01 11:52:28)
>> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Tony Ye <tony.ye@intel.com>
>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> # v7
>> Reviewed-by: Chris Wilson <chris@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.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks! I apparently messed up the actual branch and will resend once 
IGT series finished so I can play with Test-with:

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx