[igt-dev] [PATCH i-g-t 1/8] lib: Report file cache as available system memory

[igt-dev] [PATCH i-g-t 1/8] lib: Report file cache as available system memory

[Chris Wilson]

sysinfo() doesn't include all reclaimable memory. In particular it
excludes the majority of global_node_page_state(NR_FILE_PAGES),
reclaimable pages that are a copy of on-disk files It seems the only way
to obtain this counter is by parsing /proc/meminfo. For comparison,
check vm_enough_memory() which includes NR_FILE_PAGES as available
(sadly there's no way to call vm_enough_memory() directly either!)

v2: Pay attention to what one writes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/intel_os.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/lib/intel_os.c b/lib/intel_os.c
index 885ffdcec..5bd18fc52 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -84,6 +84,18 @@ intel_get_total_ram_mb(void)
 	return retval / (1024*1024);
 }
 
+static uint64_t get_meminfo(const char *info, const char *tag)
+{
+	const char *str;
+	unsigned long val;
+
+	str = strstr(info, tag);
+	if (str && sscanf(str + strlen(tag), " %lu", &val) == 1)
+		return (uint64_t)val << 10;
+
+	return 0;
+}
+
 /**
  * intel_get_avail_ram_mb:
  *
@@ -96,17 +108,31 @@ intel_get_avail_ram_mb(void)
 	uint64_t retval;
 
 #ifdef HAVE_STRUCT_SYSINFO_TOTALRAM /* Linux */
-	struct sysinfo sysinf;
+	char *info;
 	int fd;
 
 	fd = drm_open_driver(DRIVER_INTEL);
 	intel_purge_vm_caches(fd);
 	close(fd);
 
-	igt_assert(sysinfo(&sysinf) == 0);
-	retval = sysinf.freeram;
-	retval += min(sysinf.freeswap, sysinf.bufferram);
-	retval *= sysinf.mem_unit;
+	fd = open("/proc", O_RDONLY);
+	info = igt_sysfs_get(fd, "meminfo");
+	close(fd);
+
+	if (info) {
+		retval  = get_meminfo(info, "MemAvailable: ");
+		retval += get_meminfo(info, "Buffers: ");
+		retval += get_meminfo(info, "Cached: ");
+		retval += get_meminfo(info, "SwapCached: ");
+		free(info);
+	} else {
+		struct sysinfo sysinf;
+
+		igt_assert(sysinfo(&sysinf) == 0);
+		retval = sysinf.freeram;
+		retval += min(sysinf.freeswap, sysinf.bufferram);
+		retval *= sysinf.mem_unit;
+	}
 #elif defined(_SC_PAGESIZE) && defined(_SC_AVPHYS_PAGES) /* Solaris */
 	long pagesize, npages;
 
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 2/8] igt/gem_tiled_partial_pwrite_pread: Check for known swizzling

[Chris Wilson]

As we want to compare a templated tiling pattern against the target_bo,
we need to know that the swizzling is compatible. Or else the two
tiling pattern may differ due to underlying page address that we cannot
know, and so the test may sporadically fail.

References: https://bugs.freedesktop.org/show_bug.cgi?id=102575
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c
index fe573c37c..83c57c07d 100644
--- a/tests/gem_tiled_partial_pwrite_pread.c
+++ b/tests/gem_tiled_partial_pwrite_pread.c
@@ -249,6 +249,24 @@ static void test_partial_read_writes(void)
 	}
 }
 
+static bool known_swizzling(uint32_t handle)
+{
+	struct drm_i915_gem_get_tiling2 {
+		uint32_t handle;
+		uint32_t tiling_mode;
+		uint32_t swizzle_mode;
+		uint32_t phys_swizzle_mode;
+	} arg = {
+		.handle = handle,
+	};
+#define DRM_IOCTL_I915_GEM_GET_TILING2	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct drm_i915_gem_get_tiling2)
+
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg))
+		return false;
+
+	return arg.phys_swizzle_mode == arg.swizzle_mode;
+}
+
 igt_main
 {
 	uint32_t tiling_mode = I915_TILING_X;
@@ -271,6 +289,12 @@ igt_main
 						      &tiling_mode, &scratch_pitch, 0);
 		igt_assert(tiling_mode == I915_TILING_X);
 		igt_assert(scratch_pitch == 4096);
+
+		/*
+		 * As we want to compare our template tiled pattern against
+		 * the target bo, we need consistent swizzling on both.
+		 */
+		igt_require(known_swizzling(scratch_bo->handle));
 		staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096);
 		tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024,
 							    BO_SIZE/4096, 4,
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 3/8] igt/gem_set_tiling_vs_pwrite: Show the erroneous value

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_set_tiling_vs_pwrite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/gem_set_tiling_vs_pwrite.c b/tests/gem_set_tiling_vs_pwrite.c
index 006edfe4e..f0126b648 100644
--- a/tests/gem_set_tiling_vs_pwrite.c
+++ b/tests/gem_set_tiling_vs_pwrite.c
@@ -75,7 +75,7 @@ igt_simple_main
 	memset(data, 0, OBJECT_SIZE);
 	gem_read(fd, handle, 0, data, OBJECT_SIZE);
 	for (i = 0; i < OBJECT_SIZE/4; i++)
-		igt_assert(i == data[i]);
+		igt_assert_eq_u32(data[i], i);
 
 	/* touch it before changing the tiling, so that the fence sticks around */
 	gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
@@ -88,7 +88,7 @@ igt_simple_main
 	memset(data, 0, OBJECT_SIZE);
 	gem_read(fd, handle, 0, data, OBJECT_SIZE);
 	for (i = 0; i < OBJECT_SIZE/4; i++)
-		igt_assert(i == data[i]);
+		igt_assert_eq_u32(data[i], i);
 
 	munmap(ptr, OBJECT_SIZE);
 
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 4/8] lib: Convert spin batch constructor to a factory

[Chris Wilson]

In order to make adding more options easier, expose the full set of
options to the caller.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_dummyload.c            | 147 +++++++++------------------------
 lib/igt_dummyload.h            |  42 +++++-----
 tests/drv_missed_irq.c         |   2 +-
 tests/gem_busy.c               |  17 ++--
 tests/gem_ctx_isolation.c      |  26 +++---
 tests/gem_eio.c                |  13 ++-
 tests/gem_exec_fence.c         |  16 ++--
 tests/gem_exec_latency.c       |  18 +++-
 tests/gem_exec_nop.c           |   4 +-
 tests/gem_exec_reloc.c         |  10 ++-
 tests/gem_exec_schedule.c      |  27 ++++--
 tests/gem_exec_suspend.c       |   2 +-
 tests/gem_fenced_exec_thrash.c |   2 +-
 tests/gem_shrink.c             |   4 +-
 tests/gem_spin_batch.c         |   4 +-
 tests/gem_sync.c               |   5 +-
 tests/gem_wait.c               |   4 +-
 tests/kms_busy.c               |  10 ++-
 tests/kms_cursor_legacy.c      |   7 +-
 tests/perf_pmu.c               |  33 +++++---
 tests/pm_rps.c                 |   9 +-
 21 files changed, 189 insertions(+), 213 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 3809b4e61..94efdf745 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -75,12 +75,9 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
 	reloc->write_domain = write_domains;
 }
 
-#define OUT_FENCE	(1 << 0)
-#define POLL_RUN	(1 << 1)
-
 static int
-emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine,
-		     uint32_t dep, unsigned int flags)
+emit_recursive_batch(igt_spin_t *spin,
+		     int fd, const struct igt_spin_factory *opts)
 {
 #define SCRATCH 0
 #define BATCH 1
@@ -95,21 +92,18 @@ emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine,
 	int i;
 
 	nengine = 0;
-	if (engine == ALL_ENGINES) {
-		for_each_engine(fd, engine) {
-			if (engine) {
-			if (flags & POLL_RUN)
-				igt_require(!(flags & POLL_RUN) ||
-					    gem_can_store_dword(fd, engine));
-
-				engines[nengine++] = engine;
-			}
+	if (opts->engine == ALL_ENGINES) {
+		unsigned int engine;
+
+		for_each_physical_engine(fd, engine) {
+			if (opts->flags & IGT_SPIN_POLL_RUN &&
+			    !gem_can_store_dword(fd, engine))
+				continue;
+
+			engines[nengine++] = engine;
 		}
 	} else {
-		gem_require_ring(fd, engine);
-		igt_require(!(flags & POLL_RUN) ||
-			    gem_can_store_dword(fd, engine));
-		engines[nengine++] = engine;
+		engines[nengine++] = opts->engine;
 	}
 	igt_require(nengine);
 
@@ -130,20 +124,20 @@ emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine,
 	execbuf->buffer_count++;
 	batch_start = batch;
 
-	if (dep) {
-		igt_assert(!(flags & POLL_RUN));
+	if (opts->dependency) {
+		igt_assert(!(opts->flags & IGT_SPIN_POLL_RUN));
 
 		/* dummy write to dependency */
-		obj[SCRATCH].handle = dep;
+		obj[SCRATCH].handle = opts->dependency;
 		fill_reloc(&relocs[obj[BATCH].relocation_count++],
-			   dep, 1020,
+			   opts->dependency, 1020,
 			   I915_GEM_DOMAIN_RENDER,
 			   I915_GEM_DOMAIN_RENDER);
 		execbuf->buffer_count++;
-	} else if (flags & POLL_RUN) {
+	} else if (opts->flags & IGT_SPIN_POLL_RUN) {
 		unsigned int offset;
 
-		igt_assert(!dep);
+		igt_assert(!opts->dependency);
 
 		if (gen == 4 || gen == 5) {
 			execbuf->flags |= I915_EXEC_SECURE;
@@ -231,9 +225,9 @@ emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine,
 
 	execbuf->buffers_ptr = to_user_pointer(obj +
 					       (2 - execbuf->buffer_count));
-	execbuf->rsvd1 = ctx;
+	execbuf->rsvd1 = opts->ctx;
 
-	if (flags & OUT_FENCE)
+	if (opts->flags & IGT_SPIN_FENCE_OUT)
 		execbuf->flags |= I915_EXEC_FENCE_OUT;
 
 	for (i = 0; i < nengine; i++) {
@@ -242,7 +236,7 @@ emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine,
 
 		gem_execbuf_wr(fd, execbuf);
 
-		if (flags & OUT_FENCE) {
+		if (opts->flags & IGT_SPIN_FENCE_OUT) {
 			int _fd = execbuf->rsvd2 >> 32;
 
 			igt_assert(_fd >= 0);
@@ -271,16 +265,14 @@ emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine,
 }
 
 static igt_spin_t *
-___igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep,
-		      unsigned int flags)
+spin_batch_create(int fd, const struct igt_spin_factory *opts)
 {
 	igt_spin_t *spin;
 
 	spin = calloc(1, sizeof(struct igt_spin));
 	igt_assert(spin);
 
-	spin->out_fence = emit_recursive_batch(spin, fd, ctx, engine, dep,
-					       flags);
+	spin->out_fence = emit_recursive_batch(spin, fd, opts);
 
 	pthread_mutex_lock(&list_lock);
 	igt_list_add(&spin->link, &spin_list);
@@ -290,18 +282,15 @@ ___igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep,
 }
 
 igt_spin_t *
-__igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
+__igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts)
 {
-	return ___igt_spin_batch_new(fd, ctx, engine, dep, 0);
+	return spin_batch_create(fd, opts);
 }
 
 /**
- * igt_spin_batch_new:
+ * igt_spin_batch_factory:
  * @fd: open i915 drm file descriptor
- * @engine: Ring to execute batch OR'd with execbuf flags. If value is less
- *          than 0, execute on all available rings.
- * @dep: handle to a buffer object dependency. If greater than 0, add a
- *              relocation entry to this buffer within the batch.
+ * @opts: controlling options such as context, engine, dependencies etc
  *
  * Start a recursive batch on a ring. Immediately returns a #igt_spin_t that
  * contains the batch's handle that can be waited upon. The returned structure
@@ -311,86 +300,26 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
  * Structure with helper internal state for igt_spin_batch_free().
  */
 igt_spin_t *
-igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
+igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts)
 {
 	igt_spin_t *spin;
 
 	igt_require_gem(fd);
 
-	spin = __igt_spin_batch_new(fd, ctx, engine, dep);
-	igt_assert(gem_bo_busy(fd, spin->handle));
-
-	return spin;
-}
-
-igt_spin_t *
-__igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
-{
-	return ___igt_spin_batch_new(fd, ctx, engine, 0, OUT_FENCE);
-}
+	if (opts->engine != ALL_ENGINES) {
+		gem_require_ring(fd, opts->engine);
+		if (opts->flags & IGT_SPIN_POLL_RUN)
+			igt_require(gem_can_store_dword(fd, opts->engine));
+	}
 
-/**
- * igt_spin_batch_new_fence:
- * @fd: open i915 drm file descriptor
- * @engine: Ring to execute batch OR'd with execbuf flags. If value is less
- *          than 0, execute on all available rings.
- *
- * Start a recursive batch on a ring. Immediately returns a #igt_spin_t that
- * contains the batch's handle that can be waited upon. The returned structure
- * must be passed to igt_spin_batch_free() for post-processing.
- *
- * igt_spin_t will contain an output fence associtated with this batch.
- *
- * Returns:
- * Structure with helper internal state for igt_spin_batch_free().
- */
-igt_spin_t *
-igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
-{
-	igt_spin_t *spin;
+	spin = spin_batch_create(fd, opts);
 
-	igt_require_gem(fd);
-	igt_require(gem_has_exec_fence(fd));
-
-	spin = __igt_spin_batch_new_fence(fd, ctx, engine);
 	igt_assert(gem_bo_busy(fd, spin->handle));
-	igt_assert(poll(&(struct pollfd){spin->out_fence, POLLIN}, 1, 0) == 0);
-
-	return spin;
-}
-
-igt_spin_t *
-__igt_spin_batch_new_poll(int fd, uint32_t ctx, unsigned engine)
-{
-	return ___igt_spin_batch_new(fd, ctx, engine, 0, POLL_RUN);
-}
+	if (opts->flags & IGT_SPIN_FENCE_OUT) {
+		struct pollfd pfd = { spin->out_fence, POLLIN };
 
-/**
- * igt_spin_batch_new_poll:
- * @fd: open i915 drm file descriptor
- * @engine: Ring to execute batch OR'd with execbuf flags. If value is less
- *          than 0, execute on all available rings.
- *
- * Start a recursive batch on a ring. Immediately returns a #igt_spin_t that
- * contains the batch's handle that can be waited upon. The returned structure
- * must be passed to igt_spin_batch_free() for post-processing.
- *
- * igt_spin_t->running will containt a pointer which target will change from
- * zero to one once the spinner actually starts executing on the GPU.
- *
- * Returns:
- * Structure with helper internal state for igt_spin_batch_free().
- */
-igt_spin_t *
-igt_spin_batch_new_poll(int fd, uint32_t ctx, unsigned engine)
-{
-	igt_spin_t *spin;
-
-	igt_require_gem(fd);
-	igt_require(gem_mmap__has_wc(fd));
-
-	spin = __igt_spin_batch_new_poll(fd, ctx, engine);
-	igt_assert(gem_bo_busy(fd, spin->handle));
+		igt_assert(poll(&pfd, 1, 0) == 0);
+	}
 
 	return spin;
 }
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index c6ccc2936..c794f2544 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -43,29 +43,25 @@ typedef struct igt_spin {
 	bool *running;
 } igt_spin_t;
 
-igt_spin_t *__igt_spin_batch_new(int fd,
-				 uint32_t ctx,
-				 unsigned engine,
-				 uint32_t  dep);
-igt_spin_t *igt_spin_batch_new(int fd,
-			       uint32_t ctx,
-			       unsigned engine,
-			       uint32_t  dep);
-
-igt_spin_t *__igt_spin_batch_new_fence(int fd,
-				       uint32_t ctx,
-				       unsigned engine);
-
-igt_spin_t *igt_spin_batch_new_fence(int fd,
-				     uint32_t ctx,
-				     unsigned engine);
-
-igt_spin_t *__igt_spin_batch_new_poll(int fd,
-				       uint32_t ctx,
-				       unsigned engine);
-igt_spin_t *igt_spin_batch_new_poll(int fd,
-				    uint32_t ctx,
-				    unsigned engine);
+struct igt_spin_factory {
+	uint32_t ctx;
+	uint32_t dependency;
+	unsigned int engine;
+	unsigned int flags;
+};
+
+#define IGT_SPIN_FENCE_OUT (1 << 0)
+#define IGT_SPIN_POLL_RUN  (1 << 1)
+
+igt_spin_t *
+__igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts);
+igt_spin_t *
+igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts);
+
+#define __igt_spin_batch_new(fd, ...) \
+	__igt_spin_batch_factory(fd, &((struct igt_spin_factory){__VA_ARGS__}))
+#define igt_spin_batch_new(fd, ...) \
+	igt_spin_batch_factory(fd, &((struct igt_spin_factory){__VA_ARGS__}))
 
 void igt_spin_batch_set_timeout(igt_spin_t *spin, int64_t ns);
 void igt_spin_batch_end(igt_spin_t *spin);
diff --git a/tests/drv_missed_irq.c b/tests/drv_missed_irq.c
index 791ee51fb..78690c36a 100644
--- a/tests/drv_missed_irq.c
+++ b/tests/drv_missed_irq.c
@@ -33,7 +33,7 @@ IGT_TEST_DESCRIPTION("Inject missed interrupts and make sure they are caught");
 
 static void trigger_missed_interrupt(int fd, unsigned ring)
 {
-	igt_spin_t *spin = __igt_spin_batch_new(fd, 0, ring, 0);
+	igt_spin_t *spin = __igt_spin_batch_new(fd, .engine = ring);
 	uint32_t go;
 	int link[2];
 
diff --git a/tests/gem_busy.c b/tests/gem_busy.c
index f564651ba..76b44a5d4 100644
--- a/tests/gem_busy.c
+++ b/tests/gem_busy.c
@@ -114,7 +114,9 @@ static void semaphore(int fd, unsigned ring, uint32_t flags)
 
 	/* Create a long running batch which we can use to hog the GPU */
 	handle[BUSY] = gem_create(fd, 4096);
-	spin = igt_spin_batch_new(fd, 0, ring, handle[BUSY]);
+	spin = igt_spin_batch_new(fd,
+				  .engine = ring,
+				  .dependency = handle[BUSY]);
 
 	/* Queue a batch after the busy, it should block and remain "busy" */
 	igt_assert(exec_noop(fd, handle, ring | flags, false));
@@ -363,17 +365,16 @@ static void close_race(int fd)
 		igt_assert(sched_setscheduler(getpid(), SCHED_RR, &rt) == 0);
 
 		for (i = 0; i < nhandles; i++) {
-			spin[i] = __igt_spin_batch_new(fd, 0,
-						       engines[rand() % nengine], 0);
+			spin[i] = __igt_spin_batch_new(fd,
+						       .engine = engines[rand() % nengine]);
 			handles[i] = spin[i]->handle;
 		}
 
 		igt_until_timeout(20) {
 			for (i = 0; i < nhandles; i++) {
 				igt_spin_batch_free(fd, spin[i]);
-				spin[i] = __igt_spin_batch_new(fd, 0,
-							       engines[rand() % nengine],
-							       0);
+				spin[i] = __igt_spin_batch_new(fd,
+							       .engine = engines[rand() % nengine]);
 				handles[i] = spin[i]->handle;
 				__sync_synchronize();
 			}
@@ -415,7 +416,7 @@ static bool has_semaphores(int fd)
 
 static bool has_extended_busy_ioctl(int fd)
 {
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, I915_EXEC_RENDER, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd, .engine = I915_EXEC_RENDER);
 	uint32_t read, write;
 
 	__gem_busy(fd, spin->handle, &read, &write);
@@ -426,7 +427,7 @@ static bool has_extended_busy_ioctl(int fd)
 
 static void basic(int fd, unsigned ring, unsigned flags)
 {
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, ring, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd, .engine = ring);
 	struct timespec tv;
 	int timeout;
 	bool busy;
diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
index fe7d3490c..2e19e8c03 100644
--- a/tests/gem_ctx_isolation.c
+++ b/tests/gem_ctx_isolation.c
@@ -502,7 +502,7 @@ static void isolation(int fd,
 		ctx[0] = gem_context_create(fd);
 		regs[0] = read_regs(fd, ctx[0], e, flags);
 
-		spin = igt_spin_batch_new(fd, ctx[0], engine, 0);
+		spin = igt_spin_batch_new(fd, .ctx = ctx[0], .engine = engine);
 
 		if (flags & DIRTY1) {
 			igt_debug("%s[%d]: Setting all registers of ctx 0 to 0x%08x\n",
@@ -557,8 +557,11 @@ static void isolation(int fd,
 
 static void inject_reset_context(int fd, unsigned int engine)
 {
+	struct igt_spin_factory opts = {
+		.ctx = gem_context_create(fd),
+		.engine = engine,
+	};
 	igt_spin_t *spin;
-	uint32_t ctx;
 
 	/*
 	 * Force a context switch before triggering the reset, or else
@@ -566,19 +569,20 @@ static void inject_reset_context(int fd, unsigned int engine)
 	 * HW for screwing up if the context was already broken.
 	 */
 
-	ctx = gem_context_create(fd);
-	if (gem_can_store_dword(fd, engine)) {
-		spin = __igt_spin_batch_new_poll(fd, ctx, engine);
+	if (gem_can_store_dword(fd, engine))
+		opts.flags |= IGT_SPIN_POLL_RUN;
+
+	spin = __igt_spin_batch_factory(fd, &opts);
+
+	if (spin->running)
 		igt_spin_busywait_until_running(spin);
-	} else {
-		spin = __igt_spin_batch_new(fd, ctx, engine, 0);
+	else
 		usleep(1000); /* better than nothing */
-	}
 
 	igt_force_gpu_reset(fd);
 
 	igt_spin_batch_free(fd, spin);
-	gem_context_destroy(fd, ctx);
+	gem_context_destroy(fd, opts.ctx);
 }
 
 static void preservation(int fd,
@@ -604,7 +608,7 @@ static void preservation(int fd,
 	gem_quiescent_gpu(fd);
 
 	ctx[num_values] = gem_context_create(fd);
-	spin = igt_spin_batch_new(fd, ctx[num_values], engine, 0);
+	spin = igt_spin_batch_new(fd, .ctx = ctx[num_values], .engine = engine);
 	regs[num_values][0] = read_regs(fd, ctx[num_values], e, flags);
 	for (int v = 0; v < num_values; v++) {
 		ctx[v] = gem_context_create(fd);
@@ -644,7 +648,7 @@ static void preservation(int fd,
 		break;
 	}
 
-	spin = igt_spin_batch_new(fd, ctx[num_values], engine, 0);
+	spin = igt_spin_batch_new(fd, .ctx = ctx[num_values], .engine = engine);
 	for (int v = 0; v < num_values; v++)
 		regs[v][1] = read_regs(fd, ctx[v], e, flags);
 	regs[num_values][1] = read_regs(fd, ctx[num_values], e, flags);
diff --git a/tests/gem_eio.c b/tests/gem_eio.c
index 5faf7502b..0ec1aaec9 100644
--- a/tests/gem_eio.c
+++ b/tests/gem_eio.c
@@ -157,10 +157,15 @@ static int __gem_wait(int fd, uint32_t handle, int64_t timeout)
 
 static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
 {
-	if (gem_can_store_dword(fd, flags))
-		return __igt_spin_batch_new_poll(fd, ctx, flags);
-	else
-		return __igt_spin_batch_new(fd, ctx, flags, 0);
+	struct igt_spin_factory opts = {
+		.ctx = ctx,
+		.engine = flags,
+	};
+
+	if (gem_can_store_dword(fd, opts.engine))
+		opts.flags |= IGT_SPIN_POLL_RUN;
+
+	return __igt_spin_batch_factory(fd, &opts);
 }
 
 static void __spin_wait(int fd, igt_spin_t *spin)
diff --git a/tests/gem_exec_fence.c b/tests/gem_exec_fence.c
index eb93308d1..ba46595d3 100644
--- a/tests/gem_exec_fence.c
+++ b/tests/gem_exec_fence.c
@@ -468,7 +468,7 @@ static void test_parallel(int fd, unsigned int master)
 	/* Fill the queue with many requests so that the next one has to
 	 * wait before it can be executed by the hardware.
 	 */
-	spin = igt_spin_batch_new(fd, 0, master, plug);
+	spin = igt_spin_batch_new(fd, .engine = master, .dependency = plug);
 	resubmit(fd, spin->handle, master, 16);
 
 	/* Now queue the master request and its secondaries */
@@ -651,7 +651,7 @@ static void test_keep_in_fence(int fd, unsigned int engine, unsigned int flags)
 	igt_spin_t *spin;
 	int fence;
 
-	spin = igt_spin_batch_new(fd, 0, engine, 0);
+	spin = igt_spin_batch_new(fd, .engine = engine);
 
 	gem_execbuf_wr(fd, &execbuf);
 	fence = upper_32_bits(execbuf.rsvd2);
@@ -1070,7 +1070,7 @@ static void test_syncobj_unused_fence(int fd)
 	struct local_gem_exec_fence fence = {
 		.handle = syncobj_create(fd),
 	};
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, 0, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd);
 
 	/* sanity check our syncobj_to_sync_file interface */
 	igt_assert_eq(__syncobj_to_sync_file(fd, 0), -ENOENT);
@@ -1162,7 +1162,7 @@ static void test_syncobj_signal(int fd)
 	struct local_gem_exec_fence fence = {
 		.handle = syncobj_create(fd),
 	};
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, 0, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd);
 
 	/* Check that the syncobj is signaled only when our request/fence is */
 
@@ -1212,7 +1212,7 @@ static void test_syncobj_wait(int fd)
 
 	gem_quiescent_gpu(fd);
 
-	spin = igt_spin_batch_new(fd, 0, 0, 0);
+	spin = igt_spin_batch_new(fd);
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(&obj);
@@ -1282,7 +1282,7 @@ static void test_syncobj_export(int fd)
 		.handle = syncobj_create(fd),
 	};
 	int export[2];
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, 0, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd);
 
 	/* Check that if we export the syncobj prior to use it picks up
 	 * the later fence. This allows a syncobj to establish a channel
@@ -1340,7 +1340,7 @@ static void test_syncobj_repeat(int fd)
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct local_gem_exec_fence *fence;
 	int export;
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, 0, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd);
 
 	/* Check that we can wait on the same fence multiple times */
 	fence = calloc(nfences, sizeof(*fence));
@@ -1395,7 +1395,7 @@ static void test_syncobj_import(int fd)
 	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 obj;
 	struct drm_i915_gem_execbuffer2 execbuf;
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, 0, 0);
+	igt_spin_t *spin = igt_spin_batch_new(fd);
 	uint32_t sync = syncobj_create(fd);
 	int fence;
 
diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c
index ea2e4c681..75811f325 100644
--- a/tests/gem_exec_latency.c
+++ b/tests/gem_exec_latency.c
@@ -63,6 +63,10 @@ static unsigned int ring_size;
 static void
 poll_ring(int fd, unsigned ring, const char *name)
 {
+	const struct igt_spin_factory opts = {
+		.engine = ring,
+		.flags = IGT_SPIN_POLL_RUN,
+	};
 	struct timespec tv = {};
 	unsigned long cycles;
 	igt_spin_t *spin[2];
@@ -72,11 +76,11 @@ poll_ring(int fd, unsigned ring, const char *name)
 	gem_require_ring(fd, ring);
 	igt_require(gem_can_store_dword(fd, ring));
 
-	spin[0] = __igt_spin_batch_new_poll(fd, 0, ring);
+	spin[0] = __igt_spin_batch_factory(fd, &opts);
 	igt_assert(spin[0]->running);
 	cmd = *spin[0]->batch;
 
-	spin[1] = __igt_spin_batch_new_poll(fd, 0, ring);
+	spin[1] = __igt_spin_batch_factory(fd, &opts);
 	igt_assert(spin[1]->running);
 	igt_assert(cmd == *spin[1]->batch);
 
@@ -312,7 +316,9 @@ static void latency_from_ring(int fd,
 			       I915_GEM_DOMAIN_GTT);
 
 		if (flags & PREEMPT)
-			spin = __igt_spin_batch_new(fd, ctx[0], ring, 0);
+			spin = __igt_spin_batch_new(fd,
+						    .ctx = ctx[0],
+						    .engine = ring);
 
 		if (flags & CORK) {
 			obj[0].handle = igt_cork_plug(&c, fd);
@@ -456,6 +462,10 @@ rthog_latency_on_ring(int fd, unsigned int engine, const char *name, unsigned in
 	};
 #define NPASS ARRAY_SIZE(passname)
 #define MMAP_SZ (64 << 10)
+	const struct igt_spin_factory opts = {
+		.engine = engine,
+		.flags = IGT_SPIN_POLL_RUN,
+	};
 	struct rt_pkt *results;
 	unsigned int engines[16];
 	const char *names[16];
@@ -513,7 +523,7 @@ rthog_latency_on_ring(int fd, unsigned int engine, const char *name, unsigned in
 
 			usleep(250);
 
-			spin = __igt_spin_batch_new_poll(fd, 0, engine);
+			spin = __igt_spin_batch_factory(fd, &opts);
 			if (!spin) {
 				igt_warn("Failed to create spinner! (%s)\n",
 					 passname[pass]);
diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index 0523b1c02..74d27522d 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -709,7 +709,9 @@ static void preempt(int fd, uint32_t handle,
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	do {
 		igt_spin_t *spin =
-			__igt_spin_batch_new(fd, ctx[0], ring_id, 0);
+			__igt_spin_batch_new(fd,
+					     .ctx = ctx[0],
+					     .engine = ring_id);
 
 		for (int loop = 0; loop < 1024; loop++)
 			gem_execbuf(fd, &execbuf);
diff --git a/tests/gem_exec_reloc.c b/tests/gem_exec_reloc.c
index 91c6691af..837f60a6c 100644
--- a/tests/gem_exec_reloc.c
+++ b/tests/gem_exec_reloc.c
@@ -388,7 +388,9 @@ static void basic_reloc(int fd, unsigned before, unsigned after, unsigned flags)
 		}
 
 		if (flags & ACTIVE) {
-			spin = igt_spin_batch_new(fd, 0, I915_EXEC_DEFAULT, obj.handle);
+			spin = igt_spin_batch_new(fd,
+						  .engine = I915_EXEC_DEFAULT,
+						  .dependency = obj.handle);
 			if (!(flags & HANG))
 				igt_spin_batch_set_timeout(spin, NSEC_PER_SEC/100);
 			igt_assert(gem_bo_busy(fd, obj.handle));
@@ -454,7 +456,9 @@ static void basic_reloc(int fd, unsigned before, unsigned after, unsigned flags)
 		}
 
 		if (flags & ACTIVE) {
-			spin = igt_spin_batch_new(fd, 0, I915_EXEC_DEFAULT, obj.handle);
+			spin = igt_spin_batch_new(fd,
+						  .engine = I915_EXEC_DEFAULT,
+						  .dependency = obj.handle);
 			if (!(flags & HANG))
 				igt_spin_batch_set_timeout(spin, NSEC_PER_SEC/100);
 			igt_assert(gem_bo_busy(fd, obj.handle));
@@ -581,7 +585,7 @@ static void basic_range(int fd, unsigned flags)
 	execbuf.buffer_count = n + 1;
 
 	if (flags & ACTIVE) {
-		spin = igt_spin_batch_new(fd, 0, 0, obj[n].handle);
+		spin = igt_spin_batch_new(fd, .dependency = obj[n].handle);
 		if (!(flags & HANG))
 			igt_spin_batch_set_timeout(spin, NSEC_PER_SEC/100);
 		igt_assert(gem_bo_busy(fd, obj[n].handle));
diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
index 1f43147f7..35a44ab10 100644
--- a/tests/gem_exec_schedule.c
+++ b/tests/gem_exec_schedule.c
@@ -132,9 +132,12 @@ static void unplug_show_queue(int fd, struct igt_cork *c, unsigned int engine)
 	igt_spin_t *spin[MAX_ELSP_QLEN];
 
 	for (int n = 0; n < ARRAY_SIZE(spin); n++) {
-		uint32_t ctx = create_highest_priority(fd);
-		spin[n] = __igt_spin_batch_new(fd, ctx, engine, 0);
-		gem_context_destroy(fd, ctx);
+		const struct igt_spin_factory opts = {
+			.ctx = create_highest_priority(fd),
+			.engine = engine,
+		};
+		spin[n] = __igt_spin_batch_factory(fd, &opts);
+		gem_context_destroy(fd, opts.ctx);
 	}
 
 	igt_cork_unplug(c); /* batches will now be queued on the engine */
@@ -196,7 +199,7 @@ static void independent(int fd, unsigned int engine)
 			continue;
 
 		if (spin == NULL) {
-			spin = __igt_spin_batch_new(fd, 0, other, 0);
+			spin = __igt_spin_batch_new(fd, .engine = other);
 		} else {
 			struct drm_i915_gem_exec_object2 obj = {
 				.handle = spin->handle,
@@ -428,7 +431,9 @@ static void preempt(int fd, unsigned ring, unsigned flags)
 			ctx[LO] = gem_context_create(fd);
 			gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 		}
-		spin[n] = __igt_spin_batch_new(fd, ctx[LO], ring, 0);
+		spin[n] = __igt_spin_batch_new(fd,
+					       .ctx = ctx[LO],
+					       .engine = ring);
 		igt_debug("spin[%d].handle=%d\n", n, spin[n]->handle);
 
 		store_dword(fd, ctx[HI], ring, result, 0, n + 1, 0, I915_GEM_DOMAIN_RENDER);
@@ -462,7 +467,9 @@ static igt_spin_t *__noise(int fd, uint32_t ctx, int prio, igt_spin_t *spin)
 
 	for_each_physical_engine(fd, other) {
 		if (spin == NULL) {
-			spin = __igt_spin_batch_new(fd, ctx, other, 0);
+			spin = __igt_spin_batch_new(fd,
+						    .ctx = ctx,
+						    .engine = other);
 		} else {
 			struct drm_i915_gem_exec_object2 obj = {
 				.handle = spin->handle,
@@ -672,7 +679,9 @@ static void preempt_self(int fd, unsigned ring)
 	n = 0;
 	gem_context_set_priority(fd, ctx[HI], MIN_PRIO);
 	for_each_physical_engine(fd, other) {
-		spin[n] = __igt_spin_batch_new(fd, ctx[NOISE], other, 0);
+		spin[n] = __igt_spin_batch_new(fd,
+					       .ctx = ctx[NOISE],
+					       .engine = other);
 		store_dword(fd, ctx[HI], other,
 			    result, (n + 1)*sizeof(uint32_t), n + 1,
 			    0, I915_GEM_DOMAIN_RENDER);
@@ -714,7 +723,9 @@ static void preemptive_hang(int fd, unsigned ring)
 		ctx[LO] = gem_context_create(fd);
 		gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 
-		spin[n] = __igt_spin_batch_new(fd, ctx[LO], ring, 0);
+		spin[n] = __igt_spin_batch_new(fd,
+					       .ctx = ctx[LO],
+					       .engine = ring);
 
 		gem_context_destroy(fd, ctx[LO]);
 	}
diff --git a/tests/gem_exec_suspend.c b/tests/gem_exec_suspend.c
index db2bca262..43c52d105 100644
--- a/tests/gem_exec_suspend.c
+++ b/tests/gem_exec_suspend.c
@@ -189,7 +189,7 @@ static void run_test(int fd, unsigned engine, unsigned flags)
 	}
 
 	if (flags & HANG)
-		spin = igt_spin_batch_new(fd, 0, engine, 0);
+		spin = igt_spin_batch_new(fd, .engine = engine);
 
 	switch (mode(flags)) {
 	case NOSLEEP:
diff --git a/tests/gem_fenced_exec_thrash.c b/tests/gem_fenced_exec_thrash.c
index 385790ada..7248d310d 100644
--- a/tests/gem_fenced_exec_thrash.c
+++ b/tests/gem_fenced_exec_thrash.c
@@ -132,7 +132,7 @@ static void run_test(int fd, int num_fences, int expected_errno,
 			igt_spin_t *spin = NULL;
 
 			if (flags & BUSY_LOAD)
-				spin = __igt_spin_batch_new(fd, 0, 0, 0);
+				spin = __igt_spin_batch_new(fd);
 
 			igt_while_interruptible(flags & INTERRUPTIBLE) {
 				igt_assert_eq(__gem_execbuf(fd, &execbuf[i]),
diff --git a/tests/gem_shrink.c b/tests/gem_shrink.c
index 3d33453aa..929e0426a 100644
--- a/tests/gem_shrink.c
+++ b/tests/gem_shrink.c
@@ -346,9 +346,9 @@ static void reclaim(unsigned engine, int timeout)
 		} while (!*shared);
 	}
 
-	spin = igt_spin_batch_new(fd, 0, engine, 0);
+	spin = igt_spin_batch_new(fd, .engine = engine);
 	igt_until_timeout(timeout) {
-		igt_spin_t *next = __igt_spin_batch_new(fd, 0, engine, 0);
+		igt_spin_t *next = __igt_spin_batch_new(fd, .engine = engine);
 
 		igt_spin_batch_set_timeout(spin, timeout_100ms);
 		gem_sync(fd, spin->handle);
diff --git a/tests/gem_spin_batch.c b/tests/gem_spin_batch.c
index cffeb6d71..52410010b 100644
--- a/tests/gem_spin_batch.c
+++ b/tests/gem_spin_batch.c
@@ -41,9 +41,9 @@ static void spin(int fd, unsigned int engine, unsigned int timeout_sec)
 	struct timespec itv = { };
 	uint64_t elapsed;
 
-	spin = __igt_spin_batch_new(fd, 0, engine, 0);
+	spin = __igt_spin_batch_new(fd, .engine = engine);
 	while ((elapsed = igt_nsec_elapsed(&tv)) >> 30 < timeout_sec) {
-		igt_spin_t *next = __igt_spin_batch_new(fd, 0, engine, 0);
+		igt_spin_t *next = __igt_spin_batch_new(fd, .engine = engine);
 
 		igt_spin_batch_set_timeout(spin,
 					   timeout_100ms - igt_nsec_elapsed(&itv));
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index 1e2e089a1..2fcb9aa01 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -715,9 +715,8 @@ preempt(int fd, unsigned ring, int num_children, int timeout)
 		do {
 			igt_spin_t *spin =
 				__igt_spin_batch_new(fd,
-						     ctx[0],
-						     execbuf.flags,
-						     0);
+						     .ctx = ctx[0],
+						     .engine = execbuf.flags);
 
 			do {
 				gem_execbuf(fd, &execbuf);
diff --git a/tests/gem_wait.c b/tests/gem_wait.c
index 61d8a4059..7914c9365 100644
--- a/tests/gem_wait.c
+++ b/tests/gem_wait.c
@@ -74,7 +74,9 @@ static void basic(int fd, unsigned engine, unsigned flags)
 	IGT_CORK_HANDLE(cork);
 	uint32_t plug =
 		flags & (WRITE | AWAIT) ? igt_cork_plug(&cork, fd) : 0;
-	igt_spin_t *spin = igt_spin_batch_new(fd, 0, engine, plug);
+	igt_spin_t *spin = igt_spin_batch_new(fd,
+					      .engine = engine,
+					      .dependency = plug);
 	struct drm_i915_gem_wait wait = {
 		flags & WRITE ? plug : spin->handle
 	};
diff --git a/tests/kms_busy.c b/tests/kms_busy.c
index 4a4e0e156..abf39828b 100644
--- a/tests/kms_busy.c
+++ b/tests/kms_busy.c
@@ -84,7 +84,8 @@ static void flip_to_fb(igt_display_t *dpy, int pipe,
 	struct drm_event_vblank ev;
 
 	igt_spin_t *t = igt_spin_batch_new(dpy->drm_fd,
-					   0, ring, fb->gem_handle);
+					   .engine = ring,
+					   .dependency = fb->gem_handle);
 
 	if (modeset) {
 		/*
@@ -200,7 +201,8 @@ static void test_atomic_commit_hang(igt_display_t *dpy, igt_plane_t *primary,
 				    struct igt_fb *busy_fb, unsigned ring)
 {
 	igt_spin_t *t = igt_spin_batch_new(dpy->drm_fd,
-					   0, ring, busy_fb->gem_handle);
+					   .engine = ring,
+					   .dependency = busy_fb->gem_handle);
 	struct pollfd pfd = { .fd = dpy->drm_fd, .events = POLLIN };
 	unsigned flags = 0;
 	struct drm_event_vblank ev;
@@ -287,7 +289,9 @@ static void test_pageflip_modeset_hang(igt_display_t *dpy,
 
 	igt_display_commit2(dpy, dpy->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
-	t = igt_spin_batch_new(dpy->drm_fd, 0, ring, fb.gem_handle);
+	t = igt_spin_batch_new(dpy->drm_fd,
+			       .engine = ring,
+			       .dependency = fb.gem_handle);
 
 	do_or_die(drmModePageFlip(dpy->drm_fd, dpy->pipes[pipe].crtc_id, fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, &fb));
 
diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index d0a28b3c4..85340d43e 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -532,7 +532,8 @@ static void basic_flip_cursor(igt_display_t *display,
 
 		spin = NULL;
 		if (flags & BASIC_BUSY)
-			spin = igt_spin_batch_new(display->drm_fd, 0, 0, fb_info.gem_handle);
+			spin = igt_spin_batch_new(display->drm_fd,
+						  .dependency = fb_info.gem_handle);
 
 		/* Start with a synchronous query to align with the vblank */
 		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
@@ -1323,8 +1324,8 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
 	for (int i = 1; i >= 0; i--) {
 		igt_spin_t *spin;
 
-		spin = igt_spin_batch_new(display->drm_fd, 0, 0,
-					  fb_info[1].gem_handle);
+		spin = igt_spin_batch_new(display->drm_fd,
+					  .dependency = fb_info[1].gem_handle);
 
 		vblank_start = get_vblank(display->drm_fd, pipe, DRM_VBLANK_NEXTONMISS);
 
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 4570f926d..a1d36ac4f 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -172,10 +172,15 @@ static unsigned int e2ring(int gem_fd, const struct intel_execution_engine2 *e)
 
 static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
 {
+	struct igt_spin_factory opts = {
+		.ctx = ctx,
+		.engine = flags,
+	};
+
 	if (gem_can_store_dword(fd, flags))
-		return __igt_spin_batch_new_poll(fd, ctx, flags);
-	else
-		return __igt_spin_batch_new(fd, ctx, flags, 0);
+		opts.flags |= IGT_SPIN_POLL_RUN;
+
+	return __igt_spin_batch_factory(fd, &opts);
 }
 
 static unsigned long __spin_wait(int fd, igt_spin_t *spin)
@@ -356,7 +361,9 @@ busy_double_start(int gem_fd, const struct intel_execution_engine2 *e)
 	 */
 	spin[0] = __spin_sync(gem_fd, 0, e2ring(gem_fd, e));
 	usleep(500e3);
-	spin[1] = __igt_spin_batch_new(gem_fd, ctx, e2ring(gem_fd, e), 0);
+	spin[1] = __igt_spin_batch_new(gem_fd,
+				       .ctx = ctx,
+				       .engine = e2ring(gem_fd, e));
 
 	/*
 	 * Open PMU as fast as possible after the second spin batch in attempt
@@ -1045,8 +1052,8 @@ static void cpu_hotplug(int gem_fd)
 	 * Create two spinners so test can ensure shorter gaps in engine
 	 * busyness as it is terminating one and re-starting the other.
 	 */
-	spin[0] = igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER, 0);
-	spin[1] = __igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER, 0);
+	spin[0] = igt_spin_batch_new(gem_fd, .engine = I915_EXEC_RENDER);
+	spin[1] = __igt_spin_batch_new(gem_fd, .engine = I915_EXEC_RENDER);
 
 	val = __pmu_read_single(fd, &ts[0]);
 
@@ -1129,8 +1136,8 @@ static void cpu_hotplug(int gem_fd)
 			break;
 
 		igt_spin_batch_free(gem_fd, spin[cur]);
-		spin[cur] = __igt_spin_batch_new(gem_fd, 0, I915_EXEC_RENDER,
-						 0);
+		spin[cur] = __igt_spin_batch_new(gem_fd,
+						 .engine = I915_EXEC_RENDER);
 		cur ^= 1;
 	}
 
@@ -1167,8 +1174,9 @@ test_interrupts(int gem_fd)
 
 	/* Queue spinning batches. */
 	for (int i = 0; i < target; i++) {
-		spin[i] = __igt_spin_batch_new_fence(gem_fd,
-						     0, I915_EXEC_RENDER);
+		spin[i] = __igt_spin_batch_new(gem_fd,
+					       .engine = I915_EXEC_RENDER,
+					       .flags = IGT_SPIN_FENCE_OUT);
 		if (i == 0) {
 			fence_fd = spin[i]->out_fence;
 		} else {
@@ -1229,7 +1237,8 @@ test_interrupts_sync(int gem_fd)
 
 	/* Queue spinning batches. */
 	for (int i = 0; i < target; i++)
-		spin[i] = __igt_spin_batch_new_fence(gem_fd, 0, 0);
+		spin[i] = __igt_spin_batch_new(gem_fd,
+					       .flags = IGT_SPIN_FENCE_OUT);
 
 	/* Wait for idle state. */
 	idle = pmu_read_single(fd);
@@ -1550,7 +1559,7 @@ accuracy(int gem_fd, const struct intel_execution_engine2 *e,
 		igt_spin_t *spin;
 
 		/* Allocate our spin batch and idle it. */
-		spin = igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
+		spin = igt_spin_batch_new(gem_fd, .engine = e2ring(gem_fd, e));
 		igt_spin_batch_end(spin);
 		gem_sync(gem_fd, spin->handle);
 
diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 006d084b8..202132b1c 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -235,9 +235,9 @@ static void load_helper_run(enum load load)
 
 		igt_debug("Applying %s load...\n", lh.load ? "high" : "low");
 
-		spin[0] = __igt_spin_batch_new(drm_fd, 0, 0, 0);
+		spin[0] = __igt_spin_batch_new(drm_fd);
 		if (lh.load == HIGH)
-			spin[1] = __igt_spin_batch_new(drm_fd, 0, 0, 0);
+			spin[1] = __igt_spin_batch_new(drm_fd);
 		while (!lh.exit) {
 			handle = spin[0]->handle;
 			igt_spin_batch_end(spin[0]);
@@ -248,8 +248,7 @@ static void load_helper_run(enum load load)
 			usleep(100);
 
 			spin[0] = spin[1];
-			spin[lh.load == HIGH] =
-				__igt_spin_batch_new(drm_fd, 0, 0, 0);
+			spin[lh.load == HIGH] = __igt_spin_batch_new(drm_fd);
 		}
 
 		handle = spin[0]->handle;
@@ -510,7 +509,7 @@ static void boost_freq(int fd, int *boost_freqs)
 	int64_t timeout = 1;
 	igt_spin_t *load;
 
-	load = igt_spin_batch_new(fd, 0, 0, 0);
+	load = igt_spin_batch_new(fd);
 	resubmit_batch(fd, load->handle, 16);
 
 	/* Waiting will grant us a boost to maximum */
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 5/8] lib: Spin fast, retire early

[Chris Wilson]

When using the pollable spinner, we often want to use it as a means of
ensuring the task is running on the GPU before switching to something
else. In which case we don't want to add extra delay inside the spinner,
but the current 1000 NOPs add on order of 5us, which is often larger
than the target latency.

v2: Don't change perf_pmu as that is sensitive to the extra CPU latency
from a tight GPU spinner.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com> #v1
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_dummyload.c       | 3 ++-
 lib/igt_dummyload.h       | 1 +
 tests/gem_ctx_isolation.c | 1 +
 tests/gem_eio.c           | 1 +
 tests/gem_exec_latency.c  | 4 ++--
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 94efdf745..7beb66244 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -199,7 +199,8 @@ emit_recursive_batch(igt_spin_t *spin,
 	 * between function calls, that appears enough to keep SNB out of
 	 * trouble. See https://bugs.freedesktop.org/show_bug.cgi?id=102262
 	 */
-	batch += 1000;
+	if (!(opts->flags & IGT_SPIN_FAST))
+		batch += 1000;
 
 	/* recurse */
 	r = &relocs[obj[BATCH].relocation_count++];
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index c794f2544..e80a12451 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -52,6 +52,7 @@ struct igt_spin_factory {
 
 #define IGT_SPIN_FENCE_OUT (1 << 0)
 #define IGT_SPIN_POLL_RUN  (1 << 1)
+#define IGT_SPIN_FAST      (1 << 2)
 
 igt_spin_t *
 __igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts);
diff --git a/tests/gem_ctx_isolation.c b/tests/gem_ctx_isolation.c
index 2e19e8c03..4325e1c28 100644
--- a/tests/gem_ctx_isolation.c
+++ b/tests/gem_ctx_isolation.c
@@ -560,6 +560,7 @@ static void inject_reset_context(int fd, unsigned int engine)
 	struct igt_spin_factory opts = {
 		.ctx = gem_context_create(fd),
 		.engine = engine,
+		.flags = IGT_SPIN_FAST,
 	};
 	igt_spin_t *spin;
 
diff --git a/tests/gem_eio.c b/tests/gem_eio.c
index 0ec1aaec9..3162a3170 100644
--- a/tests/gem_eio.c
+++ b/tests/gem_eio.c
@@ -160,6 +160,7 @@ static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
 	struct igt_spin_factory opts = {
 		.ctx = ctx,
 		.engine = flags,
+		.flags = IGT_SPIN_FAST,
 	};
 
 	if (gem_can_store_dword(fd, opts.engine))
diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c
index 75811f325..de16322a6 100644
--- a/tests/gem_exec_latency.c
+++ b/tests/gem_exec_latency.c
@@ -65,7 +65,7 @@ poll_ring(int fd, unsigned ring, const char *name)
 {
 	const struct igt_spin_factory opts = {
 		.engine = ring,
-		.flags = IGT_SPIN_POLL_RUN,
+		.flags = IGT_SPIN_POLL_RUN | IGT_SPIN_FAST,
 	};
 	struct timespec tv = {};
 	unsigned long cycles;
@@ -464,7 +464,7 @@ rthog_latency_on_ring(int fd, unsigned int engine, const char *name, unsigned in
 #define MMAP_SZ (64 << 10)
 	const struct igt_spin_factory opts = {
 		.engine = engine,
-		.flags = IGT_SPIN_POLL_RUN,
+		.flags = IGT_SPIN_POLL_RUN | IGT_SPIN_FAST,
 	};
 	struct rt_pkt *results;
 	unsigned int engines[16];
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 6/8] igt/gem_sync: Alternate stress for nop+sync

[Chris Wilson]

Apply a different sort of stress by timing how long it takes to sync a
second nop batch in the pipeline. We first start a spinner on the
engine, then when we know the GPU is active, we submit the second nop;
start timing as we then release the spinner and wait for the nop to
complete.

As with every other gem_sync test, it serves two roles. The first is
that it checks that we do not miss a wakeup under common stressful
conditions (the more conditions we check, the happier we will be that
they do not occur in practice). And the second role it fulfils, is that
it provides a very crude estimate for how long it takes for a nop to
execute from a running start (we already have a complimentary estimate
for an idle start).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 tests/gem_sync.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index 2fcb9aa01..747412c5d 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -177,6 +177,95 @@ idle_ring(int fd, unsigned ring, int timeout)
 	gem_close(fd, object.handle);
 }
 
+static void
+wakeup_ring(int fd, unsigned ring, int timeout)
+{
+	unsigned engines[16];
+	const char *names[16];
+	int num_engines = 0;
+
+	if (ring == ALL_ENGINES) {
+		for_each_physical_engine(fd, ring) {
+			if (!gem_can_store_dword(fd, ring))
+				continue;
+
+			names[num_engines] = e__->name;
+			engines[num_engines++] = ring;
+			if (num_engines == ARRAY_SIZE(engines))
+				break;
+		}
+		igt_require(num_engines);
+	} else {
+		gem_require_ring(fd, ring);
+		igt_require(gem_can_store_dword(fd, ring));
+		names[num_engines] = NULL;
+		engines[num_engines++] = ring;
+	}
+
+	intel_detect_and_clear_missed_interrupts(fd);
+	igt_fork(child, num_engines) {
+		const uint32_t bbe = MI_BATCH_BUFFER_END;
+		struct drm_i915_gem_exec_object2 object;
+		struct drm_i915_gem_execbuffer2 execbuf;
+		double end, this, elapsed, now;
+		unsigned long cycles;
+		uint32_t cmd;
+		igt_spin_t *spin;
+
+		memset(&object, 0, sizeof(object));
+		object.handle = gem_create(fd, 4096);
+		gem_write(fd, object.handle, 0, &bbe, sizeof(bbe));
+
+		memset(&execbuf, 0, sizeof(execbuf));
+		execbuf.buffers_ptr = to_user_pointer(&object);
+		execbuf.buffer_count = 1;
+		execbuf.flags = engines[child % num_engines];
+
+		spin = __igt_spin_batch_new(fd,
+					    .engine = execbuf.flags,
+					    .flags = (IGT_SPIN_POLL_RUN |
+						      IGT_SPIN_FAST));
+		igt_assert(spin->running);
+		cmd = *spin->batch;
+
+		gem_execbuf(fd, &execbuf);
+
+		igt_spin_batch_end(spin);
+		gem_sync(fd, object.handle);
+
+		end = gettime() + timeout;
+		elapsed = 0;
+		cycles = 0;
+		do {
+			*spin->batch = cmd;
+			*spin->running = 0;
+			gem_execbuf(fd, &spin->execbuf);
+			while (!READ_ONCE(*spin->running))
+				;
+
+			gem_execbuf(fd, &execbuf);
+
+			this = gettime();
+			igt_spin_batch_end(spin);
+			gem_sync(fd, object.handle);
+			now = gettime();
+
+			elapsed += now - this;
+			cycles++;
+		} while (now < end);
+
+		igt_info("%s%sompleted %ld cycles: %.3f us\n",
+			 names[child % num_engines] ?: "",
+			 names[child % num_engines] ? " c" : "C",
+			 cycles, elapsed*1e6/cycles);
+
+		igt_spin_batch_free(fd, spin);
+		gem_close(fd, object.handle);
+	}
+	igt_waitchildren_timeout(timeout+10, NULL);
+	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
+}
+
 static void
 store_ring(int fd, unsigned ring, int num_children, int timeout)
 {
@@ -761,6 +850,8 @@ igt_main
 			sync_ring(fd, e->exec_id | e->flags, 1, 150);
 		igt_subtest_f("idle-%s", e->name)
 			idle_ring(fd, e->exec_id | e->flags, 150);
+		igt_subtest_f("wakeup-%s", e->name)
+			wakeup_ring(fd, e->exec_id | e->flags, 150);
 		igt_subtest_f("store-%s", e->name)
 			store_ring(fd, e->exec_id | e->flags, 1, 150);
 		igt_subtest_f("many-%s", e->name)
@@ -781,6 +872,8 @@ igt_main
 		sync_ring(fd, ALL_ENGINES, ncpus, 150);
 	igt_subtest("forked-store-each")
 		store_ring(fd, ALL_ENGINES, ncpus, 150);
+	igt_subtest("wakeup-each")
+		wakeup_ring(fd, ALL_ENGINES, 150);
 
 	igt_subtest("basic-all")
 		sync_all(fd, 1, 5);
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 7/8] igt/gem_sync: Double the wakeups, twice the pain

[Chris Wilson]

To further defeat any contemplated spin-optimisations to avoid the irq
latency for synchronous wakeups, increase the queue length.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 tests/gem_sync.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index 747412c5d..60d61a02e 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -178,7 +178,7 @@ idle_ring(int fd, unsigned ring, int timeout)
 }
 
 static void
-wakeup_ring(int fd, unsigned ring, int timeout)
+wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 {
 	unsigned engines[16];
 	const char *names[16];
@@ -243,7 +243,8 @@ wakeup_ring(int fd, unsigned ring, int timeout)
 			while (!READ_ONCE(*spin->running))
 				;
 
-			gem_execbuf(fd, &execbuf);
+			for (int n = 0; n < wlen; n++)
+				gem_execbuf(fd, &execbuf);
 
 			this = gettime();
 			igt_spin_batch_end(spin);
@@ -851,7 +852,9 @@ igt_main
 		igt_subtest_f("idle-%s", e->name)
 			idle_ring(fd, e->exec_id | e->flags, 150);
 		igt_subtest_f("wakeup-%s", e->name)
-			wakeup_ring(fd, e->exec_id | e->flags, 150);
+			wakeup_ring(fd, e->exec_id | e->flags, 150, 1);
+		igt_subtest_f("double-wakeup-%s", e->name)
+			wakeup_ring(fd, e->exec_id | e->flags, 150, 2);
 		igt_subtest_f("store-%s", e->name)
 			store_ring(fd, e->exec_id | e->flags, 1, 150);
 		igt_subtest_f("many-%s", e->name)
@@ -873,7 +876,9 @@ igt_main
 	igt_subtest("forked-store-each")
 		store_ring(fd, ALL_ENGINES, ncpus, 150);
 	igt_subtest("wakeup-each")
-		wakeup_ring(fd, ALL_ENGINES, 150);
+		wakeup_ring(fd, ALL_ENGINES, 150, 1);
+	igt_subtest("double-wakeup-each")
+		wakeup_ring(fd, ALL_ENGINES, 150, 2);
 
 	igt_subtest("basic-all")
 		sync_all(fd, 1, 5);
-- 
2.18.0

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

[igt-dev] [PATCH i-g-t 8/8] igt/gem_sync: Show the baseline poll latency for wakeups

[Chris Wilson]

Distinguish between the latency required to switch away from the
pollable spinner into the target nops from the client wakeup of
synchronisation on the last nop.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 tests/gem_sync.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index 60d61a02e..493ae61df 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -207,7 +207,7 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 		const uint32_t bbe = MI_BATCH_BUFFER_END;
 		struct drm_i915_gem_exec_object2 object;
 		struct drm_i915_gem_execbuffer2 execbuf;
-		double end, this, elapsed, now;
+		double end, this, elapsed, now, baseline;
 		unsigned long cycles;
 		uint32_t cmd;
 		igt_spin_t *spin;
@@ -233,6 +233,32 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 		igt_spin_batch_end(spin);
 		gem_sync(fd, object.handle);
 
+		for (int warmup = 0; warmup <= 1; warmup++) {
+			end = gettime() + timeout/10.;
+			elapsed = 0;
+			cycles = 0;
+			do {
+				*spin->batch = cmd;
+				*spin->running = 0;
+				gem_execbuf(fd, &spin->execbuf);
+				while (!READ_ONCE(*spin->running))
+					;
+
+				this = gettime();
+				igt_spin_batch_end(spin);
+				gem_sync(fd, spin->handle);
+				now = gettime();
+
+				elapsed += now - this;
+				cycles++;
+			} while (now < end);
+			baseline = elapsed / cycles;
+		}
+		igt_info("%s%saseline %ld cycles: %.3f us\n",
+			 names[child % num_engines] ?: "",
+			 names[child % num_engines] ? " b" : "B",
+			 cycles, elapsed*1e6/cycles);
+
 		end = gettime() + timeout;
 		elapsed = 0;
 		cycles = 0;
@@ -254,16 +280,17 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 			elapsed += now - this;
 			cycles++;
 		} while (now < end);
+		elapsed -= cycles * baseline;
 
-		igt_info("%s%sompleted %ld cycles: %.3f us\n",
+		igt_info("%s%sompleted %ld cycles: %.3f + %.3f us\n",
 			 names[child % num_engines] ?: "",
 			 names[child % num_engines] ? " c" : "C",
-			 cycles, elapsed*1e6/cycles);
+			 cycles, 1e6*baseline, elapsed*1e6/cycles);
 
 		igt_spin_batch_free(fd, spin);
 		gem_close(fd, object.handle);
 	}
-	igt_waitchildren_timeout(timeout+10, NULL);
+	igt_waitchildren_timeout(2*timeout, NULL);
 	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
 }
 
-- 
2.18.0

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

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/8] lib: Report file cache as available system memory

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/8] lib: Report file cache as available system memory
URL   : https://patchwork.freedesktop.org/series/45427/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4373 -> IGTPW_1502 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1502 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1502, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45427/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1502:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-no-display:
      fi-snb-2600:        PASS -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in IGTPW_1502 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ringfill@basic-default-hang:
      fi-pnv-d510:        NOTRUN -> DMESG-WARN (fdo#101600)

    
    ==== Possible fixes ====

    igt@gem_exec_gttfill@basic:
      fi-byt-n2820:       FAIL (fdo#106744) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

    
  fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744


== Participating hosts (43 -> 39) ==

  Additional (1): fi-pnv-d510 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4529 -> IGTPW_1502

  CI_DRM_4373: be7193758db79443ad5dc45072a166746819ba7e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1502: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1502/
  IGT_4529: 23d50a49413aff619d00ec50fc2e051e9b45baa5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_sync@double-wakeup-blt
+igt@gem_sync@double-wakeup-bsd
+igt@gem_sync@double-wakeup-bsd1
+igt@gem_sync@double-wakeup-bsd2
+igt@gem_sync@double-wakeup-default
+igt@gem_sync@double-wakeup-each
+igt@gem_sync@double-wakeup-render
+igt@gem_sync@double-wakeup-vebox
+igt@gem_sync@wakeup-blt
+igt@gem_sync@wakeup-bsd
+igt@gem_sync@wakeup-bsd1
+igt@gem_sync@wakeup-bsd2
+igt@gem_sync@wakeup-default
+igt@gem_sync@wakeup-each
+igt@gem_sync@wakeup-render
+igt@gem_sync@wakeup-vebox

== Logs ==

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

Re: [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/8] lib: Report file cache as available system memory

[Chris Wilson]

Quoting Patchwork (2018-06-26 20:02:33)
> == Series Details ==
> 
> Series: series starting with [i-g-t,1/8] lib: Report file cache as available system memory
> URL   : https://patchwork.freedesktop.org/series/45427/
> State : failure
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4373 -> IGTPW_1502 =
> 
> == Summary - FAILURE ==
> 
>   Serious unknown changes coming with IGTPW_1502 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in IGTPW_1502, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/45427/revisions/1/mbox/
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in IGTPW_1502:
> 
>   === IGT changes ===
> 
>     ==== Possible regressions ====
> 
>     igt@drv_module_reload@basic-no-display:
>       fi-snb-2600:        PASS -> INCOMPLETE

Just bad timing to hit a recursive deadlock in hdmi_present_sense().

<6>[  456.622589] snd_hda_intel 0000:00:1b.0: failed to add i915 component master (-19)
<6>[  456.634323] snd_hda_codec_realtek hdaudioC0D0: autoconfig for ALC887-VD: line_outs=4 (0x14/0x15/0x16/0x17/0x0) type:line
<6>[  456.634332] snd_hda_codec_realtek hdaudioC0D0:    speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
<6>[  456.634334] snd_hda_codec_realtek hdaudioC0D0:    hp_outs=1 (0x1b/0x0/0x0/0x0/0x0)
<6>[  456.634336] snd_hda_codec_realtek hdaudioC0D0:    mono: mono_out=0x0
<6>[  456.634338] snd_hda_codec_realtek hdaudioC0D0:    dig-out=0x1e/0x0
<6>[  456.634340] snd_hda_codec_realtek hdaudioC0D0:    inputs:
<6>[  456.634343] snd_hda_codec_realtek hdaudioC0D0:      Front Mic=0x19
<6>[  456.634345] snd_hda_codec_realtek hdaudioC0D0:      Rear Mic=0x18
<6>[  456.634347] snd_hda_codec_realtek hdaudioC0D0:      Line=0x1a
<6>[  456.654732] snd_hda_codec_hdmi hdaudioC0D3: No i915 binding for Intel HDMI/DP codec
<6>[  456.659357] snd_hda_codec_hdmi hdaudioC0D3: No i915 binding for Intel HDMI/DP codec

<4>[  456.665407] ============================================
<4>[  456.665411] WARNING: possible recursive locking detected
<4>[  456.665416] 4.18.0-rc2-CI-CI_DRM_4373+ #1 Tainted: G     U           
<4>[  456.665421] --------------------------------------------
<4>[  456.665425] kworker/3:2/396 is trying to acquire lock:
<4>[  456.665430] 0000000026312358 (&spec->pcm_lock){+.+.}, at: hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665443] 
                  but task is already holding lock:
<4>[  456.665447] 0000000026312358 (&spec->pcm_lock){+.+.}, at: hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665458] 
                  other info that might help us debug this:
<4>[  456.665463]  Possible unsafe locking scenario:

<4>[  456.665468]        CPU0
<4>[  456.665470]        ----
<4>[  456.665473]   lock(&spec->pcm_lock);
<4>[  456.665477]   lock(&spec->pcm_lock);
<4>[  456.665481] 
                   *** DEADLOCK ***

<4>[  456.665488]  May be due to missing lock nesting notation

<4>[  456.665494] 3 locks held by kworker/3:2/396:
<4>[  456.665497]  #0: 000000006a321a6b ((wq_completion)"events"){+.+.}, at: process_one_work+0x1c2/0x6c0
<4>[  456.665510]  #1: 00000000c4e02304 ((work_completion)(&bus->unsol_work)){+.+.}, at: process_one_work+0x1c2/0x6c0
<4>[  456.665521]  #2: 0000000026312358 (&spec->pcm_lock){+.+.}, at: hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665532] 
                  stack backtrace:
<4>[  456.665539] CPU: 3 PID: 396 Comm: kworker/3:2 Tainted: G     U            4.18.0-rc2-CI-CI_DRM_4373+ #1
<4>[  456.665545] Hardware name: Dell Inc. XPS 8300  /0Y2MRG, BIOS A06 10/17/2011
<4>[  456.665555] Workqueue: events process_unsol_events [snd_hda_core]
<4>[  456.665561] Call Trace:
<4>[  456.665567]  dump_stack+0x67/0x9b
<4>[  456.665573]  __lock_acquire+0xc67/0x1b50
<4>[  456.665586]  ? mark_held_locks+0x50/0x80
<4>[  456.665596]  ? codec_exec_verb+0x94/0x100 [snd_hda_codec]
<4>[  456.665602]  ? lock_acquire+0xa6/0x210
<4>[  456.665607]  lock_acquire+0xa6/0x210
<4>[  456.665612]  ? hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665620]  __mutex_lock+0x8c/0x9c0
<4>[  456.665625]  ? hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665633]  ? hda_reg_write+0x20e/0x2f0 [snd_hda_core]
<4>[  456.665639]  ? hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665646]  ? regcache_sync+0x113/0x3f0
<4>[  456.665651]  ? regcache_sync_block+0xcb/0x260
<4>[  456.665657]  ? hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665664]  hdmi_present_sense+0x46/0x370 [snd_hda_codec_hdmi]
<4>[  456.665671]  ? rcu_read_lock_sched_held+0x6f/0x80
<4>[  456.665676]  ? regcache_sync+0x328/0x3f0
<4>[  456.665684]  ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec]
<4>[  456.665691]  generic_hdmi_resume+0x44/0x50 [snd_hda_codec_hdmi]
<4>[  456.665699]  hda_call_codec_resume+0xb6/0x110 [snd_hda_codec]
<4>[  456.665707]  hda_codec_runtime_resume+0x2c/0x40 [snd_hda_codec]
<4>[  456.665714]  __rpm_callback+0xb3/0x1b0
<4>[  456.665719]  rpm_callback+0x1a/0x70
<4>[  456.665727]  ? hda_call_codec_resume+0x110/0x110 [snd_hda_codec]
<4>[  456.665733]  rpm_resume+0x4f7/0x850
<4>[  456.665739]  __pm_runtime_resume+0x42/0x80
<4>[  456.665747]  codec_exec_verb+0x57/0x100 [snd_hda_codec]
<4>[  456.665755]  codec_read+0x39/0x70 [snd_hda_core]
<4>[  456.665764]  read_pin_sense+0x39/0xa0 [snd_hda_codec]
<4>[  456.665772]  jack_detect_update+0x8d/0xc0 [snd_hda_codec]
<4>[  456.665781]  snd_hda_pin_sense+0x55/0x60 [snd_hda_codec]
<4>[  456.665787]  hdmi_present_sense+0x162/0x370 [snd_hda_codec_hdmi]
<4>[  456.665795]  check_presence_and_report+0x59/0x80 [snd_hda_codec_hdmi]
<4>[  456.665802]  process_unsol_events+0x5d/0x70 [snd_hda_core]
<4>[  456.665809]  process_one_work+0x248/0x6c0
<4>[  456.665815]  worker_thread+0x37/0x380
<4>[  456.665821]  ? process_one_work+0x6c0/0x6c0
<4>[  456.665826]  kthread+0x119/0x130
<4>[  456.665831]  ? kthread_flush_work_fn+0x10/0x10
<4>[  456.665836]  ret_from_fork+0x3a/0x50
<6>[  456.667401] input: HDA Intel PCH Front Mic as /devices/pci0000:00/0000:00:1b.0/sound/card0/input29
<6>[  456.667703] input: HDA Intel PCH Rear Mic as /devices/pci0000:00/0000:00:1b.0/sound/card0/input30
<6>[  456.668004] input: HDA Intel PCH Line as /devices/pci0000:00/0000:00:1b.0/sound/card0/input31
<6>[  456.668317] input: HDA Intel PCH Line Out Front as /devices/pci0000:00/0000:00:1b.0/sound/card0/input32
<6>[  456.668614] input: HDA Intel PCH Line Out Surround as /devices/pci0000:00/0000:00:1b.0/sound/card0/input33
<6>[  456.668905] input: HDA Intel PCH Line Out CLFE as /devices/pci0000:00/0000:00:1b.0/sound/card0/input34
<6>[  456.669463] input: HDA Intel PCH Line Out Side as /devices/pci0000:00/0000:00:1b.0/sound/card0/input35
<6>[  456.669964] input: HDA Intel PCH Front Headphone as /devices/pci0000:00/0000:00:1b.0/sound/card0/input36
<6>[  456.670463] input: HDA Intel PCH HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1b.0/sound/card0/input37

hdmi_present_sense() is not handling a possible error,

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 7d7eb1354eee..3ca98c9fcaee 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1632,8 +1632,8 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
        int ret;
 
        /* no temporary power up/down needed for component notifier */
-       if (!codec_has_acomp(codec))
-               snd_hda_power_up_pm(codec);
+       if (!codec_has_acomp(codec) && snd_hda_power_up_pm(codec))
+               return false;
 
        mutex_lock(&spec->pcm_lock);
        if (codec_has_acomp(codec)) {

but

int snd_hdac_power_up_pm(struct hdac_device *codec)
{
        if (!atomic_inc_not_zero(&codec->in_pm))
                return snd_hdac_power_up(codec);
        return 0;
}

looks weird as how is in_pm actually incremented?

$ git grep in_pm -- sound/hda/
sound/hda/hdac_device.c:        atomic_set(&codec->in_pm, 0);
sound/hda/hdac_device.c:        if (!atomic_inc_not_zero(&codec->in_pm))
sound/hda/hdac_device.c:        if (!atomic_inc_not_zero(&codec->in_pm)) {
sound/hda/hdac_device.c:        if (atomic_dec_if_positive(&codec->in_pm) < 0)

as I read that in_pm always remains 0.
-Chris

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

Re: [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/8] lib: Report file cache as available system memory

[Takashi Iwai]

On Tue, 26 Jun 2018 21:25:58 +0200,
Chris Wilson wrote:
> 
> Just bad timing to hit a recursive deadlock in hdmi_present_sense().

Yeah, the runtime PM return value should be checked like your patch.
Care to send a proper patch?

However...

> <6>[  456.622589] snd_hda_intel 0000:00:1b.0: failed to add i915 component master (-19)
(snip)
> <6>[  456.654732] snd_hda_codec_hdmi hdaudioC0D3: No i915 binding for Intel HDMI/DP codec
> <6>[  456.659357] snd_hda_codec_hdmi hdaudioC0D3: No i915 binding for Intel HDMI/DP codec

... so i915 binding failed but still the HDMI codec got probed.
That's the unexpected point.  We should have stopped probing in such a
case.

The patch below prevents the generic driver binding in the case of
i915 binding error.

(snip)
> but
> 
> int snd_hdac_power_up_pm(struct hdac_device *codec)
> {
>         if (!atomic_inc_not_zero(&codec->in_pm))
>                 return snd_hdac_power_up(codec);
>         return 0;
> }
> 
> looks weird as how is in_pm actually incremented?
> 
> $ git grep in_pm -- sound/hda/
> sound/hda/hdac_device.c:        atomic_set(&codec->in_pm, 0);
> sound/hda/hdac_device.c:        if (!atomic_inc_not_zero(&codec->in_pm))
> sound/hda/hdac_device.c:        if (!atomic_inc_not_zero(&codec->in_pm)) {
> sound/hda/hdac_device.c:        if (atomic_dec_if_positive(&codec->in_pm) < 0)
> 
> as I read that in_pm always remains 0.

It's set in the codec driver side, i.e. in sound/pci/hda/hda_codec.c.


thanks,

Takashi

---
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 98e1c411c56a..6aab2aad9f8d 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2542,6 +2542,8 @@ static int alloc_intel_hdmi(struct hda_codec *codec)
 	/* requires i915 binding */
 	if (!codec->bus->core.audio_component) {
 		codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n");
+		/* set probe_id here to prevent generic fallback binding */
+		codec->probe_id = HDA_CODEC_ID_GENERIC_HDMI:
 		return -ENODEV;
 	}
 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev