[PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Chris Wilson]

We multiply the memfd 64k to create a 2G arena which we then attempt to
write into after marking read-only. Howver, when it comes to unlock the
arena after the test, performance tanks as the kernel tries to resolve
the 64k repeated mappings onto the same set of pages. (Must not be a
very common operation!) We can get away with just mlocking the backing
store to prevent its eviction, which should prevent the arena mapping
from being freed as well.

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

diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
index 909dd19df..c4e60ba92 100644
--- a/tests/i915/gem_userptr_blits.c
+++ b/tests/i915/gem_userptr_blits.c
@@ -1124,7 +1124,7 @@ static void test_readonly(int i915)
 		*(uint32_t *)(space + offset) = offset;
 	}
 	igt_assert_eq_u32(*(uint32_t *)pages, (uint32_t)(total - sz));
-	igt_assert(mlock(space, total) == 0);
+	igt_assert(mlock(pages, sz) == 0);
 	close(memfd);
 
 	/* Check we can create a normal userptr bo wrapping the wrapper */
@@ -1176,6 +1176,7 @@ static void test_readonly(int i915)
 	}
 	igt_waitchildren();
 
+	munlock(pages, sz);
 	munmap(space, total);
 	munmap(pages, sz);
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Chris Wilson]

We multiply the memfd 64k to create a 2G arena which we then attempt to
write into after marking read-only. Howver, when it comes to unlock the
arena after the test, performance tanks as the kernel tries to resolve
the 64k repeated mappings onto the same set of pages. (Must not be a
very common operation!) We can get away with just mlocking the backing
store to prevent its eviction, which should prevent the arena mapping
from being freed as well.

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

diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
index 909dd19df..c4e60ba92 100644
--- a/tests/i915/gem_userptr_blits.c
+++ b/tests/i915/gem_userptr_blits.c
@@ -1124,7 +1124,7 @@ static void test_readonly(int i915)
 		*(uint32_t *)(space + offset) = offset;
 	}
 	igt_assert_eq_u32(*(uint32_t *)pages, (uint32_t)(total - sz));
-	igt_assert(mlock(space, total) == 0);
+	igt_assert(mlock(pages, sz) == 0);
 	close(memfd);
 
 	/* Check we can create a normal userptr bo wrapping the wrapper */
@@ -1176,6 +1176,7 @@ static void test_readonly(int i915)
 	}
 	igt_waitchildren();
 
+	munlock(pages, sz);
 	munmap(space, total);
 	munmap(pages, sz);
 }
-- 
2.20.1

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

[PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Chris Wilson]

Use another sensitive CPU reloc to emit a chained batch from inside the
updated buffer to reduce the workload on slow machines to fit within the
CI timeout.

References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
 1 file changed, 189 insertions(+), 158 deletions(-)

diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
index 882c312d4..33c4e4e0f 100644
--- a/tests/i915/gem_cpu_reloc.c
+++ b/tests/i915/gem_cpu_reloc.c
@@ -59,214 +59,245 @@
 
 #include "intel_bufmgr.h"
 
-IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
+#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
 
-static uint32_t use_blt;
+IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
 
-static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
+static uint32_t *
+gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_relocation_entry gem_reloc[2];
-	struct drm_i915_gem_exec_object2 gem_exec[3];
-
-	gem_reloc[0].offset = 4 * sizeof(uint32_t);
-	gem_reloc[0].delta = 0;
-	gem_reloc[0].target_handle = dst;
-	gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
-	gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
-	gem_reloc[0].presumed_offset = -1;
-
-	gem_reloc[1].offset = 7 * sizeof(uint32_t);
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		gem_reloc[1].offset += sizeof(uint32_t);
-	gem_reloc[1].delta = 0;
-	gem_reloc[1].target_handle = src;
-	gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
-	gem_reloc[1].write_domain = 0;
-	gem_reloc[1].presumed_offset = -1;
-
-	memset(gem_exec, 0, sizeof(gem_exec));
-	gem_exec[0].handle = src;
-	gem_exec[1].handle = dst;
-	gem_exec[2].handle = batch;
-	gem_exec[2].relocation_count = 2;
-	gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
-
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(gem_exec);
-	execbuf.buffer_count = 3;
-	execbuf.batch_len = 4096;
-	execbuf.flags = use_blt;
-
-	gem_execbuf(fd, &execbuf);
+	*cs++ = MI_STORE_DWORD_IMM - 1;
+	addr->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	cs += 1; /* value: implicit 0xffffffff */
+	return cs;
+}
+static uint32_t *
+gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
+{
+	*cs++ = MI_STORE_DWORD_IMM;
+	*cs++ = 0;
+	addr->offset += 2 * sizeof(*cs);
+	cs += 1; /* addr */
+	cs += 1; /* value: implicit 0xffffffff */
+	return cs;
+}
+static uint32_t *
+gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
+{
+	*cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
+	addr->offset += sizeof(*cs);
+	igt_assert((addr->delta & 7) == 0);
+	cs += 2; /* addr */
+	cs += 2; /* value: implicit 0xffffffffffffffff */
+	return cs;
 }
 
-static void exec(int fd, uint32_t handle)
+static uint32_t *
+gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 gem_exec;
+	*cs++ = MI_BATCH_BUFFER_START | 2 << 6;
+	reloc->offset += sizeof(*cs);
+	reloc->delta += 1;
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
+	reloc->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	*cs++ = MI_BATCH_BUFFER_START | 1 << 8;
+	reloc->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
+	reloc->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	if (((uintptr_t)cs & 7) == 0) {
+		*cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
+		reloc->offset += sizeof(*cs);
+	}
 
-	memset(&gem_exec, 0, sizeof(gem_exec));
-	gem_exec.handle = handle;
+	*cs++ = MI_BATCH_BUFFER_START + 1;
+	reloc->offset += sizeof(*cs);
+	cs += 2; /* addr */
 
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(&gem_exec);
-	execbuf.buffer_count = 1;
-	execbuf.batch_len = 4096;
+	return cs;
+}
 
-	gem_execbuf(fd, &execbuf);
+static void *
+create_tmpl(int i915, struct drm_i915_gem_relocation_entry *reloc)
+{
+	const uint32_t devid = intel_get_drm_devid(i915);
+	const int gen = intel_gen(devid);
+	uint32_t *(*emit_store_addr)(uint32_t *cs,
+				   struct drm_i915_gem_relocation_entry *addr);
+	uint32_t *(*emit_bb_start)(uint32_t *cs,
+				   struct drm_i915_gem_relocation_entry *reloc);
+	void *tmpl;
+
+	/* could use BLT_FILL instead for gen2 */
+	igt_require(gem_can_store_dword(i915, 0));
+
+	if (gen >= 8)
+		emit_store_addr = gen8_emit_store_addr;
+	else if (gen >= 4)
+		emit_store_addr = gen4_emit_store_addr;
+	else
+		emit_store_addr = gen2_emit_store_addr;
+
+	if (gen >= 8)
+		emit_bb_start = gen8_emit_bb_start;
+	else if (IS_HASWELL(devid))
+		emit_bb_start = hsw_emit_bb_start;
+	else if (gen >= 6)
+		emit_bb_start = gen6_emit_bb_start;
+	else if (gen >= 4)
+		emit_bb_start = gen4_emit_bb_start;
+	else
+		emit_bb_start = gen2_emit_bb_start;
+
+	tmpl = malloc(4096);
+	igt_assert(tmpl);
+	memset(tmpl, 0xff, 4096);
+
+	/* Jump over the booby traps to the end */
+	reloc[0].delta = 64;
+	emit_bb_start(tmpl, &reloc[0]);
+
+	/* Restore the bad address to catch missing relocs */
+	reloc[1].offset = 64;
+	reloc[1].delta = reloc[0].offset;
+	*emit_store_addr(tmpl + 64, &reloc[1]) = MI_BATCH_BUFFER_END;
+
+	return tmpl;
 }
 
-uint32_t gen6_batch[] = {
-	(XY_SRC_COPY_BLT_CMD | 6 |
-	 XY_SRC_COPY_BLT_WRITE_ALPHA |
-	 XY_SRC_COPY_BLT_WRITE_RGB),
-	(3 << 24 | /* 32 bits */
-	 0xcc << 16 | /* copy ROP */
-	 4096),
-	0 << 16 | 0, /* dst x1, y1 */
-	1 << 16 | 2,
-	0, /* dst relocation */
-	0 << 16 | 0, /* src x1, y1 */
-	4096,
-	0, /* src relocation */
-	MI_BATCH_BUFFER_END,
-};
-
-uint32_t gen8_batch[] = {
-	(XY_SRC_COPY_BLT_CMD | 8 |
-	 XY_SRC_COPY_BLT_WRITE_ALPHA |
-	 XY_SRC_COPY_BLT_WRITE_RGB),
-	(3 << 24 | /* 32 bits */
-	 0xcc << 16 | /* copy ROP */
-	 4096),
-	0 << 16 | 0, /* dst x1, y1 */
-	1 << 16 | 2,
-	0, /* dst relocation */
-	0, /* FIXME */
-	0 << 16 | 0, /* src x1, y1 */
-	4096,
-	0, /* src relocation */
-	0, /* FIXME */
-	MI_BATCH_BUFFER_END,
-};
-
-uint32_t *batch = gen6_batch;
-uint32_t batch_size = sizeof(gen6_batch);
-
-static void run_test(int fd, int count)
+static void run_test(int i915, int count)
 {
-	const uint32_t hang[] = {-1, -1, -1, -1};
-	const uint32_t end[] = {MI_BATCH_BUFFER_END, 0};
-	uint32_t noop;
-	uint32_t *handles;
-	int i;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct drm_i915_gem_relocation_entry reloc[2];
+	struct drm_i915_gem_exec_object2 obj;
 
-	noop = intel_get_drm_devid(fd);
+	uint32_t *handles;
+	uint32_t *tmpl;
 
-	use_blt = 0;
-	if (intel_gen(noop) >= 6)
-		use_blt = I915_EXEC_BLT;
+	handles = malloc(count * sizeof(uint32_t));
+	igt_assert(handles);
 
-	if (intel_gen(noop) >= 8) {
-		batch = gen8_batch;
-		batch_size += 2 * 4;
+	memset(reloc, 0, sizeof(reloc));
+	tmpl = create_tmpl(i915, reloc);
+	for (int i = 0; i < count; i++) {
+		handles[i] = gem_create(i915, 4096);
+		gem_write(i915, handles[i], 0, tmpl, 4096);
 	}
+	free(tmpl);
 
-	handles = malloc (count * sizeof(uint32_t));
-	igt_assert(handles);
+	memset(&obj, 0, sizeof(obj));
+	obj.relocs_ptr = to_user_pointer(reloc);
+	obj.relocation_count = ARRAY_SIZE(reloc);
 
-	noop = gem_create(fd, 4096);
-	gem_write(fd, noop, 0, end, sizeof(end));
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(&obj);
+	execbuf.buffer_count = 1;
 
 	/* fill the entire gart with batches and run them */
-	for (i = 0; i < count; i++) {
-		uint32_t bad;
-
-		handles[i] = gem_create(fd, 4096);
-		gem_write(fd, handles[i], 0, batch, batch_size);
-
-		bad = gem_create(fd, 4096);
-		gem_write(fd, bad, 0, hang, sizeof(hang));
-		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
+	for (int i = 0; i < count; i++) {
+		obj.handle = handles[i];
 
-		/* launch the newly created batch */
-		copy(fd, handles[i], noop, bad);
-		exec(fd, bad);
-		gem_close(fd, bad);
+		reloc[0].target_handle = obj.handle;
+		reloc[0].presumed_offset = -1;
+		reloc[1].target_handle = obj.handle;
+		reloc[1].presumed_offset = -1;
 
-		igt_progress("gem_cpu_reloc: ", i, 2*count);
+		gem_execbuf(i915, &execbuf);
 	}
 
 	/* And again in reverse to try and catch the relocation code out */
-	for (i = 0; i < count; i++) {
-		uint32_t bad;
+	for (int i = 0; i < count; i++) {
+		obj.handle = handles[count - i - 1];
 
-		bad = gem_create(fd, 4096);
-		gem_write(fd, bad, 0, hang, sizeof(hang));
-		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
+		reloc[0].target_handle = obj.handle;
+		reloc[0].presumed_offset = -1;
+		reloc[1].target_handle = obj.handle;
+		reloc[1].presumed_offset = -1;
 
-		/* launch the newly created batch */
-		copy(fd, handles[count-i-1], noop, bad);
-		exec(fd, bad);
-		gem_close(fd, bad);
-
-		igt_progress("gem_cpu_reloc: ", count+i, 3*count);
+		gem_execbuf(i915, &execbuf);
 	}
 
-	/* Third time lucky? */
-	for (i = 0; i < count; i++) {
-		uint32_t bad;
+	/* Third time unlucky? */
+	for (int i = 0; i < count; i++) {
+		obj.handle = handles[i];
 
-		bad = gem_create(fd, 4096);
-		gem_write(fd, bad, 0, hang, sizeof(hang));
-		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
+		reloc[0].target_handle = obj.handle;
+		reloc[0].presumed_offset = -1;
+		reloc[1].target_handle = obj.handle;
+		reloc[1].presumed_offset = -1;
 
-		/* launch the newly created batch */
-		gem_set_domain(fd, handles[i],
-			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-		copy(fd, handles[i], noop, bad);
-		exec(fd, bad);
-		gem_close(fd, bad);
+		gem_set_domain(i915, obj.handle,
+			       I915_GEM_DOMAIN_CPU,
+			       I915_GEM_DOMAIN_CPU);
 
-		igt_progress("gem_cpu_reloc: ", 2*count+i, 3*count);
+		gem_execbuf(i915, &execbuf);
 	}
 
-	igt_info("Subtest suceeded, cleanup up - this might take a while.\n");
-	for (i = 0; i < count; i++) {
-		gem_close(fd, handles[i]);
-	}
-	gem_close(fd, noop);
+	for (int i = 0; i < count; i++)
+		gem_close(i915, handles[i]);
 	free(handles);
 }
 
 igt_main
 {
-	uint64_t aper_size;
-	int fd, count;
+	int i915;
 
 	igt_fixture {
-		fd = drm_open_driver(DRIVER_INTEL);
-		igt_require_gem(fd);
-	}
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
 
-	igt_subtest("basic") {
-		run_test (fd, 10);
+		igt_fork_hang_detector(i915);
 	}
 
+	igt_subtest("basic")
+		run_test(i915, 1);
 
 	igt_subtest("full") {
-		aper_size = gem_mappable_aperture_size();
-		count = aper_size / 4096 * 2;
+		uint64_t aper_size = gem_mappable_aperture_size();
+		unsigned long count = aper_size / 4096 + 1;
+
+		intel_require_memory(count, 4096, CHECK_RAM);
+
+		run_test(i915, count);
+	}
+
+	igt_subtest("forked") {
+		uint64_t aper_size = gem_mappable_aperture_size();
+		unsigned long count = aper_size / 4096 + 1;
+		int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
 
-		/* count + 2 (noop & bad) buffers. A gem object appears to
-                   require about 2kb + buffer + kernel overhead */
-		intel_require_memory(2+count, 2048+4096, CHECK_RAM);
+		intel_require_memory(count, 4096, CHECK_RAM);
 
-		run_test (fd, count);
+		igt_fork(child, ncpus)
+			run_test(i915, count / ncpus + 1);
+		igt_waitchildren();
 	}
 
 	igt_fixture {
-		close(fd);
+		igt_stop_hang_detector();
 	}
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Chris Wilson]

Use another sensitive CPU reloc to emit a chained batch from inside the
updated buffer to reduce the workload on slow machines to fit within the
CI timeout.

References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
 1 file changed, 189 insertions(+), 158 deletions(-)

diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
index 882c312d4..33c4e4e0f 100644
--- a/tests/i915/gem_cpu_reloc.c
+++ b/tests/i915/gem_cpu_reloc.c
@@ -59,214 +59,245 @@
 
 #include "intel_bufmgr.h"
 
-IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
+#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
 
-static uint32_t use_blt;
+IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
 
-static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
+static uint32_t *
+gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_relocation_entry gem_reloc[2];
-	struct drm_i915_gem_exec_object2 gem_exec[3];
-
-	gem_reloc[0].offset = 4 * sizeof(uint32_t);
-	gem_reloc[0].delta = 0;
-	gem_reloc[0].target_handle = dst;
-	gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
-	gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
-	gem_reloc[0].presumed_offset = -1;
-
-	gem_reloc[1].offset = 7 * sizeof(uint32_t);
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
-		gem_reloc[1].offset += sizeof(uint32_t);
-	gem_reloc[1].delta = 0;
-	gem_reloc[1].target_handle = src;
-	gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
-	gem_reloc[1].write_domain = 0;
-	gem_reloc[1].presumed_offset = -1;
-
-	memset(gem_exec, 0, sizeof(gem_exec));
-	gem_exec[0].handle = src;
-	gem_exec[1].handle = dst;
-	gem_exec[2].handle = batch;
-	gem_exec[2].relocation_count = 2;
-	gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
-
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(gem_exec);
-	execbuf.buffer_count = 3;
-	execbuf.batch_len = 4096;
-	execbuf.flags = use_blt;
-
-	gem_execbuf(fd, &execbuf);
+	*cs++ = MI_STORE_DWORD_IMM - 1;
+	addr->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	cs += 1; /* value: implicit 0xffffffff */
+	return cs;
+}
+static uint32_t *
+gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
+{
+	*cs++ = MI_STORE_DWORD_IMM;
+	*cs++ = 0;
+	addr->offset += 2 * sizeof(*cs);
+	cs += 1; /* addr */
+	cs += 1; /* value: implicit 0xffffffff */
+	return cs;
+}
+static uint32_t *
+gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
+{
+	*cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
+	addr->offset += sizeof(*cs);
+	igt_assert((addr->delta & 7) == 0);
+	cs += 2; /* addr */
+	cs += 2; /* value: implicit 0xffffffffffffffff */
+	return cs;
 }
 
-static void exec(int fd, uint32_t handle)
+static uint32_t *
+gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
 {
-	struct drm_i915_gem_execbuffer2 execbuf;
-	struct drm_i915_gem_exec_object2 gem_exec;
+	*cs++ = MI_BATCH_BUFFER_START | 2 << 6;
+	reloc->offset += sizeof(*cs);
+	reloc->delta += 1;
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
+	reloc->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	*cs++ = MI_BATCH_BUFFER_START | 1 << 8;
+	reloc->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
+	reloc->offset += sizeof(*cs);
+	cs += 1; /* addr */
+	return cs;
+}
+static uint32_t *
+gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
+{
+	if (((uintptr_t)cs & 7) == 0) {
+		*cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
+		reloc->offset += sizeof(*cs);
+	}
 
-	memset(&gem_exec, 0, sizeof(gem_exec));
-	gem_exec.handle = handle;
+	*cs++ = MI_BATCH_BUFFER_START + 1;
+	reloc->offset += sizeof(*cs);
+	cs += 2; /* addr */
 
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(&gem_exec);
-	execbuf.buffer_count = 1;
-	execbuf.batch_len = 4096;
+	return cs;
+}
 
-	gem_execbuf(fd, &execbuf);
+static void *
+create_tmpl(int i915, struct drm_i915_gem_relocation_entry *reloc)
+{
+	const uint32_t devid = intel_get_drm_devid(i915);
+	const int gen = intel_gen(devid);
+	uint32_t *(*emit_store_addr)(uint32_t *cs,
+				   struct drm_i915_gem_relocation_entry *addr);
+	uint32_t *(*emit_bb_start)(uint32_t *cs,
+				   struct drm_i915_gem_relocation_entry *reloc);
+	void *tmpl;
+
+	/* could use BLT_FILL instead for gen2 */
+	igt_require(gem_can_store_dword(i915, 0));
+
+	if (gen >= 8)
+		emit_store_addr = gen8_emit_store_addr;
+	else if (gen >= 4)
+		emit_store_addr = gen4_emit_store_addr;
+	else
+		emit_store_addr = gen2_emit_store_addr;
+
+	if (gen >= 8)
+		emit_bb_start = gen8_emit_bb_start;
+	else if (IS_HASWELL(devid))
+		emit_bb_start = hsw_emit_bb_start;
+	else if (gen >= 6)
+		emit_bb_start = gen6_emit_bb_start;
+	else if (gen >= 4)
+		emit_bb_start = gen4_emit_bb_start;
+	else
+		emit_bb_start = gen2_emit_bb_start;
+
+	tmpl = malloc(4096);
+	igt_assert(tmpl);
+	memset(tmpl, 0xff, 4096);
+
+	/* Jump over the booby traps to the end */
+	reloc[0].delta = 64;
+	emit_bb_start(tmpl, &reloc[0]);
+
+	/* Restore the bad address to catch missing relocs */
+	reloc[1].offset = 64;
+	reloc[1].delta = reloc[0].offset;
+	*emit_store_addr(tmpl + 64, &reloc[1]) = MI_BATCH_BUFFER_END;
+
+	return tmpl;
 }
 
-uint32_t gen6_batch[] = {
-	(XY_SRC_COPY_BLT_CMD | 6 |
-	 XY_SRC_COPY_BLT_WRITE_ALPHA |
-	 XY_SRC_COPY_BLT_WRITE_RGB),
-	(3 << 24 | /* 32 bits */
-	 0xcc << 16 | /* copy ROP */
-	 4096),
-	0 << 16 | 0, /* dst x1, y1 */
-	1 << 16 | 2,
-	0, /* dst relocation */
-	0 << 16 | 0, /* src x1, y1 */
-	4096,
-	0, /* src relocation */
-	MI_BATCH_BUFFER_END,
-};
-
-uint32_t gen8_batch[] = {
-	(XY_SRC_COPY_BLT_CMD | 8 |
-	 XY_SRC_COPY_BLT_WRITE_ALPHA |
-	 XY_SRC_COPY_BLT_WRITE_RGB),
-	(3 << 24 | /* 32 bits */
-	 0xcc << 16 | /* copy ROP */
-	 4096),
-	0 << 16 | 0, /* dst x1, y1 */
-	1 << 16 | 2,
-	0, /* dst relocation */
-	0, /* FIXME */
-	0 << 16 | 0, /* src x1, y1 */
-	4096,
-	0, /* src relocation */
-	0, /* FIXME */
-	MI_BATCH_BUFFER_END,
-};
-
-uint32_t *batch = gen6_batch;
-uint32_t batch_size = sizeof(gen6_batch);
-
-static void run_test(int fd, int count)
+static void run_test(int i915, int count)
 {
-	const uint32_t hang[] = {-1, -1, -1, -1};
-	const uint32_t end[] = {MI_BATCH_BUFFER_END, 0};
-	uint32_t noop;
-	uint32_t *handles;
-	int i;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct drm_i915_gem_relocation_entry reloc[2];
+	struct drm_i915_gem_exec_object2 obj;
 
-	noop = intel_get_drm_devid(fd);
+	uint32_t *handles;
+	uint32_t *tmpl;
 
-	use_blt = 0;
-	if (intel_gen(noop) >= 6)
-		use_blt = I915_EXEC_BLT;
+	handles = malloc(count * sizeof(uint32_t));
+	igt_assert(handles);
 
-	if (intel_gen(noop) >= 8) {
-		batch = gen8_batch;
-		batch_size += 2 * 4;
+	memset(reloc, 0, sizeof(reloc));
+	tmpl = create_tmpl(i915, reloc);
+	for (int i = 0; i < count; i++) {
+		handles[i] = gem_create(i915, 4096);
+		gem_write(i915, handles[i], 0, tmpl, 4096);
 	}
+	free(tmpl);
 
-	handles = malloc (count * sizeof(uint32_t));
-	igt_assert(handles);
+	memset(&obj, 0, sizeof(obj));
+	obj.relocs_ptr = to_user_pointer(reloc);
+	obj.relocation_count = ARRAY_SIZE(reloc);
 
-	noop = gem_create(fd, 4096);
-	gem_write(fd, noop, 0, end, sizeof(end));
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(&obj);
+	execbuf.buffer_count = 1;
 
 	/* fill the entire gart with batches and run them */
-	for (i = 0; i < count; i++) {
-		uint32_t bad;
-
-		handles[i] = gem_create(fd, 4096);
-		gem_write(fd, handles[i], 0, batch, batch_size);
-
-		bad = gem_create(fd, 4096);
-		gem_write(fd, bad, 0, hang, sizeof(hang));
-		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
+	for (int i = 0; i < count; i++) {
+		obj.handle = handles[i];
 
-		/* launch the newly created batch */
-		copy(fd, handles[i], noop, bad);
-		exec(fd, bad);
-		gem_close(fd, bad);
+		reloc[0].target_handle = obj.handle;
+		reloc[0].presumed_offset = -1;
+		reloc[1].target_handle = obj.handle;
+		reloc[1].presumed_offset = -1;
 
-		igt_progress("gem_cpu_reloc: ", i, 2*count);
+		gem_execbuf(i915, &execbuf);
 	}
 
 	/* And again in reverse to try and catch the relocation code out */
-	for (i = 0; i < count; i++) {
-		uint32_t bad;
+	for (int i = 0; i < count; i++) {
+		obj.handle = handles[count - i - 1];
 
-		bad = gem_create(fd, 4096);
-		gem_write(fd, bad, 0, hang, sizeof(hang));
-		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
+		reloc[0].target_handle = obj.handle;
+		reloc[0].presumed_offset = -1;
+		reloc[1].target_handle = obj.handle;
+		reloc[1].presumed_offset = -1;
 
-		/* launch the newly created batch */
-		copy(fd, handles[count-i-1], noop, bad);
-		exec(fd, bad);
-		gem_close(fd, bad);
-
-		igt_progress("gem_cpu_reloc: ", count+i, 3*count);
+		gem_execbuf(i915, &execbuf);
 	}
 
-	/* Third time lucky? */
-	for (i = 0; i < count; i++) {
-		uint32_t bad;
+	/* Third time unlucky? */
+	for (int i = 0; i < count; i++) {
+		obj.handle = handles[i];
 
-		bad = gem_create(fd, 4096);
-		gem_write(fd, bad, 0, hang, sizeof(hang));
-		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
+		reloc[0].target_handle = obj.handle;
+		reloc[0].presumed_offset = -1;
+		reloc[1].target_handle = obj.handle;
+		reloc[1].presumed_offset = -1;
 
-		/* launch the newly created batch */
-		gem_set_domain(fd, handles[i],
-			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-		copy(fd, handles[i], noop, bad);
-		exec(fd, bad);
-		gem_close(fd, bad);
+		gem_set_domain(i915, obj.handle,
+			       I915_GEM_DOMAIN_CPU,
+			       I915_GEM_DOMAIN_CPU);
 
-		igt_progress("gem_cpu_reloc: ", 2*count+i, 3*count);
+		gem_execbuf(i915, &execbuf);
 	}
 
-	igt_info("Subtest suceeded, cleanup up - this might take a while.\n");
-	for (i = 0; i < count; i++) {
-		gem_close(fd, handles[i]);
-	}
-	gem_close(fd, noop);
+	for (int i = 0; i < count; i++)
+		gem_close(i915, handles[i]);
 	free(handles);
 }
 
 igt_main
 {
-	uint64_t aper_size;
-	int fd, count;
+	int i915;
 
 	igt_fixture {
-		fd = drm_open_driver(DRIVER_INTEL);
-		igt_require_gem(fd);
-	}
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
 
-	igt_subtest("basic") {
-		run_test (fd, 10);
+		igt_fork_hang_detector(i915);
 	}
 
+	igt_subtest("basic")
+		run_test(i915, 1);
 
 	igt_subtest("full") {
-		aper_size = gem_mappable_aperture_size();
-		count = aper_size / 4096 * 2;
+		uint64_t aper_size = gem_mappable_aperture_size();
+		unsigned long count = aper_size / 4096 + 1;
+
+		intel_require_memory(count, 4096, CHECK_RAM);
+
+		run_test(i915, count);
+	}
+
+	igt_subtest("forked") {
+		uint64_t aper_size = gem_mappable_aperture_size();
+		unsigned long count = aper_size / 4096 + 1;
+		int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
 
-		/* count + 2 (noop & bad) buffers. A gem object appears to
-                   require about 2kb + buffer + kernel overhead */
-		intel_require_memory(2+count, 2048+4096, CHECK_RAM);
+		intel_require_memory(count, 4096, CHECK_RAM);
 
-		run_test (fd, count);
+		igt_fork(child, ncpus)
+			run_test(i915, count / ncpus + 1);
+		igt_waitchildren();
 	}
 
 	igt_fixture {
-		close(fd);
+		igt_stop_hang_detector();
 	}
 }
-- 
2.20.1

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

[PATCH i-g-t 3/3] igt/drv_missed_irq: Skip if the kernel reports no rings available to test

[Chris Wilson]

Some setups (e.g. guc and gen10+) can not disable the MI_USER_INTERRUPT
generation and so can not simulate missed interrupts. These tests would
fail, so skip when the kernel reports no tests available.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_gt.c            | 2 +-
 tests/i915/missed_irq.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index a20619246..dd28d821d 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -535,7 +535,7 @@ unsigned intel_detect_and_clear_missed_interrupts(int fd)
 	dir = igt_debugfs_dir(fd);
 
 	missed = 0;
-	igt_assert(igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed) == 1);
+	igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed);
 	if (missed)
 		igt_sysfs_set(dir, "i915_ring_missed_irq", "0");
 
diff --git a/tests/i915/missed_irq.c b/tests/i915/missed_irq.c
index 78690c36a..cade3f371 100644
--- a/tests/i915/missed_irq.c
+++ b/tests/i915/missed_irq.c
@@ -113,6 +113,7 @@ igt_simple_main
 	debugfs = igt_debugfs_dir(device);
 
 	expect_rings = engine_mask(debugfs);
+	igt_require(expect_rings);
 
 	igt_debug("Clearing rings %x\n", expect_rings);
 	intel_detect_and_clear_missed_interrupts(device);
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 3/3] igt/drv_missed_irq: Skip if the kernel reports no rings available to test

[Chris Wilson]

Some setups (e.g. guc and gen10+) can not disable the MI_USER_INTERRUPT
generation and so can not simulate missed interrupts. These tests would
fail, so skip when the kernel reports no tests available.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_gt.c            | 2 +-
 tests/i915/missed_irq.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index a20619246..dd28d821d 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -535,7 +535,7 @@ unsigned intel_detect_and_clear_missed_interrupts(int fd)
 	dir = igt_debugfs_dir(fd);
 
 	missed = 0;
-	igt_assert(igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed) == 1);
+	igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed);
 	if (missed)
 		igt_sysfs_set(dir, "i915_ring_missed_irq", "0");
 
diff --git a/tests/i915/missed_irq.c b/tests/i915/missed_irq.c
index 78690c36a..cade3f371 100644
--- a/tests/i915/missed_irq.c
+++ b/tests/i915/missed_irq.c
@@ -113,6 +113,7 @@ igt_simple_main
 	debugfs = igt_debugfs_dir(device);
 
 	expect_rings = engine_mask(debugfs);
+	igt_require(expect_rings);
 
 	igt_debug("Clearing rings %x\n", expect_rings);
 	intel_detect_and_clear_missed_interrupts(device);
-- 
2.20.1

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

Re: [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> We multiply the memfd 64k to create a 2G arena which we then attempt to
> write into after marking read-only. Howver, when it comes to unlock the

s/Howver/However

> arena after the test, performance tanks as the kernel tries to resolve
> the 64k repeated mappings onto the same set of pages. (Must not be a
> very common operation!) We can get away with just mlocking the backing
> store to prevent its eviction, which should prevent the arena mapping
> from being freed as well.

hmm should. How are they bound?

-Mika

>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=108887
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_userptr_blits.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
> index 909dd19df..c4e60ba92 100644
> --- a/tests/i915/gem_userptr_blits.c
> +++ b/tests/i915/gem_userptr_blits.c
> @@ -1124,7 +1124,7 @@ static void test_readonly(int i915)
>  		*(uint32_t *)(space + offset) = offset;
>  	}
>  	igt_assert_eq_u32(*(uint32_t *)pages, (uint32_t)(total - sz));
> -	igt_assert(mlock(space, total) == 0);
> +	igt_assert(mlock(pages, sz) == 0);
>  	close(memfd);
>  
>  	/* Check we can create a normal userptr bo wrapping the wrapper */
> @@ -1176,6 +1176,7 @@ static void test_readonly(int i915)
>  	}
>  	igt_waitchildren();
>  
> +	munlock(pages, sz);
>  	munmap(space, total);
>  	munmap(pages, sz);
>  }
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> We multiply the memfd 64k to create a 2G arena which we then attempt to
> write into after marking read-only. Howver, when it comes to unlock the

s/Howver/However

> arena after the test, performance tanks as the kernel tries to resolve
> the 64k repeated mappings onto the same set of pages. (Must not be a
> very common operation!) We can get away with just mlocking the backing
> store to prevent its eviction, which should prevent the arena mapping
> from being freed as well.

hmm should. How are they bound?

-Mika

>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=108887
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_userptr_blits.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
> index 909dd19df..c4e60ba92 100644
> --- a/tests/i915/gem_userptr_blits.c
> +++ b/tests/i915/gem_userptr_blits.c
> @@ -1124,7 +1124,7 @@ static void test_readonly(int i915)
>  		*(uint32_t *)(space + offset) = offset;
>  	}
>  	igt_assert_eq_u32(*(uint32_t *)pages, (uint32_t)(total - sz));
> -	igt_assert(mlock(space, total) == 0);
> +	igt_assert(mlock(pages, sz) == 0);
>  	close(memfd);
>  
>  	/* Check we can create a normal userptr bo wrapping the wrapper */
> @@ -1176,6 +1176,7 @@ static void test_readonly(int i915)
>  	}
>  	igt_waitchildren();
>  
> +	munlock(pages, sz);
>  	munmap(space, total);
>  	munmap(pages, sz);
>  }
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Chris Wilson]

Quoting Mika Kuoppala (2019-01-16 09:47:27)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > We multiply the memfd 64k to create a 2G arena which we then attempt to
> > write into after marking read-only. Howver, when it comes to unlock the
> 
> s/Howver/However
> 
> > arena after the test, performance tanks as the kernel tries to resolve
> > the 64k repeated mappings onto the same set of pages. (Must not be a
> > very common operation!) We can get away with just mlocking the backing
> > store to prevent its eviction, which should prevent the arena mapping
> > from being freed as well.
> 
> hmm should. How are they bound?

All I'm worried about are the allocs for the pud/pmd etc, which aiui are
not freed until the pte are removed and the pte shouldn't be reaped
because the struct page are locked. However, I haven't actually verified
that mlocking the underlying pages is enough to be sure that the page
tables of the various mappings are safe from eviction. On the other
hand, munlock_vma_range doesn't scale to the abuse we put it to, and
that is causing issues for CI!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Chris Wilson]

Quoting Mika Kuoppala (2019-01-16 09:47:27)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > We multiply the memfd 64k to create a 2G arena which we then attempt to
> > write into after marking read-only. Howver, when it comes to unlock the
> 
> s/Howver/However
> 
> > arena after the test, performance tanks as the kernel tries to resolve
> > the 64k repeated mappings onto the same set of pages. (Must not be a
> > very common operation!) We can get away with just mlocking the backing
> > store to prevent its eviction, which should prevent the arena mapping
> > from being freed as well.
> 
> hmm should. How are they bound?

All I'm worried about are the allocs for the pud/pmd etc, which aiui are
not freed until the pte are removed and the pte shouldn't be reaped
because the struct page are locked. However, I haven't actually verified
that mlocking the underlying pages is enough to be sure that the page
tables of the various mappings are safe from eviction. On the other
hand, munlock_vma_range doesn't scale to the abuse we put it to, and
that is causing issues for CI!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena
URL   : https://patchwork.freedesktop.org/series/55283/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5432 -> IGTPW_2244
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         PASS -> FAIL [fdo#104008]

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (46 -> 39)
------------------------------

  Additional (1): fi-hsw-4770r 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-skl-6770hq fi-byt-squawks fi-bsw-cyan fi-whl-u fi-ivb-3770 fi-skl-6600u 


Build changes
-------------

    * IGT: IGT_4773 -> IGTPW_2244

  CI_DRM_5432: e46e3295ab2a45098fd98ed1201d63578f53191b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2244: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2244/
  IGT_4773: 951e2b1a016b750544d0f42459b13b9c70631c68 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_cpu_reloc@forked

== Logs ==

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

Re: [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-01-16 09:47:27)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > We multiply the memfd 64k to create a 2G arena which we then attempt to
>> > write into after marking read-only. Howver, when it comes to unlock the
>> 
>> s/Howver/However
>> 
>> > arena after the test, performance tanks as the kernel tries to resolve
>> > the 64k repeated mappings onto the same set of pages. (Must not be a
>> > very common operation!) We can get away with just mlocking the backing
>> > store to prevent its eviction, which should prevent the arena mapping
>> > from being freed as well.
>> 
>> hmm should. How are they bound?
>
> All I'm worried about are the allocs for the pud/pmd etc, which aiui are
> not freed until the pte are removed and the pte shouldn't be reaped
> because the struct page are locked. However, I haven't actually verified
> that mlocking the underlying pages is enough to be sure that the page
> tables of the various mappings are safe from eviction. On the other
> hand, munlock_vma_range doesn't scale to the abuse we put it to, and
> that is causing issues for CI!

If we can dodge it with this, great.
Noticed there is also typo in preceeding code,
the comment when mapping the arena. s/usuable/usable.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-01-16 09:47:27)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > We multiply the memfd 64k to create a 2G arena which we then attempt to
>> > write into after marking read-only. Howver, when it comes to unlock the
>> 
>> s/Howver/However
>> 
>> > arena after the test, performance tanks as the kernel tries to resolve
>> > the 64k repeated mappings onto the same set of pages. (Must not be a
>> > very common operation!) We can get away with just mlocking the backing
>> > store to prevent its eviction, which should prevent the arena mapping
>> > from being freed as well.
>> 
>> hmm should. How are they bound?
>
> All I'm worried about are the allocs for the pud/pmd etc, which aiui are
> not freed until the pte are removed and the pte shouldn't be reaped
> because the struct page are locked. However, I haven't actually verified
> that mlocking the underlying pages is enough to be sure that the page
> tables of the various mappings are safe from eviction. On the other
> hand, munlock_vma_range doesn't scale to the abuse we put it to, and
> that is causing issues for CI!

If we can dodge it with this, great.
Noticed there is also typo in preceeding code,
the comment when mapping the arena. s/usuable/usable.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Chris Wilson]

Quoting Mika Kuoppala (2019-01-16 10:35:59)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-01-16 09:47:27)
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> 
> >> > We multiply the memfd 64k to create a 2G arena which we then attempt to
> >> > write into after marking read-only. Howver, when it comes to unlock the
> >> 
> >> s/Howver/However
> >> 
> >> > arena after the test, performance tanks as the kernel tries to resolve
> >> > the 64k repeated mappings onto the same set of pages. (Must not be a
> >> > very common operation!) We can get away with just mlocking the backing
> >> > store to prevent its eviction, which should prevent the arena mapping
> >> > from being freed as well.
> >> 
> >> hmm should. How are they bound?
> >
> > All I'm worried about are the allocs for the pud/pmd etc, which aiui are
> > not freed until the pte are removed and the pte shouldn't be reaped
> > because the struct page are locked. However, I haven't actually verified
> > that mlocking the underlying pages is enough to be sure that the page
> > tables of the various mappings are safe from eviction. On the other
> > hand, munlock_vma_range doesn't scale to the abuse we put it to, and
> > that is causing issues for CI!
> 
> If we can dodge it with this, great.

To be fair, it is just an optimisation to make sure we can use the whole
arena (checking against available address space) and that it won't be
change (and be refaulted, so the results should be transparent although
expensive) under testing. I can't recall any other reason for sticking
mlock in there.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Chris Wilson]

Quoting Mika Kuoppala (2019-01-16 10:35:59)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-01-16 09:47:27)
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> 
> >> > We multiply the memfd 64k to create a 2G arena which we then attempt to
> >> > write into after marking read-only. Howver, when it comes to unlock the
> >> 
> >> s/Howver/However
> >> 
> >> > arena after the test, performance tanks as the kernel tries to resolve
> >> > the 64k repeated mappings onto the same set of pages. (Must not be a
> >> > very common operation!) We can get away with just mlocking the backing
> >> > store to prevent its eviction, which should prevent the arena mapping
> >> > from being freed as well.
> >> 
> >> hmm should. How are they bound?
> >
> > All I'm worried about are the allocs for the pud/pmd etc, which aiui are
> > not freed until the pte are removed and the pte shouldn't be reaped
> > because the struct page are locked. However, I haven't actually verified
> > that mlocking the underlying pages is enough to be sure that the page
> > tables of the various mappings are safe from eviction. On the other
> > hand, munlock_vma_range doesn't scale to the abuse we put it to, and
> > that is causing issues for CI!
> 
> If we can dodge it with this, great.

To be fair, it is just an optimisation to make sure we can use the whole
arena (checking against available address space) and that it won't be
change (and be refaulted, so the results should be transparent although
expensive) under testing. I can't recall any other reason for sticking
mlock in there.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/3] i915/gem_userptr_blits: Only mlock the memfd once, not the arena
URL   : https://patchwork.freedesktop.org/series/55283/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5432_full -> IGTPW_2244_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]
    - shard-snb:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145] +1
    - shard-kbl:          PASS -> FAIL [fdo#107725] / [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-kbl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-glk:          PASS -> FAIL [fdo#103167] +5

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +2
    - shard-kbl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#108950]

  * igt@kms_setmode@basic:
    - shard-hsw:          PASS -> FAIL [fdo#99912]

  * igt@perf@short-reads:
    - shard-kbl:          PASS -> FAIL [fdo#103183]

  * igt@pm_rpm@system-suspend-execbuf:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  
#### Possible fixes ####

  * igt@gem_softpin@noreloc-s3:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-kbl:          FAIL [fdo#103167] -> PASS +2
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          FAIL [fdo#103167] -> PASS +5

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-glk:          INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2
    - shard-apl:          FAIL [fdo#103166] -> PASS +2
    - shard-kbl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          DMESG-FAIL [fdo#105763] / [fdo#106538] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-snb:          {SKIP} [fdo#109271] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103183]: https://bugs.freedesktop.org/show_bug.cgi?id=103183
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4773 -> IGTPW_2244
    * Piglit: piglit_4509 -> None

  CI_DRM_5432: e46e3295ab2a45098fd98ed1201d63578f53191b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2244: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2244/
  IGT_4773: 951e2b1a016b750544d0f42459b13b9c70631c68 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Use another sensitive CPU reloc to emit a chained batch from inside the
> updated buffer to reduce the workload on slow machines to fit within the
> CI timeout.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
>  1 file changed, 189 insertions(+), 158 deletions(-)
>
> diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
> index 882c312d4..33c4e4e0f 100644
> --- a/tests/i915/gem_cpu_reloc.c
> +++ b/tests/i915/gem_cpu_reloc.c
> @@ -59,214 +59,245 @@
>  
>  #include "intel_bufmgr.h"
>  
> -IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
> +#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
>  
> -static uint32_t use_blt;
> +IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
>  
> -static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
> +static uint32_t *
> +gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>  {
> -	struct drm_i915_gem_execbuffer2 execbuf;
> -	struct drm_i915_gem_relocation_entry gem_reloc[2];
> -	struct drm_i915_gem_exec_object2 gem_exec[3];
> -
> -	gem_reloc[0].offset = 4 * sizeof(uint32_t);
> -	gem_reloc[0].delta = 0;
> -	gem_reloc[0].target_handle = dst;
> -	gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> -	gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> -	gem_reloc[0].presumed_offset = -1;
> -
> -	gem_reloc[1].offset = 7 * sizeof(uint32_t);
> -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -		gem_reloc[1].offset += sizeof(uint32_t);
> -	gem_reloc[1].delta = 0;
> -	gem_reloc[1].target_handle = src;
> -	gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> -	gem_reloc[1].write_domain = 0;
> -	gem_reloc[1].presumed_offset = -1;
> -
> -	memset(gem_exec, 0, sizeof(gem_exec));
> -	gem_exec[0].handle = src;
> -	gem_exec[1].handle = dst;
> -	gem_exec[2].handle = batch;
> -	gem_exec[2].relocation_count = 2;
> -	gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
> -
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(gem_exec);
> -	execbuf.buffer_count = 3;
> -	execbuf.batch_len = 4096;
> -	execbuf.flags = use_blt;
> -
> -	gem_execbuf(fd, &execbuf);
> +	*cs++ = MI_STORE_DWORD_IMM - 1;
> +	addr->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	cs += 1; /* value: implicit 0xffffffff */
> +	return cs;
> +}
> +static uint32_t *
> +gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> +{
> +	*cs++ = MI_STORE_DWORD_IMM;
> +	*cs++ = 0;
> +	addr->offset += 2 * sizeof(*cs);
> +	cs += 1; /* addr */
> +	cs += 1; /* value: implicit 0xffffffff */
> +	return cs;
> +}
> +static uint32_t *
> +gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> +{
> +	*cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
> +	addr->offset += sizeof(*cs);
> +	igt_assert((addr->delta & 7) == 0);
> +	cs += 2; /* addr */
> +	cs += 2; /* value: implicit 0xffffffffffffffff */
> +	return cs;
>  }
>  
> -static void exec(int fd, uint32_t handle)
> +static uint32_t *
> +gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>  {
> -	struct drm_i915_gem_execbuffer2 execbuf;
> -	struct drm_i915_gem_exec_object2 gem_exec;
> +	*cs++ = MI_BATCH_BUFFER_START | 2 << 6;
> +	reloc->offset += sizeof(*cs);
> +	reloc->delta += 1;
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
> +	reloc->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	*cs++ = MI_BATCH_BUFFER_START | 1 << 8;
> +	reloc->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
> +	reloc->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	if (((uintptr_t)cs & 7) == 0) {
> +		*cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
> +		reloc->offset += sizeof(*cs);
> +	}

Align it so that after the bb start emitted we are in right alignment?
Otherwise it looks it should have '!' in it.
-Mika

>  
> -	memset(&gem_exec, 0, sizeof(gem_exec));
> -	gem_exec.handle = handle;
> +	*cs++ = MI_BATCH_BUFFER_START + 1;
> +	reloc->offset += sizeof(*cs);
> +	cs += 2; /* addr */
>  
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(&gem_exec);
> -	execbuf.buffer_count = 1;
> -	execbuf.batch_len = 4096;
> +	return cs;
> +}
>  
> -	gem_execbuf(fd, &execbuf);
> +static void *
> +create_tmpl(int i915, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	const uint32_t devid = intel_get_drm_devid(i915);
> +	const int gen = intel_gen(devid);
> +	uint32_t *(*emit_store_addr)(uint32_t *cs,
> +				   struct drm_i915_gem_relocation_entry *addr);
> +	uint32_t *(*emit_bb_start)(uint32_t *cs,
> +				   struct drm_i915_gem_relocation_entry *reloc);
> +	void *tmpl;
> +
> +	/* could use BLT_FILL instead for gen2 */
> +	igt_require(gem_can_store_dword(i915, 0));
> +
> +	if (gen >= 8)
> +		emit_store_addr = gen8_emit_store_addr;
> +	else if (gen >= 4)
> +		emit_store_addr = gen4_emit_store_addr;
> +	else
> +		emit_store_addr = gen2_emit_store_addr;
> +
> +	if (gen >= 8)
> +		emit_bb_start = gen8_emit_bb_start;
> +	else if (IS_HASWELL(devid))
> +		emit_bb_start = hsw_emit_bb_start;
> +	else if (gen >= 6)
> +		emit_bb_start = gen6_emit_bb_start;
> +	else if (gen >= 4)
> +		emit_bb_start = gen4_emit_bb_start;
> +	else
> +		emit_bb_start = gen2_emit_bb_start;
> +
> +	tmpl = malloc(4096);
> +	igt_assert(tmpl);
> +	memset(tmpl, 0xff, 4096);
> +
> +	/* Jump over the booby traps to the end */
> +	reloc[0].delta = 64;
> +	emit_bb_start(tmpl, &reloc[0]);
> +
> +	/* Restore the bad address to catch missing relocs */
> +	reloc[1].offset = 64;
> +	reloc[1].delta = reloc[0].offset;
> +	*emit_store_addr(tmpl + 64, &reloc[1]) = MI_BATCH_BUFFER_END;
> +
> +	return tmpl;
>  }
>  
> -uint32_t gen6_batch[] = {
> -	(XY_SRC_COPY_BLT_CMD | 6 |
> -	 XY_SRC_COPY_BLT_WRITE_ALPHA |
> -	 XY_SRC_COPY_BLT_WRITE_RGB),
> -	(3 << 24 | /* 32 bits */
> -	 0xcc << 16 | /* copy ROP */
> -	 4096),
> -	0 << 16 | 0, /* dst x1, y1 */
> -	1 << 16 | 2,
> -	0, /* dst relocation */
> -	0 << 16 | 0, /* src x1, y1 */
> -	4096,
> -	0, /* src relocation */
> -	MI_BATCH_BUFFER_END,
> -};
> -
> -uint32_t gen8_batch[] = {
> -	(XY_SRC_COPY_BLT_CMD | 8 |
> -	 XY_SRC_COPY_BLT_WRITE_ALPHA |
> -	 XY_SRC_COPY_BLT_WRITE_RGB),
> -	(3 << 24 | /* 32 bits */
> -	 0xcc << 16 | /* copy ROP */
> -	 4096),
> -	0 << 16 | 0, /* dst x1, y1 */
> -	1 << 16 | 2,
> -	0, /* dst relocation */
> -	0, /* FIXME */
> -	0 << 16 | 0, /* src x1, y1 */
> -	4096,
> -	0, /* src relocation */
> -	0, /* FIXME */
> -	MI_BATCH_BUFFER_END,
> -};
> -
> -uint32_t *batch = gen6_batch;
> -uint32_t batch_size = sizeof(gen6_batch);
> -
> -static void run_test(int fd, int count)
> +static void run_test(int i915, int count)
>  {
> -	const uint32_t hang[] = {-1, -1, -1, -1};
> -	const uint32_t end[] = {MI_BATCH_BUFFER_END, 0};
> -	uint32_t noop;
> -	uint32_t *handles;
> -	int i;
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	struct drm_i915_gem_relocation_entry reloc[2];
> +	struct drm_i915_gem_exec_object2 obj;
>  
> -	noop = intel_get_drm_devid(fd);
> +	uint32_t *handles;
> +	uint32_t *tmpl;
>  
> -	use_blt = 0;
> -	if (intel_gen(noop) >= 6)
> -		use_blt = I915_EXEC_BLT;
> +	handles = malloc(count * sizeof(uint32_t));
> +	igt_assert(handles);
>  
> -	if (intel_gen(noop) >= 8) {
> -		batch = gen8_batch;
> -		batch_size += 2 * 4;
> +	memset(reloc, 0, sizeof(reloc));
> +	tmpl = create_tmpl(i915, reloc);
> +	for (int i = 0; i < count; i++) {
> +		handles[i] = gem_create(i915, 4096);
> +		gem_write(i915, handles[i], 0, tmpl, 4096);
>  	}
> +	free(tmpl);
>  
> -	handles = malloc (count * sizeof(uint32_t));
> -	igt_assert(handles);
> +	memset(&obj, 0, sizeof(obj));
> +	obj.relocs_ptr = to_user_pointer(reloc);
> +	obj.relocation_count = ARRAY_SIZE(reloc);
>  
> -	noop = gem_create(fd, 4096);
> -	gem_write(fd, noop, 0, end, sizeof(end));
> +	memset(&execbuf, 0, sizeof(execbuf));
> +	execbuf.buffers_ptr = to_user_pointer(&obj);
> +	execbuf.buffer_count = 1;
>  
>  	/* fill the entire gart with batches and run them */
> -	for (i = 0; i < count; i++) {
> -		uint32_t bad;
> -
> -		handles[i] = gem_create(fd, 4096);
> -		gem_write(fd, handles[i], 0, batch, batch_size);
> -
> -		bad = gem_create(fd, 4096);
> -		gem_write(fd, bad, 0, hang, sizeof(hang));
> -		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
> +	for (int i = 0; i < count; i++) {
> +		obj.handle = handles[i];
>  
> -		/* launch the newly created batch */
> -		copy(fd, handles[i], noop, bad);
> -		exec(fd, bad);
> -		gem_close(fd, bad);
> +		reloc[0].target_handle = obj.handle;
> +		reloc[0].presumed_offset = -1;
> +		reloc[1].target_handle = obj.handle;
> +		reloc[1].presumed_offset = -1;
>  
> -		igt_progress("gem_cpu_reloc: ", i, 2*count);
> +		gem_execbuf(i915, &execbuf);
>  	}
>  
>  	/* And again in reverse to try and catch the relocation code out */
> -	for (i = 0; i < count; i++) {
> -		uint32_t bad;
> +	for (int i = 0; i < count; i++) {
> +		obj.handle = handles[count - i - 1];
>  
> -		bad = gem_create(fd, 4096);
> -		gem_write(fd, bad, 0, hang, sizeof(hang));
> -		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
> +		reloc[0].target_handle = obj.handle;
> +		reloc[0].presumed_offset = -1;
> +		reloc[1].target_handle = obj.handle;
> +		reloc[1].presumed_offset = -1;
>  
> -		/* launch the newly created batch */
> -		copy(fd, handles[count-i-1], noop, bad);
> -		exec(fd, bad);
> -		gem_close(fd, bad);
> -
> -		igt_progress("gem_cpu_reloc: ", count+i, 3*count);
> +		gem_execbuf(i915, &execbuf);
>  	}
>  
> -	/* Third time lucky? */
> -	for (i = 0; i < count; i++) {
> -		uint32_t bad;
> +	/* Third time unlucky? */
> +	for (int i = 0; i < count; i++) {
> +		obj.handle = handles[i];
>  
> -		bad = gem_create(fd, 4096);
> -		gem_write(fd, bad, 0, hang, sizeof(hang));
> -		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
> +		reloc[0].target_handle = obj.handle;
> +		reloc[0].presumed_offset = -1;
> +		reloc[1].target_handle = obj.handle;
> +		reloc[1].presumed_offset = -1;
>  
> -		/* launch the newly created batch */
> -		gem_set_domain(fd, handles[i],
> -			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> -		copy(fd, handles[i], noop, bad);
> -		exec(fd, bad);
> -		gem_close(fd, bad);
> +		gem_set_domain(i915, obj.handle,
> +			       I915_GEM_DOMAIN_CPU,
> +			       I915_GEM_DOMAIN_CPU);
>  
> -		igt_progress("gem_cpu_reloc: ", 2*count+i, 3*count);
> +		gem_execbuf(i915, &execbuf);
>  	}
>  
> -	igt_info("Subtest suceeded, cleanup up - this might take a while.\n");
> -	for (i = 0; i < count; i++) {
> -		gem_close(fd, handles[i]);
> -	}
> -	gem_close(fd, noop);
> +	for (int i = 0; i < count; i++)
> +		gem_close(i915, handles[i]);
>  	free(handles);
>  }
>  
>  igt_main
>  {
> -	uint64_t aper_size;
> -	int fd, count;
> +	int i915;
>  
>  	igt_fixture {
> -		fd = drm_open_driver(DRIVER_INTEL);
> -		igt_require_gem(fd);
> -	}
> +		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_require_gem(i915);
>  
> -	igt_subtest("basic") {
> -		run_test (fd, 10);
> +		igt_fork_hang_detector(i915);
>  	}
>  
> +	igt_subtest("basic")
> +		run_test(i915, 1);
>  
>  	igt_subtest("full") {
> -		aper_size = gem_mappable_aperture_size();
> -		count = aper_size / 4096 * 2;
> +		uint64_t aper_size = gem_mappable_aperture_size();
> +		unsigned long count = aper_size / 4096 + 1;
> +
> +		intel_require_memory(count, 4096, CHECK_RAM);
> +
> +		run_test(i915, count);
> +	}
> +
> +	igt_subtest("forked") {
> +		uint64_t aper_size = gem_mappable_aperture_size();
> +		unsigned long count = aper_size / 4096 + 1;
> +		int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
>  
> -		/* count + 2 (noop & bad) buffers. A gem object appears to
> -                   require about 2kb + buffer + kernel overhead */
> -		intel_require_memory(2+count, 2048+4096, CHECK_RAM);
> +		intel_require_memory(count, 4096, CHECK_RAM);
>  
> -		run_test (fd, count);
> +		igt_fork(child, ncpus)
> +			run_test(i915, count / ncpus + 1);
> +		igt_waitchildren();
>  	}
>  
>  	igt_fixture {
> -		close(fd);
> +		igt_stop_hang_detector();
>  	}
>  }
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Use another sensitive CPU reloc to emit a chained batch from inside the
> updated buffer to reduce the workload on slow machines to fit within the
> CI timeout.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
>  1 file changed, 189 insertions(+), 158 deletions(-)
>
> diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
> index 882c312d4..33c4e4e0f 100644
> --- a/tests/i915/gem_cpu_reloc.c
> +++ b/tests/i915/gem_cpu_reloc.c
> @@ -59,214 +59,245 @@
>  
>  #include "intel_bufmgr.h"
>  
> -IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
> +#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
>  
> -static uint32_t use_blt;
> +IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
>  
> -static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
> +static uint32_t *
> +gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>  {
> -	struct drm_i915_gem_execbuffer2 execbuf;
> -	struct drm_i915_gem_relocation_entry gem_reloc[2];
> -	struct drm_i915_gem_exec_object2 gem_exec[3];
> -
> -	gem_reloc[0].offset = 4 * sizeof(uint32_t);
> -	gem_reloc[0].delta = 0;
> -	gem_reloc[0].target_handle = dst;
> -	gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> -	gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> -	gem_reloc[0].presumed_offset = -1;
> -
> -	gem_reloc[1].offset = 7 * sizeof(uint32_t);
> -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> -		gem_reloc[1].offset += sizeof(uint32_t);
> -	gem_reloc[1].delta = 0;
> -	gem_reloc[1].target_handle = src;
> -	gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> -	gem_reloc[1].write_domain = 0;
> -	gem_reloc[1].presumed_offset = -1;
> -
> -	memset(gem_exec, 0, sizeof(gem_exec));
> -	gem_exec[0].handle = src;
> -	gem_exec[1].handle = dst;
> -	gem_exec[2].handle = batch;
> -	gem_exec[2].relocation_count = 2;
> -	gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
> -
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(gem_exec);
> -	execbuf.buffer_count = 3;
> -	execbuf.batch_len = 4096;
> -	execbuf.flags = use_blt;
> -
> -	gem_execbuf(fd, &execbuf);
> +	*cs++ = MI_STORE_DWORD_IMM - 1;
> +	addr->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	cs += 1; /* value: implicit 0xffffffff */
> +	return cs;
> +}
> +static uint32_t *
> +gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> +{
> +	*cs++ = MI_STORE_DWORD_IMM;
> +	*cs++ = 0;
> +	addr->offset += 2 * sizeof(*cs);
> +	cs += 1; /* addr */
> +	cs += 1; /* value: implicit 0xffffffff */
> +	return cs;
> +}
> +static uint32_t *
> +gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> +{
> +	*cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
> +	addr->offset += sizeof(*cs);
> +	igt_assert((addr->delta & 7) == 0);
> +	cs += 2; /* addr */
> +	cs += 2; /* value: implicit 0xffffffffffffffff */
> +	return cs;
>  }
>  
> -static void exec(int fd, uint32_t handle)
> +static uint32_t *
> +gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>  {
> -	struct drm_i915_gem_execbuffer2 execbuf;
> -	struct drm_i915_gem_exec_object2 gem_exec;
> +	*cs++ = MI_BATCH_BUFFER_START | 2 << 6;
> +	reloc->offset += sizeof(*cs);
> +	reloc->delta += 1;
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
> +	reloc->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	*cs++ = MI_BATCH_BUFFER_START | 1 << 8;
> +	reloc->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	*cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
> +	reloc->offset += sizeof(*cs);
> +	cs += 1; /* addr */
> +	return cs;
> +}
> +static uint32_t *
> +gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	if (((uintptr_t)cs & 7) == 0) {
> +		*cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
> +		reloc->offset += sizeof(*cs);
> +	}

Align it so that after the bb start emitted we are in right alignment?
Otherwise it looks it should have '!' in it.
-Mika

>  
> -	memset(&gem_exec, 0, sizeof(gem_exec));
> -	gem_exec.handle = handle;
> +	*cs++ = MI_BATCH_BUFFER_START + 1;
> +	reloc->offset += sizeof(*cs);
> +	cs += 2; /* addr */
>  
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(&gem_exec);
> -	execbuf.buffer_count = 1;
> -	execbuf.batch_len = 4096;
> +	return cs;
> +}
>  
> -	gem_execbuf(fd, &execbuf);
> +static void *
> +create_tmpl(int i915, struct drm_i915_gem_relocation_entry *reloc)
> +{
> +	const uint32_t devid = intel_get_drm_devid(i915);
> +	const int gen = intel_gen(devid);
> +	uint32_t *(*emit_store_addr)(uint32_t *cs,
> +				   struct drm_i915_gem_relocation_entry *addr);
> +	uint32_t *(*emit_bb_start)(uint32_t *cs,
> +				   struct drm_i915_gem_relocation_entry *reloc);
> +	void *tmpl;
> +
> +	/* could use BLT_FILL instead for gen2 */
> +	igt_require(gem_can_store_dword(i915, 0));
> +
> +	if (gen >= 8)
> +		emit_store_addr = gen8_emit_store_addr;
> +	else if (gen >= 4)
> +		emit_store_addr = gen4_emit_store_addr;
> +	else
> +		emit_store_addr = gen2_emit_store_addr;
> +
> +	if (gen >= 8)
> +		emit_bb_start = gen8_emit_bb_start;
> +	else if (IS_HASWELL(devid))
> +		emit_bb_start = hsw_emit_bb_start;
> +	else if (gen >= 6)
> +		emit_bb_start = gen6_emit_bb_start;
> +	else if (gen >= 4)
> +		emit_bb_start = gen4_emit_bb_start;
> +	else
> +		emit_bb_start = gen2_emit_bb_start;
> +
> +	tmpl = malloc(4096);
> +	igt_assert(tmpl);
> +	memset(tmpl, 0xff, 4096);
> +
> +	/* Jump over the booby traps to the end */
> +	reloc[0].delta = 64;
> +	emit_bb_start(tmpl, &reloc[0]);
> +
> +	/* Restore the bad address to catch missing relocs */
> +	reloc[1].offset = 64;
> +	reloc[1].delta = reloc[0].offset;
> +	*emit_store_addr(tmpl + 64, &reloc[1]) = MI_BATCH_BUFFER_END;
> +
> +	return tmpl;
>  }
>  
> -uint32_t gen6_batch[] = {
> -	(XY_SRC_COPY_BLT_CMD | 6 |
> -	 XY_SRC_COPY_BLT_WRITE_ALPHA |
> -	 XY_SRC_COPY_BLT_WRITE_RGB),
> -	(3 << 24 | /* 32 bits */
> -	 0xcc << 16 | /* copy ROP */
> -	 4096),
> -	0 << 16 | 0, /* dst x1, y1 */
> -	1 << 16 | 2,
> -	0, /* dst relocation */
> -	0 << 16 | 0, /* src x1, y1 */
> -	4096,
> -	0, /* src relocation */
> -	MI_BATCH_BUFFER_END,
> -};
> -
> -uint32_t gen8_batch[] = {
> -	(XY_SRC_COPY_BLT_CMD | 8 |
> -	 XY_SRC_COPY_BLT_WRITE_ALPHA |
> -	 XY_SRC_COPY_BLT_WRITE_RGB),
> -	(3 << 24 | /* 32 bits */
> -	 0xcc << 16 | /* copy ROP */
> -	 4096),
> -	0 << 16 | 0, /* dst x1, y1 */
> -	1 << 16 | 2,
> -	0, /* dst relocation */
> -	0, /* FIXME */
> -	0 << 16 | 0, /* src x1, y1 */
> -	4096,
> -	0, /* src relocation */
> -	0, /* FIXME */
> -	MI_BATCH_BUFFER_END,
> -};
> -
> -uint32_t *batch = gen6_batch;
> -uint32_t batch_size = sizeof(gen6_batch);
> -
> -static void run_test(int fd, int count)
> +static void run_test(int i915, int count)
>  {
> -	const uint32_t hang[] = {-1, -1, -1, -1};
> -	const uint32_t end[] = {MI_BATCH_BUFFER_END, 0};
> -	uint32_t noop;
> -	uint32_t *handles;
> -	int i;
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	struct drm_i915_gem_relocation_entry reloc[2];
> +	struct drm_i915_gem_exec_object2 obj;
>  
> -	noop = intel_get_drm_devid(fd);
> +	uint32_t *handles;
> +	uint32_t *tmpl;
>  
> -	use_blt = 0;
> -	if (intel_gen(noop) >= 6)
> -		use_blt = I915_EXEC_BLT;
> +	handles = malloc(count * sizeof(uint32_t));
> +	igt_assert(handles);
>  
> -	if (intel_gen(noop) >= 8) {
> -		batch = gen8_batch;
> -		batch_size += 2 * 4;
> +	memset(reloc, 0, sizeof(reloc));
> +	tmpl = create_tmpl(i915, reloc);
> +	for (int i = 0; i < count; i++) {
> +		handles[i] = gem_create(i915, 4096);
> +		gem_write(i915, handles[i], 0, tmpl, 4096);
>  	}
> +	free(tmpl);
>  
> -	handles = malloc (count * sizeof(uint32_t));
> -	igt_assert(handles);
> +	memset(&obj, 0, sizeof(obj));
> +	obj.relocs_ptr = to_user_pointer(reloc);
> +	obj.relocation_count = ARRAY_SIZE(reloc);
>  
> -	noop = gem_create(fd, 4096);
> -	gem_write(fd, noop, 0, end, sizeof(end));
> +	memset(&execbuf, 0, sizeof(execbuf));
> +	execbuf.buffers_ptr = to_user_pointer(&obj);
> +	execbuf.buffer_count = 1;
>  
>  	/* fill the entire gart with batches and run them */
> -	for (i = 0; i < count; i++) {
> -		uint32_t bad;
> -
> -		handles[i] = gem_create(fd, 4096);
> -		gem_write(fd, handles[i], 0, batch, batch_size);
> -
> -		bad = gem_create(fd, 4096);
> -		gem_write(fd, bad, 0, hang, sizeof(hang));
> -		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
> +	for (int i = 0; i < count; i++) {
> +		obj.handle = handles[i];
>  
> -		/* launch the newly created batch */
> -		copy(fd, handles[i], noop, bad);
> -		exec(fd, bad);
> -		gem_close(fd, bad);
> +		reloc[0].target_handle = obj.handle;
> +		reloc[0].presumed_offset = -1;
> +		reloc[1].target_handle = obj.handle;
> +		reloc[1].presumed_offset = -1;
>  
> -		igt_progress("gem_cpu_reloc: ", i, 2*count);
> +		gem_execbuf(i915, &execbuf);
>  	}
>  
>  	/* And again in reverse to try and catch the relocation code out */
> -	for (i = 0; i < count; i++) {
> -		uint32_t bad;
> +	for (int i = 0; i < count; i++) {
> +		obj.handle = handles[count - i - 1];
>  
> -		bad = gem_create(fd, 4096);
> -		gem_write(fd, bad, 0, hang, sizeof(hang));
> -		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
> +		reloc[0].target_handle = obj.handle;
> +		reloc[0].presumed_offset = -1;
> +		reloc[1].target_handle = obj.handle;
> +		reloc[1].presumed_offset = -1;
>  
> -		/* launch the newly created batch */
> -		copy(fd, handles[count-i-1], noop, bad);
> -		exec(fd, bad);
> -		gem_close(fd, bad);
> -
> -		igt_progress("gem_cpu_reloc: ", count+i, 3*count);
> +		gem_execbuf(i915, &execbuf);
>  	}
>  
> -	/* Third time lucky? */
> -	for (i = 0; i < count; i++) {
> -		uint32_t bad;
> +	/* Third time unlucky? */
> +	for (int i = 0; i < count; i++) {
> +		obj.handle = handles[i];
>  
> -		bad = gem_create(fd, 4096);
> -		gem_write(fd, bad, 0, hang, sizeof(hang));
> -		gem_write(fd, bad, 4096-sizeof(end), end, sizeof(end));
> +		reloc[0].target_handle = obj.handle;
> +		reloc[0].presumed_offset = -1;
> +		reloc[1].target_handle = obj.handle;
> +		reloc[1].presumed_offset = -1;
>  
> -		/* launch the newly created batch */
> -		gem_set_domain(fd, handles[i],
> -			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> -		copy(fd, handles[i], noop, bad);
> -		exec(fd, bad);
> -		gem_close(fd, bad);
> +		gem_set_domain(i915, obj.handle,
> +			       I915_GEM_DOMAIN_CPU,
> +			       I915_GEM_DOMAIN_CPU);
>  
> -		igt_progress("gem_cpu_reloc: ", 2*count+i, 3*count);
> +		gem_execbuf(i915, &execbuf);
>  	}
>  
> -	igt_info("Subtest suceeded, cleanup up - this might take a while.\n");
> -	for (i = 0; i < count; i++) {
> -		gem_close(fd, handles[i]);
> -	}
> -	gem_close(fd, noop);
> +	for (int i = 0; i < count; i++)
> +		gem_close(i915, handles[i]);
>  	free(handles);
>  }
>  
>  igt_main
>  {
> -	uint64_t aper_size;
> -	int fd, count;
> +	int i915;
>  
>  	igt_fixture {
> -		fd = drm_open_driver(DRIVER_INTEL);
> -		igt_require_gem(fd);
> -	}
> +		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_require_gem(i915);
>  
> -	igt_subtest("basic") {
> -		run_test (fd, 10);
> +		igt_fork_hang_detector(i915);
>  	}
>  
> +	igt_subtest("basic")
> +		run_test(i915, 1);
>  
>  	igt_subtest("full") {
> -		aper_size = gem_mappable_aperture_size();
> -		count = aper_size / 4096 * 2;
> +		uint64_t aper_size = gem_mappable_aperture_size();
> +		unsigned long count = aper_size / 4096 + 1;
> +
> +		intel_require_memory(count, 4096, CHECK_RAM);
> +
> +		run_test(i915, count);
> +	}
> +
> +	igt_subtest("forked") {
> +		uint64_t aper_size = gem_mappable_aperture_size();
> +		unsigned long count = aper_size / 4096 + 1;
> +		int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
>  
> -		/* count + 2 (noop & bad) buffers. A gem object appears to
> -                   require about 2kb + buffer + kernel overhead */
> -		intel_require_memory(2+count, 2048+4096, CHECK_RAM);
> +		intel_require_memory(count, 4096, CHECK_RAM);
>  
> -		run_test (fd, count);
> +		igt_fork(child, ncpus)
> +			run_test(i915, count / ncpus + 1);
> +		igt_waitchildren();
>  	}
>  
>  	igt_fixture {
> -		close(fd);
> +		igt_stop_hang_detector();
>  	}
>  }
> -- 
> 2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Chris Wilson]

Quoting Mika Kuoppala (2019-01-16 14:22:59)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Use another sensitive CPU reloc to emit a chained batch from inside the
> > updated buffer to reduce the workload on slow machines to fit within the
> > CI timeout.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
> >  1 file changed, 189 insertions(+), 158 deletions(-)
> >
> > diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
> > index 882c312d4..33c4e4e0f 100644
> > --- a/tests/i915/gem_cpu_reloc.c
> > +++ b/tests/i915/gem_cpu_reloc.c
> > @@ -59,214 +59,245 @@
> >  
> >  #include "intel_bufmgr.h"
> >  
> > -IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
> > +#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
> >  
> > -static uint32_t use_blt;
> > +IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
> >  
> > -static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
> > +static uint32_t *
> > +gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> >  {
> > -     struct drm_i915_gem_execbuffer2 execbuf;
> > -     struct drm_i915_gem_relocation_entry gem_reloc[2];
> > -     struct drm_i915_gem_exec_object2 gem_exec[3];
> > -
> > -     gem_reloc[0].offset = 4 * sizeof(uint32_t);
> > -     gem_reloc[0].delta = 0;
> > -     gem_reloc[0].target_handle = dst;
> > -     gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> > -     gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> > -     gem_reloc[0].presumed_offset = -1;
> > -
> > -     gem_reloc[1].offset = 7 * sizeof(uint32_t);
> > -     if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -             gem_reloc[1].offset += sizeof(uint32_t);
> > -     gem_reloc[1].delta = 0;
> > -     gem_reloc[1].target_handle = src;
> > -     gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> > -     gem_reloc[1].write_domain = 0;
> > -     gem_reloc[1].presumed_offset = -1;
> > -
> > -     memset(gem_exec, 0, sizeof(gem_exec));
> > -     gem_exec[0].handle = src;
> > -     gem_exec[1].handle = dst;
> > -     gem_exec[2].handle = batch;
> > -     gem_exec[2].relocation_count = 2;
> > -     gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
> > -
> > -     memset(&execbuf, 0, sizeof(execbuf));
> > -     execbuf.buffers_ptr = to_user_pointer(gem_exec);
> > -     execbuf.buffer_count = 3;
> > -     execbuf.batch_len = 4096;
> > -     execbuf.flags = use_blt;
> > -
> > -     gem_execbuf(fd, &execbuf);
> > +     *cs++ = MI_STORE_DWORD_IMM - 1;
> > +     addr->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     cs += 1; /* value: implicit 0xffffffff */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> > +{
> > +     *cs++ = MI_STORE_DWORD_IMM;
> > +     *cs++ = 0;
> > +     addr->offset += 2 * sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     cs += 1; /* value: implicit 0xffffffff */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> > +{
> > +     *cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
> > +     addr->offset += sizeof(*cs);
> > +     igt_assert((addr->delta & 7) == 0);
> > +     cs += 2; /* addr */
> > +     cs += 2; /* value: implicit 0xffffffffffffffff */
> > +     return cs;
> >  }
> >  
> > -static void exec(int fd, uint32_t handle)
> > +static uint32_t *
> > +gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> >  {
> > -     struct drm_i915_gem_execbuffer2 execbuf;
> > -     struct drm_i915_gem_exec_object2 gem_exec;
> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6;
> > +     reloc->offset += sizeof(*cs);
> > +     reloc->delta += 1;
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
> > +     reloc->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     *cs++ = MI_BATCH_BUFFER_START | 1 << 8;
> > +     reloc->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
> > +     reloc->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     if (((uintptr_t)cs & 7) == 0) {
> > +             *cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
> > +             reloc->offset += sizeof(*cs);
> > +     }
> 
> Align it so that after the bb start emitted we are in right alignment?
> Otherwise it looks it should have '!' in it.

It's so the batch buffer address is aligned to the qword (since we use a
qword MI_STORE_DATA_IMM which requires destination alignment)

Imagine s/reloc/addr/ might be a better clue.
-chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Chris Wilson]

Quoting Mika Kuoppala (2019-01-16 14:22:59)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Use another sensitive CPU reloc to emit a chained batch from inside the
> > updated buffer to reduce the workload on slow machines to fit within the
> > CI timeout.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
> >  1 file changed, 189 insertions(+), 158 deletions(-)
> >
> > diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
> > index 882c312d4..33c4e4e0f 100644
> > --- a/tests/i915/gem_cpu_reloc.c
> > +++ b/tests/i915/gem_cpu_reloc.c
> > @@ -59,214 +59,245 @@
> >  
> >  #include "intel_bufmgr.h"
> >  
> > -IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
> > +#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
> >  
> > -static uint32_t use_blt;
> > +IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
> >  
> > -static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
> > +static uint32_t *
> > +gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> >  {
> > -     struct drm_i915_gem_execbuffer2 execbuf;
> > -     struct drm_i915_gem_relocation_entry gem_reloc[2];
> > -     struct drm_i915_gem_exec_object2 gem_exec[3];
> > -
> > -     gem_reloc[0].offset = 4 * sizeof(uint32_t);
> > -     gem_reloc[0].delta = 0;
> > -     gem_reloc[0].target_handle = dst;
> > -     gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> > -     gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> > -     gem_reloc[0].presumed_offset = -1;
> > -
> > -     gem_reloc[1].offset = 7 * sizeof(uint32_t);
> > -     if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> > -             gem_reloc[1].offset += sizeof(uint32_t);
> > -     gem_reloc[1].delta = 0;
> > -     gem_reloc[1].target_handle = src;
> > -     gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> > -     gem_reloc[1].write_domain = 0;
> > -     gem_reloc[1].presumed_offset = -1;
> > -
> > -     memset(gem_exec, 0, sizeof(gem_exec));
> > -     gem_exec[0].handle = src;
> > -     gem_exec[1].handle = dst;
> > -     gem_exec[2].handle = batch;
> > -     gem_exec[2].relocation_count = 2;
> > -     gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
> > -
> > -     memset(&execbuf, 0, sizeof(execbuf));
> > -     execbuf.buffers_ptr = to_user_pointer(gem_exec);
> > -     execbuf.buffer_count = 3;
> > -     execbuf.batch_len = 4096;
> > -     execbuf.flags = use_blt;
> > -
> > -     gem_execbuf(fd, &execbuf);
> > +     *cs++ = MI_STORE_DWORD_IMM - 1;
> > +     addr->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     cs += 1; /* value: implicit 0xffffffff */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> > +{
> > +     *cs++ = MI_STORE_DWORD_IMM;
> > +     *cs++ = 0;
> > +     addr->offset += 2 * sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     cs += 1; /* value: implicit 0xffffffff */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
> > +{
> > +     *cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
> > +     addr->offset += sizeof(*cs);
> > +     igt_assert((addr->delta & 7) == 0);
> > +     cs += 2; /* addr */
> > +     cs += 2; /* value: implicit 0xffffffffffffffff */
> > +     return cs;
> >  }
> >  
> > -static void exec(int fd, uint32_t handle)
> > +static uint32_t *
> > +gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> >  {
> > -     struct drm_i915_gem_execbuffer2 execbuf;
> > -     struct drm_i915_gem_exec_object2 gem_exec;
> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6;
> > +     reloc->offset += sizeof(*cs);
> > +     reloc->delta += 1;
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
> > +     reloc->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     *cs++ = MI_BATCH_BUFFER_START | 1 << 8;
> > +     reloc->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
> > +     reloc->offset += sizeof(*cs);
> > +     cs += 1; /* addr */
> > +     return cs;
> > +}
> > +static uint32_t *
> > +gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
> > +{
> > +     if (((uintptr_t)cs & 7) == 0) {
> > +             *cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
> > +             reloc->offset += sizeof(*cs);
> > +     }
> 
> Align it so that after the bb start emitted we are in right alignment?
> Otherwise it looks it should have '!' in it.

It's so the batch buffer address is aligned to the qword (since we use a
qword MI_STORE_DATA_IMM which requires destination alignment)

Imagine s/reloc/addr/ might be a better clue.
-chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 3/3] igt/drv_missed_irq: Skip if the kernel reports no rings available to test

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Some setups (e.g. guc and gen10+) can not disable the MI_USER_INTERRUPT
> generation and so can not simulate missed interrupts. These tests would
> fail, so skip when the kernel reports no tests available.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  lib/igt_gt.c            | 2 +-
>  tests/i915/missed_irq.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index a20619246..dd28d821d 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -535,7 +535,7 @@ unsigned intel_detect_and_clear_missed_interrupts(int fd)
>  	dir = igt_debugfs_dir(fd);
>  
>  	missed = 0;
> -	igt_assert(igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed) == 1);
> +	igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed);
>  	if (missed)
>  		igt_sysfs_set(dir, "i915_ring_missed_irq", "0");
>  
> diff --git a/tests/i915/missed_irq.c b/tests/i915/missed_irq.c
> index 78690c36a..cade3f371 100644
> --- a/tests/i915/missed_irq.c
> +++ b/tests/i915/missed_irq.c
> @@ -113,6 +113,7 @@ igt_simple_main
>  	debugfs = igt_debugfs_dir(device);
>  
>  	expect_rings = engine_mask(debugfs);
> +	igt_require(expect_rings);
>  
>  	igt_debug("Clearing rings %x\n", expect_rings);
>  	intel_detect_and_clear_missed_interrupts(device);
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 3/3] igt/drv_missed_irq: Skip if the kernel reports no rings available to test

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Some setups (e.g. guc and gen10+) can not disable the MI_USER_INTERRUPT
> generation and so can not simulate missed interrupts. These tests would
> fail, so skip when the kernel reports no tests available.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  lib/igt_gt.c            | 2 +-
>  tests/i915/missed_irq.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index a20619246..dd28d821d 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -535,7 +535,7 @@ unsigned intel_detect_and_clear_missed_interrupts(int fd)
>  	dir = igt_debugfs_dir(fd);
>  
>  	missed = 0;
> -	igt_assert(igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed) == 1);
> +	igt_sysfs_scanf(dir, "i915_ring_missed_irq", "%x", &missed);
>  	if (missed)
>  		igt_sysfs_set(dir, "i915_ring_missed_irq", "0");
>  
> diff --git a/tests/i915/missed_irq.c b/tests/i915/missed_irq.c
> index 78690c36a..cade3f371 100644
> --- a/tests/i915/missed_irq.c
> +++ b/tests/i915/missed_irq.c
> @@ -113,6 +113,7 @@ igt_simple_main
>  	debugfs = igt_debugfs_dir(device);
>  
>  	expect_rings = engine_mask(debugfs);
> +	igt_require(expect_rings);
>  
>  	igt_debug("Clearing rings %x\n", expect_rings);
>  	intel_detect_and_clear_missed_interrupts(device);
> -- 
> 2.20.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-01-16 14:22:59)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Use another sensitive CPU reloc to emit a chained batch from inside the
>> > updated buffer to reduce the workload on slow machines to fit within the
>> > CI timeout.
>> >
>> > References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
>> >  1 file changed, 189 insertions(+), 158 deletions(-)
>> >
>> > diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
>> > index 882c312d4..33c4e4e0f 100644
>> > --- a/tests/i915/gem_cpu_reloc.c
>> > +++ b/tests/i915/gem_cpu_reloc.c
>> > @@ -59,214 +59,245 @@
>> >  
>> >  #include "intel_bufmgr.h"
>> >  
>> > -IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
>> > +#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
>> >  
>> > -static uint32_t use_blt;
>> > +IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
>> >  
>> > -static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
>> > +static uint32_t *
>> > +gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>> >  {
>> > -     struct drm_i915_gem_execbuffer2 execbuf;
>> > -     struct drm_i915_gem_relocation_entry gem_reloc[2];
>> > -     struct drm_i915_gem_exec_object2 gem_exec[3];
>> > -
>> > -     gem_reloc[0].offset = 4 * sizeof(uint32_t);
>> > -     gem_reloc[0].delta = 0;
>> > -     gem_reloc[0].target_handle = dst;
>> > -     gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
>> > -     gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
>> > -     gem_reloc[0].presumed_offset = -1;
>> > -
>> > -     gem_reloc[1].offset = 7 * sizeof(uint32_t);
>> > -     if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>> > -             gem_reloc[1].offset += sizeof(uint32_t);
>> > -     gem_reloc[1].delta = 0;
>> > -     gem_reloc[1].target_handle = src;
>> > -     gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
>> > -     gem_reloc[1].write_domain = 0;
>> > -     gem_reloc[1].presumed_offset = -1;
>> > -
>> > -     memset(gem_exec, 0, sizeof(gem_exec));
>> > -     gem_exec[0].handle = src;
>> > -     gem_exec[1].handle = dst;
>> > -     gem_exec[2].handle = batch;
>> > -     gem_exec[2].relocation_count = 2;
>> > -     gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
>> > -
>> > -     memset(&execbuf, 0, sizeof(execbuf));
>> > -     execbuf.buffers_ptr = to_user_pointer(gem_exec);
>> > -     execbuf.buffer_count = 3;
>> > -     execbuf.batch_len = 4096;
>> > -     execbuf.flags = use_blt;
>> > -
>> > -     gem_execbuf(fd, &execbuf);
>> > +     *cs++ = MI_STORE_DWORD_IMM - 1;
>> > +     addr->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     cs += 1; /* value: implicit 0xffffffff */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>> > +{
>> > +     *cs++ = MI_STORE_DWORD_IMM;
>> > +     *cs++ = 0;
>> > +     addr->offset += 2 * sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     cs += 1; /* value: implicit 0xffffffff */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>> > +{
>> > +     *cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
>> > +     addr->offset += sizeof(*cs);
>> > +     igt_assert((addr->delta & 7) == 0);
>> > +     cs += 2; /* addr */
>> > +     cs += 2; /* value: implicit 0xffffffffffffffff */
>> > +     return cs;
>> >  }
>> >  
>> > -static void exec(int fd, uint32_t handle)
>> > +static uint32_t *
>> > +gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> >  {
>> > -     struct drm_i915_gem_execbuffer2 execbuf;
>> > -     struct drm_i915_gem_exec_object2 gem_exec;
>> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6;
>> > +     reloc->offset += sizeof(*cs);
>> > +     reloc->delta += 1;
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
>> > +     reloc->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     *cs++ = MI_BATCH_BUFFER_START | 1 << 8;
>> > +     reloc->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
>> > +     reloc->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     if (((uintptr_t)cs & 7) == 0) {
>> > +             *cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
>> > +             reloc->offset += sizeof(*cs);
>> > +     }
>> 
>> Align it so that after the bb start emitted we are in right alignment?
>> Otherwise it looks it should have '!' in it.
>
> It's so the batch buffer address is aligned to the qword (since we use a
> qword MI_STORE_DATA_IMM which requires destination alignment)
>
> Imagine s/reloc/addr/ might be a better clue.

Ok. Some undefined bits on bb starts. But sometimes magic is warranted,
as in here distinction between trickery and magic is blurred. Alerting
and warming up the reader to avoid sprains.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> -chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/3] i915/gem_cpu_reloc: Use a self-modifying chained batch

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-01-16 14:22:59)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Use another sensitive CPU reloc to emit a chained batch from inside the
>> > updated buffer to reduce the workload on slow machines to fit within the
>> > CI timeout.
>> >
>> > References: https://bugs.freedesktop.org/show_bug.cgi?id=108248
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  tests/i915/gem_cpu_reloc.c | 347 ++++++++++++++++++++-----------------
>> >  1 file changed, 189 insertions(+), 158 deletions(-)
>> >
>> > diff --git a/tests/i915/gem_cpu_reloc.c b/tests/i915/gem_cpu_reloc.c
>> > index 882c312d4..33c4e4e0f 100644
>> > --- a/tests/i915/gem_cpu_reloc.c
>> > +++ b/tests/i915/gem_cpu_reloc.c
>> > @@ -59,214 +59,245 @@
>> >  
>> >  #include "intel_bufmgr.h"
>> >  
>> > -IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
>> > +#define MI_INSTR(opcode, flags) ((opcode) << 23 | (flags))
>> >  
>> > -static uint32_t use_blt;
>> > +IGT_TEST_DESCRIPTION("Test the relocations through the CPU domain.");
>> >  
>> > -static void copy(int fd, uint32_t batch, uint32_t src, uint32_t dst)
>> > +static uint32_t *
>> > +gen2_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>> >  {
>> > -     struct drm_i915_gem_execbuffer2 execbuf;
>> > -     struct drm_i915_gem_relocation_entry gem_reloc[2];
>> > -     struct drm_i915_gem_exec_object2 gem_exec[3];
>> > -
>> > -     gem_reloc[0].offset = 4 * sizeof(uint32_t);
>> > -     gem_reloc[0].delta = 0;
>> > -     gem_reloc[0].target_handle = dst;
>> > -     gem_reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
>> > -     gem_reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
>> > -     gem_reloc[0].presumed_offset = -1;
>> > -
>> > -     gem_reloc[1].offset = 7 * sizeof(uint32_t);
>> > -     if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>> > -             gem_reloc[1].offset += sizeof(uint32_t);
>> > -     gem_reloc[1].delta = 0;
>> > -     gem_reloc[1].target_handle = src;
>> > -     gem_reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
>> > -     gem_reloc[1].write_domain = 0;
>> > -     gem_reloc[1].presumed_offset = -1;
>> > -
>> > -     memset(gem_exec, 0, sizeof(gem_exec));
>> > -     gem_exec[0].handle = src;
>> > -     gem_exec[1].handle = dst;
>> > -     gem_exec[2].handle = batch;
>> > -     gem_exec[2].relocation_count = 2;
>> > -     gem_exec[2].relocs_ptr = to_user_pointer(gem_reloc);
>> > -
>> > -     memset(&execbuf, 0, sizeof(execbuf));
>> > -     execbuf.buffers_ptr = to_user_pointer(gem_exec);
>> > -     execbuf.buffer_count = 3;
>> > -     execbuf.batch_len = 4096;
>> > -     execbuf.flags = use_blt;
>> > -
>> > -     gem_execbuf(fd, &execbuf);
>> > +     *cs++ = MI_STORE_DWORD_IMM - 1;
>> > +     addr->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     cs += 1; /* value: implicit 0xffffffff */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen4_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>> > +{
>> > +     *cs++ = MI_STORE_DWORD_IMM;
>> > +     *cs++ = 0;
>> > +     addr->offset += 2 * sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     cs += 1; /* value: implicit 0xffffffff */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen8_emit_store_addr(uint32_t *cs, struct drm_i915_gem_relocation_entry *addr)
>> > +{
>> > +     *cs++ = (MI_STORE_DWORD_IMM | 1 << 21) + 1;
>> > +     addr->offset += sizeof(*cs);
>> > +     igt_assert((addr->delta & 7) == 0);
>> > +     cs += 2; /* addr */
>> > +     cs += 2; /* value: implicit 0xffffffffffffffff */
>> > +     return cs;
>> >  }
>> >  
>> > -static void exec(int fd, uint32_t handle)
>> > +static uint32_t *
>> > +gen2_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> >  {
>> > -     struct drm_i915_gem_execbuffer2 execbuf;
>> > -     struct drm_i915_gem_exec_object2 gem_exec;
>> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6;
>> > +     reloc->offset += sizeof(*cs);
>> > +     reloc->delta += 1;
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen4_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8;
>> > +     reloc->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen6_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     *cs++ = MI_BATCH_BUFFER_START | 1 << 8;
>> > +     reloc->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +hsw_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     *cs++ = MI_BATCH_BUFFER_START | 2 << 6 | 1 << 8 | 1 << 13;
>> > +     reloc->offset += sizeof(*cs);
>> > +     cs += 1; /* addr */
>> > +     return cs;
>> > +}
>> > +static uint32_t *
>> > +gen8_emit_bb_start(uint32_t *cs, struct drm_i915_gem_relocation_entry *reloc)
>> > +{
>> > +     if (((uintptr_t)cs & 7) == 0) {
>> > +             *cs++ = MI_NOOP; /* align addr for MI_STORE_DWORD_IMM */
>> > +             reloc->offset += sizeof(*cs);
>> > +     }
>> 
>> Align it so that after the bb start emitted we are in right alignment?
>> Otherwise it looks it should have '!' in it.
>
> It's so the batch buffer address is aligned to the qword (since we use a
> qword MI_STORE_DATA_IMM which requires destination alignment)
>
> Imagine s/reloc/addr/ might be a better clue.

Ok. Some undefined bits on bb starts. But sometimes magic is warranted,
as in here distinction between trickery and magic is blurred. Alerting
and warming up the reader to avoid sprains.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

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