[igt-dev] [PATCH i-g-t 1/2] lib/dummyload: Allow spin batches to be restarted

[igt-dev] [PATCH i-g-t 1/2] lib/dummyload: Allow spin batches to be restarted

[Tvrtko Ursulin]

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

This adds the igt_spin_batch_restart API so same spin batch can be
re-used in tests which care about low setup cost.

Batch will be re-submited in the manner completely identical to as
it was originally submitted.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_dummyload.c | 150 +++++++++++++++++++++++++++++++++-------------------
 lib/igt_dummyload.h |   9 ++++
 2 files changed, 106 insertions(+), 53 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 27eb402bb699..102efd197e5b 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -71,6 +71,35 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
 	reloc->write_domain = write_domains;
 }
 
+static int __igt_spin_batch_submit(int fd, igt_spin_t *spin)
+{
+	int fence_fd = -1;
+	unsigned int i;
+
+	for (i = 0; i < spin->nengine; i++) {
+		spin->execbuf.flags &= ~ENGINE_MASK;
+		spin->execbuf.flags |= spin->engines[i];
+		gem_execbuf_wr(fd, &spin->execbuf);
+		if (spin->out_fence_requested) {
+			int _fd = spin->execbuf.rsvd2 >> 32;
+
+			igt_assert(_fd >= 0);
+			if (fence_fd == -1) {
+				fence_fd = _fd;
+			} else {
+				int old_fd = fence_fd;
+
+				fence_fd = sync_fence_merge(old_fd, _fd);
+				close(old_fd);
+				close(_fd);
+			}
+			igt_assert(fence_fd >= 0);
+		}
+	}
+
+	return fence_fd;
+}
+
 static int emit_recursive_batch(igt_spin_t *spin,
 				int fd, uint32_t ctx, unsigned engine,
 				uint32_t dep, bool out_fence)
@@ -78,52 +107,46 @@ static int emit_recursive_batch(igt_spin_t *spin,
 #define SCRATCH 0
 #define BATCH 1
 	const int gen = intel_gen(intel_get_drm_devid(fd));
-	struct drm_i915_gem_exec_object2 obj[2];
-	struct drm_i915_gem_relocation_entry relocs[2];
-	struct drm_i915_gem_execbuffer2 execbuf;
-	unsigned int engines[16];
-	unsigned int nengine;
-	int fence_fd = -1;
 	uint32_t *batch;
-	int i;
 
-	nengine = 0;
+	spin->nengine = 0;
 	if (engine == -1) {
 		for_each_engine(fd, engine)
 			if (engine)
-				engines[nengine++] = engine;
+				spin->engines[spin->nengine++] = engine;
 	} else {
 		gem_require_ring(fd, engine);
-		engines[nengine++] = engine;
+		spin->engines[spin->nengine++] = engine;
 	}
-	igt_require(nengine);
+	igt_require(spin->nengine);
 
-	memset(&execbuf, 0, sizeof(execbuf));
-	memset(obj, 0, sizeof(obj));
-	memset(relocs, 0, sizeof(relocs));
+	memset(&spin->execbuf, 0, sizeof(spin->execbuf));
+	memset(spin->obj, 0, sizeof(spin->obj));
+	memset(spin->relocs, 0, sizeof(spin->relocs));
 
-	obj[BATCH].handle = gem_create(fd, BATCH_SIZE);
-	batch = __gem_mmap__wc(fd, obj[BATCH].handle,
+	spin->obj[BATCH].handle = gem_create(fd, BATCH_SIZE);
+	batch = __gem_mmap__wc(fd, spin->obj[BATCH].handle,
 			       0, BATCH_SIZE, PROT_WRITE);
 	if (!batch)
-		batch = __gem_mmap__gtt(fd, obj[BATCH].handle,
+		batch = __gem_mmap__gtt(fd, spin->obj[BATCH].handle,
 				       	BATCH_SIZE, PROT_WRITE);
-	gem_set_domain(fd, obj[BATCH].handle,
+	gem_set_domain(fd, spin->obj[BATCH].handle,
 			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-	execbuf.buffer_count++;
+	spin->execbuf.buffer_count++;
 
 	if (dep) {
 		/* dummy write to dependency */
-		obj[SCRATCH].handle = dep;
-		fill_reloc(&relocs[obj[BATCH].relocation_count++],
+		spin->obj[SCRATCH].handle = dep;
+		fill_reloc(&spin->relocs[spin->obj[BATCH].relocation_count++],
 			   dep, 1020,
 			   I915_GEM_DOMAIN_RENDER,
 			   I915_GEM_DOMAIN_RENDER);
-		execbuf.buffer_count++;
+		spin->execbuf.buffer_count++;
 	}
 
 	spin->batch = batch;
-	spin->handle = obj[BATCH].handle;
+	spin->handle = spin->obj[BATCH].handle;
+	spin->out_fence_requested = out_fence;
 
 	/* Allow ourselves to be preempted */
 	*batch++ = MI_ARB_CHK;
@@ -142,8 +165,8 @@ static int emit_recursive_batch(igt_spin_t *spin,
 	batch += 1000;
 
 	/* recurse */
-	fill_reloc(&relocs[obj[BATCH].relocation_count],
-		   obj[BATCH].handle, (batch - spin->batch) + 1,
+	fill_reloc(&spin->relocs[spin->obj[BATCH].relocation_count],
+		   spin->obj[BATCH].handle, (batch - spin->batch) + 1,
 		   I915_GEM_DOMAIN_COMMAND, 0);
 	if (gen >= 8) {
 		*batch++ = MI_BATCH_BUFFER_START | 1 << 8 | 1;
@@ -157,41 +180,21 @@ static int emit_recursive_batch(igt_spin_t *spin,
 		*batch = 0;
 		if (gen < 4) {
 			*batch |= 1;
-			relocs[obj[BATCH].relocation_count].delta = 1;
+			spin->relocs[spin->obj[BATCH].relocation_count].delta = 1;
 		}
 		batch++;
 	}
-	obj[BATCH].relocation_count++;
-	obj[BATCH].relocs_ptr = to_user_pointer(relocs);
+	spin->obj[BATCH].relocation_count++;
+	spin->obj[BATCH].relocs_ptr = to_user_pointer(spin->relocs);
 
-	execbuf.buffers_ptr = to_user_pointer(obj + (2 - execbuf.buffer_count));
-	execbuf.rsvd1 = ctx;
+	spin->execbuf.buffers_ptr =
+		to_user_pointer(spin->obj + (2 - spin->execbuf.buffer_count));
+	spin->execbuf.rsvd1 = ctx;
 
 	if (out_fence)
-		execbuf.flags |= I915_EXEC_FENCE_OUT;
-
-	for (i = 0; i < nengine; i++) {
-		execbuf.flags &= ~ENGINE_MASK;
-		execbuf.flags |= engines[i];
-		gem_execbuf_wr(fd, &execbuf);
-		if (out_fence) {
-			int _fd = execbuf.rsvd2 >> 32;
-
-			igt_assert(_fd >= 0);
-			if (fence_fd == -1) {
-				fence_fd = _fd;
-			} else {
-				int old_fd = fence_fd;
-
-				fence_fd = sync_fence_merge(old_fd, _fd);
-				close(old_fd);
-				close(_fd);
-			}
-			igt_assert(fence_fd >= 0);
-		}
-	}
+		spin->execbuf.flags |= I915_EXEC_FENCE_OUT;
 
-	return fence_fd;
+	return __igt_spin_batch_submit(fd, spin);
 }
 
 static igt_spin_t *
@@ -273,6 +276,47 @@ igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
 	return __igt_spin_batch_new_fence(fd, ctx, engine);
 }
 
+/**
+ * __igt_spin_batch_restart:
+ * @fd: open i915 drm file descriptor
+ * @spin: spin batch state from igt_spin_batch_new()
+ *
+ * Restarts the spin batch which was previously ended either explicitly
+ * or via timeout.
+ *
+ * This version does not verify that the batch is currently idle.
+ *
+ * Returns:
+ * New fence fd if spin batch was originaly created as requesting the
+ * output fence.
+ */
+int __igt_spin_batch_restart(int fd, igt_spin_t *spin)
+{
+	*spin->batch = MI_ARB_CHK;
+	__sync_synchronize();
+
+	return __igt_spin_batch_submit(fd, spin);
+}
+
+/**
+ * igt_spin_batch_restart:
+ * @fd: open i915 drm file descriptor
+ * @spin: spin batch state from igt_spin_batch_new()
+ *
+ * Restarts the spin batch which was previously ended either explicitly
+ * or via timeout.
+ *
+ * Returns:
+ * New fence fd if spin batch was originaly created as requesting the
+ * output fence.
+ */
+int igt_spin_batch_restart(int fd, igt_spin_t *spin)
+{
+	igt_assert(!gem_bo_busy(fd, spin->handle));
+
+	return __igt_spin_batch_restart(fd, spin);
+}
+
 static void notify(union sigval arg)
 {
 	igt_spin_t *spin = arg.sival_ptr;
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index ffa7e351dea3..b9f201d4afb6 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -36,6 +36,12 @@ typedef struct igt_spin {
 	struct igt_list link;
 	uint32_t *batch;
 	int out_fence;
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry relocs[2];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	unsigned int engines[16];
+	unsigned int nengine;
+	bool out_fence_requested;
 } igt_spin_t;
 
 igt_spin_t *__igt_spin_batch_new(int fd,
@@ -55,6 +61,9 @@ igt_spin_t *igt_spin_batch_new_fence(int fd,
 				     uint32_t ctx,
 				     unsigned engine);
 
+int __igt_spin_batch_restart(int fd, igt_spin_t *spin);
+int igt_spin_batch_restart(int fd, igt_spin_t *spin);
+
 void igt_spin_batch_set_timeout(igt_spin_t *spin, int64_t ns);
 void igt_spin_batch_end(igt_spin_t *spin);
 void igt_spin_batch_free(int fd, igt_spin_t *spin);
-- 
2.14.1

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

[igt-dev] [PATCH i-g-t 2/2] tests/perf_pmu: Verify engine busyness accuracy

[Tvrtko Ursulin]

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

A subtest to verify that the engine busyness is reported with expected
accuracy on platforms where the feature is available.

We test three patterns: 2%, 50% and 98% load per engine.

v2:
 * Use spin batch instead of nop calibration.
 * Various tweaks.

v3:
 * Change loops to be time based.
 * Use __igt_spin_batch_new inside timing sensitive loops.
 * Fixed PWM sleep handling.

v4:
 * Use restarting spin batch.
 * Calibrate more carefully by looking at the real PWM loop.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/perf_pmu.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 167 insertions(+)

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 2f7d33414a53..ffba750ef3a5 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -35,6 +35,7 @@
 #include <dirent.h>
 #include <time.h>
 #include <poll.h>
+#include <sched.h>
 
 #include "igt.h"
 #include "igt_core.h"
@@ -1169,6 +1170,162 @@ test_rc6(int gem_fd)
 	assert_within_epsilon(busy - prev, 0.0, tolerance);
 }
 
+static uint64_t __pmu_read_single(int fd, uint64_t *ts)
+{
+	uint64_t data[2];
+
+	igt_assert_eq(read(fd, data, sizeof(data)), sizeof(data));
+
+	*ts = data[1];
+
+	return data[0];
+}
+
+static double __error(double val, double ref)
+{
+	igt_assert(ref != 0.0);
+	return (100.0 * val / ref) - 100.0;
+}
+
+static void debug_error(const char *str, double val, double ref)
+{
+	igt_debug("%s=%.2f%% (%.2f/%.2f)\n", str, __error(val, ref), val, ref);
+}
+
+static void log_error(const char *str, double val, double ref)
+{
+	debug_error(str, val, ref);
+	igt_info("%s=%.2f%%\n", str, __error(val, ref));
+}
+
+#define div_round_up(a, b) (((a) + (b) - 1) / (b))
+
+static void
+accuracy(int gem_fd, const struct intel_execution_engine2 *e,
+	 unsigned long target_busy_pct)
+{
+	const unsigned int min_test_loops = 7;
+	const unsigned long min_test_us = 1e6;
+	unsigned long busy_us = 2500;
+	unsigned long idle_us = 100 * (busy_us - target_busy_pct *
+				busy_us / 100) / target_busy_pct;
+	unsigned long pwm_calibration_us;
+	unsigned long test_us;
+	double busy_r;
+	uint64_t val[2];
+	uint64_t ts[2];
+	int fd;
+
+	/* Sampling platforms cannot reach the high accuracy criteria. */
+	igt_require(intel_gen(intel_get_drm_devid(gem_fd)) >= 8);
+
+	while (idle_us < 2500) {
+		busy_us *= 2;
+		idle_us *= 2;
+	}
+
+	pwm_calibration_us = min_test_loops * (busy_us + idle_us);
+	while (pwm_calibration_us < min_test_us)
+		pwm_calibration_us += busy_us + idle_us;
+	test_us = min_test_loops * (idle_us + busy_us);
+	while (test_us < min_test_us)
+		test_us += busy_us + idle_us;
+
+	igt_info("calibration=%luus, test=%luus; busy=%luus, idle=%luus\n",
+		 pwm_calibration_us, test_us, busy_us, idle_us);
+
+	assert_within_epsilon((double)busy_us / (busy_us + idle_us),
+				(double)target_busy_pct / 100.0, tolerance);
+
+	/* Emit PWM pattern on the engine from a child. */
+	igt_fork(child, 1) {
+		struct sched_param rt = { .sched_priority = 99 };
+		const unsigned long timeout[] = { pwm_calibration_us * 1000,
+						  test_us * 2 * 1000 };
+		unsigned long sleep_busy = busy_us;
+		unsigned long sleep_idle = idle_us;
+		igt_spin_t *spin;
+
+		/* We need the best sleep accuracy we can get. */
+		igt_require(sched_setscheduler(0,
+					       SCHED_FIFO | SCHED_RESET_ON_FORK,
+					       &rt) == 0);
+
+		/* Allocate our spin batch and idle it. */
+		spin = igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
+		igt_spin_batch_end(spin);
+		gem_sync(gem_fd, spin->handle);
+
+		/* 1st pass is calibration, second pass is the test. */
+		for (int pass = 0; pass < ARRAY_SIZE(timeout); pass++) {
+			unsigned long busy_ns = 0, idle_ns = 0;
+			struct timespec test_start = { };
+			unsigned long loops = 0;
+			double err_busy, err_idle;
+			long get_time_d;
+
+			igt_nsec_elapsed(&test_start);
+			do {
+				struct timespec t_busy = { };
+
+				igt_nsec_elapsed(&t_busy);
+				__igt_spin_batch_restart(gem_fd, spin);
+				measured_usleep(sleep_busy);
+				igt_spin_batch_end(spin);
+				gem_sync(gem_fd, spin->handle);
+				busy_ns += igt_nsec_elapsed(&t_busy);
+
+				idle_ns += measured_usleep(sleep_idle);
+
+				loops++;
+			} while (igt_nsec_elapsed(&test_start) < timeout[pass]);
+
+			/* igt_nsec_elapsed overhead from measured_usleep. */
+			get_time_d = idle_ns - loops * sleep_idle * 1000;
+			busy_ns -= get_time_d;
+			busy_ns = div_round_up(busy_ns, loops);
+			idle_ns = div_round_up(idle_ns, loops);
+
+			err_busy = __error(busy_ns / 1000, busy_us);
+			err_idle = __error(idle_ns / 1000, idle_us);
+
+			igt_info("%u: busy %lu/%lu %.2f%%, idle %lu/%lu %.2f%%\n",
+				 pass,
+				 busy_ns / 1000, busy_us, err_busy,
+				 idle_ns / 1000, idle_us, err_idle);
+
+			if (pass == 0) {
+				sleep_busy = (double)busy_us -
+					     (double)busy_us * err_busy / 100.0;
+				sleep_idle = (double)idle_us -
+					     (double)idle_us * err_idle / 100.0;
+				igt_info("calibrated sleeps: busy=%lu, idle=%lu\n",
+					 sleep_busy, sleep_idle);
+			}
+		}
+
+		igt_spin_batch_free(gem_fd, spin);
+	}
+
+	/* Let the child run. */
+	usleep(pwm_calibration_us * 2);
+
+	/* Collect engine busyness for an interesting part of child runtime. */
+	fd = open_pmu(I915_PMU_ENGINE_BUSY(e->class, e->instance));
+	val[0] = __pmu_read_single(fd, &ts[0]);
+	usleep(test_us / 2);
+	val[1] = __pmu_read_single(fd, &ts[1]);
+	close(fd);
+
+	igt_waitchildren();
+
+	busy_r = (double)(val[1] - val[0]) / (ts[1] - ts[0]);
+
+	log_error("error", busy_r, target_busy_pct / 100.0);
+
+	assert_within_epsilon(busy_r, (double)target_busy_pct / 100.0, 0.15);
+}
+
 igt_main
 {
 	const unsigned int num_other_metrics =
@@ -1197,6 +1354,8 @@ igt_main
 		invalid_init();
 
 	for_each_engine_class_instance(fd, e) {
+		const unsigned int pct[] = { 2, 50, 98 };
+
 		/**
 		 * Test that a single engine metric can be initialized.
 		 */
@@ -1277,6 +1436,14 @@ igt_main
 		 */
 		igt_subtest_f("busy-double-start-%s", e->name)
 			busy_double_start(fd, e);
+
+		/**
+		 * Check engine busyness accuracy is as expected.
+		 */
+		for (i = 0; i < ARRAY_SIZE(pct); i++) {
+			igt_subtest_f("busy-accuracy-%u-%s", pct[i], e->name)
+				accuracy(fd, e, pct[i]);
+		}
 	}
 
 	/**
-- 
2.14.1

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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted
URL   : https://patchwork.freedesktop.org/series/37406/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
3f820260ce660cdff7fb803237c57554a29498c0 tests/kms_atomic_transition: Don't abuse the HSKEW flag to force a modeset

with latest DRM-Tip kernel build CI_DRM_3707
badc0398a060 drm-tip: 2018y-01m-31d-12h-17m-12s UTC integration manifest

Testlist changes:
+igt@perf_pmu@busy-accuracy-2-bcs0
+igt@perf_pmu@busy-accuracy-2-rcs0
+igt@perf_pmu@busy-accuracy-2-vcs0
+igt@perf_pmu@busy-accuracy-2-vcs1
+igt@perf_pmu@busy-accuracy-2-vecs0
+igt@perf_pmu@busy-accuracy-50-bcs0
+igt@perf_pmu@busy-accuracy-50-rcs0
+igt@perf_pmu@busy-accuracy-50-vcs0
+igt@perf_pmu@busy-accuracy-50-vcs1
+igt@perf_pmu@busy-accuracy-50-vecs0
+igt@perf_pmu@busy-accuracy-98-bcs0
+igt@perf_pmu@busy-accuracy-98-rcs0
+igt@perf_pmu@busy-accuracy-98-vcs0
+igt@perf_pmu@busy-accuracy-98-vcs1
+igt@perf_pmu@busy-accuracy-98-vecs0

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> FAIL       (fi-elk-e7500) fdo#103989 +1

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:420s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:425s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:378s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:489s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:481s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:487s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:470s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:456s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:566s
fi-elk-e7500     total:224  pass:167  dwarn:10  dfail:0   fail:1   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:280s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:521s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:391s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:399s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:414s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:457s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:412s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:459s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:494s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:503s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:579s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:429s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:525s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:487s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:417s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:398s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:471s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_841/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/2] lib/dummyload: Allow spin batches to be restarted

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-01-31 12:34:40)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This adds the igt_spin_batch_restart API so same spin batch can be
> re-used in tests which care about low setup cost.
> 
> Batch will be re-submited in the manner completely identical to as
> it was originally submitted.

Heh, I would have just marked the objects as EXEC_OBJECT_PINNED after
execution and dropped the relocs :)

> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/igt_dummyload.c | 150 +++++++++++++++++++++++++++++++++-------------------
>  lib/igt_dummyload.h |   9 ++++
>  2 files changed, 106 insertions(+), 53 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 27eb402bb699..102efd197e5b 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -71,6 +71,35 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
>         reloc->write_domain = write_domains;
>  }
>  
> +static int __igt_spin_batch_submit(int fd, igt_spin_t *spin)
> +{
> +       int fence_fd = -1;
> +       unsigned int i;
> +
> +       for (i = 0; i < spin->nengine; i++) {
> +               spin->execbuf.flags &= ~ENGINE_MASK;
> +               spin->execbuf.flags |= spin->engines[i];
> +               gem_execbuf_wr(fd, &spin->execbuf);
> +               if (spin->out_fence_requested) {
> +                       int _fd = spin->execbuf.rsvd2 >> 32;
> +
> +                       igt_assert(_fd >= 0);
> +                       if (fence_fd == -1) {
> +                               fence_fd = _fd;
> +                       } else {
> +                               int old_fd = fence_fd;
> +
> +                               fence_fd = sync_fence_merge(old_fd, _fd);
> +                               close(old_fd);
> +                               close(_fd);
> +                       }
> +                       igt_assert(fence_fd >= 0);
> +               }
> +       }
> +
> +       return fence_fd;
> +}
> +
>  static int emit_recursive_batch(igt_spin_t *spin,
>                                 int fd, uint32_t ctx, unsigned engine,
>                                 uint32_t dep, bool out_fence)
> @@ -78,52 +107,46 @@ static int emit_recursive_batch(igt_spin_t *spin,
>  #define SCRATCH 0
>  #define BATCH 1
>         const int gen = intel_gen(intel_get_drm_devid(fd));
> -       struct drm_i915_gem_exec_object2 obj[2];
> -       struct drm_i915_gem_relocation_entry relocs[2];
> -       struct drm_i915_gem_execbuffer2 execbuf;
> -       unsigned int engines[16];
> -       unsigned int nengine;
> -       int fence_fd = -1;
>         uint32_t *batch;
> -       int i;
>  
> -       nengine = 0;
> +       spin->nengine = 0;
>         if (engine == -1) {
>                 for_each_engine(fd, engine)
>                         if (engine)
> -                               engines[nengine++] = engine;
> +                               spin->engines[spin->nengine++] = engine;
>         } else {
>                 gem_require_ring(fd, engine);
> -               engines[nengine++] = engine;
> +               spin->engines[spin->nengine++] = engine;
>         }
> -       igt_require(nengine);
> +       igt_require(spin->nengine);
>  
> -       memset(&execbuf, 0, sizeof(execbuf));
> -       memset(obj, 0, sizeof(obj));
> -       memset(relocs, 0, sizeof(relocs));
> +       memset(&spin->execbuf, 0, sizeof(spin->execbuf));
> +       memset(spin->obj, 0, sizeof(spin->obj));
> +       memset(spin->relocs, 0, sizeof(spin->relocs));
>  
> -       obj[BATCH].handle = gem_create(fd, BATCH_SIZE);
> -       batch = __gem_mmap__wc(fd, obj[BATCH].handle,
> +       spin->obj[BATCH].handle = gem_create(fd, BATCH_SIZE);
> +       batch = __gem_mmap__wc(fd, spin->obj[BATCH].handle,
>                                0, BATCH_SIZE, PROT_WRITE);
>         if (!batch)
> -               batch = __gem_mmap__gtt(fd, obj[BATCH].handle,
> +               batch = __gem_mmap__gtt(fd, spin->obj[BATCH].handle,
>                                         BATCH_SIZE, PROT_WRITE);
> -       gem_set_domain(fd, obj[BATCH].handle,
> +       gem_set_domain(fd, spin->obj[BATCH].handle,
>                         I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> -       execbuf.buffer_count++;
> +       spin->execbuf.buffer_count++;
>  
>         if (dep) {
>                 /* dummy write to dependency */
> -               obj[SCRATCH].handle = dep;
> -               fill_reloc(&relocs[obj[BATCH].relocation_count++],
> +               spin->obj[SCRATCH].handle = dep;
> +               fill_reloc(&spin->relocs[spin->obj[BATCH].relocation_count++],
>                            dep, 1020,
>                            I915_GEM_DOMAIN_RENDER,
>                            I915_GEM_DOMAIN_RENDER);
> -               execbuf.buffer_count++;
> +               spin->execbuf.buffer_count++;
>         }
>  
>         spin->batch = batch;
> -       spin->handle = obj[BATCH].handle;
> +       spin->handle = spin->obj[BATCH].handle;
> +       spin->out_fence_requested = out_fence;
>  
>         /* Allow ourselves to be preempted */
>         *batch++ = MI_ARB_CHK;
> @@ -142,8 +165,8 @@ static int emit_recursive_batch(igt_spin_t *spin,
>         batch += 1000;
>  
>         /* recurse */
> -       fill_reloc(&relocs[obj[BATCH].relocation_count],
> -                  obj[BATCH].handle, (batch - spin->batch) + 1,
> +       fill_reloc(&spin->relocs[spin->obj[BATCH].relocation_count],
> +                  spin->obj[BATCH].handle, (batch - spin->batch) + 1,
>                    I915_GEM_DOMAIN_COMMAND, 0);
>         if (gen >= 8) {
>                 *batch++ = MI_BATCH_BUFFER_START | 1 << 8 | 1;
> @@ -157,41 +180,21 @@ static int emit_recursive_batch(igt_spin_t *spin,
>                 *batch = 0;
>                 if (gen < 4) {
>                         *batch |= 1;
> -                       relocs[obj[BATCH].relocation_count].delta = 1;
> +                       spin->relocs[spin->obj[BATCH].relocation_count].delta = 1;
>                 }
>                 batch++;
>         }
> -       obj[BATCH].relocation_count++;
> -       obj[BATCH].relocs_ptr = to_user_pointer(relocs);
> +       spin->obj[BATCH].relocation_count++;
> +       spin->obj[BATCH].relocs_ptr = to_user_pointer(spin->relocs);
>  
> -       execbuf.buffers_ptr = to_user_pointer(obj + (2 - execbuf.buffer_count));
> -       execbuf.rsvd1 = ctx;
> +       spin->execbuf.buffers_ptr =
> +               to_user_pointer(spin->obj + (2 - spin->execbuf.buffer_count));
> +       spin->execbuf.rsvd1 = ctx;
>  
>         if (out_fence)
> -               execbuf.flags |= I915_EXEC_FENCE_OUT;
> -
> -       for (i = 0; i < nengine; i++) {
> -               execbuf.flags &= ~ENGINE_MASK;
> -               execbuf.flags |= engines[i];
> -               gem_execbuf_wr(fd, &execbuf);
> -               if (out_fence) {
> -                       int _fd = execbuf.rsvd2 >> 32;
> -
> -                       igt_assert(_fd >= 0);
> -                       if (fence_fd == -1) {
> -                               fence_fd = _fd;
> -                       } else {
> -                               int old_fd = fence_fd;
> -
> -                               fence_fd = sync_fence_merge(old_fd, _fd);
> -                               close(old_fd);
> -                               close(_fd);
> -                       }
> -                       igt_assert(fence_fd >= 0);
> -               }
> -       }
> +               spin->execbuf.flags |= I915_EXEC_FENCE_OUT;
>  
> -       return fence_fd;
> +       return __igt_spin_batch_submit(fd, spin);
>  }
>  
>  static igt_spin_t *
> @@ -273,6 +276,47 @@ igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
>         return __igt_spin_batch_new_fence(fd, ctx, engine);
>  }
>  
> +/**
> + * __igt_spin_batch_restart:
> + * @fd: open i915 drm file descriptor
> + * @spin: spin batch state from igt_spin_batch_new()
> + *
> + * Restarts the spin batch which was previously ended either explicitly
> + * or via timeout.
> + *
> + * This version does not verify that the batch is currently idle.
> + *
> + * Returns:
> + * New fence fd if spin batch was originaly created as requesting the
> + * output fence.
> + */
> +int __igt_spin_batch_restart(int fd, igt_spin_t *spin)
> +{
> +       *spin->batch = MI_ARB_CHK;
> +       __sync_synchronize();

Hmm, I would maybe assert(*spin->batch == MI_BBE), but the mfence here
is a part of the following submit (i.e. as this batch is idle, and can
only be to work reliably, we don't need an explicit mfence as that is
handled for us by submit.)

> +
> +       return __igt_spin_batch_submit(fd, spin);
> +}
> +
> +/**
> + * igt_spin_batch_restart:
> + * @fd: open i915 drm file descriptor
> + * @spin: spin batch state from igt_spin_batch_new()
> + *
> + * Restarts the spin batch which was previously ended either explicitly
> + * or via timeout.
> + *
> + * Returns:
> + * New fence fd if spin batch was originaly created as requesting the
> + * output fence.
> + */
> +int igt_spin_batch_restart(int fd, igt_spin_t *spin)
> +{
> +       igt_assert(!gem_bo_busy(fd, spin->handle));
> +
> +       return __igt_spin_batch_restart(fd, spin);
> +}
> +
>  static void notify(union sigval arg)
>  {
>         igt_spin_t *spin = arg.sival_ptr;
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index ffa7e351dea3..b9f201d4afb6 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -36,6 +36,12 @@ typedef struct igt_spin {
>         struct igt_list link;
>         uint32_t *batch;
>         int out_fence;
> +       struct drm_i915_gem_exec_object2 obj[2];
> +       struct drm_i915_gem_relocation_entry relocs[2];
> +       struct drm_i915_gem_execbuffer2 execbuf;

I'm dubious whether we want to emit the dependency obj on resubmit. I
can see where it may be desired, and where it may be a hindrance.

I think I'm coming down on the side that to reemit the dependency is
surprising, and would argue that if that is desired it should be an
explicit parameter to restart().
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/2] lib/dummyload: Allow spin batches to be restarted

[Chris Wilson]

Quoting Chris Wilson (2018-01-31 15:44:41)
> Quoting Tvrtko Ursulin (2018-01-31 12:34:40)
> > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> > index ffa7e351dea3..b9f201d4afb6 100644
> > --- a/lib/igt_dummyload.h
> > +++ b/lib/igt_dummyload.h
> > @@ -36,6 +36,12 @@ typedef struct igt_spin {
> >         struct igt_list link;
> >         uint32_t *batch;
> >         int out_fence;
> > +       struct drm_i915_gem_exec_object2 obj[2];
> > +       struct drm_i915_gem_relocation_entry relocs[2];
> > +       struct drm_i915_gem_execbuffer2 execbuf;
> 
> I'm dubious whether we want to emit the dependency obj on resubmit. I
> can see where it may be desired, and where it may be a hindrance.
> 
> I think I'm coming down on the side that to reemit the dependency is
> surprising, and would argue that if that is desired it should be an
> explicit parameter to restart().

Stronger hint that it is surprising to remit the dependency obj is that
it is owned by the caller, who may not realise it has been captured by
the spin batch and will call gem_close() on it.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted
URL   : https://patchwork.freedesktop.org/series/37406/
State : failure

== Summary ==

Test perf_pmu:
        Subgroup semaphore-wait-bcs0:
                fail       -> PASS       (shard-apl) fdo#104829
        Subgroup busy-check-all-vcs0:
                pass       -> FAIL       (shard-apl)
Test drv_suspend:
        Subgroup forcewake:
                pass       -> SKIP       (shard-snb)
Test kms_flip:
        Subgroup plain-flip-fb-recreate:
                pass       -> FAIL       (shard-hsw) fdo#100368
        Subgroup blocking-wf_vblank:
                pass       -> FAIL       (shard-hsw) fdo#103928
Test kms_flip_tiling:
        Subgroup flip-changes-tiling-y:
                pass       -> FAIL       (shard-apl) fdo#103822

fdo#104829 https://bugs.freedesktop.org/show_bug.cgi?id=104829
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822

shard-apl        total:2853 pass:1763 dwarn:1   dfail:0   fail:22  skip:1067 time:12801s
shard-hsw        total:2853 pass:1734 dwarn:1   dfail:0   fail:12  skip:1105 time:11954s
shard-snb        total:2853 pass:1329 dwarn:1   dfail:0   fail:10  skip:1513 time:6568s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_841/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/2] lib/dummyload: Allow spin batches to be restarted

[Tvrtko Ursulin]

On 31/01/2018 15:49, Chris Wilson wrote:
> Quoting Chris Wilson (2018-01-31 15:44:41)
>> Quoting Tvrtko Ursulin (2018-01-31 12:34:40)
>>> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
>>> index ffa7e351dea3..b9f201d4afb6 100644
>>> --- a/lib/igt_dummyload.h
>>> +++ b/lib/igt_dummyload.h
>>> @@ -36,6 +36,12 @@ typedef struct igt_spin {
>>>          struct igt_list link;
>>>          uint32_t *batch;
>>>          int out_fence;
>>> +       struct drm_i915_gem_exec_object2 obj[2];
>>> +       struct drm_i915_gem_relocation_entry relocs[2];
>>> +       struct drm_i915_gem_execbuffer2 execbuf;
>>
>> I'm dubious whether we want to emit the dependency obj on resubmit. I
>> can see where it may be desired, and where it may be a hindrance.
>>
>> I think I'm coming down on the side that to reemit the dependency is
>> surprising, and would argue that if that is desired it should be an
>> explicit parameter to restart().
> 
> Stronger hint that it is surprising to remit the dependency obj is that
> it is owned by the caller, who may not realise it has been captured by
> the spin batch and will call gem_close() on it.

Hm, I kind of cannot bring myself to care, or at least see it as a 
problem. My thinking is, that if userspace uses restarts and nukes the 
dependency object in the meantime, it will get to know its mistake soon 
enough.

In other words, the only way in which the dependency has been "captured" 
is if caller explicitly uses the API.

So to me it still sounds logical to repeat the exactly same submission 
as was the original one.

But ultimately I care much more about getting this test done over API 
details so can change this if you insist.

Regards,

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

Re: [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted

[Tvrtko Ursulin]

On 31/01/2018 16:47, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted
> URL   : https://patchwork.freedesktop.org/series/37406/
> State : failure
> 
> == Summary ==
> 
> Test perf_pmu:
>          Subgroup semaphore-wait-bcs0:
>                  fail       -> PASS       (shard-apl) fdo#104829
>          Subgroup busy-check-all-vcs0:
>                  pass       -> FAIL       (shard-apl)

Bah this needs a revisit.

But the accuracy tests have all passes on APL!

To bad KBL is AWOL(?!) so I cannot marvel at the PWM accuracy on a big 
core. :)

Regards,

Tvrtko

> Test drv_suspend:
>          Subgroup forcewake:
>                  pass       -> SKIP       (shard-snb)
> Test kms_flip:
>          Subgroup plain-flip-fb-recreate:
>                  pass       -> FAIL       (shard-hsw) fdo#100368
>          Subgroup blocking-wf_vblank:
>                  pass       -> FAIL       (shard-hsw) fdo#103928
> Test kms_flip_tiling:
>          Subgroup flip-changes-tiling-y:
>                  pass       -> FAIL       (shard-apl) fdo#103822
> 
> fdo#104829 https://bugs.freedesktop.org/show_bug.cgi?id=104829
> fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
> fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
> fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
> 
> shard-apl        total:2853 pass:1763 dwarn:1   dfail:0   fail:22  skip:1067 time:12801s
> shard-hsw        total:2853 pass:1734 dwarn:1   dfail:0   fail:12  skip:1105 time:11954s
> shard-snb        total:2853 pass:1329 dwarn:1   dfail:0   fail:10  skip:1513 time:6568s
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_841/shards.html
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-01-31 17:26:24)
> 
> On 31/01/2018 16:47, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [i-g-t,1/2] lib/dummyload: Allow spin batches to be restarted
> > URL   : https://patchwork.freedesktop.org/series/37406/
> > State : failure
> > 
> > == Summary ==
> > 
> > Test perf_pmu:
> >          Subgroup semaphore-wait-bcs0:
> >                  fail       -> PASS       (shard-apl) fdo#104829
> >          Subgroup busy-check-all-vcs0:
> >                  pass       -> FAIL       (shard-apl)
> 
> Bah this needs a revisit.

Throw in the towel and run as RT? But take a look through them and see
if you can spot where we lose track of time. :|
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev