[Intel-gfx] [RFC v4 1/2] drm/i915: Engine discovery uAPI
Chris Wilson
chris at chris-wilson.co.uk
Wed Jun 28 11:27:03 UTC 2017
Quoting Tvrtko Ursulin (2017-06-26 16:47:41)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Engine discovery uAPI allows userspace to probe for engine
> configuration and features without needing to maintain the
> internal PCI id based database.
>
> This enables removal of code duplications across userspace
> components.
>
> Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl
> which returns the number and information on the specified engine
> class.
>
> 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.
>
> 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.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin 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>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++
> include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++
> 4 files changed, 109 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e9d8e9ee51b2..44dd2f37937f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW),
> };
>
> static struct drm_driver driver = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 427d10c7eca5..496565e1753f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine,
> struct i915_gem_context *ctx,
> uint32_t *reg_state);
>
> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file);
> +
> /* i915_gem_evict.c */
> int __must_check i915_gem_evict_something(struct i915_address_space *vm,
> u64 min_size, u64 alignment,
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 3b46c1f7b88b..a98669f6ad85 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -25,6 +25,7 @@
> #include "i915_drv.h"
> #include "intel_ringbuffer.h"
> #include "intel_lrc.h"
> +#include <uapi/drm/i915_drm.h>
>
> /* Haswell does have the CXT_SIZE register however it does not appear to be
> * valid. Now, docs explain in dwords what is in the context object. The full
> @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
> }
> }
>
> +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = {
> + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS,
> + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS,
> + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS,
> + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS,
> + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS,
> +};
> +
> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
> +{
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct drm_i915_gem_engine_info *args = data;
> + struct drm_i915_engine_info __user *user_info =
> + u64_to_user_ptr(args->info_ptr);
> + unsigned int info_size = args->num_engines;
> + struct drm_i915_engine_info info;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + u8 class;
> +
> + if (args->rsvd)
> + return -EINVAL;
> +
> + switch (args->engine_class) {
> + case I915_ENGINE_CLASS_OTHER:
> + case I915_ENGINE_CLASS_RENDER:
> + case I915_ENGINE_CLASS_COPY:
> + case I915_ENGINE_CLASS_VIDEO:
> + case I915_ENGINE_CLASS_VIDEO_ENHANCE:
> + class = user_class_map[args->engine_class];
> + break;
> + case I915_ENGINE_CLASS_MAX:
> + default:
> + return -EINVAL;
> + };
> +
> + args->num_engines = 0;
> +
> + if (args->version != 1) {
> + args->version = 1;
> + return 0;
My gut feeling says we want to report both the version and count in the
first pass. Otherwise we have to do a version check, followed by a count
query, followed by the actual retrieval. If we can combine the version +
count check into one pass, it may be a little less hassle?
-Chris
More information about the Intel-gfx
mailing list