[RFC,v7,2/5] lib: implement new engine discovery interface

Submitted by Tomi Valkeinen via igt-dev on Feb. 11, 2019, 11:08 p.m.

Details

Message ID 20190211230811.24955-3-andi.shyti@intel.com
State New
Headers show
Series "new engine discovery interface" ( rev: 1 ) in IGT

Not browsing as part of any series.

Commit Message

Tomi Valkeinen via igt-dev Feb. 11, 2019, 11:08 p.m.
Kernel commits:

[1] ae8f4544dd8f ("drm/i915: Engine discovery query")
[2] 31e7d35667a0 ("drm/i915: Allow a context to define its set of engines")

implement a new uapi for engine discovery that consist in first
querying the driver about the engines in the gpu [1] and then
binding a context to the set of engines that it can access [2].

In igt the classic way for discovering engines is done through
the for_each_physical_engine() macro, that would be replaced by
the new for_each_engine_ctx().

A new function gem_init_engine_list() is added in the library
that initializes the active engine list by interrogating the
driver. The list is stored in intel_active_engines2.

In case of missing GET/SETPARAM ioctl command
intel_active_engines2 will point to the existing
intel_execution_engines2 that has a static list of engines.

gem_init_engine_list() returns

 * '0' on success
 * -EINVAL if GET/SETPARAM are not present in the running kernel
 * errno in case of other failure cases

Signed-off-by: Andi Shyti <andi.shyti@intel.com>
---
 lib/igt_gt.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/igt_gt.h |  22 +++++++-
 2 files changed, 163 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 646696727ee4..6c2c46562760 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -577,7 +577,7 @@  bool gem_can_store_dword(int fd, unsigned int engine)
 	return true;
 }
 
-const struct intel_execution_engine2 intel_execution_engines2[] = {
+struct intel_execution_engine2 intel_execution_engines2[] = {
 	{ "rcs0", I915_ENGINE_CLASS_RENDER, 0 },
 	{ "bcs0", I915_ENGINE_CLASS_COPY, 0 },
 	{ "vcs0", I915_ENGINE_CLASS_VIDEO, 0 },
@@ -586,6 +586,8 @@  const struct intel_execution_engine2 intel_execution_engines2[] = {
 	{ }
 };
 
+struct intel_execution_engine2 *intel_active_engines2;
+
 unsigned int
 gem_class_instance_to_eb_flags(int gem_fd,
 			       enum drm_i915_gem_engine_class class,
@@ -650,3 +652,143 @@  bool gem_ring_has_physical_engine(int fd, unsigned ring)
 
 	return gem_has_ring(fd, ring);
 }
+
+static int __gem_query(int fd, struct drm_i915_query *q)
+{
+	return igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q) ? -errno : 0;
+}
+
+void gem_query(int fd, struct drm_i915_query *q)
+{
+	igt_assert(!__gem_query(fd, q));
+}
+
+static int __gem_get_set_param(int fd, unsigned long request,
+			       struct drm_i915_gem_context_param *p)
+{
+	return igt_ioctl(fd, request, p) ?  -errno : 0;
+}
+
+void gem_get_set_param(int fd, unsigned long request,
+		       struct drm_i915_gem_context_param *p)
+{
+	igt_assert(!__gem_get_set_param(fd, request, p));
+}
+
+bool gem_is_get_set_param(void)
+{
+	return intel_active_engines2 != intel_execution_engines2;
+}
+
+static struct drm_i915_query_engine_info *query_engines(int fd)
+{
+	struct drm_i915_query query = { };
+	struct drm_i915_query_item item = { };
+	struct drm_i915_query_engine_info *query_engines;
+
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	query.items_ptr = to_user_pointer(&item);
+	query.num_items = 1;
+	item.length = sizeof(*query_engines) +
+		      64 * sizeof(struct drm_i915_engine_info);
+
+	igt_assert((query_engines = calloc(1, item.length)));
+	item.data_ptr = to_user_pointer(query_engines);
+
+	gem_query(fd, &query);
+
+	return query_engines;
+}
+
+void __set_ctx_engine_map(int fd, uint32_t ctx_id)
+{
+	int i;
+	const struct intel_execution_engine2 *e2;
+	struct drm_i915_gem_context_param ctx_param;
+	struct i915_context_param_engines *ctx_engine;
+
+	if (!gem_is_get_set_param())
+		return;
+
+	ctx_param.ctx_id = ctx_id;
+	ctx_param.param = I915_CONTEXT_PARAM_ENGINES;
+	ctx_param.size = sizeof(*ctx_engine) +
+			 (I915_EXEC_RING_MASK - 1) *
+			 sizeof(*ctx_engine->class_instance);
+
+	igt_assert((ctx_engine = calloc(1, ctx_param.size)));
+
+	ctx_engine->extensions = 0;
+	for (i = 0, e2 = intel_active_engines2; e2->name; i++, e2++) {
+		ctx_engine->class_instance[i].engine_class = e2->class;
+		ctx_engine->class_instance[i].engine_instance = e2->instance;
+	}
+
+	ctx_param.value = to_user_pointer(ctx_engine);
+
+	gem_setparam(fd, &ctx_param);
+
+	free(ctx_engine);
+}
+
+/*
+ * Initializes the list of engines.
+ *
+ * Returns:
+ *
+ *  - 0 in case of success and the get/setparam ioctls are implemented
+ *  - -EINVAL in case of success but get/setparam ioctls are not implemented
+ *  - errno in case of failure
+ */
+int gem_init_engine_list(int fd)
+{
+	int i, ret;
+	struct drm_i915_query_engine_info *query_engine = query_engines(fd);
+	const char *engine_names[] = { "rcs", "bcs", "vcs", "vecs" };
+	struct drm_i915_gem_context_param ctx_param = {
+		.param = I915_CONTEXT_PARAM_ENGINES,
+	};
+
+	/* the list is already initialized */
+	if (intel_active_engines2)
+		return gem_is_get_set_param() ? 0 : -EINVAL;
+
+	/*
+	 * we check first whether the new engine discovery uapi
+	 * is in the current kernel, if not, the
+	 * DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM will fail with
+	 * errno = EINVAL. In this case, we fall back to using
+	 * the previous engine discovery way
+	 */
+	ret = __gem_get_set_param(fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM,
+				  &ctx_param);
+	if (ret) {
+		if (ret == -EINVAL)
+			intel_active_engines2 = intel_execution_engines2;
+		return ret;
+	}
+
+	igt_assert((intel_active_engines2 =
+		    calloc(query_engine->num_engines + 1,
+			   sizeof(*intel_active_engines2))));
+
+	for (i = 0; i < query_engine->num_engines; i++) {
+		char *name;
+		int class = query_engine->engines[i].class;
+		int instance = query_engine->engines[i].instance;
+
+		igt_assert(class < ARRAY_SIZE(engine_names) && class >= 0);
+		igt_assert(engine_names[class]);
+
+		intel_active_engines2[i].class = class;
+		intel_active_engines2[i].instance = instance;
+
+		igt_assert(asprintf(&name, "%s%d",
+				    engine_names[class], instance) > 0);
+		intel_active_engines2[i].name = name;
+	}
+
+	free(query_engine);
+
+	return 0;
+}
diff --git a/lib/igt_gt.h b/lib/igt_gt.h
index 54e95da98084..28350252d80c 100644
--- a/lib/igt_gt.h
+++ b/lib/igt_gt.h
@@ -86,16 +86,34 @@  extern const struct intel_execution_engine {
 	     e__++) \
 		for_if (gem_ring_has_physical_engine(fd__, flags__ = e__->exec_id | e__->flags))
 
+#define for_each_engine_ctx(fd, ctx, e) \
+		for (__set_ctx_engine_map(fd, ctx_id), \
+		     e = intel_active_engines2; e->name; e++) \
+			for_if (gem_is_get_set_param() || \
+				gem_has_engine(fd, e->class, e->instance))
+
 bool gem_ring_is_physical_engine(int fd, unsigned int ring);
 bool gem_ring_has_physical_engine(int fd, unsigned int ring);
 
 bool gem_can_store_dword(int fd, unsigned int engine);
 
-extern const struct intel_execution_engine2 {
+struct intel_execution_engine2 {
 	const char *name;
 	int class;
 	int instance;
-} intel_execution_engines2[];
+};
+
+extern struct intel_execution_engine2 *intel_active_engines2;
+extern struct intel_execution_engine2 intel_execution_engines2[];
+
+bool gem_is_get_set_param(void);
+void gem_query(int fd, struct drm_i915_query *q);
+int gem_init_engine_list(int fd);
+void __set_ctx_engine_map(int fd, uint32_t ctx_id);
+void gem_get_set_param(int fd, unsigned long request, struct drm_i915_gem_context_param *p);
+
+#define gem_setparam(f, p) gem_get_set_param(f, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, p)
+#define gem_getparam(f, p) gem_get_set_param(f, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, p)
 
 unsigned int
 gem_class_instance_to_eb_flags(int gem_fd,

Comments

Quoting Andi Shyti (2019-02-11 23:08:08)
> Kernel commits:
> 
> [1] ae8f4544dd8f ("drm/i915: Engine discovery query")
> [2] 31e7d35667a0 ("drm/i915: Allow a context to define its set of engines")
> 
> implement a new uapi for engine discovery that consist in first
> querying the driver about the engines in the gpu [1] and then
> binding a context to the set of engines that it can access [2].
> 
> In igt the classic way for discovering engines is done through
> the for_each_physical_engine() macro, that would be replaced by
> the new for_each_engine_ctx().
> 
> A new function gem_init_engine_list() is added in the library
> that initializes the active engine list by interrogating the
> driver. The list is stored in intel_active_engines2.
> 
> In case of missing GET/SETPARAM ioctl command
> intel_active_engines2 will point to the existing
> intel_execution_engines2 that has a static list of engines.
> 
> gem_init_engine_list() returns
> 
>  * '0' on success
>  * -EINVAL if GET/SETPARAM are not present in the running kernel
>  * errno in case of other failure cases
> 
> Signed-off-by: Andi Shyti <andi.shyti@intel.com>
> ---
>  lib/igt_gt.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  lib/igt_gt.h |  22 +++++++-
>  2 files changed, 163 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index 646696727ee4..6c2c46562760 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -577,7 +577,7 @@ bool gem_can_store_dword(int fd, unsigned int engine)
>         return true;
>  }
>  
> -const struct intel_execution_engine2 intel_execution_engines2[] = {
> +struct intel_execution_engine2 intel_execution_engines2[] = {
>         { "rcs0", I915_ENGINE_CLASS_RENDER, 0 },
>         { "bcs0", I915_ENGINE_CLASS_COPY, 0 },
>         { "vcs0", I915_ENGINE_CLASS_VIDEO, 0 },
> @@ -586,6 +586,8 @@ const struct intel_execution_engine2 intel_execution_engines2[] = {
>         { }
>  };
>  
> +struct intel_execution_engine2 *intel_active_engines2;
> +
>  unsigned int
>  gem_class_instance_to_eb_flags(int gem_fd,
>                                enum drm_i915_gem_engine_class class,
> @@ -650,3 +652,143 @@ bool gem_ring_has_physical_engine(int fd, unsigned ring)
>  
>         return gem_has_ring(fd, ring);
>  }
> +
> +static int __gem_query(int fd, struct drm_i915_query *q)
> +{
> +       return igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q) ? -errno : 0;
> +}
> +
> +void gem_query(int fd, struct drm_i915_query *q)
> +{
> +       igt_assert(!__gem_query(fd, q));
> +}
> +
> +static int __gem_get_set_param(int fd, unsigned long request,
> +                              struct drm_i915_gem_context_param *p)
> +{
> +       return igt_ioctl(fd, request, p) ?  -errno : 0;
> +}
> +
> +void gem_get_set_param(int fd, unsigned long request,
> +                      struct drm_i915_gem_context_param *p)

Sigh. gem_context_set_param et al. Which should remind you that this is
i915/ specific...
-Chris