Re: [PATCH 1/4] drm/i915: introduce query info uAPI

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Wed, 2017-11-08 at 16:22 +0000, Lionel Landwerlin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: "Gong, Zhipeng" <zhipeng.gong@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 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.).

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx