new engine discovery interface

lib/i915/gem_engine_topology.c line 282

Quoted 4 lines

lib/i915/gem_engine_topology.c new file +4 -0

+void gem_unset_context_engines(int fd, uint32_t ctx)
+{
+       gem_context_destroy(fd, ctx);
+}

That's a little surprising. I certainly would not expect my context to be destroyed just to reset the engines back to default.

On the Rusty scale that would be
-7. The obvious use is wrong.

Quoted 10 lines

lib/i915/gem_engine_topology.c line 282

Quoted 4 lines

lib/i915/gem_engine_topology.c new file +4 -0

+void gem_unset_context_engines(int fd, uint32_t ctx)
+{
+       gem_context_destroy(fd, ctx);
+}

That's a little surprising. I certainly would not expect my context to be destroyed just to reset the engines back to default.

On the Rusty scale that would be
-7. The obvious use is wrong.

if I have a context creator 'gem_make_context_set_all_engines()' (which you already didn't like at the previous version), I should have a context destroyer.

I understand your concern, I will remove the context creation and destruction from gem_engine_topology.

lib/igt_dummyload.c +11 -5

  			if (opts->flags & IGT_SPIN_POLL_RUN &&
-			    !gem_can_store_dword(fd, engine))
+			    !gem_class_can_store_dword(fd, engine->class))
  				continue;
 
-			engines[nengine++] = engine;
+			flags[nengine++] = engine->flags;
  		}
  	} else {
-		engines[nengine++] = opts->engine;
+		flags[nengine++] = opts->engine;
  	}
  	igt_require(nengine);
 

@@ -234,7 +235,7 @@ emit_recursive_batch(igt_spin_t *spin,
 
  	for (i = 0; i < nengine; i++) {
  		execbuf->flags &= ~ENGINE_MASK;
-		execbuf->flags |= engines[i];
+		execbuf->flags |= flags[i];
 
  		gem_execbuf_wr(fd, execbuf);
 

@@ -309,9 +310,19 @@ igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts)
  	igt_require_gem(fd);
 
  	if (opts->engine != ALL_ENGINES) {
-		gem_require_ring(fd, opts->engine);
+		struct intel_execution_engine2 e;
+		int class;
+
+		if (!gem_context_lookup_engine(fd, opts->engine,
+					       opts->ctx, &e)) {
+			class = e.class;
+		} else {

<snip>

Quoted 7 lines

lib/igt_dummyload.c +1 -1

@@ -94,17 +95,17 @@ emit_recursive_batch(igt_spin_t *spin,
 
      nengine = 0;
      if (opts->engine == ALL_ENGINES) {
-             unsigned int engine;
+             struct intel_execution_engine2 *engine;
 

lib/igt_dummyload.c emit_recursive_batch(igt_spin_t *spin, · line 100

Quoted 2 lines

lib/igt_dummyload.c +1 -1

-             for_each_physical_engine(fd, engine) {
+             for_each_context_engine(fd, opts->ctx, engine) {

On a kernel that has the new I915_CONTEXT_PARAM_ENGINES, wouldn't this implicitly update opts->ctx to use it (via the ctx_map_engines in intel_init_engine_list)? What if the caller then tries to submit with that ctx using an execbuf flag?only an issue until all the callers are updated I guess.

<snip>

lib/igt_dummyload.c emit_recursive_batch(igt_spin_t *spin, · line 98

Quoted 7 lines

lib/igt_dummyload.c +2 -2

@@ -94,17 +95,17 @@ emit_recursive_batch(igt_spin_t *spin,
       nengine = 0;
       if (opts->engine == ALL_ENGINES) {
-        unsigned int engine;
+        struct intel_execution_engine2 *engine;
-        for_each_physical_engine(fd, engine) {
+        for_each_context_engine(fd, opts->ctx, engine) {

On a kernel that has the new I915_CONTEXT_PARAM_ENGINES, wouldn't this implicitly update opts->ctx to use it (via the ctx_map_engines in intel_init_engine_list)? What if the caller then tries to submit with that ctx using an execbuf flag?only an issue until all the callers are updated I guess.

Thanks,
Daniele

Quoted 31 lines

              if (opts->flags & IGT_SPIN_POLL_RUN && -                !gem_can_store_dword(fd, engine))
+                !gem_class_can_store_dword(fd, engine->class)) continue;
-            engines[nengine++] = engine;
+            flags[nengine++] = engine->flags;
          }
      } else {
-        engines[nengine++] = opts->engine;
+        flags[nengine++] = opts->engine;
      }
      igt_require(nengine);

lib/igt_dummyload.c +9 -2

@@ -234,7 +235,7 @@ emit_recursive_batch(igt_spin_t *spin,
       for (i = 0; i < nengine; i++) {
           execbuf->flags &= ~ENGINE_MASK;
-        execbuf->flags |= engines[i];
+        execbuf->flags |= flags[i];
           gem_execbuf_wr(fd, execbuf);

@@ -309,9 +310,19 @@ igt_spin_batch_factory(int fd, const struct 
 igt_spin_factory *opts)
       igt_require_gem(fd);
       if (opts->engine != ALL_ENGINES) {
-        gem_require_ring(fd, opts->engine);
+        struct intel_execution_engine2 e;
+        int class;
+
+        if (!gem_context_lookup_engine(fd, opts->engine,
+                           opts->ctx, &e)) {
+            class = e.class;
+        } else {
+            gem_require_ring(fd, opts->engine);

lib/igt_dummyload.c igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts) · line 328

Quoted 8 lines

lib/igt_dummyload.c +4 -1

+            class = gem_execbuf_flags_to_engine_class(opts->engine);
+        }
+
           if (opts->flags & IGT_SPIN_POLL_RUN)
-            igt_require(gem_can_store_dword(fd, opts->engine));
+            igt_require(gem_class_can_store_dword(fd, class));
       }
       spin = spin_batch_create(fd, opts);

So what to do to preserve compatibility with eb.flags and not mess up the context in here.. use the static iterator for contexts wo/ maps and legacy eb? But what does ALL_ENGINES mean then? Phase out ALL_ENGINES support in spin batch? Needs an audit of how many call sites we have and how do they look.

Quoting Tvrtko Ursulin (2019-04-17 17:14:45)

Quoted 5 lines

So what to do to preserve compatibility with eb.flags and not mess up the context in here.. use the static iterator for contexts wo/ maps and legacy eb? But what does ALL_ENGINES mean then? Phase out ALL_ENGINES support in spin batch? Needs an audit of how many call sites we have and how do they look.

Actually, we've^I've open-coded several callsite that could do with ALL_ENGINES. And I think that's a good thing -- as the setup requires a bit of finesse and resubmitting the spin batch works quite well. No doubt someone else will be tempted to refactor.

Replace the legacy for_each_engine* defines with the ones implemented in the gem_engine_topology library.

Use whenever possible gem_engine_can_store_dword() that checks class instead of flags.

Now the __for_each_engine_class_instance and for_each_engine_class_instance are unused, remove them.

Suggested-by Tvrtko Ursulin <[email protected]>

Signed-off-by Andi Shyti <[email protected]>

Cc Tvrtko Ursulin <[email protected]>

Reviewed-by Tvrtko Ursulin <[email protected]>

---
  lib/igt_gt.h     |   7 ---
  tests/perf_pmu.c | 148 ++++++++++++++++++++++++++++-------------------
  2 files changed, 90 insertions(+), 65 deletions(-)

lib/igt_gt.h +0 -7

index 0b5c7fcb4c3c..77318e2a82b8 100644
@@ -119,11 +119,4 @@ void gem_require_engine(int gem_fd,
  	igt_require(gem_has_engine(gem_fd, class, instance));
  }
 
-#define __for_each_engine_class_instance(e__) \
-	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)
-
-#define for_each_engine_class_instance(fd__, e__) \
-	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \
-		for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance))
-
  #endif /* IGT_GT_H */

tests/perf_pmu.c +57 -46

index 4f552bc2ae28..93e8efc9a645 100644
@@ -72,7 +72,7 @@ static int open_group(uint64_t config, int group)
  }
 
  static void
-init(int gem_fd, const struct intel_execution_engine2 *e, uint8_t sample)
+init(int gem_fd, struct intel_execution_engine2 *e, uint8_t sample)
  {
  	int fd, err = 0;
  	bool exists;

@@ -82,7 +82,7 @@ init(int gem_fd, const struct intel_execution_engine2 *e, uint8_t sample)
  	if (fd < 0)
  		err = errno;
 
-	exists = gem_has_engine(gem_fd, e->class, e->instance);
+	exists = gem_context_has_engine(gem_fd, 0, e->flags);
  	if (intel_gen(intel_get_drm_devid(gem_fd)) < 6 &&
  	    sample == I915_SAMPLE_SEMA)
  		exists = false;

@@ -158,11 +158,6 @@ static unsigned int measured_usleep(unsigned int usec)
  	return igt_nsec_elapsed(&ts);
  }
 
-static unsigned int e2ring(int gem_fd, const struct intel_execution_engine2 *e)
-{
-	return gem_class_instance_to_eb_flags(gem_fd, e->class, e->instance);
-}
-
  #define TEST_BUSY (1)
  #define FLAG_SYNC (2)
  #define TEST_TRAILING_IDLE (4)

@@ -170,14 +165,15 @@ static unsigned int e2ring(int gem_fd, const struct intel_execution_engine2 *e)
  #define FLAG_LONG (16)
  #define FLAG_HANG (32)
 
-static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
+static igt_spin_t * __spin_poll(int fd, uint32_t ctx,
+				struct intel_execution_engine2 *e)
  {
  	struct igt_spin_factory opts = {
  		.ctx = ctx,
-		.engine = flags,
+		.engine = e->flags,
  	};
 
-	if (gem_can_store_dword(fd, flags))
+	if (gem_class_can_store_dword(fd, e->class))
  		opts.flags |= IGT_SPIN_POLL_RUN;
 
  	return __igt_spin_batch_factory(fd, &opts);

@@ -209,20 +205,34 @@ static unsigned long __spin_wait(int fd, igt_spin_t *spin)
  	return igt_nsec_elapsed(&start);
  }
 
-static igt_spin_t * __spin_sync(int fd, uint32_t ctx, unsigned long flags)
+static igt_spin_t * __spin_sync(int fd, uint32_t ctx,
+				struct intel_execution_engine2 *e)
  {
-	igt_spin_t *spin = __spin_poll(fd, ctx, flags);
+	igt_spin_t *spin = __spin_poll(fd, ctx, e);
 
  	__spin_wait(fd, spin);
 
  	return spin;
  }
 
-static igt_spin_t * spin_sync(int fd, uint32_t ctx, unsigned long flags)
+static igt_spin_t * spin_sync(int fd, uint32_t ctx,
+			      struct intel_execution_engine2 *e)
  {
  	igt_require_gem(fd);
 
-	return __spin_sync(fd, ctx, flags);
+	return __spin_sync(fd, ctx, e);
+}
+
+static igt_spin_t * spin_sync_flags(int fd, uint32_t ctx, unsigned int flags)
+{
+	struct intel_execution_engine2 e = { };
+
+	e.class = gem_execbuf_flags_to_engine_class(flags);
+	e.instance = (flags & (I915_EXEC_BSD_MASK | I915_EXEC_RING_MASK)) ==
+		     (I915_EXEC_BSD | I915_EXEC_BSD_RING2) ? 1 : 0;
+	e.flags = flags;
+
+	return spin_sync(fd, ctx, &e);
  }
 
  static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)

@@ -257,7 +267,7 @@ static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
  }
 
  static void
-single(int gem_fd, const struct intel_execution_engine2 *e, unsigned int flags)
+single(int gem_fd, struct intel_execution_engine2 *e, unsigned int flags)
  {
  	unsigned long slept;
  	igt_spin_t *spin;

@@ -267,7 +277,7 @@ single(int gem_fd, const struct intel_execution_engine2 *e, unsigned int flags)
  	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
 
  	if (flags & TEST_BUSY)
-		spin = spin_sync(gem_fd, 0, e2ring(gem_fd, e));
+		spin = spin_sync(gem_fd, 0, e);
  	else
  		spin = NULL;
 

@@ -303,7 +313,7 @@ single(int gem_fd, const struct intel_execution_engine2 *e, unsigned int flags)
  }
 
  static void
-busy_start(int gem_fd, const struct intel_execution_engine2 *e)
+busy_start(int gem_fd, struct intel_execution_engine2 *e)
  {
  	unsigned long slept;
  	uint64_t val, ts[2];

@@ -316,7 +326,7 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
  	 */
  	sleep(2);
 
-	spin = __spin_sync(gem_fd, 0, e2ring(gem_fd, e));
+	spin = __spin_sync(gem_fd, 0, e);
 
  	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
 

@@ -338,7 +348,7 @@ busy_start(int gem_fd, const struct intel_execution_engine2 *e)
   * will depend on the CI systems running it a lot to detect issues.
   */
  static void
-busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
+busy_double_start(int gem_fd, struct intel_execution_engine2 *e)
  {
  	unsigned long slept;
  	uint64_t val, val2, ts[2];

@@ -346,7 +356,7 @@ busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
  	uint32_t ctx;
  	int fd;
 
-	ctx = gem_context_create(gem_fd);
+	ctx = gem_make_context_set_all_engines(gem_fd);
 
  	/*
  	 * Defeat the busy stats delayed disable, we need to guarantee we are

@@ -359,11 +369,11 @@ busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
  	 * re-submission in execlists mode. Make sure busyness is correctly
  	 * reported with the engine busy, and after the engine went idle.
  	 */
-	spin[0] = __spin_sync(gem_fd, 0, e2ring(gem_fd, e));
+	spin[0] = __spin_sync(gem_fd, 0, e);
  	usleep(500e3);
  	spin[1] = __igt_spin_batch_new(gem_fd,
  				       .ctx = ctx,
-				       .engine = e2ring(gem_fd, e));
+				       .engine = e->flags);
 
  	/*
  	 * Open PMU as fast as possible after the second spin batch in attempt

@@ -393,7 +403,7 @@ busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
 
  	close(fd);
 
-	gem_context_destroy(gem_fd, ctx);
+	gem_unset_context_engines(gem_fd, ctx);
 
  	assert_within_epsilon(val, ts[1] - ts[0], tolerance);
  	igt_assert_eq(val2, 0);

@@ -421,10 +431,10 @@ static void log_busy(unsigned int num_engines, uint64_t *val)
  }
 
  static void
-busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
+busy_check_all(int gem_fd, struct intel_execution_engine2 *e,
  	       const unsigned int num_engines, unsigned int flags)
  {
-	const struct intel_execution_engine2 *e_;
+	struct intel_execution_engine2 *e_;
  	uint64_t tval[2][num_engines];
  	unsigned int busy_idx = 0, i;
  	uint64_t val[num_engines];

@@ -434,8 +444,8 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
 
  	i = 0;
  	fd[0] = -1;
-	for_each_engine_class_instance(gem_fd, e_) {
-		if (e == e_)
+	__for_each_physical_engine(gem_fd, e_) {
+		if (e->class == e_->class && e->instance == e_->instance)
  			busy_idx = i;
 
  		fd[i++] = open_group(I915_PMU_ENGINE_BUSY(e_->class,

@@ -445,7 +455,7 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
 
  	igt_assert_eq(i, num_engines);
 
-	spin = spin_sync(gem_fd, 0, e2ring(gem_fd, e));
+	spin = spin_sync(gem_fd, 0, e);
  	pmu_read_multi(fd[0], num_engines, tval[0]);
  	slept = measured_usleep(batch_duration_ns / 1000);
  	if (flags & TEST_TRAILING_IDLE)

@@ -472,23 +482,23 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
 
  static void
  __submit_spin_batch(int gem_fd, igt_spin_t *spin,
-		    const struct intel_execution_engine2 *e,
+		    struct intel_execution_engine2 *e,
  		    int offset)
  {
  	struct drm_i915_gem_execbuffer2 eb = spin->execbuf;
 
  	eb.flags &= ~(0x3f | I915_EXEC_BSD_MASK);
-	eb.flags |= e2ring(gem_fd, e) | I915_EXEC_NO_RELOC;
+	eb.flags |= e->flags | I915_EXEC_NO_RELOC;
  	eb.batch_start_offset += offset;
 
  	gem_execbuf(gem_fd, &eb);
  }
 
  static void
-most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
+most_busy_check_all(int gem_fd, struct intel_execution_engine2 *e,
  		    const unsigned int num_engines, unsigned int flags)
  {
-	const struct intel_execution_engine2 *e_;
+	struct intel_execution_engine2 *e_;
  	uint64_t tval[2][num_engines];
  	uint64_t val[num_engines];
  	int fd[num_engines];

@@ -497,13 +507,13 @@ most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
  	unsigned int idle_idx, i;
 
  	i = 0;
-	for_each_engine_class_instance(gem_fd, e_) {
-		if (e == e_)
+	__for_each_physical_engine(gem_fd, e_) {
+		if (e->class == e_->class && e->instance == e_->instance)
  			idle_idx = i;
  		else if (spin)
  			__submit_spin_batch(gem_fd, spin, e_, 64);
  		else
-			spin = __spin_poll(gem_fd, 0, e2ring(gem_fd, e_));
+			spin = __spin_poll(gem_fd, 0, e_);
 
  		val[i++] = I915_PMU_ENGINE_BUSY(e_->class, e_->instance);
  	}

@@ -545,7 +555,7 @@ static void
  all_busy_check_all(int gem_fd, const unsigned int num_engines,
  		   unsigned int flags)
  {
-	const struct intel_execution_engine2 *e;
+	struct intel_execution_engine2 *e;
  	uint64_t tval[2][num_engines];
  	uint64_t val[num_engines];
  	int fd[num_engines];

@@ -554,11 +564,11 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines,
  	unsigned int i;
 
  	i = 0;
-	for_each_engine_class_instance(gem_fd, e) {
+	__for_each_physical_engine(gem_fd, e) {
  		if (spin)
  			__submit_spin_batch(gem_fd, spin, e, 64);
  		else
-			spin = __spin_poll(gem_fd, 0, e2ring(gem_fd, e));
+			spin = __spin_poll(gem_fd, 0, e);
 
  		val[i++] = I915_PMU_ENGINE_BUSY(e->class, e->instance);
  	}

@@ -592,7 +602,7 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines,
  }
 
  static void
-no_sema(int gem_fd, const struct intel_execution_engine2 *e, unsigned int flags)
+no_sema(int gem_fd, struct intel_execution_engine2 *e, unsigned int flags)
  {
  	igt_spin_t *spin;
  	uint64_t val[2][2];

@@ -602,7 +612,7 @@ no_sema(int gem_fd, const struct intel_execution_engine2 *e, unsigned int flags)
  	open_group(I915_PMU_ENGINE_WAIT(e->class, e->instance), fd);
 
  	if (flags & TEST_BUSY)
-		spin = spin_sync(gem_fd, 0, e2ring(gem_fd, e));
+		spin = spin_sync(gem_fd, 0, e);
  	else
  		spin = NULL;
 

@@ -631,7 +641,7 @@ no_sema(int gem_fd, const struct intel_execution_engine2 *e, unsigned int flags)
  #define   MI_SEMAPHORE_SAD_GTE_SDD	(1<<12)
 
  static void
-sema_wait(int gem_fd, const struct intel_execution_engine2 *e,
+sema_wait(int gem_fd, struct intel_execution_engine2 *e,
  	  unsigned int flags)
  {
  	struct drm_i915_gem_relocation_entry reloc[2] = {};

@@ -689,7 +699,7 @@ sema_wait(int gem_fd, const struct intel_execution_engine2 *e,
 
  	eb.buffer_count = 2;
  	eb.buffers_ptr = to_user_pointer(obj);
-	eb.flags = e2ring(gem_fd, e);
+	eb.flags = e->flags;
 
  	/**
  	 * Start the semaphore wait PMU and after some known time let the above

@@ -792,7 +802,7 @@ static int wait_vblank(int fd, union drm_wait_vblank *vbl)
  }
 
  static void
-event_wait(int gem_fd, const struct intel_execution_engine2 *e)
+event_wait(int gem_fd, struct intel_execution_engine2 *e)
  {
  	struct drm_i915_gem_exec_object2 obj = { };
  	struct drm_i915_gem_execbuffer2 eb = { };

@@ -845,7 +855,7 @@ event_wait(int gem_fd, const struct intel_execution_engine2 *e)
 
  	eb.buffer_count = 1;
  	eb.buffers_ptr = to_user_pointer(&obj);
-	eb.flags = e2ring(gem_fd, e) | I915_EXEC_SECURE;
+	eb.flags = e->flags | I915_EXEC_SECURE;
 
  	for_each_pipe_with_valid_output(&data.display, p, output) {
  		struct igt_helper_process waiter = { };

@@ -917,7 +927,7 @@ event_wait(int gem_fd, const struct intel_execution_engine2 *e)
  }
 
  static void
-multi_client(int gem_fd, const struct intel_execution_engine2 *e)
+multi_client(int gem_fd, struct intel_execution_engine2 *e)
  {
  	uint64_t config = I915_PMU_ENGINE_BUSY(e->class, e->instance);
  	unsigned long slept[2];

@@ -936,7 +946,7 @@ multi_client(int gem_fd, const struct intel_execution_engine2 *e)
  	 */
  	fd[1] = open_pmu(config);
 
-	spin = spin_sync(gem_fd, 0, e2ring(gem_fd, e));
+	spin = spin_sync(gem_fd, 0, e);
 
  	val[0] = val[1] = __pmu_read_single(fd[0], &ts[0]);
  	slept[1] = measured_usleep(batch_duration_ns / 1000);

@@ -1039,6 +1049,7 @@ static void cpu_hotplug(int gem_fd)
  	igt_spin_t *spin[2];
  	uint64_t ts[2];
  	uint64_t val;
+	uint32_t ctx;
  	int link[2];
  	int fd, ret;
  	int cur = 0;

tests/perf_pmu.c +31 -11

+
  	fd = open_pmu(I915_PMU_ENGINE_BUSY(I915_ENGINE_CLASS_RENDER, 0));
 
  	/*
  	 * Create two spinners so test can ensure shorter gaps in engine
  	 * busyness as it is terminating one and re-starting the other.
  	 */
-	spin[0] = igt_spin_batch_new(gem_fd, .engine = I915_EXEC_RENDER);
-	spin[1] = __igt_spin_batch_new(gem_fd, .engine = I915_EXEC_RENDER);
+	spin[0] = igt_spin_batch_new(gem_fd,
+				     .engine = I915_EXEC_RENDER, .ctx = ctx);
+	spin[1] = __igt_spin_batch_new(gem_fd,
+				       .engine = I915_EXEC_RENDER, .ctx = ctx);
 
  	val = __pmu_read_single(fd, &ts[0]);
 

@@ -1137,6 +1152,7 @@ static void cpu_hotplug(int gem_fd)
 
  		igt_spin_batch_free(gem_fd, spin[cur]);
  		spin[cur] = __igt_spin_batch_new(gem_fd,
+						 .ctx = ctx,
  						 .engine = I915_EXEC_RENDER);
  		cur ^= 1;
  	}

@@ -1150,6 +1166,7 @@ static void cpu_hotplug(int gem_fd)
  	igt_waitchildren();
  	close(fd);
  	close(link[0]);
+	gem_context_destroy(gem_fd, ctx);
 
  	/* Skip if child signals a problem with offlining a CPU. */
  	igt_skip_on(buf == 's');

@@ -1165,17 +1182,21 @@ test_interrupts(int gem_fd)
  	igt_spin_t *spin[target];
  	struct pollfd pfd;
  	uint64_t idle, busy;
+	uint32_t ctx;
  	int fence_fd;
  	int fd;
 
  	gem_quiescent_gpu(gem_fd);
 
+	ctx = gem_context_create(gem_fd);
+
  	fd = open_pmu(I915_PMU_INTERRUPTS);
 
  	/* Queue spinning batches. */
  	for (int i = 0; i < target; i++) {
  		spin[i] = __igt_spin_batch_new(gem_fd,
  					       .engine = I915_EXEC_RENDER,
+					       .ctx = ctx,
  					       .flags = IGT_SPIN_FENCE_OUT);
  		if (i == 0) {
  			fence_fd = spin[i]->out_fence;

@@ -1217,6 +1238,7 @@ test_interrupts(int gem_fd)
  	/* Check at least as many interrupts has been generated. */
  	busy = pmu_read_single(fd) - idle;
  	close(fd);
+	gem_context_destroy(gem_fd, ctx);
 
  	igt_assert_lte(target, busy);
  }

@@ -1229,15 +1251,19 @@ test_interrupts_sync(int gem_fd)
  	igt_spin_t *spin[target];
  	struct pollfd pfd;
  	uint64_t idle, busy;
+	uint32_t ctx;
  	int fd;
 
  	gem_quiescent_gpu(gem_fd);
 
+	ctx = gem_context_create(gem_fd);
+
  	fd = open_pmu(I915_PMU_INTERRUPTS);
 
  	/* Queue spinning batches. */
  	for (int i = 0; i < target; i++)
  		spin[i] = __igt_spin_batch_new(gem_fd,
+					       .ctx = ctx,
  					       .flags = IGT_SPIN_FENCE_OUT);
 
  	/* Wait for idle state. */

@@ -1262,6 +1288,7 @@ test_interrupts_sync(int gem_fd)
  	/* Check at least as many interrupts has been generated. */
  	busy = pmu_read_single(fd) - idle;
  	close(fd);
+	gem_context_destroy(gem_fd, ctx);
 
  	igt_assert_lte(target, busy);
  }

@@ -1274,6 +1301,9 @@ test_frequency(int gem_fd)
  	double min[2], max[2];
  	igt_spin_t *spin;
  	int fd, sysfs;
+	uint32_t ctx;
+
+	ctx = gem_context_create(gem_fd);
 
  	sysfs = igt_sysfs_open(gem_fd);
  	igt_require(sysfs >= 0);

@@ -1301,7 +1331,7 @@ test_frequency(int gem_fd)
  	igt_require(igt_sysfs_get_u32(sysfs, "gt_boost_freq_mhz") == min_freq);
 
  	gem_quiescent_gpu(gem_fd); /* Idle to be sure the change takes effect */
-	spin = spin_sync(gem_fd, 0, I915_EXEC_RENDER);
+	spin = spin_sync_flags(gem_fd, ctx, I915_EXEC_RENDER);
 
  	slept = pmu_read_multi(fd, 2, start);
  	measured_usleep(batch_duration_ns / 1000);

@@ -1327,7 +1357,7 @@ test_frequency(int gem_fd)
  	igt_require(igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz") == max_freq);
 
  	gem_quiescent_gpu(gem_fd);
-	spin = spin_sync(gem_fd, 0, I915_EXEC_RENDER);
+	spin = spin_sync_flags(gem_fd, ctx, I915_EXEC_RENDER);
 
  	slept = pmu_read_multi(fd, 2, start);
  	measured_usleep(batch_duration_ns / 1000);

@@ -1348,6 +1378,8 @@ test_frequency(int gem_fd)
  			 min_freq, igt_sysfs_get_u32(sysfs, "gt_min_freq_mhz"));
  	close(fd);
 
+	gem_context_destroy(gem_fd, ctx);
+
  	igt_info("Min frequency: requested %.1f, actual %.1f\n",
  		 min[0], min[1]);
  	igt_info("Max frequency: requested %.1f, actual %.1f\n",

@@ -1448,7 +1480,7 @@ test_rc6(int gem_fd, unsigned int flags)
  }
 
  static void
-test_enable_race(int gem_fd, const struct intel_execution_engine2 *e)
+test_enable_race(int gem_fd, struct intel_execution_engine2 *e)
  {
  	uint64_t config = I915_PMU_ENGINE_BUSY(e->class, e->instance);
  	struct igt_helper_process engine_load = { };

@@ -1465,7 +1497,7 @@ test_enable_race(int gem_fd, const struct intel_execution_engine2 *e)
 
  	eb.buffer_count = 1;
  	eb.buffers_ptr = to_user_pointer(&obj);
-	eb.flags = e2ring(gem_fd, e);
+	eb.flags = e->flags;
 
  	/*
  	 * This test is probabilistic so run in a few times to increase the

@@ -1520,7 +1552,7 @@ static void __rearm_spin_batch(igt_spin_t *spin)
  	__assert_within(x, ref, tolerance, tolerance)
 
  static void
-accuracy(int gem_fd, const struct intel_execution_engine2 *e,
+accuracy(int gem_fd, struct intel_execution_engine2 *e,
  	 unsigned long target_busy_pct,
  	 unsigned long target_iters)
  {

@@ -1570,7 +1602,7 @@ accuracy(int gem_fd, const struct intel_execution_engine2 *e,
  		igt_spin_t *spin;
 
  		/* Allocate our spin batch and idle it. */
-		spin = igt_spin_batch_new(gem_fd, .engine = e2ring(gem_fd, e));
+		spin = igt_spin_batch_new(gem_fd, .engine = e->flags);
  		igt_spin_batch_end(spin);
  		gem_sync(gem_fd, spin->handle);
 

@@ -1674,7 +1706,7 @@ igt_main
  				I915_PMU_LAST - __I915_PMU_OTHER(0) + 1;
  	unsigned int num_engines = 0;
  	int fd = -1;
-	const struct intel_execution_engine2 *e;
+	struct intel_execution_engine2 *e;
  	unsigned int i;
 
  	igt_fixture {

@@ -1683,7 +1715,7 @@ igt_main
  		igt_require_gem(fd);
  		igt_require(i915_type_id() > 0);
 
-		for_each_engine_class_instance(fd, e)
+		__for_each_physical_engine(fd, e)
  			num_engines++;
  	}
 

@@ -1693,7 +1725,7 @@ igt_main
  	igt_subtest("invalid-init")
  		invalid_init();
 
-	__for_each_engine_class_instance(e) {
+	__for_each_physical_engine(fd, e) {
  		const unsigned int pct[] = { 2, 50, 98 };
 
  		/**

tests/perf_pmu.c static void cpu_hotplug(int gem_fd) · line 1058

Quoted 3 lines

tests/perf_pmu.c +1 -0

@@ -1046,14 +1057,18 @@ static void cpu_hotplug(int gem_fd)
  	igt_require(cpu0_hotplug_support());
+	ctx = gem_context_create(gem_fd);

TODO left was to try without new contexts for a handful of tests where I added them. Instead try I915_EXEC_DEFAULT when submitting and (ab)use the fact RCS is at index zero for foreseeable future. When it fails we'll know about it. This applies to hotplug, frequency, interrupts subtests AFAIR.

Quoted 8 lines

tests/perf_pmu.c static void cpu_hotplug(int gem_fd) · line 1058

Quoted 3 lines

tests/perf_pmu.c +1 -0

@@ -1046,14 +1057,18 @@ static void cpu_hotplug(int gem_fd)
   	igt_require(cpu0_hotplug_support());
+	ctx = gem_context_create(gem_fd);

TODO left was to try without new contexts for a handful of tests where I added them. Instead try I915_EXEC_DEFAULT when submitting and (ab)use the fact RCS is at index zero for foreseeable future. When it fails we'll know about it. This applies to hotplug, frequency, interrupts subtests AFAIR.

yes, I was also thinking about adding a few wrappers around.

I managed to build quite a hefty list of things to do in these 21 versions (22 with the coming)

But I would happily send them once this series gets accepted to not procrastinate it further.