[Intel-gfx] [PATCH 1/4] drm/i915: introduce query info uAPI

[Lionel Landwerlin]

On 09/11/17 15:57, Joonas Lahtinen wrote:
> On Wed, 2017-11-08 at 16:22 +0000, Lionel Landwerlin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Query info uAPI allows userspace to probe for a number of properties
>> of the GPU. This partially removes the need for userspace to maintain
>> the internal PCI id based database (as well as code duplication across
>> userspace components).
>>
>> This first version enables engine configuration and features to be
>> queried.
>>
>> Probing is done via the new DRM_IOCTL_I915_QUERY_INFO ioctl which
>> takes a query ID as well as query arguments.
>>
>> Currently only general engine configuration and HEVC feature of the
>> VCS engine can be probed but the uAPI is designed to be generic and
>> extensible.
>>
>> Code is based almost exactly on the earlier proposal on the topic by
>> Jon Bloomfield. Engine class and instance refactoring made recently by
>> Daniele Ceraolo Spurio enabled this to be implemented in an elegant
>> fashion.
>>
>> To probe configuration userspace sets the engine class it wants to
>> query (struct drm_i915_gem_engine_info) and provides an array of
>> drm_i915_engine_info structs which will be filled in by the driver.
>> Userspace also has to tell i915 how many elements are in the array,
>> and the driver will report back the total number of engine instances
>> in any case.
>>
>> Pseudo-code example of userspace using the uAPI:
>>
>>    struct drm_i915_query_info info = {};
>>    struct drm_i915_engine_info *engines;
>>    int i, ret;
>>
>>    info.version = 1;
>>    info.query = I915_QUERY_INFO_ENGINE;
>>    info.query_params[0] = I915_ENGINE_CLASS_VIDEO;
>>    info.info_ptr_len = 0;
>>    ret = ioctl(..., &info);
>>    assert(ret == 0);
>>
>>    engines = malloc(info.info_ptr_len);
>>    info.info_ptr = to_user_pointer(engines);
>>    ret = ioctl(..., &info);
>>    assert(ret == 0);
>>
>>    for (i = 0; i < info.info_ptr / sizeof(*engines); i++)
>>        printf("VCS%u: flags=%x\n",
>>               engines[i].instance,
>>               engines[i].info);
>>
>> First time with info.info_ptr_len set to zero, so the kernel reports
>> back the size of the data to be written into info.info_ptr. Then a
>> second time with the info.info_ptr_len field set to the correct
>> length.
>>
>> v2:
>>   * Add a version field and consolidate to one engine count.
>>     (Chris Wilson)
>>   * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO.
>>     (Gong Zhipeng)
>>
>> v3:
>>   * Drop the DRM_ prefix from uAPI enums. (Chris Wilson)
>>   * Only reserve 8-bits for the engine info for time being.
>>
>> v4:
>>   * Made user_class_map static.
>>
>> v5:
>>   * Example usage added to commit msg. (Chris Wilson)
>>   * Report engine count in case of version mismatch. (Chris Wilson)
>>
>> v6:
>>   * Return API to query_info to make it more generic (Lionel)
>>   * Add query ID & query params (Lionel)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Cc: Ben Widawsky <ben at bwidawsk.net>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Daniel Vetter <daniel.vetter at intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Cc: Jon Bloomfield <jon.bloomfield at intel.com>
>> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin at intel.com>
>> Cc: Oscar Mateo <oscar.mateo at intel.com>
>> Cc: "Gong, Zhipeng" <zhipeng.gong at intel.com>
> <SNIP>
>
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -2776,6 +2776,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
>>   	DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),
>>   	DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>   	DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>> +	DRM_IOCTL_DEF_DRV(I915_QUERY_INFO, i915_query_info_ioctl, DRM_RENDER_ALLOW),
> I'm not a fan. This is bit much to the direction of I915_META_IOCTL.

So you would prefer a different ioctl for every bit of information we 
would want to query from the kernel?

>
> So can we keep this as engine info query without versioning, and if you
> need to query something else, lets have another ioctl for that.
>
> (I heard a rumour of this being about RCS topology, which could be in
> the engine info.).

I'm not sure to understand what you mean by "in the engine info". Would 
you mind to give an example?

>
> Regards, Joonas