On Tue, May 18, 2021 at 3:12 AM Zbigniew Kempczyński
[off-list ref] wrote:

On Fri, Apr 23, 2021 at 04:47:37PM -0500, Jason Ekstrand wrote:

quoted

v2 (Jason Ekstrand):
 - Add documentation

Signed-off-by: Jason Ekstrand <redacted>
Reviewed-by: Daniel Vetter <redacted>
---
 lib/i915/gem_engine_topology.c | 69 ++++++++++++++++++++++++++++++++++
 lib/i915/gem_engine_topology.h | 29 +++++++++++++-
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index 6bceb909..ac707cb3 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c

@@ -39,6 +39,47 @@
  *
  * This helper library contains functions used for querying and dealing
  * with engines in GEM contexts.
+ *
+ * Combined with intel_ctx_t, these helpers give a pretty standard pattern
+ * for testing every engine in a device:
+ * |[<!-- language="C" -->
+ *   const struct intel_execution_engine2 *e;
+ *   const intel_ctx_t *ctx = intel_ctx_create_all_physical(fd);
+ *
+ *   igt_subtest_with_dynamic("basic") {
+ *           for_each_ctx_engine(fd, ctx, e) {
+ *                   igt_dynamic_f("%s", e->name)
+ *                   run_ctx_test(fd, ctx, e);
+ *           }
+ *   }
+ * ]|
+ * This pattern works regardless of whether or not the engines topology API
+ * is available and regardless of whether or not your platform supports
+ * contexts.  If engines are unavailable, it falls back to a legacy context
+ * and if contexts are unavailable, intel_ctx_create_all_physial() will

                                                            ^^^^^^^ - typo

Fixed.

quoted

+ * return a wrapper around ctx0.
+ *
+ * If, for some reason, you want to create a second identical context to
+ * use with your engine iterator, duplicating the context is easy:
+ * |[<!-- language="C" -->
+ *   const intel_ctx_t *ctx2 = intel_ctx_create(fd, &ctx->cfg);
+ * ]|
+ *
+ * If you want each subtest to always create its own contexts, there are
+ * also iterators which work only on a context config.  As long as all
+ * contexts are created from that config, or from one with an identical set
+ * of engines, the iterator will be valid for those contexts.
+ * |[<!-- language="C" -->
+ *   const struct intel_execution_engine2 *e;
+ *   intel_ctx_cfg_t cfg = intel_ctx_cfg_all_physical(fd);
+ *
+ *   igt_subtest_with_dynamic("basic") {
+ *           for_each_ctx_cfg_engine(fd, &cfg, e) {
+ *                   igt_dynamic_f("%s", e->name)
+ *                   run_ctx_cfg_test(fd, &cfg, e);
+ *           }
+ *   }
+ * ]|
  */

 /*

@@ -256,6 +297,34 @@ struct intel_engine_data intel_engine_list_of_physical(int fd)
      return intel_engine_list_for_static(fd);
 }

+/**
+ * intel_engine_list_for_ctx_cfg:
+ * @fd: open i915 drm file descriptor
+ * @cfg: Context config
+ *
+ * Returns the list of all engines in the context config
+ */
+struct intel_engine_data
+intel_engine_list_for_ctx_cfg(int fd, const intel_ctx_cfg_t *cfg)
+{

I would add igt_assert(cfg) here especially we expect not-null
cfg pointer here.

Done.

quoted

+     if (fd >= 0 && cfg->num_engines) {
+             struct intel_engine_data engine_data = { };
+             int i;
+
+             engine_data.nengines = cfg->num_engines;
+             for (i = 0; i < cfg->num_engines; i++)
+                     init_engine(&engine_data.engines[i],
+                                 cfg->engines[i].engine_class,
+                                 cfg->engines[i].engine_instance,
+                                 i);

What's the reason of passing incrementing value here? It's confusing
especially it is not correspond to legacy engine ids.

The final parameter to init_engine() is the "flags" parameter to use
with it.  For userspace-specified engines, the flags parameter is an
engine index, hence i.  For legacy contexts it's something like
I915_EXEC_RENDER or I915_EXEC_BLT.

--Jason

--
Zbigniew

quoted

+
+             return engine_data;
+     } else {
+             /* This is a legacy context */
+             return intel_engine_list_for_static(fd);
+     }
+}
+
 static int gem_topology_get_param(int fd,
                                struct drm_i915_gem_context_param *p)
 {

diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
index bd93e410..92d9a479 100644
--- a/lib/i915/gem_engine_topology.h
+++ b/lib/i915/gem_engine_topology.h

@@ -26,8 +26,7 @@

 #include "igt_gt.h"
 #include "i915_drm.h"
-
-#define GEM_MAX_ENGINES              I915_EXEC_RING_MASK + 1
+#include "intel_ctx.h"

 int __gem_query_engines(int fd,
                      struct drm_i915_query_engine_info *query_engines,

@@ -51,6 +50,7 @@ struct intel_engine_data {

 bool gem_has_engine_topology(int fd);
 struct intel_engine_data intel_engine_list_of_physical(int fd);
+struct intel_engine_data intel_engine_list_for_ctx_cfg(int fd, const intel_ctx_cfg_t *cfg);
 struct intel_engine_data intel_init_engine_list(int fd, uint32_t ctx_id);

 /* iteration functions */

@@ -81,6 +81,31 @@ struct intel_execution_engine2 gem_eb_flags_to_engine(unsigned int flags);
 #define __for_each_static_engine(e__) \
      for ((e__) = intel_execution_engines2; (e__)->name[0]; (e__)++)

+/**
+ * for_each_ctx_cfg_engine
+ * @fd__: open i915 drm file descriptor
+ * @ctx_cfg__: Intel context config
+ * @e__: struct intel_execution_engine2 iterator
+ *
+ * Iterates over each physical engine in the context config
+ */
+#define for_each_ctx_cfg_engine(fd__, ctx_cfg__, e__) \
+     for (struct intel_engine_data i__##e__ = \
+                     intel_engine_list_for_ctx_cfg(fd__, ctx_cfg__); \
+          ((e__) = intel_get_current_engine(&i__##e__)); \
+          intel_next_engine(&i__##e__))
+
+/**
+ * for_each_ctx_engine
+ * @fd__: open i915 drm file descriptor
+ * @ctx__: Intel context wrapper
+ * @e__: struct intel_execution_engine2 iterator
+ *
+ * Iterates over each physical engine in the context
+ */
+#define for_each_ctx_engine(fd__, ctx__, e__) \
+     for_each_ctx_cfg_engine(fd__, &(ctx__)->cfg, e__)
+
 #define for_each_context_engine(fd__, ctx__, e__) \
      for (struct intel_engine_data i__ = intel_init_engine_list(fd__, ctx__); \
           ((e__) = intel_get_current_engine(&i__)); \
--

2.31.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev