intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API
@ 2023-07-11 16:02 Tvrtko Ursulin
  2023-07-11 16:02 ` [Intel-gfx] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds Tvrtko Ursulin
  2023-07-14 17:22 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Rodrigo Vivi
  0 siblings, 2 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2023-07-11 16:02 UTC (permalink / raw)
  To: igt-dev, Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Sync spinner API is identical and compatible with regular spinners just
that it tries to make sure spinner is actually running on the hardware
before returning from the constructor.

A few tests already use it, one more will, so lets promote it into
common library.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 lib/igt_dummyload.c     | 105 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_dummyload.h     |  11 +++++
 tests/i915/drm_fdinfo.c |  81 ++++---------------------------
 tests/i915/gem_eio.c    |  15 ++----
 tests/i915/perf_pmu.c   |  84 +++++---------------------------
 5 files changed, 140 insertions(+), 156 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 9f941cef73e6..d3cee91540b6 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -33,6 +33,7 @@
 #include "drmtest.h"
 #include "i915/gem_create.h"
 #include "i915/gem_engine_topology.h"
+#include "i915/gem.h"
 #include "i915/gem_mman.h"
 #include "i915/gem_submission.h"
 #include "igt_aux.h"
@@ -715,6 +716,110 @@ void igt_unshare_spins(void)
 	IGT_INIT_LIST_HEAD(&spin_list);
 }
 
+/**
+ * __igt_sync_spin_poll:
+ * @i915: open i915 drm file descriptor
+ * @ahnd: allocator handle
+ * @ctx: intel_ctx_t context
+ * @e: engine to execute on
+ *
+ * Starts a recursive batch on an engine.
+ *
+ * Returns a #igt_spin_t which can be used with both standard and igt_sync_spin
+ * API family. Callers should consult @gem_class_can_store_dword to whether
+ * the target platform+engine can reliably support the igt_sync_spin API.
+ */
+igt_spin_t *
+__igt_sync_spin_poll(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
+		     const struct intel_execution_engine2 *e)
+{
+	struct igt_spin_factory opts = {
+		.ahnd = ahnd,
+		.ctx = ctx,
+		.engine = e ? e->flags : 0,
+	};
+
+	if (!e || gem_class_can_store_dword(i915, e->class))
+		opts.flags |= IGT_SPIN_POLL_RUN;
+
+	return __igt_spin_factory(i915, &opts);
+}
+
+/**
+ * __igt_sync_spin_wait:
+ * @i915: open i915 drm file descriptor
+ * @spin: previously create sync spinner
+ *
+ * Waits for a spinner to be running on the hardware.
+ *
+ * Callers should consult @gem_class_can_store_dword to whether the target
+ * platform+engine can reliably support the igt_sync_spin API.
+ */
+unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin)
+{
+	struct timespec start = { };
+
+	igt_nsec_elapsed(&start);
+
+	if (igt_spin_has_poll(spin)) {
+		unsigned long timeout = 0;
+
+		while (!igt_spin_has_started(spin)) {
+			unsigned long t = igt_nsec_elapsed(&start);
+
+			igt_assert(gem_bo_busy(i915, spin->handle));
+			if ((t - timeout) > 250e6) {
+				timeout = t;
+				igt_warn("Spinner not running after %.2fms\n",
+					 (double)t / 1e6);
+				igt_assert(t < 2e9);
+			}
+		}
+	} else {
+		igt_debug("__spin_wait - usleep mode\n");
+		usleep(500e3); /* Better than nothing! */
+	}
+
+	igt_assert(gem_bo_busy(i915, spin->handle));
+	return igt_nsec_elapsed(&start);
+}
+
+igt_spin_t *
+__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
+		const struct intel_execution_engine2 *e)
+{
+	igt_spin_t *spin = __igt_sync_spin_poll(i915, ahnd, ctx, e);
+
+	__igt_sync_spin_wait(i915, spin);
+
+	return spin;
+}
+
+/**
+ * igt_sync_spin:
+ * @i915: open i915 drm file descriptor
+ * @ahnd: allocator handle
+ * @ctx: intel_ctx_t context
+ * @e: engine to execute on
+ *
+ * Starts a recursive batch on an engine.
+ *
+ * Returns a #igt_spin_t and tries to guarantee it to be running at the time of
+ * the return. Otherwise it does a best effort only. Callers should check for
+ * @gem_class_can_store_dword if they want to be sure guarantee can be given.
+ *
+ * Both standard and igt_sync_spin API family can be used on the returned
+ * spinner object.
+ */
+igt_spin_t *
+igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
+	      const struct intel_execution_engine2 *e)
+{
+	igt_require_gem(i915);
+
+	return __igt_sync_spin(i915, ahnd, ctx, e);
+}
+
 static uint32_t plug_vgem_handle(struct igt_cork *cork, int fd)
 {
 	struct vgem_bo bo;
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index 6eb3f2e696bb..b771011af74f 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -143,6 +143,17 @@ void igt_terminate_spins(void);
 void igt_unshare_spins(void);
 void igt_free_spins(int i915);
 
+struct intel_execution_engine2;
+
+igt_spin_t *__igt_sync_spin_poll(int i915, uint64_t ahnd,
+				 const intel_ctx_t *ctx,
+				 const struct intel_execution_engine2 *e);
+unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin);
+igt_spin_t *__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
+			    const struct intel_execution_engine2 *e);
+igt_spin_t *igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
+			  const struct intel_execution_engine2 *e);
+
 enum igt_cork_type {
 	CORK_SYNC_FD = 1,
 	CORK_VGEM_HANDLE
diff --git a/tests/i915/drm_fdinfo.c b/tests/i915/drm_fdinfo.c
index c0e0ba1905f1..5cafa0e469e2 100644
--- a/tests/i915/drm_fdinfo.c
+++ b/tests/i915/drm_fdinfo.c
@@ -138,68 +138,6 @@ static unsigned int measured_usleep(unsigned int usec)
 #define FLAG_HANG (8)
 #define TEST_ISOLATION (16)
 
-static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
-			       const struct intel_execution_engine2 *e)
-{
-	struct igt_spin_factory opts = {
-		.ahnd = ahnd,
-		.ctx = ctx,
-		.engine = e ? e->flags : 0,
-	};
-
-	if (!e || gem_class_can_store_dword(fd, e->class))
-		opts.flags |= IGT_SPIN_POLL_RUN;
-
-	return __igt_spin_factory(fd, &opts);
-}
-
-static unsigned long __spin_wait(int fd, igt_spin_t *spin)
-{
-	struct timespec start = { };
-
-	igt_nsec_elapsed(&start);
-
-	if (igt_spin_has_poll(spin)) {
-		unsigned long timeout = 0;
-
-		while (!igt_spin_has_started(spin)) {
-			unsigned long t = igt_nsec_elapsed(&start);
-
-			igt_assert(gem_bo_busy(fd, spin->handle));
-			if ((t - timeout) > 250e6) {
-				timeout = t;
-				igt_warn("Spinner not running after %.2fms\n",
-					 (double)t / 1e6);
-				igt_assert(t < 2e9);
-			}
-		}
-	} else {
-		igt_debug("__spin_wait - usleep mode\n");
-		usleep(500e3); /* Better than nothing! */
-	}
-
-	igt_assert(gem_bo_busy(fd, spin->handle));
-	return igt_nsec_elapsed(&start);
-}
-
-static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
-			       const struct intel_execution_engine2 *e)
-{
-	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
-
-	__spin_wait(fd, spin);
-
-	return spin;
-}
-
-static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
-			     const struct intel_execution_engine2 *e)
-{
-	igt_require_gem(fd);
-
-	return __spin_sync(fd, ahnd, ctx, e);
-}
-
 static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
 {
 	if (!spin)
@@ -264,7 +202,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
 	ahnd = get_reloc_ahnd(spin_fd, ctx->id);
 
 	if (flags & TEST_BUSY)
-		spin = spin_sync(spin_fd, ahnd, ctx, e);
+		spin = igt_sync_spin(spin_fd, ahnd, ctx, e);
 	else
 		spin = NULL;
 
@@ -349,7 +287,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 
 	memset(tval, 0, sizeof(tval));
 
-	spin = spin_sync(gem_fd, ahnd, ctx, e);
+	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
 
 	read_busy_all(gem_fd, tval[0]);
 	slept = measured_usleep(batch_duration_ns / 1000);
@@ -418,14 +356,14 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 			__submit_spin(gem_fd, spin, e_, 64);
 			busy_class[e_->class]++;
 		} else {
-			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
+			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
 			busy_class[e_->class]++;
 		}
 	}
 	igt_require(spin); /* at least one busy engine */
 
 	/* Small delay to allow engines to start. */
-	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
+	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
 
 	read_busy_all(gem_fd, tval[0]);
 	slept = measured_usleep(batch_duration_ns / 1000);
@@ -475,12 +413,12 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 		if (spin)
 			__submit_spin(gem_fd, spin, e, 64);
 		else
-			spin = __spin_poll(gem_fd, ahnd, ctx, e);
+			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
 		busy_class[e->class]++;
 	}
 
 	/* Small delay to allow engines to start. */
-	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
+	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
 
 	read_busy_all(gem_fd, tval[0]);
 	slept = measured_usleep(batch_duration_ns / 1000);
@@ -624,7 +562,7 @@ virtual(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
 			ahnd = get_reloc_ahnd(i915, ctx->id);
 
 			if (flags & TEST_BUSY)
-				spin = spin_sync(i915, ahnd, ctx, NULL);
+				spin = igt_sync_spin(i915, ahnd, ctx, NULL);
 			else
 				spin = NULL;
 
@@ -732,11 +670,12 @@ virtual_all(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
 			if (spin)
 				__virt_submit_spin(i915, spin, ctx[i], 64);
 			else
-				spin = __spin_poll(i915, ahnd, ctx[i], NULL);
+				spin = __igt_sync_spin_poll(i915, ahnd, ctx[i],
+							    NULL);
 		}
 
 		/* Small delay to allow engines to start. */
-		usleep(__spin_wait(i915, spin) * count / 1e3);
+		usleep(__igt_sync_spin_wait(i915, spin) * count / 1e3);
 
 		val = read_busy(i915, class);
 		slept = measured_usleep(batch_duration_ns / 1000);
diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index d889d67dcebd..6d4b8f7df909 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -47,6 +47,7 @@
 #include "i915/gem_ring.h"
 #include "igt.h"
 #include "igt_device.h"
+#include "igt_dummyload.h"
 #include "igt_fb.h"
 #include "igt_kms.h"
 #include "igt_stats.h"
@@ -429,22 +430,12 @@ static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
 	return __igt_spin_factory(fd, &opts);
 }
 
-static void __spin_wait(int fd, igt_spin_t *spin)
-{
-	if (igt_spin_has_poll(spin)) {
-		igt_spin_busywait_until_started(spin);
-	} else {
-		igt_debug("__spin_wait - usleep mode\n");
-		usleep(500e3); /* Better than nothing! */
-	}
-}
-
 static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
 			     unsigned long flags)
 {
 	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, flags);
 
-	__spin_wait(fd, spin);
+	__igt_sync_spin_wait(fd, spin);
 
 	return spin;
 }
@@ -963,7 +954,7 @@ static void test_inflight_external(int fd)
 	fence = execbuf.rsvd2 >> 32;
 	igt_assert(fence != -1);
 
-	__spin_wait(fd, hang);
+	__igt_sync_spin_wait(fd, hang);
 	manual_hang(fd);
 
 	gem_sync(fd, hang->handle); /* wedged, with an unready batch */
diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index 0806a8e545b5..a888027ad9af 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -377,68 +377,6 @@ static unsigned int measured_usleep(unsigned int usec)
 #define TEST_OTHER (128)
 #define TEST_ALL   (256)
 
-static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
-			       const struct intel_execution_engine2 *e)
-{
-	struct igt_spin_factory opts = {
-		.ahnd = ahnd,
-		.ctx = ctx,
-		.engine = e->flags,
-	};
-
-	if (gem_class_can_store_dword(fd, e->class))
-		opts.flags |= IGT_SPIN_POLL_RUN;
-
-	return __igt_spin_factory(fd, &opts);
-}
-
-static unsigned long __spin_wait(int fd, igt_spin_t *spin)
-{
-	struct timespec start = { };
-
-	igt_nsec_elapsed(&start);
-
-	if (igt_spin_has_poll(spin)) {
-		unsigned long timeout = 0;
-
-		while (!igt_spin_has_started(spin)) {
-			unsigned long t = igt_nsec_elapsed(&start);
-
-			igt_assert(gem_bo_busy(fd, spin->handle));
-			if ((t - timeout) > 250e6) {
-				timeout = t;
-				igt_warn("Spinner not running after %.2fms\n",
-					 (double)t / 1e6);
-				igt_assert(t < 2e9);
-			}
-		}
-	} else {
-		igt_debug("__spin_wait - usleep mode\n");
-		usleep(500e3); /* Better than nothing! */
-	}
-
-	igt_assert(gem_bo_busy(fd, spin->handle));
-	return igt_nsec_elapsed(&start);
-}
-
-static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
-			       const struct intel_execution_engine2 *e)
-{
-	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
-
-	__spin_wait(fd, spin);
-
-	return spin;
-}
-
-static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
-			     const struct intel_execution_engine2 *e)
-{
-	igt_require_gem(fd);
-
-	return __spin_sync(fd, ahnd, ctx, e);
-}
-
 static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
 {
 	if (!spin)
@@ -484,7 +422,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
 	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
 
 	if (flags & TEST_BUSY)
-		spin = spin_sync(gem_fd, ahnd, ctx, e);
+		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
 	else
 		spin = NULL;
 
@@ -536,7 +474,7 @@ busy_start(int gem_fd, const intel_ctx_t *ctx,
 	 */
 	sleep(2);
 
-	spin = __spin_sync(gem_fd, ahnd, ctx, e);
+	spin = __igt_sync_spin(gem_fd, ahnd, ctx, e);
 
 	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
 
@@ -583,7 +521,7 @@ busy_double_start(int gem_fd, const intel_ctx_t *ctx,
 	 * 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, ahnd, ctx, e);
+	spin[0] = __igt_sync_spin(gem_fd, ahnd, ctx, e);
 	usleep(500e3);
 	spin[1] = __igt_spin_new(gem_fd,
 				 .ahnd = ahndN,
@@ -675,7 +613,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 
 	igt_assert_eq(i, num_engines);
 
-	spin = spin_sync(gem_fd, ahnd, ctx, e);
+	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
 	pmu_read_multi(fd[0], num_engines, tval[0]);
 	slept = measured_usleep(batch_duration_ns / 1000);
 	if (flags & TEST_TRAILING_IDLE)
@@ -737,7 +675,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 		else if (spin)
 			__submit_spin(gem_fd, spin, e_, 64);
 		else
-			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
+			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
 
 		val[i++] = I915_PMU_ENGINE_BUSY(e_->class, e_->instance);
 	}
@@ -749,7 +687,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 		fd[i] = open_group(gem_fd, val[i], fd[0]);
 
 	/* Small delay to allow engines to start. */
-	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
+	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
 
 	pmu_read_multi(fd[0], num_engines, tval[0]);
 	slept = measured_usleep(batch_duration_ns / 1000);
@@ -796,7 +734,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 		if (spin)
 			__submit_spin(gem_fd, spin, e, 64);
 		else
-			spin = __spin_poll(gem_fd, ahnd, ctx, e);
+			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
 
 		val[i++] = I915_PMU_ENGINE_BUSY(e->class, e->instance);
 	}
@@ -807,7 +745,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
 		fd[i] = open_group(gem_fd, val[i], fd[0]);
 
 	/* Small delay to allow engines to start. */
-	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
+	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
 
 	pmu_read_multi(fd[0], num_engines, tval[0]);
 	slept = measured_usleep(batch_duration_ns / 1000);
@@ -848,7 +786,7 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
 			   fd[0]);
 
 	if (flags & TEST_BUSY)
-		spin = spin_sync(gem_fd, ahnd, ctx, e);
+		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
 	else
 		spin = NULL;
 
@@ -1406,7 +1344,7 @@ multi_client(int gem_fd, const intel_ctx_t *ctx,
 	 */
 	fd[1] = open_pmu(gem_fd, config);
 
-	spin = spin_sync(gem_fd, ahnd, ctx, e);
+	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
 
 	val[0] = val[1] = __pmu_read_single(fd[0], &ts[0]);
 	slept[1] = measured_usleep(batch_duration_ns / 1000);
@@ -1776,7 +1714,7 @@ static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
 
 	igt_debug("Using engine %u:%u\n", e.class, e.instance);
 
-	return spin_sync(i915, ahnd, *ctx, &e);
+	return igt_sync_spin(i915, ahnd, *ctx, &e);
 }
 
 static void
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Intel-gfx] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds
  2023-07-11 16:02 [Intel-gfx] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Tvrtko Ursulin
@ 2023-07-11 16:02 ` Tvrtko Ursulin
  2023-07-14 17:26   ` [Intel-gfx] [igt-dev] " Rodrigo Vivi
  2023-07-14 17:22 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Rodrigo Vivi
  1 sibling, 1 reply; 7+ messages in thread
From: Tvrtko Ursulin @ 2023-07-11 16:02 UTC (permalink / raw)
  To: igt-dev, Intel-gfx; +Cc: Rodrigo Vivi

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Exercise a bunch of up and down rps thresholds to verify hardware
is happy with them all.

To limit the overall runtime relies on probability and number of runs
to approach complete coverage.

v2:
 * Common sync spinner code now in library.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/i915/i915_pm_rps.c | 194 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 194 insertions(+)

diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c
index 7044fcd81c56..8c370b35c85b 100644
--- a/tests/i915/i915_pm_rps.c
+++ b/tests/i915/i915_pm_rps.c
@@ -39,8 +39,10 @@
 #include "i915/gem.h"
 #include "i915/gem_create.h"
 #include "igt.h"
+#include "igt_aux.h"
 #include "igt_dummyload.h"
 #include "igt_perf.h"
+#include "igt_rand.h"
 #include "igt_sysfs.h"
 /**
  * TEST: i915 pm rps
@@ -81,6 +83,22 @@
  * SUBTEST: waitboost
  * Feature: pm_rps
  * Run type: FULL
+ *
+ * SUBTEST: thresholds
+ * Feature: pm_rps
+ * Run type: FULL
+ *
+ * SUBTEST: thresholds-idle
+ * Feature: pm_rps
+ * Run type: FULL
+ *
+ * SUBTEST: thresholds-idle-park
+ * Feature: pm_rps
+ * Run type: FULL
+ *
+ * SUBTEST: thresholds-park
+ * Feature: pm_rps
+ * Run type: FULL
  */
 
 IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
@@ -920,6 +938,146 @@ static void pm_rps_exit_handler(int sig)
 	drm_close_driver(drm_fd);
 }
 
+static struct i915_engine_class_instance
+find_dword_engine(int i915, const unsigned int gt)
+{
+	struct i915_engine_class_instance *engines, ci = { -1, -1 };
+	unsigned int i, count;
+
+	engines = gem_list_engines(i915, 1u << gt, ~0u, &count);
+	igt_assert(engines);
+
+	for (i = 0; i < count; i++) {
+		if (!gem_class_can_store_dword(i915, engines[i].engine_class))
+			continue;
+
+		ci = engines[i];
+		break;
+	}
+
+	free(engines);
+
+	return ci;
+}
+
+static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
+				const intel_ctx_t **ctx)
+{
+	struct i915_engine_class_instance ci = { -1, -1 };
+	struct intel_execution_engine2 e = { };
+
+	ci = find_dword_engine(i915, gt);
+
+	igt_require(ci.engine_class != (uint16_t)I915_ENGINE_CLASS_INVALID);
+
+	if (gem_has_contexts(i915)) {
+		e.class = ci.engine_class;
+		e.instance = ci.engine_instance;
+		e.flags = 0;
+		*ctx = intel_ctx_create_for_engine(i915, e.class, e.instance);
+	} else {
+		igt_require(gt == 0); /* Impossible anyway. */
+		e.class = gem_execbuf_flags_to_engine_class(I915_EXEC_DEFAULT);
+		e.instance = 0;
+		e.flags = I915_EXEC_DEFAULT;
+		*ctx = intel_ctx_0(i915);
+	}
+
+	igt_debug("Using engine %u:%u\n", e.class, e.instance);
+
+	return __igt_sync_spin(i915, ahnd, *ctx, &e);
+}
+
+#define TEST_IDLE 0x1
+#define TEST_PARK 0x2
+static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
+{
+	uint64_t ahnd = get_reloc_ahnd(i915, 0);
+	const unsigned int points = 10;
+	unsigned int def_up, def_down;
+	igt_spin_t *spin = NULL;
+	const intel_ctx_t *ctx;
+	unsigned int *ta, *tb;
+	unsigned int i;
+	int sysfs;
+
+	sysfs = igt_sysfs_gt_open(i915, gt);
+	igt_require(sysfs >= 0);
+
+	/* Feature test */
+	def_up = igt_sysfs_get_u32(sysfs, "rps_up_threshold_pct");
+	def_down = igt_sysfs_get_u32(sysfs, "rps_down_threshold_pct");
+	igt_require(def_up && def_down);
+
+	/* Check invalid percentages are rejected */
+	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false);
+	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);
+
+	/*
+	 * Invent some random up-down thresholds, but always include 0 and 100
+	 * just to have some wild edge cases.
+	 */
+	ta = calloc(points, sizeof(unsigned int));
+	tb = calloc(points, sizeof(unsigned int));
+	igt_require(ta && tb);
+
+	ta[0] = tb[0] = 0;
+	ta[1] = tb[1] = 100;
+	hars_petruska_f54_1_random_seed(time(NULL));
+	for (i = 2; i < points; i++) {
+		ta[i] = hars_petruska_f54_1_random_unsafe_max(100);
+		tb[i] = hars_petruska_f54_1_random_unsafe_max(100);
+	}
+	igt_permute_array(ta, points, igt_exchange_int);
+	igt_permute_array(tb, points, igt_exchange_int);
+
+	/* Exercise the thresholds with a GPU load to trigger park/unpark etc */
+	for (i = 0; i < points; i++) {
+		igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]);
+		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true);
+		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);
+
+		if (flags & TEST_IDLE) {
+			gem_quiescent_gpu(i915);
+		} else if (spin) {
+			intel_ctx_destroy(i915, ctx);
+			igt_spin_free(i915, spin);
+			spin = NULL;
+			if (flags & TEST_PARK) {
+				gem_quiescent_gpu(i915);
+				usleep(500000);
+			}
+		}
+		spin = spin_sync_gt(i915, ahnd, gt, &ctx);
+		usleep(1000000);
+		if (flags & TEST_IDLE) {
+			intel_ctx_destroy(i915, ctx);
+			igt_spin_free(i915, spin);
+			if (flags & TEST_PARK) {
+				gem_quiescent_gpu(i915);
+				usleep(500000);
+			}
+			spin = NULL;
+		}
+	}
+
+	if (spin) {
+		intel_ctx_destroy(i915, ctx);
+		igt_spin_free(i915, spin);
+	}
+
+	gem_quiescent_gpu(i915);
+
+	/* Restore defaults */
+	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true);
+	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);
+
+	free(ta);
+	free(tb);
+	close(sysfs);
+	put_ahnd(ahnd);
+}
+
 igt_main
 {
 	igt_fixture {
@@ -1000,6 +1158,42 @@ igt_main
 		igt_disallow_hang(drm_fd, hang);
 	}
 
+	igt_subtest_with_dynamic("thresholds-idle") {
+		int tmp, gt;
+
+		i915_for_each_gt(drm_fd, tmp, gt) {
+			igt_dynamic_f("gt%u", gt)
+				test_thresholds(drm_fd, gt, TEST_IDLE);
+		}
+	}
+
+	igt_subtest_with_dynamic("thresholds") {
+		int tmp, gt;
+
+		i915_for_each_gt(drm_fd, tmp, gt) {
+			igt_dynamic_f("gt%u", gt)
+				test_thresholds(drm_fd, gt, 0);
+		}
+	}
+
+	igt_subtest_with_dynamic("thresholds-park") {
+		int tmp, gt;
+
+		i915_for_each_gt(drm_fd, tmp, gt) {
+			igt_dynamic_f("gt%u", gt)
+				test_thresholds(drm_fd, gt, TEST_PARK);
+		}
+	}
+
+	igt_subtest_with_dynamic("thresholds-idle-park") {
+		int tmp, gt;
+
+		i915_for_each_gt(drm_fd, tmp, gt) {
+			igt_dynamic_f("gt%u", gt)
+				test_thresholds(drm_fd, gt, TEST_IDLE | TEST_PARK);
+		}
+	}
+
 	igt_fixture
 		drm_close_driver(drm_fd);
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API
  2023-07-11 16:02 [Intel-gfx] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Tvrtko Ursulin
  2023-07-11 16:02 ` [Intel-gfx] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds Tvrtko Ursulin
@ 2023-07-14 17:22 ` Rodrigo Vivi
  2023-07-17  8:34   ` Tvrtko Ursulin
  1 sibling, 1 reply; 7+ messages in thread
From: Rodrigo Vivi @ 2023-07-14 17:22 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx

On Tue, Jul 11, 2023 at 05:02:13PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Sync spinner API is identical and compatible with regular spinners just
> that it tries to make sure spinner is actually running on the hardware
> before returning from the constructor.
> 
> A few tests already use it, one more will, so lets promote it into
> common library.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  lib/igt_dummyload.c     | 105 ++++++++++++++++++++++++++++++++++++++++
>  lib/igt_dummyload.h     |  11 +++++
>  tests/i915/drm_fdinfo.c |  81 ++++---------------------------
>  tests/i915/gem_eio.c    |  15 ++----
>  tests/i915/perf_pmu.c   |  84 +++++---------------------------
>  5 files changed, 140 insertions(+), 156 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 9f941cef73e6..d3cee91540b6 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c

why here?

> @@ -33,6 +33,7 @@
>  #include "drmtest.h"
>  #include "i915/gem_create.h"
>  #include "i915/gem_engine_topology.h"
> +#include "i915/gem.h"
>  #include "i915/gem_mman.h"
>  #include "i915/gem_submission.h"
>  #include "igt_aux.h"
> @@ -715,6 +716,110 @@ void igt_unshare_spins(void)
>  	IGT_INIT_LIST_HEAD(&spin_list);
>  }
>  
> +/**
> + * __igt_sync_spin_poll:
> + * @i915: open i915 drm file descriptor

anyway to make this not i915 centric?
or maybe move it to or start a lib that is i915 only?

I know that we have many more lib things that are still i915 centric,
but at some point we need to start organizing it...

> + * @ahnd: allocator handle
> + * @ctx: intel_ctx_t context
> + * @e: engine to execute on
> + *
> + * Starts a recursive batch on an engine.
> + *
> + * Returns a #igt_spin_t which can be used with both standard and igt_sync_spin
> + * API family. Callers should consult @gem_class_can_store_dword to whether
> + * the target platform+engine can reliably support the igt_sync_spin API.
> + */
> +igt_spin_t *
> +__igt_sync_spin_poll(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> +		     const struct intel_execution_engine2 *e)
> +{
> +	struct igt_spin_factory opts = {
> +		.ahnd = ahnd,
> +		.ctx = ctx,
> +		.engine = e ? e->flags : 0,
> +	};
> +
> +	if (!e || gem_class_can_store_dword(i915, e->class))
> +		opts.flags |= IGT_SPIN_POLL_RUN;
> +
> +	return __igt_spin_factory(i915, &opts);
> +}
> +
> +/**
> + * __igt_sync_spin_wait:
> + * @i915: open i915 drm file descriptor
> + * @spin: previously create sync spinner
> + *
> + * Waits for a spinner to be running on the hardware.
> + *
> + * Callers should consult @gem_class_can_store_dword to whether the target
> + * platform+engine can reliably support the igt_sync_spin API.
> + */
> +unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin)
> +{
> +	struct timespec start = { };
> +
> +	igt_nsec_elapsed(&start);
> +
> +	if (igt_spin_has_poll(spin)) {
> +		unsigned long timeout = 0;
> +
> +		while (!igt_spin_has_started(spin)) {
> +			unsigned long t = igt_nsec_elapsed(&start);
> +
> +			igt_assert(gem_bo_busy(i915, spin->handle));
> +			if ((t - timeout) > 250e6) {
> +				timeout = t;
> +				igt_warn("Spinner not running after %.2fms\n",
> +					 (double)t / 1e6);
> +				igt_assert(t < 2e9);
> +			}
> +		}
> +	} else {
> +		igt_debug("__spin_wait - usleep mode\n");
> +		usleep(500e3); /* Better than nothing! */
> +	}
> +
> +	igt_assert(gem_bo_busy(i915, spin->handle));
> +	return igt_nsec_elapsed(&start);
> +}
> +
> +igt_spin_t *
> +__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> +		const struct intel_execution_engine2 *e)
> +{
> +	igt_spin_t *spin = __igt_sync_spin_poll(i915, ahnd, ctx, e);
> +
> +	__igt_sync_spin_wait(i915, spin);
> +
> +	return spin;
> +}
> +
> +/**
> + * igt_sync_spin:
> + * @i915: open i915 drm file descriptor
> + * @ahnd: allocator handle
> + * @ctx: intel_ctx_t context
> + * @e: engine to execute on
> + *
> + * Starts a recursive batch on an engine.
> + *
> + * Returns a #igt_spin_t and tries to guarantee it to be running at the time of
> + * the return. Otherwise it does a best effort only. Callers should check for
> + * @gem_class_can_store_dword if they want to be sure guarantee can be given.
> + *
> + * Both standard and igt_sync_spin API family can be used on the returned
> + * spinner object.
> + */
> +igt_spin_t *
> +igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> +	      const struct intel_execution_engine2 *e)
> +{
> +	igt_require_gem(i915);
> +
> +	return __igt_sync_spin(i915, ahnd, ctx, e);
> +}
> +
>  static uint32_t plug_vgem_handle(struct igt_cork *cork, int fd)
>  {
>  	struct vgem_bo bo;
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index 6eb3f2e696bb..b771011af74f 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -143,6 +143,17 @@ void igt_terminate_spins(void);
>  void igt_unshare_spins(void);
>  void igt_free_spins(int i915);
>  
> +struct intel_execution_engine2;
> +
> +igt_spin_t *__igt_sync_spin_poll(int i915, uint64_t ahnd,
> +				 const intel_ctx_t *ctx,
> +				 const struct intel_execution_engine2 *e);
> +unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin);
> +igt_spin_t *__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> +			    const struct intel_execution_engine2 *e);
> +igt_spin_t *igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> +			  const struct intel_execution_engine2 *e);
> +
>  enum igt_cork_type {
>  	CORK_SYNC_FD = 1,
>  	CORK_VGEM_HANDLE
> diff --git a/tests/i915/drm_fdinfo.c b/tests/i915/drm_fdinfo.c
> index c0e0ba1905f1..5cafa0e469e2 100644
> --- a/tests/i915/drm_fdinfo.c
> +++ b/tests/i915/drm_fdinfo.c
> @@ -138,68 +138,6 @@ static unsigned int measured_usleep(unsigned int usec)
>  #define FLAG_HANG (8)
>  #define TEST_ISOLATION (16)
>  
> -static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> -			       const struct intel_execution_engine2 *e)
> -{
> -	struct igt_spin_factory opts = {
> -		.ahnd = ahnd,
> -		.ctx = ctx,
> -		.engine = e ? e->flags : 0,
> -	};
> -
> -	if (!e || gem_class_can_store_dword(fd, e->class))
> -		opts.flags |= IGT_SPIN_POLL_RUN;
> -
> -	return __igt_spin_factory(fd, &opts);
> -}
> -
> -static unsigned long __spin_wait(int fd, igt_spin_t *spin)
> -{
> -	struct timespec start = { };
> -
> -	igt_nsec_elapsed(&start);
> -
> -	if (igt_spin_has_poll(spin)) {
> -		unsigned long timeout = 0;
> -
> -		while (!igt_spin_has_started(spin)) {
> -			unsigned long t = igt_nsec_elapsed(&start);
> -
> -			igt_assert(gem_bo_busy(fd, spin->handle));
> -			if ((t - timeout) > 250e6) {
> -				timeout = t;
> -				igt_warn("Spinner not running after %.2fms\n",
> -					 (double)t / 1e6);
> -				igt_assert(t < 2e9);
> -			}
> -		}
> -	} else {
> -		igt_debug("__spin_wait - usleep mode\n");
> -		usleep(500e3); /* Better than nothing! */
> -	}
> -
> -	igt_assert(gem_bo_busy(fd, spin->handle));
> -	return igt_nsec_elapsed(&start);
> -}
> -
> -static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> -			       const struct intel_execution_engine2 *e)
> -{
> -	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
> -
> -	__spin_wait(fd, spin);
> -
> -	return spin;
> -}
> -
> -static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> -			     const struct intel_execution_engine2 *e)
> -{
> -	igt_require_gem(fd);
> -
> -	return __spin_sync(fd, ahnd, ctx, e);
> -}
> -
>  static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
>  {
>  	if (!spin)
> @@ -264,7 +202,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
>  	ahnd = get_reloc_ahnd(spin_fd, ctx->id);
>  
>  	if (flags & TEST_BUSY)
> -		spin = spin_sync(spin_fd, ahnd, ctx, e);
> +		spin = igt_sync_spin(spin_fd, ahnd, ctx, e);
>  	else
>  		spin = NULL;
>  
> @@ -349,7 +287,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  
>  	memset(tval, 0, sizeof(tval));
>  
> -	spin = spin_sync(gem_fd, ahnd, ctx, e);
> +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>  
>  	read_busy_all(gem_fd, tval[0]);
>  	slept = measured_usleep(batch_duration_ns / 1000);
> @@ -418,14 +356,14 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  			__submit_spin(gem_fd, spin, e_, 64);
>  			busy_class[e_->class]++;
>  		} else {
> -			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
>  			busy_class[e_->class]++;
>  		}
>  	}
>  	igt_require(spin); /* at least one busy engine */
>  
>  	/* Small delay to allow engines to start. */
> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>  
>  	read_busy_all(gem_fd, tval[0]);
>  	slept = measured_usleep(batch_duration_ns / 1000);
> @@ -475,12 +413,12 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  		if (spin)
>  			__submit_spin(gem_fd, spin, e, 64);
>  		else
> -			spin = __spin_poll(gem_fd, ahnd, ctx, e);
> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
>  		busy_class[e->class]++;
>  	}
>  
>  	/* Small delay to allow engines to start. */
> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>  
>  	read_busy_all(gem_fd, tval[0]);
>  	slept = measured_usleep(batch_duration_ns / 1000);
> @@ -624,7 +562,7 @@ virtual(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
>  			ahnd = get_reloc_ahnd(i915, ctx->id);
>  
>  			if (flags & TEST_BUSY)
> -				spin = spin_sync(i915, ahnd, ctx, NULL);
> +				spin = igt_sync_spin(i915, ahnd, ctx, NULL);
>  			else
>  				spin = NULL;
>  
> @@ -732,11 +670,12 @@ virtual_all(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
>  			if (spin)
>  				__virt_submit_spin(i915, spin, ctx[i], 64);
>  			else
> -				spin = __spin_poll(i915, ahnd, ctx[i], NULL);
> +				spin = __igt_sync_spin_poll(i915, ahnd, ctx[i],
> +							    NULL);
>  		}
>  
>  		/* Small delay to allow engines to start. */
> -		usleep(__spin_wait(i915, spin) * count / 1e3);
> +		usleep(__igt_sync_spin_wait(i915, spin) * count / 1e3);
>  
>  		val = read_busy(i915, class);
>  		slept = measured_usleep(batch_duration_ns / 1000);
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index d889d67dcebd..6d4b8f7df909 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -47,6 +47,7 @@
>  #include "i915/gem_ring.h"
>  #include "igt.h"
>  #include "igt_device.h"
> +#include "igt_dummyload.h"
>  #include "igt_fb.h"
>  #include "igt_kms.h"
>  #include "igt_stats.h"
> @@ -429,22 +430,12 @@ static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>  	return __igt_spin_factory(fd, &opts);
>  }
>  
> -static void __spin_wait(int fd, igt_spin_t *spin)
> -{
> -	if (igt_spin_has_poll(spin)) {
> -		igt_spin_busywait_until_started(spin);
> -	} else {
> -		igt_debug("__spin_wait - usleep mode\n");
> -		usleep(500e3); /* Better than nothing! */
> -	}
> -}
> -
>  static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>  			     unsigned long flags)
>  {
>  	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, flags);
>  
> -	__spin_wait(fd, spin);
> +	__igt_sync_spin_wait(fd, spin);
>  
>  	return spin;
>  }
> @@ -963,7 +954,7 @@ static void test_inflight_external(int fd)
>  	fence = execbuf.rsvd2 >> 32;
>  	igt_assert(fence != -1);
>  
> -	__spin_wait(fd, hang);
> +	__igt_sync_spin_wait(fd, hang);
>  	manual_hang(fd);
>  
>  	gem_sync(fd, hang->handle); /* wedged, with an unready batch */
> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> index 0806a8e545b5..a888027ad9af 100644
> --- a/tests/i915/perf_pmu.c
> +++ b/tests/i915/perf_pmu.c
> @@ -377,68 +377,6 @@ static unsigned int measured_usleep(unsigned int usec)
>  #define TEST_OTHER (128)
>  #define TEST_ALL   (256)
>  
> -static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> -			       const struct intel_execution_engine2 *e)
> -{
> -	struct igt_spin_factory opts = {
> -		.ahnd = ahnd,
> -		.ctx = ctx,
> -		.engine = e->flags,
> -	};
> -
> -	if (gem_class_can_store_dword(fd, e->class))
> -		opts.flags |= IGT_SPIN_POLL_RUN;
> -
> -	return __igt_spin_factory(fd, &opts);
> -}
> -
> -static unsigned long __spin_wait(int fd, igt_spin_t *spin)
> -{
> -	struct timespec start = { };
> -
> -	igt_nsec_elapsed(&start);
> -
> -	if (igt_spin_has_poll(spin)) {
> -		unsigned long timeout = 0;
> -
> -		while (!igt_spin_has_started(spin)) {
> -			unsigned long t = igt_nsec_elapsed(&start);
> -
> -			igt_assert(gem_bo_busy(fd, spin->handle));
> -			if ((t - timeout) > 250e6) {
> -				timeout = t;
> -				igt_warn("Spinner not running after %.2fms\n",
> -					 (double)t / 1e6);
> -				igt_assert(t < 2e9);
> -			}
> -		}
> -	} else {
> -		igt_debug("__spin_wait - usleep mode\n");
> -		usleep(500e3); /* Better than nothing! */
> -	}
> -
> -	igt_assert(gem_bo_busy(fd, spin->handle));
> -	return igt_nsec_elapsed(&start);
> -}
> -
> -static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> -			       const struct intel_execution_engine2 *e)
> -{
> -	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
> -
> -	__spin_wait(fd, spin);
> -
> -	return spin;
> -}
> -
> -static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> -			     const struct intel_execution_engine2 *e)
> -{
> -	igt_require_gem(fd);
> -
> -	return __spin_sync(fd, ahnd, ctx, e);
> -}
> -
>  static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
>  {
>  	if (!spin)
> @@ -484,7 +422,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
>  	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
>  
>  	if (flags & TEST_BUSY)
> -		spin = spin_sync(gem_fd, ahnd, ctx, e);
> +		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>  	else
>  		spin = NULL;
>  
> @@ -536,7 +474,7 @@ busy_start(int gem_fd, const intel_ctx_t *ctx,
>  	 */
>  	sleep(2);
>  
> -	spin = __spin_sync(gem_fd, ahnd, ctx, e);
> +	spin = __igt_sync_spin(gem_fd, ahnd, ctx, e);
>  
>  	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
>  
> @@ -583,7 +521,7 @@ busy_double_start(int gem_fd, const intel_ctx_t *ctx,
>  	 * 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, ahnd, ctx, e);
> +	spin[0] = __igt_sync_spin(gem_fd, ahnd, ctx, e);
>  	usleep(500e3);
>  	spin[1] = __igt_spin_new(gem_fd,
>  				 .ahnd = ahndN,
> @@ -675,7 +613,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  
>  	igt_assert_eq(i, num_engines);
>  
> -	spin = spin_sync(gem_fd, ahnd, ctx, e);
> +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>  	pmu_read_multi(fd[0], num_engines, tval[0]);
>  	slept = measured_usleep(batch_duration_ns / 1000);
>  	if (flags & TEST_TRAILING_IDLE)
> @@ -737,7 +675,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  		else if (spin)
>  			__submit_spin(gem_fd, spin, e_, 64);
>  		else
> -			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
>  
>  		val[i++] = I915_PMU_ENGINE_BUSY(e_->class, e_->instance);
>  	}
> @@ -749,7 +687,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  		fd[i] = open_group(gem_fd, val[i], fd[0]);
>  
>  	/* Small delay to allow engines to start. */
> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>  
>  	pmu_read_multi(fd[0], num_engines, tval[0]);
>  	slept = measured_usleep(batch_duration_ns / 1000);
> @@ -796,7 +734,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  		if (spin)
>  			__submit_spin(gem_fd, spin, e, 64);
>  		else
> -			spin = __spin_poll(gem_fd, ahnd, ctx, e);
> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
>  
>  		val[i++] = I915_PMU_ENGINE_BUSY(e->class, e->instance);
>  	}
> @@ -807,7 +745,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>  		fd[i] = open_group(gem_fd, val[i], fd[0]);
>  
>  	/* Small delay to allow engines to start. */
> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>  
>  	pmu_read_multi(fd[0], num_engines, tval[0]);
>  	slept = measured_usleep(batch_duration_ns / 1000);
> @@ -848,7 +786,7 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
>  			   fd[0]);
>  
>  	if (flags & TEST_BUSY)
> -		spin = spin_sync(gem_fd, ahnd, ctx, e);
> +		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>  	else
>  		spin = NULL;
>  
> @@ -1406,7 +1344,7 @@ multi_client(int gem_fd, const intel_ctx_t *ctx,
>  	 */
>  	fd[1] = open_pmu(gem_fd, config);
>  
> -	spin = spin_sync(gem_fd, ahnd, ctx, e);
> +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>  
>  	val[0] = val[1] = __pmu_read_single(fd[0], &ts[0]);
>  	slept[1] = measured_usleep(batch_duration_ns / 1000);
> @@ -1776,7 +1714,7 @@ static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
>  
>  	igt_debug("Using engine %u:%u\n", e.class, e.instance);
>  
> -	return spin_sync(i915, ahnd, *ctx, &e);
> +	return igt_sync_spin(i915, ahnd, *ctx, &e);
>  }
>  
>  static void
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds
  2023-07-11 16:02 ` [Intel-gfx] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds Tvrtko Ursulin
@ 2023-07-14 17:26   ` Rodrigo Vivi
  2023-07-17  8:37     ` Tvrtko Ursulin
  0 siblings, 1 reply; 7+ messages in thread
From: Rodrigo Vivi @ 2023-07-14 17:26 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx

On Tue, Jul 11, 2023 at 05:02:14PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Exercise a bunch of up and down rps thresholds to verify hardware
> is happy with them all.
> 
> To limit the overall runtime relies on probability and number of runs
> to approach complete coverage.
> 
> v2:
>  * Common sync spinner code now in library.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  tests/i915/i915_pm_rps.c | 194 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 194 insertions(+)
> 
> diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c
> index 7044fcd81c56..8c370b35c85b 100644
> --- a/tests/i915/i915_pm_rps.c
> +++ b/tests/i915/i915_pm_rps.c
> @@ -39,8 +39,10 @@
>  #include "i915/gem.h"
>  #include "i915/gem_create.h"
>  #include "igt.h"
> +#include "igt_aux.h"
>  #include "igt_dummyload.h"
>  #include "igt_perf.h"
> +#include "igt_rand.h"
>  #include "igt_sysfs.h"
>  /**
>   * TEST: i915 pm rps
> @@ -81,6 +83,22 @@
>   * SUBTEST: waitboost
>   * Feature: pm_rps
>   * Run type: FULL
> + *
> + * SUBTEST: thresholds
> + * Feature: pm_rps
> + * Run type: FULL
> + *
> + * SUBTEST: thresholds-idle
> + * Feature: pm_rps
> + * Run type: FULL
> + *
> + * SUBTEST: thresholds-idle-park
> + * Feature: pm_rps
> + * Run type: FULL
> + *
> + * SUBTEST: thresholds-park
> + * Feature: pm_rps
> + * Run type: FULL
>   */
>  
>  IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
> @@ -920,6 +938,146 @@ static void pm_rps_exit_handler(int sig)
>  	drm_close_driver(drm_fd);
>  }
>  
> +static struct i915_engine_class_instance
> +find_dword_engine(int i915, const unsigned int gt)
> +{
> +	struct i915_engine_class_instance *engines, ci = { -1, -1 };
> +	unsigned int i, count;
> +
> +	engines = gem_list_engines(i915, 1u << gt, ~0u, &count);
> +	igt_assert(engines);
> +
> +	for (i = 0; i < count; i++) {
> +		if (!gem_class_can_store_dword(i915, engines[i].engine_class))
> +			continue;
> +
> +		ci = engines[i];
> +		break;
> +	}
> +
> +	free(engines);
> +
> +	return ci;
> +}
> +
> +static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
> +				const intel_ctx_t **ctx)
> +{
> +	struct i915_engine_class_instance ci = { -1, -1 };
> +	struct intel_execution_engine2 e = { };
> +
> +	ci = find_dword_engine(i915, gt);
> +
> +	igt_require(ci.engine_class != (uint16_t)I915_ENGINE_CLASS_INVALID);
> +
> +	if (gem_has_contexts(i915)) {
> +		e.class = ci.engine_class;
> +		e.instance = ci.engine_instance;
> +		e.flags = 0;
> +		*ctx = intel_ctx_create_for_engine(i915, e.class, e.instance);
> +	} else {
> +		igt_require(gt == 0); /* Impossible anyway. */

I'm confused by the comment here... if it is impossible why we have code below?!
but why impossible?

anyway, the tests below are great for the sysfs that you are adding. Thanks

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> +		e.class = gem_execbuf_flags_to_engine_class(I915_EXEC_DEFAULT);
> +		e.instance = 0;
> +		e.flags = I915_EXEC_DEFAULT;
> +		*ctx = intel_ctx_0(i915);
> +	}
> +
> +	igt_debug("Using engine %u:%u\n", e.class, e.instance);
> +
> +	return __igt_sync_spin(i915, ahnd, *ctx, &e);
> +}
> +
> +#define TEST_IDLE 0x1
> +#define TEST_PARK 0x2
> +static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
> +{
> +	uint64_t ahnd = get_reloc_ahnd(i915, 0);
> +	const unsigned int points = 10;
> +	unsigned int def_up, def_down;
> +	igt_spin_t *spin = NULL;
> +	const intel_ctx_t *ctx;
> +	unsigned int *ta, *tb;
> +	unsigned int i;
> +	int sysfs;
> +
> +	sysfs = igt_sysfs_gt_open(i915, gt);
> +	igt_require(sysfs >= 0);
> +
> +	/* Feature test */
> +	def_up = igt_sysfs_get_u32(sysfs, "rps_up_threshold_pct");
> +	def_down = igt_sysfs_get_u32(sysfs, "rps_down_threshold_pct");
> +	igt_require(def_up && def_down);
> +
> +	/* Check invalid percentages are rejected */
> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false);
> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);
> +
> +	/*
> +	 * Invent some random up-down thresholds, but always include 0 and 100
> +	 * just to have some wild edge cases.
> +	 */
> +	ta = calloc(points, sizeof(unsigned int));
> +	tb = calloc(points, sizeof(unsigned int));
> +	igt_require(ta && tb);
> +
> +	ta[0] = tb[0] = 0;
> +	ta[1] = tb[1] = 100;
> +	hars_petruska_f54_1_random_seed(time(NULL));
> +	for (i = 2; i < points; i++) {
> +		ta[i] = hars_petruska_f54_1_random_unsafe_max(100);
> +		tb[i] = hars_petruska_f54_1_random_unsafe_max(100);
> +	}
> +	igt_permute_array(ta, points, igt_exchange_int);
> +	igt_permute_array(tb, points, igt_exchange_int);
> +
> +	/* Exercise the thresholds with a GPU load to trigger park/unpark etc */
> +	for (i = 0; i < points; i++) {
> +		igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]);
> +		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true);
> +		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);
> +
> +		if (flags & TEST_IDLE) {
> +			gem_quiescent_gpu(i915);
> +		} else if (spin) {
> +			intel_ctx_destroy(i915, ctx);
> +			igt_spin_free(i915, spin);
> +			spin = NULL;
> +			if (flags & TEST_PARK) {
> +				gem_quiescent_gpu(i915);
> +				usleep(500000);
> +			}
> +		}
> +		spin = spin_sync_gt(i915, ahnd, gt, &ctx);
> +		usleep(1000000);
> +		if (flags & TEST_IDLE) {
> +			intel_ctx_destroy(i915, ctx);
> +			igt_spin_free(i915, spin);
> +			if (flags & TEST_PARK) {
> +				gem_quiescent_gpu(i915);
> +				usleep(500000);
> +			}
> +			spin = NULL;
> +		}
> +	}
> +
> +	if (spin) {
> +		intel_ctx_destroy(i915, ctx);
> +		igt_spin_free(i915, spin);
> +	}
> +
> +	gem_quiescent_gpu(i915);
> +
> +	/* Restore defaults */
> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true);
> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);
> +
> +	free(ta);
> +	free(tb);
> +	close(sysfs);
> +	put_ahnd(ahnd);
> +}
> +
>  igt_main
>  {
>  	igt_fixture {
> @@ -1000,6 +1158,42 @@ igt_main
>  		igt_disallow_hang(drm_fd, hang);
>  	}
>  
> +	igt_subtest_with_dynamic("thresholds-idle") {
> +		int tmp, gt;
> +
> +		i915_for_each_gt(drm_fd, tmp, gt) {
> +			igt_dynamic_f("gt%u", gt)
> +				test_thresholds(drm_fd, gt, TEST_IDLE);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("thresholds") {
> +		int tmp, gt;
> +
> +		i915_for_each_gt(drm_fd, tmp, gt) {
> +			igt_dynamic_f("gt%u", gt)
> +				test_thresholds(drm_fd, gt, 0);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("thresholds-park") {
> +		int tmp, gt;
> +
> +		i915_for_each_gt(drm_fd, tmp, gt) {
> +			igt_dynamic_f("gt%u", gt)
> +				test_thresholds(drm_fd, gt, TEST_PARK);
> +		}
> +	}
> +
> +	igt_subtest_with_dynamic("thresholds-idle-park") {
> +		int tmp, gt;
> +
> +		i915_for_each_gt(drm_fd, tmp, gt) {
> +			igt_dynamic_f("gt%u", gt)
> +				test_thresholds(drm_fd, gt, TEST_IDLE | TEST_PARK);
> +		}
> +	}
> +
>  	igt_fixture
>  		drm_close_driver(drm_fd);
>  }
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API
  2023-07-14 17:22 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Rodrigo Vivi
@ 2023-07-17  8:34   ` Tvrtko Ursulin
  2023-07-17 14:55     ` Rodrigo Vivi
  0 siblings, 1 reply; 7+ messages in thread
From: Tvrtko Ursulin @ 2023-07-17  8:34 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: igt-dev, Intel-gfx


On 14/07/2023 18:22, Rodrigo Vivi wrote:
> On Tue, Jul 11, 2023 at 05:02:13PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Sync spinner API is identical and compatible with regular spinners just
>> that it tries to make sure spinner is actually running on the hardware
>> before returning from the constructor.
>>
>> A few tests already use it, one more will, so lets promote it into
>> common library.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>   lib/igt_dummyload.c     | 105 ++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_dummyload.h     |  11 +++++
>>   tests/i915/drm_fdinfo.c |  81 ++++---------------------------
>>   tests/i915/gem_eio.c    |  15 ++----
>>   tests/i915/perf_pmu.c   |  84 +++++---------------------------
>>   5 files changed, 140 insertions(+), 156 deletions(-)
>>
>> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
>> index 9f941cef73e6..d3cee91540b6 100644
>> --- a/lib/igt_dummyload.c
>> +++ b/lib/igt_dummyload.c
> 
> why here?
> 
>> @@ -33,6 +33,7 @@
>>   #include "drmtest.h"
>>   #include "i915/gem_create.h"
>>   #include "i915/gem_engine_topology.h"
>> +#include "i915/gem.h"
>>   #include "i915/gem_mman.h"
>>   #include "i915/gem_submission.h"
>>   #include "igt_aux.h"
>> @@ -715,6 +716,110 @@ void igt_unshare_spins(void)
>>   	IGT_INIT_LIST_HEAD(&spin_list);
>>   }
>>   
>> +/**
>> + * __igt_sync_spin_poll:
>> + * @i915: open i915 drm file descriptor
> 
> anyway to make this not i915 centric?
> or maybe move it to or start a lib that is i915 only?
> 
> I know that we have many more lib things that are still i915 centric,
> but at some point we need to start organizing it...

Is igt_dummyload i915/xe agnostic already? I missed that. It would be a 
big ask for me to get on top of two uapis and do that.. ugh. :(

So on the "why here?" part. Assuming taking the i915 route.. where would 
you suggest to put it?

Regards,

Tvrtko

>> + * @ahnd: allocator handle
>> + * @ctx: intel_ctx_t context
>> + * @e: engine to execute on
>> + *
>> + * Starts a recursive batch on an engine.
>> + *
>> + * Returns a #igt_spin_t which can be used with both standard and igt_sync_spin
>> + * API family. Callers should consult @gem_class_can_store_dword to whether
>> + * the target platform+engine can reliably support the igt_sync_spin API.
>> + */
>> +igt_spin_t *
>> +__igt_sync_spin_poll(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
>> +		     const struct intel_execution_engine2 *e)
>> +{
>> +	struct igt_spin_factory opts = {
>> +		.ahnd = ahnd,
>> +		.ctx = ctx,
>> +		.engine = e ? e->flags : 0,
>> +	};
>> +
>> +	if (!e || gem_class_can_store_dword(i915, e->class))
>> +		opts.flags |= IGT_SPIN_POLL_RUN;
>> +
>> +	return __igt_spin_factory(i915, &opts);
>> +}
>> +
>> +/**
>> + * __igt_sync_spin_wait:
>> + * @i915: open i915 drm file descriptor
>> + * @spin: previously create sync spinner
>> + *
>> + * Waits for a spinner to be running on the hardware.
>> + *
>> + * Callers should consult @gem_class_can_store_dword to whether the target
>> + * platform+engine can reliably support the igt_sync_spin API.
>> + */
>> +unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin)
>> +{
>> +	struct timespec start = { };
>> +
>> +	igt_nsec_elapsed(&start);
>> +
>> +	if (igt_spin_has_poll(spin)) {
>> +		unsigned long timeout = 0;
>> +
>> +		while (!igt_spin_has_started(spin)) {
>> +			unsigned long t = igt_nsec_elapsed(&start);
>> +
>> +			igt_assert(gem_bo_busy(i915, spin->handle));
>> +			if ((t - timeout) > 250e6) {
>> +				timeout = t;
>> +				igt_warn("Spinner not running after %.2fms\n",
>> +					 (double)t / 1e6);
>> +				igt_assert(t < 2e9);
>> +			}
>> +		}
>> +	} else {
>> +		igt_debug("__spin_wait - usleep mode\n");
>> +		usleep(500e3); /* Better than nothing! */
>> +	}
>> +
>> +	igt_assert(gem_bo_busy(i915, spin->handle));
>> +	return igt_nsec_elapsed(&start);
>> +}
>> +
>> +igt_spin_t *
>> +__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
>> +		const struct intel_execution_engine2 *e)
>> +{
>> +	igt_spin_t *spin = __igt_sync_spin_poll(i915, ahnd, ctx, e);
>> +
>> +	__igt_sync_spin_wait(i915, spin);
>> +
>> +	return spin;
>> +}
>> +
>> +/**
>> + * igt_sync_spin:
>> + * @i915: open i915 drm file descriptor
>> + * @ahnd: allocator handle
>> + * @ctx: intel_ctx_t context
>> + * @e: engine to execute on
>> + *
>> + * Starts a recursive batch on an engine.
>> + *
>> + * Returns a #igt_spin_t and tries to guarantee it to be running at the time of
>> + * the return. Otherwise it does a best effort only. Callers should check for
>> + * @gem_class_can_store_dword if they want to be sure guarantee can be given.
>> + *
>> + * Both standard and igt_sync_spin API family can be used on the returned
>> + * spinner object.
>> + */
>> +igt_spin_t *
>> +igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
>> +	      const struct intel_execution_engine2 *e)
>> +{
>> +	igt_require_gem(i915);
>> +
>> +	return __igt_sync_spin(i915, ahnd, ctx, e);
>> +}
>> +
>>   static uint32_t plug_vgem_handle(struct igt_cork *cork, int fd)
>>   {
>>   	struct vgem_bo bo;
>> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
>> index 6eb3f2e696bb..b771011af74f 100644
>> --- a/lib/igt_dummyload.h
>> +++ b/lib/igt_dummyload.h
>> @@ -143,6 +143,17 @@ void igt_terminate_spins(void);
>>   void igt_unshare_spins(void);
>>   void igt_free_spins(int i915);
>>   
>> +struct intel_execution_engine2;
>> +
>> +igt_spin_t *__igt_sync_spin_poll(int i915, uint64_t ahnd,
>> +				 const intel_ctx_t *ctx,
>> +				 const struct intel_execution_engine2 *e);
>> +unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin);
>> +igt_spin_t *__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
>> +			    const struct intel_execution_engine2 *e);
>> +igt_spin_t *igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
>> +			  const struct intel_execution_engine2 *e);
>> +
>>   enum igt_cork_type {
>>   	CORK_SYNC_FD = 1,
>>   	CORK_VGEM_HANDLE
>> diff --git a/tests/i915/drm_fdinfo.c b/tests/i915/drm_fdinfo.c
>> index c0e0ba1905f1..5cafa0e469e2 100644
>> --- a/tests/i915/drm_fdinfo.c
>> +++ b/tests/i915/drm_fdinfo.c
>> @@ -138,68 +138,6 @@ static unsigned int measured_usleep(unsigned int usec)
>>   #define FLAG_HANG (8)
>>   #define TEST_ISOLATION (16)
>>   
>> -static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> -			       const struct intel_execution_engine2 *e)
>> -{
>> -	struct igt_spin_factory opts = {
>> -		.ahnd = ahnd,
>> -		.ctx = ctx,
>> -		.engine = e ? e->flags : 0,
>> -	};
>> -
>> -	if (!e || gem_class_can_store_dword(fd, e->class))
>> -		opts.flags |= IGT_SPIN_POLL_RUN;
>> -
>> -	return __igt_spin_factory(fd, &opts);
>> -}
>> -
>> -static unsigned long __spin_wait(int fd, igt_spin_t *spin)
>> -{
>> -	struct timespec start = { };
>> -
>> -	igt_nsec_elapsed(&start);
>> -
>> -	if (igt_spin_has_poll(spin)) {
>> -		unsigned long timeout = 0;
>> -
>> -		while (!igt_spin_has_started(spin)) {
>> -			unsigned long t = igt_nsec_elapsed(&start);
>> -
>> -			igt_assert(gem_bo_busy(fd, spin->handle));
>> -			if ((t - timeout) > 250e6) {
>> -				timeout = t;
>> -				igt_warn("Spinner not running after %.2fms\n",
>> -					 (double)t / 1e6);
>> -				igt_assert(t < 2e9);
>> -			}
>> -		}
>> -	} else {
>> -		igt_debug("__spin_wait - usleep mode\n");
>> -		usleep(500e3); /* Better than nothing! */
>> -	}
>> -
>> -	igt_assert(gem_bo_busy(fd, spin->handle));
>> -	return igt_nsec_elapsed(&start);
>> -}
>> -
>> -static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> -			       const struct intel_execution_engine2 *e)
>> -{
>> -	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
>> -
>> -	__spin_wait(fd, spin);
>> -
>> -	return spin;
>> -}
>> -
>> -static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> -			     const struct intel_execution_engine2 *e)
>> -{
>> -	igt_require_gem(fd);
>> -
>> -	return __spin_sync(fd, ahnd, ctx, e);
>> -}
>> -
>>   static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
>>   {
>>   	if (!spin)
>> @@ -264,7 +202,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
>>   	ahnd = get_reloc_ahnd(spin_fd, ctx->id);
>>   
>>   	if (flags & TEST_BUSY)
>> -		spin = spin_sync(spin_fd, ahnd, ctx, e);
>> +		spin = igt_sync_spin(spin_fd, ahnd, ctx, e);
>>   	else
>>   		spin = NULL;
>>   
>> @@ -349,7 +287,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   
>>   	memset(tval, 0, sizeof(tval));
>>   
>> -	spin = spin_sync(gem_fd, ahnd, ctx, e);
>> +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   
>>   	read_busy_all(gem_fd, tval[0]);
>>   	slept = measured_usleep(batch_duration_ns / 1000);
>> @@ -418,14 +356,14 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   			__submit_spin(gem_fd, spin, e_, 64);
>>   			busy_class[e_->class]++;
>>   		} else {
>> -			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
>> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
>>   			busy_class[e_->class]++;
>>   		}
>>   	}
>>   	igt_require(spin); /* at least one busy engine */
>>   
>>   	/* Small delay to allow engines to start. */
>> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
>> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>>   
>>   	read_busy_all(gem_fd, tval[0]);
>>   	slept = measured_usleep(batch_duration_ns / 1000);
>> @@ -475,12 +413,12 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   		if (spin)
>>   			__submit_spin(gem_fd, spin, e, 64);
>>   		else
>> -			spin = __spin_poll(gem_fd, ahnd, ctx, e);
>> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
>>   		busy_class[e->class]++;
>>   	}
>>   
>>   	/* Small delay to allow engines to start. */
>> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
>> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>>   
>>   	read_busy_all(gem_fd, tval[0]);
>>   	slept = measured_usleep(batch_duration_ns / 1000);
>> @@ -624,7 +562,7 @@ virtual(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
>>   			ahnd = get_reloc_ahnd(i915, ctx->id);
>>   
>>   			if (flags & TEST_BUSY)
>> -				spin = spin_sync(i915, ahnd, ctx, NULL);
>> +				spin = igt_sync_spin(i915, ahnd, ctx, NULL);
>>   			else
>>   				spin = NULL;
>>   
>> @@ -732,11 +670,12 @@ virtual_all(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
>>   			if (spin)
>>   				__virt_submit_spin(i915, spin, ctx[i], 64);
>>   			else
>> -				spin = __spin_poll(i915, ahnd, ctx[i], NULL);
>> +				spin = __igt_sync_spin_poll(i915, ahnd, ctx[i],
>> +							    NULL);
>>   		}
>>   
>>   		/* Small delay to allow engines to start. */
>> -		usleep(__spin_wait(i915, spin) * count / 1e3);
>> +		usleep(__igt_sync_spin_wait(i915, spin) * count / 1e3);
>>   
>>   		val = read_busy(i915, class);
>>   		slept = measured_usleep(batch_duration_ns / 1000);
>> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
>> index d889d67dcebd..6d4b8f7df909 100644
>> --- a/tests/i915/gem_eio.c
>> +++ b/tests/i915/gem_eio.c
>> @@ -47,6 +47,7 @@
>>   #include "i915/gem_ring.h"
>>   #include "igt.h"
>>   #include "igt_device.h"
>> +#include "igt_dummyload.h"
>>   #include "igt_fb.h"
>>   #include "igt_kms.h"
>>   #include "igt_stats.h"
>> @@ -429,22 +430,12 @@ static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>>   	return __igt_spin_factory(fd, &opts);
>>   }
>>   
>> -static void __spin_wait(int fd, igt_spin_t *spin)
>> -{
>> -	if (igt_spin_has_poll(spin)) {
>> -		igt_spin_busywait_until_started(spin);
>> -	} else {
>> -		igt_debug("__spin_wait - usleep mode\n");
>> -		usleep(500e3); /* Better than nothing! */
>> -	}
>> -}
>> -
>>   static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>>   			     unsigned long flags)
>>   {
>>   	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, flags);
>>   
>> -	__spin_wait(fd, spin);
>> +	__igt_sync_spin_wait(fd, spin);
>>   
>>   	return spin;
>>   }
>> @@ -963,7 +954,7 @@ static void test_inflight_external(int fd)
>>   	fence = execbuf.rsvd2 >> 32;
>>   	igt_assert(fence != -1);
>>   
>> -	__spin_wait(fd, hang);
>> +	__igt_sync_spin_wait(fd, hang);
>>   	manual_hang(fd);
>>   
>>   	gem_sync(fd, hang->handle); /* wedged, with an unready batch */
>> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
>> index 0806a8e545b5..a888027ad9af 100644
>> --- a/tests/i915/perf_pmu.c
>> +++ b/tests/i915/perf_pmu.c
>> @@ -377,68 +377,6 @@ static unsigned int measured_usleep(unsigned int usec)
>>   #define TEST_OTHER (128)
>>   #define TEST_ALL   (256)
>>   
>> -static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> -			       const struct intel_execution_engine2 *e)
>> -{
>> -	struct igt_spin_factory opts = {
>> -		.ahnd = ahnd,
>> -		.ctx = ctx,
>> -		.engine = e->flags,
>> -	};
>> -
>> -	if (gem_class_can_store_dword(fd, e->class))
>> -		opts.flags |= IGT_SPIN_POLL_RUN;
>> -
>> -	return __igt_spin_factory(fd, &opts);
>> -}
>> -
>> -static unsigned long __spin_wait(int fd, igt_spin_t *spin)
>> -{
>> -	struct timespec start = { };
>> -
>> -	igt_nsec_elapsed(&start);
>> -
>> -	if (igt_spin_has_poll(spin)) {
>> -		unsigned long timeout = 0;
>> -
>> -		while (!igt_spin_has_started(spin)) {
>> -			unsigned long t = igt_nsec_elapsed(&start);
>> -
>> -			igt_assert(gem_bo_busy(fd, spin->handle));
>> -			if ((t - timeout) > 250e6) {
>> -				timeout = t;
>> -				igt_warn("Spinner not running after %.2fms\n",
>> -					 (double)t / 1e6);
>> -				igt_assert(t < 2e9);
>> -			}
>> -		}
>> -	} else {
>> -		igt_debug("__spin_wait - usleep mode\n");
>> -		usleep(500e3); /* Better than nothing! */
>> -	}
>> -
>> -	igt_assert(gem_bo_busy(fd, spin->handle));
>> -	return igt_nsec_elapsed(&start);
>> -}
>> -
>> -static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> -			       const struct intel_execution_engine2 *e)
>> -{
>> -	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
>> -
>> -	__spin_wait(fd, spin);
>> -
>> -	return spin;
>> -}
>> -
>> -static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> -			     const struct intel_execution_engine2 *e)
>> -{
>> -	igt_require_gem(fd);
>> -
>> -	return __spin_sync(fd, ahnd, ctx, e);
>> -}
>> -
>>   static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
>>   {
>>   	if (!spin)
>> @@ -484,7 +422,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
>>   	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
>>   
>>   	if (flags & TEST_BUSY)
>> -		spin = spin_sync(gem_fd, ahnd, ctx, e);
>> +		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   	else
>>   		spin = NULL;
>>   
>> @@ -536,7 +474,7 @@ busy_start(int gem_fd, const intel_ctx_t *ctx,
>>   	 */
>>   	sleep(2);
>>   
>> -	spin = __spin_sync(gem_fd, ahnd, ctx, e);
>> +	spin = __igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   
>>   	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
>>   
>> @@ -583,7 +521,7 @@ busy_double_start(int gem_fd, const intel_ctx_t *ctx,
>>   	 * 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, ahnd, ctx, e);
>> +	spin[0] = __igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   	usleep(500e3);
>>   	spin[1] = __igt_spin_new(gem_fd,
>>   				 .ahnd = ahndN,
>> @@ -675,7 +613,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   
>>   	igt_assert_eq(i, num_engines);
>>   
>> -	spin = spin_sync(gem_fd, ahnd, ctx, e);
>> +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   	pmu_read_multi(fd[0], num_engines, tval[0]);
>>   	slept = measured_usleep(batch_duration_ns / 1000);
>>   	if (flags & TEST_TRAILING_IDLE)
>> @@ -737,7 +675,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   		else if (spin)
>>   			__submit_spin(gem_fd, spin, e_, 64);
>>   		else
>> -			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
>> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
>>   
>>   		val[i++] = I915_PMU_ENGINE_BUSY(e_->class, e_->instance);
>>   	}
>> @@ -749,7 +687,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   		fd[i] = open_group(gem_fd, val[i], fd[0]);
>>   
>>   	/* Small delay to allow engines to start. */
>> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
>> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>>   
>>   	pmu_read_multi(fd[0], num_engines, tval[0]);
>>   	slept = measured_usleep(batch_duration_ns / 1000);
>> @@ -796,7 +734,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   		if (spin)
>>   			__submit_spin(gem_fd, spin, e, 64);
>>   		else
>> -			spin = __spin_poll(gem_fd, ahnd, ctx, e);
>> +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
>>   
>>   		val[i++] = I915_PMU_ENGINE_BUSY(e->class, e->instance);
>>   	}
>> @@ -807,7 +745,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
>>   		fd[i] = open_group(gem_fd, val[i], fd[0]);
>>   
>>   	/* Small delay to allow engines to start. */
>> -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
>> +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
>>   
>>   	pmu_read_multi(fd[0], num_engines, tval[0]);
>>   	slept = measured_usleep(batch_duration_ns / 1000);
>> @@ -848,7 +786,7 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
>>   			   fd[0]);
>>   
>>   	if (flags & TEST_BUSY)
>> -		spin = spin_sync(gem_fd, ahnd, ctx, e);
>> +		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   	else
>>   		spin = NULL;
>>   
>> @@ -1406,7 +1344,7 @@ multi_client(int gem_fd, const intel_ctx_t *ctx,
>>   	 */
>>   	fd[1] = open_pmu(gem_fd, config);
>>   
>> -	spin = spin_sync(gem_fd, ahnd, ctx, e);
>> +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
>>   
>>   	val[0] = val[1] = __pmu_read_single(fd[0], &ts[0]);
>>   	slept[1] = measured_usleep(batch_duration_ns / 1000);
>> @@ -1776,7 +1714,7 @@ static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
>>   
>>   	igt_debug("Using engine %u:%u\n", e.class, e.instance);
>>   
>> -	return spin_sync(i915, ahnd, *ctx, &e);
>> +	return igt_sync_spin(i915, ahnd, *ctx, &e);
>>   }
>>   
>>   static void
>> -- 
>> 2.39.2
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds
  2023-07-14 17:26   ` [Intel-gfx] [igt-dev] " Rodrigo Vivi
@ 2023-07-17  8:37     ` Tvrtko Ursulin
  0 siblings, 0 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2023-07-17  8:37 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: igt-dev, Intel-gfx


On 14/07/2023 18:26, Rodrigo Vivi wrote:
> On Tue, Jul 11, 2023 at 05:02:14PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Exercise a bunch of up and down rps thresholds to verify hardware
>> is happy with them all.
>>
>> To limit the overall runtime relies on probability and number of runs
>> to approach complete coverage.
>>
>> v2:
>>   * Common sync spinner code now in library.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   tests/i915/i915_pm_rps.c | 194 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 194 insertions(+)
>>
>> diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c
>> index 7044fcd81c56..8c370b35c85b 100644
>> --- a/tests/i915/i915_pm_rps.c
>> +++ b/tests/i915/i915_pm_rps.c
>> @@ -39,8 +39,10 @@
>>   #include "i915/gem.h"
>>   #include "i915/gem_create.h"
>>   #include "igt.h"
>> +#include "igt_aux.h"
>>   #include "igt_dummyload.h"
>>   #include "igt_perf.h"
>> +#include "igt_rand.h"
>>   #include "igt_sysfs.h"
>>   /**
>>    * TEST: i915 pm rps
>> @@ -81,6 +83,22 @@
>>    * SUBTEST: waitboost
>>    * Feature: pm_rps
>>    * Run type: FULL
>> + *
>> + * SUBTEST: thresholds
>> + * Feature: pm_rps
>> + * Run type: FULL
>> + *
>> + * SUBTEST: thresholds-idle
>> + * Feature: pm_rps
>> + * Run type: FULL
>> + *
>> + * SUBTEST: thresholds-idle-park
>> + * Feature: pm_rps
>> + * Run type: FULL
>> + *
>> + * SUBTEST: thresholds-park
>> + * Feature: pm_rps
>> + * Run type: FULL
>>    */
>>   
>>   IGT_TEST_DESCRIPTION("Render P-States tests - verify GPU frequency changes");
>> @@ -920,6 +938,146 @@ static void pm_rps_exit_handler(int sig)
>>   	drm_close_driver(drm_fd);
>>   }
>>   
>> +static struct i915_engine_class_instance
>> +find_dword_engine(int i915, const unsigned int gt)
>> +{
>> +	struct i915_engine_class_instance *engines, ci = { -1, -1 };
>> +	unsigned int i, count;
>> +
>> +	engines = gem_list_engines(i915, 1u << gt, ~0u, &count);
>> +	igt_assert(engines);
>> +
>> +	for (i = 0; i < count; i++) {
>> +		if (!gem_class_can_store_dword(i915, engines[i].engine_class))
>> +			continue;
>> +
>> +		ci = engines[i];
>> +		break;
>> +	}
>> +
>> +	free(engines);
>> +
>> +	return ci;
>> +}
>> +
>> +static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
>> +				const intel_ctx_t **ctx)
>> +{
>> +	struct i915_engine_class_instance ci = { -1, -1 };
>> +	struct intel_execution_engine2 e = { };
>> +
>> +	ci = find_dword_engine(i915, gt);
>> +
>> +	igt_require(ci.engine_class != (uint16_t)I915_ENGINE_CLASS_INVALID);
>> +
>> +	if (gem_has_contexts(i915)) {
>> +		e.class = ci.engine_class;
>> +		e.instance = ci.engine_instance;
>> +		e.flags = 0;
>> +		*ctx = intel_ctx_create_for_engine(i915, e.class, e.instance);
>> +	} else {
>> +		igt_require(gt == 0); /* Impossible anyway. */
> 
> I'm confused by the comment here... if it is impossible why we have code below?!
> but why impossible?

Only the gt != 0 part would be impossible in this !gem_has_context 
branch, otherwise the branch runs on old platforms (which are always 
single tile). Perhaps a case of too many asserts adding confusion?

> 
> anyway, the tests below are great for the sysfs that you are adding. Thanks
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks!

Regards,

Tvrtko

> 
>> +		e.class = gem_execbuf_flags_to_engine_class(I915_EXEC_DEFAULT);
>> +		e.instance = 0;
>> +		e.flags = I915_EXEC_DEFAULT;
>> +		*ctx = intel_ctx_0(i915);
>> +	}
>> +
>> +	igt_debug("Using engine %u:%u\n", e.class, e.instance);
>> +
>> +	return __igt_sync_spin(i915, ahnd, *ctx, &e);
>> +}
>> +
>> +#define TEST_IDLE 0x1
>> +#define TEST_PARK 0x2
>> +static void test_thresholds(int i915, unsigned int gt, unsigned int flags)
>> +{
>> +	uint64_t ahnd = get_reloc_ahnd(i915, 0);
>> +	const unsigned int points = 10;
>> +	unsigned int def_up, def_down;
>> +	igt_spin_t *spin = NULL;
>> +	const intel_ctx_t *ctx;
>> +	unsigned int *ta, *tb;
>> +	unsigned int i;
>> +	int sysfs;
>> +
>> +	sysfs = igt_sysfs_gt_open(i915, gt);
>> +	igt_require(sysfs >= 0);
>> +
>> +	/* Feature test */
>> +	def_up = igt_sysfs_get_u32(sysfs, "rps_up_threshold_pct");
>> +	def_down = igt_sysfs_get_u32(sysfs, "rps_down_threshold_pct");
>> +	igt_require(def_up && def_down);
>> +
>> +	/* Check invalid percentages are rejected */
>> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", 101), false);
>> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", 101), false);
>> +
>> +	/*
>> +	 * Invent some random up-down thresholds, but always include 0 and 100
>> +	 * just to have some wild edge cases.
>> +	 */
>> +	ta = calloc(points, sizeof(unsigned int));
>> +	tb = calloc(points, sizeof(unsigned int));
>> +	igt_require(ta && tb);
>> +
>> +	ta[0] = tb[0] = 0;
>> +	ta[1] = tb[1] = 100;
>> +	hars_petruska_f54_1_random_seed(time(NULL));
>> +	for (i = 2; i < points; i++) {
>> +		ta[i] = hars_petruska_f54_1_random_unsafe_max(100);
>> +		tb[i] = hars_petruska_f54_1_random_unsafe_max(100);
>> +	}
>> +	igt_permute_array(ta, points, igt_exchange_int);
>> +	igt_permute_array(tb, points, igt_exchange_int);
>> +
>> +	/* Exercise the thresholds with a GPU load to trigger park/unpark etc */
>> +	for (i = 0; i < points; i++) {
>> +		igt_info("Testing thresholds up %u%% and down %u%%...\n", ta[i], tb[i]);
>> +		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", ta[i]), true);
>> +		igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", tb[i]), true);
>> +
>> +		if (flags & TEST_IDLE) {
>> +			gem_quiescent_gpu(i915);
>> +		} else if (spin) {
>> +			intel_ctx_destroy(i915, ctx);
>> +			igt_spin_free(i915, spin);
>> +			spin = NULL;
>> +			if (flags & TEST_PARK) {
>> +				gem_quiescent_gpu(i915);
>> +				usleep(500000);
>> +			}
>> +		}
>> +		spin = spin_sync_gt(i915, ahnd, gt, &ctx);
>> +		usleep(1000000);
>> +		if (flags & TEST_IDLE) {
>> +			intel_ctx_destroy(i915, ctx);
>> +			igt_spin_free(i915, spin);
>> +			if (flags & TEST_PARK) {
>> +				gem_quiescent_gpu(i915);
>> +				usleep(500000);
>> +			}
>> +			spin = NULL;
>> +		}
>> +	}
>> +
>> +	if (spin) {
>> +		intel_ctx_destroy(i915, ctx);
>> +		igt_spin_free(i915, spin);
>> +	}
>> +
>> +	gem_quiescent_gpu(i915);
>> +
>> +	/* Restore defaults */
>> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_up_threshold_pct", def_up), true);
>> +	igt_assert_eq(igt_sysfs_set_u32(sysfs, "rps_down_threshold_pct", def_down), true);
>> +
>> +	free(ta);
>> +	free(tb);
>> +	close(sysfs);
>> +	put_ahnd(ahnd);
>> +}
>> +
>>   igt_main
>>   {
>>   	igt_fixture {
>> @@ -1000,6 +1158,42 @@ igt_main
>>   		igt_disallow_hang(drm_fd, hang);
>>   	}
>>   
>> +	igt_subtest_with_dynamic("thresholds-idle") {
>> +		int tmp, gt;
>> +
>> +		i915_for_each_gt(drm_fd, tmp, gt) {
>> +			igt_dynamic_f("gt%u", gt)
>> +				test_thresholds(drm_fd, gt, TEST_IDLE);
>> +		}
>> +	}
>> +
>> +	igt_subtest_with_dynamic("thresholds") {
>> +		int tmp, gt;
>> +
>> +		i915_for_each_gt(drm_fd, tmp, gt) {
>> +			igt_dynamic_f("gt%u", gt)
>> +				test_thresholds(drm_fd, gt, 0);
>> +		}
>> +	}
>> +
>> +	igt_subtest_with_dynamic("thresholds-park") {
>> +		int tmp, gt;
>> +
>> +		i915_for_each_gt(drm_fd, tmp, gt) {
>> +			igt_dynamic_f("gt%u", gt)
>> +				test_thresholds(drm_fd, gt, TEST_PARK);
>> +		}
>> +	}
>> +
>> +	igt_subtest_with_dynamic("thresholds-idle-park") {
>> +		int tmp, gt;
>> +
>> +		i915_for_each_gt(drm_fd, tmp, gt) {
>> +			igt_dynamic_f("gt%u", gt)
>> +				test_thresholds(drm_fd, gt, TEST_IDLE | TEST_PARK);
>> +		}
>> +	}
>> +
>>   	igt_fixture
>>   		drm_close_driver(drm_fd);
>>   }
>> -- 
>> 2.39.2
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API
  2023-07-17  8:34   ` Tvrtko Ursulin
@ 2023-07-17 14:55     ` Rodrigo Vivi
  0 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2023-07-17 14:55 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx

On Mon, Jul 17, 2023 at 09:34:36AM +0100, Tvrtko Ursulin wrote:
> 
> On 14/07/2023 18:22, Rodrigo Vivi wrote:
> > On Tue, Jul 11, 2023 at 05:02:13PM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > 
> > > Sync spinner API is identical and compatible with regular spinners just
> > > that it tries to make sure spinner is actually running on the hardware
> > > before returning from the constructor.
> > > 
> > > A few tests already use it, one more will, so lets promote it into
> > > common library.
> > > 
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > ---
> > >   lib/igt_dummyload.c     | 105 ++++++++++++++++++++++++++++++++++++++++
> > >   lib/igt_dummyload.h     |  11 +++++
> > >   tests/i915/drm_fdinfo.c |  81 ++++---------------------------
> > >   tests/i915/gem_eio.c    |  15 ++----
> > >   tests/i915/perf_pmu.c   |  84 +++++---------------------------
> > >   5 files changed, 140 insertions(+), 156 deletions(-)
> > > 
> > > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> > > index 9f941cef73e6..d3cee91540b6 100644
> > > --- a/lib/igt_dummyload.c
> > > +++ b/lib/igt_dummyload.c
> > 
> > why here?
> > 
> > > @@ -33,6 +33,7 @@
> > >   #include "drmtest.h"
> > >   #include "i915/gem_create.h"
> > >   #include "i915/gem_engine_topology.h"
> > > +#include "i915/gem.h"
> > >   #include "i915/gem_mman.h"
> > >   #include "i915/gem_submission.h"
> > >   #include "igt_aux.h"
> > > @@ -715,6 +716,110 @@ void igt_unshare_spins(void)
> > >   	IGT_INIT_LIST_HEAD(&spin_list);
> > >   }
> > > +/**
> > > + * __igt_sync_spin_poll:
> > > + * @i915: open i915 drm file descriptor
> > 
> > anyway to make this not i915 centric?
> > or maybe move it to or start a lib that is i915 only?
> > 
> > I know that we have many more lib things that are still i915 centric,
> > but at some point we need to start organizing it...
> 
> Is igt_dummyload i915/xe agnostic already? I missed that. It would be a big
> ask for me to get on top of two uapis and do that.. ugh. :(

no, but I also asked above why igt_dummyload to start with?

> 
> So on the "why here?" part. Assuming taking the i915 route.. where would you
> suggest to put it?

maybe something new inside ./lib/i915 ?

But, well, I actually don't have a better suggestion here, so let's
not block and any clean-up on i915 vs xe will need to come later anyway.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Regards,
> 
> Tvrtko
> 
> > > + * @ahnd: allocator handle
> > > + * @ctx: intel_ctx_t context
> > > + * @e: engine to execute on
> > > + *
> > > + * Starts a recursive batch on an engine.
> > > + *
> > > + * Returns a #igt_spin_t which can be used with both standard and igt_sync_spin
> > > + * API family. Callers should consult @gem_class_can_store_dword to whether
> > > + * the target platform+engine can reliably support the igt_sync_spin API.
> > > + */
> > > +igt_spin_t *
> > > +__igt_sync_spin_poll(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> > > +		     const struct intel_execution_engine2 *e)
> > > +{
> > > +	struct igt_spin_factory opts = {
> > > +		.ahnd = ahnd,
> > > +		.ctx = ctx,
> > > +		.engine = e ? e->flags : 0,
> > > +	};
> > > +
> > > +	if (!e || gem_class_can_store_dword(i915, e->class))
> > > +		opts.flags |= IGT_SPIN_POLL_RUN;
> > > +
> > > +	return __igt_spin_factory(i915, &opts);
> > > +}
> > > +
> > > +/**
> > > + * __igt_sync_spin_wait:
> > > + * @i915: open i915 drm file descriptor
> > > + * @spin: previously create sync spinner
> > > + *
> > > + * Waits for a spinner to be running on the hardware.
> > > + *
> > > + * Callers should consult @gem_class_can_store_dword to whether the target
> > > + * platform+engine can reliably support the igt_sync_spin API.
> > > + */
> > > +unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin)
> > > +{
> > > +	struct timespec start = { };
> > > +
> > > +	igt_nsec_elapsed(&start);
> > > +
> > > +	if (igt_spin_has_poll(spin)) {
> > > +		unsigned long timeout = 0;
> > > +
> > > +		while (!igt_spin_has_started(spin)) {
> > > +			unsigned long t = igt_nsec_elapsed(&start);
> > > +
> > > +			igt_assert(gem_bo_busy(i915, spin->handle));
> > > +			if ((t - timeout) > 250e6) {
> > > +				timeout = t;
> > > +				igt_warn("Spinner not running after %.2fms\n",
> > > +					 (double)t / 1e6);
> > > +				igt_assert(t < 2e9);
> > > +			}
> > > +		}
> > > +	} else {
> > > +		igt_debug("__spin_wait - usleep mode\n");
> > > +		usleep(500e3); /* Better than nothing! */
> > > +	}
> > > +
> > > +	igt_assert(gem_bo_busy(i915, spin->handle));
> > > +	return igt_nsec_elapsed(&start);
> > > +}
> > > +
> > > +igt_spin_t *
> > > +__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> > > +		const struct intel_execution_engine2 *e)
> > > +{
> > > +	igt_spin_t *spin = __igt_sync_spin_poll(i915, ahnd, ctx, e);
> > > +
> > > +	__igt_sync_spin_wait(i915, spin);
> > > +
> > > +	return spin;
> > > +}
> > > +
> > > +/**
> > > + * igt_sync_spin:
> > > + * @i915: open i915 drm file descriptor
> > > + * @ahnd: allocator handle
> > > + * @ctx: intel_ctx_t context
> > > + * @e: engine to execute on
> > > + *
> > > + * Starts a recursive batch on an engine.
> > > + *
> > > + * Returns a #igt_spin_t and tries to guarantee it to be running at the time of
> > > + * the return. Otherwise it does a best effort only. Callers should check for
> > > + * @gem_class_can_store_dword if they want to be sure guarantee can be given.
> > > + *
> > > + * Both standard and igt_sync_spin API family can be used on the returned
> > > + * spinner object.
> > > + */
> > > +igt_spin_t *
> > > +igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> > > +	      const struct intel_execution_engine2 *e)
> > > +{
> > > +	igt_require_gem(i915);
> > > +
> > > +	return __igt_sync_spin(i915, ahnd, ctx, e);
> > > +}
> > > +
> > >   static uint32_t plug_vgem_handle(struct igt_cork *cork, int fd)
> > >   {
> > >   	struct vgem_bo bo;
> > > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> > > index 6eb3f2e696bb..b771011af74f 100644
> > > --- a/lib/igt_dummyload.h
> > > +++ b/lib/igt_dummyload.h
> > > @@ -143,6 +143,17 @@ void igt_terminate_spins(void);
> > >   void igt_unshare_spins(void);
> > >   void igt_free_spins(int i915);
> > > +struct intel_execution_engine2;
> > > +
> > > +igt_spin_t *__igt_sync_spin_poll(int i915, uint64_t ahnd,
> > > +				 const intel_ctx_t *ctx,
> > > +				 const struct intel_execution_engine2 *e);
> > > +unsigned long __igt_sync_spin_wait(int i915, igt_spin_t *spin);
> > > +igt_spin_t *__igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> > > +			    const struct intel_execution_engine2 *e);
> > > +igt_spin_t *igt_sync_spin(int i915, uint64_t ahnd, const intel_ctx_t *ctx,
> > > +			  const struct intel_execution_engine2 *e);
> > > +
> > >   enum igt_cork_type {
> > >   	CORK_SYNC_FD = 1,
> > >   	CORK_VGEM_HANDLE
> > > diff --git a/tests/i915/drm_fdinfo.c b/tests/i915/drm_fdinfo.c
> > > index c0e0ba1905f1..5cafa0e469e2 100644
> > > --- a/tests/i915/drm_fdinfo.c
> > > +++ b/tests/i915/drm_fdinfo.c
> > > @@ -138,68 +138,6 @@ static unsigned int measured_usleep(unsigned int usec)
> > >   #define FLAG_HANG (8)
> > >   #define TEST_ISOLATION (16)
> > > -static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > > -			       const struct intel_execution_engine2 *e)
> > > -{
> > > -	struct igt_spin_factory opts = {
> > > -		.ahnd = ahnd,
> > > -		.ctx = ctx,
> > > -		.engine = e ? e->flags : 0,
> > > -	};
> > > -
> > > -	if (!e || gem_class_can_store_dword(fd, e->class))
> > > -		opts.flags |= IGT_SPIN_POLL_RUN;
> > > -
> > > -	return __igt_spin_factory(fd, &opts);
> > > -}
> > > -
> > > -static unsigned long __spin_wait(int fd, igt_spin_t *spin)
> > > -{
> > > -	struct timespec start = { };
> > > -
> > > -	igt_nsec_elapsed(&start);
> > > -
> > > -	if (igt_spin_has_poll(spin)) {
> > > -		unsigned long timeout = 0;
> > > -
> > > -		while (!igt_spin_has_started(spin)) {
> > > -			unsigned long t = igt_nsec_elapsed(&start);
> > > -
> > > -			igt_assert(gem_bo_busy(fd, spin->handle));
> > > -			if ((t - timeout) > 250e6) {
> > > -				timeout = t;
> > > -				igt_warn("Spinner not running after %.2fms\n",
> > > -					 (double)t / 1e6);
> > > -				igt_assert(t < 2e9);
> > > -			}
> > > -		}
> > > -	} else {
> > > -		igt_debug("__spin_wait - usleep mode\n");
> > > -		usleep(500e3); /* Better than nothing! */
> > > -	}
> > > -
> > > -	igt_assert(gem_bo_busy(fd, spin->handle));
> > > -	return igt_nsec_elapsed(&start);
> > > -}
> > > -
> > > -static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > > -			       const struct intel_execution_engine2 *e)
> > > -{
> > > -	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
> > > -
> > > -	__spin_wait(fd, spin);
> > > -
> > > -	return spin;
> > > -}
> > > -
> > > -static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > > -			     const struct intel_execution_engine2 *e)
> > > -{
> > > -	igt_require_gem(fd);
> > > -
> > > -	return __spin_sync(fd, ahnd, ctx, e);
> > > -}
> > > -
> > >   static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
> > >   {
> > >   	if (!spin)
> > > @@ -264,7 +202,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
> > >   	ahnd = get_reloc_ahnd(spin_fd, ctx->id);
> > >   	if (flags & TEST_BUSY)
> > > -		spin = spin_sync(spin_fd, ahnd, ctx, e);
> > > +		spin = igt_sync_spin(spin_fd, ahnd, ctx, e);
> > >   	else
> > >   		spin = NULL;
> > > @@ -349,7 +287,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   	memset(tval, 0, sizeof(tval));
> > > -	spin = spin_sync(gem_fd, ahnd, ctx, e);
> > > +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	read_busy_all(gem_fd, tval[0]);
> > >   	slept = measured_usleep(batch_duration_ns / 1000);
> > > @@ -418,14 +356,14 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   			__submit_spin(gem_fd, spin, e_, 64);
> > >   			busy_class[e_->class]++;
> > >   		} else {
> > > -			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
> > > +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
> > >   			busy_class[e_->class]++;
> > >   		}
> > >   	}
> > >   	igt_require(spin); /* at least one busy engine */
> > >   	/* Small delay to allow engines to start. */
> > > -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> > > +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
> > >   	read_busy_all(gem_fd, tval[0]);
> > >   	slept = measured_usleep(batch_duration_ns / 1000);
> > > @@ -475,12 +413,12 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   		if (spin)
> > >   			__submit_spin(gem_fd, spin, e, 64);
> > >   		else
> > > -			spin = __spin_poll(gem_fd, ahnd, ctx, e);
> > > +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
> > >   		busy_class[e->class]++;
> > >   	}
> > >   	/* Small delay to allow engines to start. */
> > > -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> > > +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
> > >   	read_busy_all(gem_fd, tval[0]);
> > >   	slept = measured_usleep(batch_duration_ns / 1000);
> > > @@ -624,7 +562,7 @@ virtual(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
> > >   			ahnd = get_reloc_ahnd(i915, ctx->id);
> > >   			if (flags & TEST_BUSY)
> > > -				spin = spin_sync(i915, ahnd, ctx, NULL);
> > > +				spin = igt_sync_spin(i915, ahnd, ctx, NULL);
> > >   			else
> > >   				spin = NULL;
> > > @@ -732,11 +670,12 @@ virtual_all(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
> > >   			if (spin)
> > >   				__virt_submit_spin(i915, spin, ctx[i], 64);
> > >   			else
> > > -				spin = __spin_poll(i915, ahnd, ctx[i], NULL);
> > > +				spin = __igt_sync_spin_poll(i915, ahnd, ctx[i],
> > > +							    NULL);
> > >   		}
> > >   		/* Small delay to allow engines to start. */
> > > -		usleep(__spin_wait(i915, spin) * count / 1e3);
> > > +		usleep(__igt_sync_spin_wait(i915, spin) * count / 1e3);
> > >   		val = read_busy(i915, class);
> > >   		slept = measured_usleep(batch_duration_ns / 1000);
> > > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> > > index d889d67dcebd..6d4b8f7df909 100644
> > > --- a/tests/i915/gem_eio.c
> > > +++ b/tests/i915/gem_eio.c
> > > @@ -47,6 +47,7 @@
> > >   #include "i915/gem_ring.h"
> > >   #include "igt.h"
> > >   #include "igt_device.h"
> > > +#include "igt_dummyload.h"
> > >   #include "igt_fb.h"
> > >   #include "igt_kms.h"
> > >   #include "igt_stats.h"
> > > @@ -429,22 +430,12 @@ static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > >   	return __igt_spin_factory(fd, &opts);
> > >   }
> > > -static void __spin_wait(int fd, igt_spin_t *spin)
> > > -{
> > > -	if (igt_spin_has_poll(spin)) {
> > > -		igt_spin_busywait_until_started(spin);
> > > -	} else {
> > > -		igt_debug("__spin_wait - usleep mode\n");
> > > -		usleep(500e3); /* Better than nothing! */
> > > -	}
> > > -}
> > > -
> > >   static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > >   			     unsigned long flags)
> > >   {
> > >   	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, flags);
> > > -	__spin_wait(fd, spin);
> > > +	__igt_sync_spin_wait(fd, spin);
> > >   	return spin;
> > >   }
> > > @@ -963,7 +954,7 @@ static void test_inflight_external(int fd)
> > >   	fence = execbuf.rsvd2 >> 32;
> > >   	igt_assert(fence != -1);
> > > -	__spin_wait(fd, hang);
> > > +	__igt_sync_spin_wait(fd, hang);
> > >   	manual_hang(fd);
> > >   	gem_sync(fd, hang->handle); /* wedged, with an unready batch */
> > > diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> > > index 0806a8e545b5..a888027ad9af 100644
> > > --- a/tests/i915/perf_pmu.c
> > > +++ b/tests/i915/perf_pmu.c
> > > @@ -377,68 +377,6 @@ static unsigned int measured_usleep(unsigned int usec)
> > >   #define TEST_OTHER (128)
> > >   #define TEST_ALL   (256)
> > > -static igt_spin_t *__spin_poll(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > > -			       const struct intel_execution_engine2 *e)
> > > -{
> > > -	struct igt_spin_factory opts = {
> > > -		.ahnd = ahnd,
> > > -		.ctx = ctx,
> > > -		.engine = e->flags,
> > > -	};
> > > -
> > > -	if (gem_class_can_store_dword(fd, e->class))
> > > -		opts.flags |= IGT_SPIN_POLL_RUN;
> > > -
> > > -	return __igt_spin_factory(fd, &opts);
> > > -}
> > > -
> > > -static unsigned long __spin_wait(int fd, igt_spin_t *spin)
> > > -{
> > > -	struct timespec start = { };
> > > -
> > > -	igt_nsec_elapsed(&start);
> > > -
> > > -	if (igt_spin_has_poll(spin)) {
> > > -		unsigned long timeout = 0;
> > > -
> > > -		while (!igt_spin_has_started(spin)) {
> > > -			unsigned long t = igt_nsec_elapsed(&start);
> > > -
> > > -			igt_assert(gem_bo_busy(fd, spin->handle));
> > > -			if ((t - timeout) > 250e6) {
> > > -				timeout = t;
> > > -				igt_warn("Spinner not running after %.2fms\n",
> > > -					 (double)t / 1e6);
> > > -				igt_assert(t < 2e9);
> > > -			}
> > > -		}
> > > -	} else {
> > > -		igt_debug("__spin_wait - usleep mode\n");
> > > -		usleep(500e3); /* Better than nothing! */
> > > -	}
> > > -
> > > -	igt_assert(gem_bo_busy(fd, spin->handle));
> > > -	return igt_nsec_elapsed(&start);
> > > -}
> > > -
> > > -static igt_spin_t *__spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > > -			       const struct intel_execution_engine2 *e)
> > > -{
> > > -	igt_spin_t *spin = __spin_poll(fd, ahnd, ctx, e);
> > > -
> > > -	__spin_wait(fd, spin);
> > > -
> > > -	return spin;
> > > -}
> > > -
> > > -static igt_spin_t *spin_sync(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
> > > -			     const struct intel_execution_engine2 *e)
> > > -{
> > > -	igt_require_gem(fd);
> > > -
> > > -	return __spin_sync(fd, ahnd, ctx, e);
> > > -}
> > > -
> > >   static void end_spin(int fd, igt_spin_t *spin, unsigned int flags)
> > >   {
> > >   	if (!spin)
> > > @@ -484,7 +422,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
> > >   	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
> > >   	if (flags & TEST_BUSY)
> > > -		spin = spin_sync(gem_fd, ahnd, ctx, e);
> > > +		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	else
> > >   		spin = NULL;
> > > @@ -536,7 +474,7 @@ busy_start(int gem_fd, const intel_ctx_t *ctx,
> > >   	 */
> > >   	sleep(2);
> > > -	spin = __spin_sync(gem_fd, ahnd, ctx, e);
> > > +	spin = __igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	fd = open_pmu(gem_fd, I915_PMU_ENGINE_BUSY(e->class, e->instance));
> > > @@ -583,7 +521,7 @@ busy_double_start(int gem_fd, const intel_ctx_t *ctx,
> > >   	 * 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, ahnd, ctx, e);
> > > +	spin[0] = __igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	usleep(500e3);
> > >   	spin[1] = __igt_spin_new(gem_fd,
> > >   				 .ahnd = ahndN,
> > > @@ -675,7 +613,7 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   	igt_assert_eq(i, num_engines);
> > > -	spin = spin_sync(gem_fd, ahnd, ctx, e);
> > > +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	pmu_read_multi(fd[0], num_engines, tval[0]);
> > >   	slept = measured_usleep(batch_duration_ns / 1000);
> > >   	if (flags & TEST_TRAILING_IDLE)
> > > @@ -737,7 +675,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   		else if (spin)
> > >   			__submit_spin(gem_fd, spin, e_, 64);
> > >   		else
> > > -			spin = __spin_poll(gem_fd, ahnd, ctx, e_);
> > > +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e_);
> > >   		val[i++] = I915_PMU_ENGINE_BUSY(e_->class, e_->instance);
> > >   	}
> > > @@ -749,7 +687,7 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   		fd[i] = open_group(gem_fd, val[i], fd[0]);
> > >   	/* Small delay to allow engines to start. */
> > > -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> > > +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
> > >   	pmu_read_multi(fd[0], num_engines, tval[0]);
> > >   	slept = measured_usleep(batch_duration_ns / 1000);
> > > @@ -796,7 +734,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   		if (spin)
> > >   			__submit_spin(gem_fd, spin, e, 64);
> > >   		else
> > > -			spin = __spin_poll(gem_fd, ahnd, ctx, e);
> > > +			spin = __igt_sync_spin_poll(gem_fd, ahnd, ctx, e);
> > >   		val[i++] = I915_PMU_ENGINE_BUSY(e->class, e->instance);
> > >   	}
> > > @@ -807,7 +745,7 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> > >   		fd[i] = open_group(gem_fd, val[i], fd[0]);
> > >   	/* Small delay to allow engines to start. */
> > > -	usleep(__spin_wait(gem_fd, spin) * num_engines / 1e3);
> > > +	usleep(__igt_sync_spin_wait(gem_fd, spin) * num_engines / 1e3);
> > >   	pmu_read_multi(fd[0], num_engines, tval[0]);
> > >   	slept = measured_usleep(batch_duration_ns / 1000);
> > > @@ -848,7 +786,7 @@ no_sema(int gem_fd, const intel_ctx_t *ctx,
> > >   			   fd[0]);
> > >   	if (flags & TEST_BUSY)
> > > -		spin = spin_sync(gem_fd, ahnd, ctx, e);
> > > +		spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	else
> > >   		spin = NULL;
> > > @@ -1406,7 +1344,7 @@ multi_client(int gem_fd, const intel_ctx_t *ctx,
> > >   	 */
> > >   	fd[1] = open_pmu(gem_fd, config);
> > > -	spin = spin_sync(gem_fd, ahnd, ctx, e);
> > > +	spin = igt_sync_spin(gem_fd, ahnd, ctx, e);
> > >   	val[0] = val[1] = __pmu_read_single(fd[0], &ts[0]);
> > >   	slept[1] = measured_usleep(batch_duration_ns / 1000);
> > > @@ -1776,7 +1714,7 @@ static igt_spin_t *spin_sync_gt(int i915, uint64_t ahnd, unsigned int gt,
> > >   	igt_debug("Using engine %u:%u\n", e.class, e.instance);
> > > -	return spin_sync(i915, ahnd, *ctx, &e);
> > > +	return igt_sync_spin(i915, ahnd, *ctx, &e);
> > >   }
> > >   static void
> > > -- 
> > > 2.39.2
> > > 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-07-17 14:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-11 16:02 [Intel-gfx] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Tvrtko Ursulin
2023-07-11 16:02 ` [Intel-gfx] [PATCH i-g-t 2/2] tests/i915_pm_rps: Exercise sysfs thresholds Tvrtko Ursulin
2023-07-14 17:26   ` [Intel-gfx] [igt-dev] " Rodrigo Vivi
2023-07-17  8:37     ` Tvrtko Ursulin
2023-07-14 17:22 ` [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] lib/igt_dummyload: Extract sync spinner API Rodrigo Vivi
2023-07-17  8:34   ` Tvrtko Ursulin
2023-07-17 14:55     ` Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).