[CI 1/5] igt/gem_workarounds: Read the workaround registers from the active context

[CI 1/5] igt/gem_workarounds: Read the workaround registers from the active context

[Chris Wilson]

The workarounds are only valid whilst the GPU is active. To be sure we
are reading the registers in the right state, issue the reads from the GPU.

v2: Show ignored write-only failures as debug.

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

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 5e30a7b8..95ec250a 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -61,20 +61,6 @@ static struct write_only_list {
 static struct intel_wa_reg *wa_regs;
 static int num_wa_regs;
 
-static void wait_gpu(void)
-{
-	int fd = drm_open_driver(DRIVER_INTEL);
-	gem_quiescent_gpu(fd);
-	close(fd);
-}
-
-static void test_hang_gpu(void)
-{
-	int fd = drm_open_driver(DRIVER_INTEL);
-	igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT));
-	close(fd);
-}
-
 static void test_suspend_resume(void)
 {
 	igt_info("Suspending the device ...\n");
@@ -96,49 +82,95 @@ static bool write_only(const uint32_t addr)
 	return false;
 }
 
-static int workaround_fail_count(void)
-{
-	int i, fail_count = 0;
-
-	/* There is a small delay after coming ot of rc6 to the correct
-	   render context values will get loaded by hardware (bdw,chv).
-	   This here ensures that we have the correct context loaded before
-	   we start to read values */
-	wait_gpu();
+#define MI_STORE_REGISTER_MEM (0x24 << 23)
 
-	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
+static int workaround_fail_count(int fd)
+{
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry *reloc;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	uint32_t result_sz, batch_sz;
+	uint32_t *base, *out;
+	int fail_count = 0;
+
+	reloc = calloc(num_wa_regs, sizeof(*reloc));
+	igt_assert(reloc);
+
+	result_sz = 4 * num_wa_regs;
+	result_sz = (result_sz + 4095) & -4096;
+
+	batch_sz = 16 * num_wa_regs;
+	batch_sz = (batch_sz + 4 + 4095) & -4096;
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(fd, result_sz);
+	gem_set_caching(fd, obj[0].handle, 1);
+	obj[1].handle = gem_create(fd, batch_sz);
+	obj[1].relocs_ptr = to_user_pointer(reloc);
+	obj[1].relocation_count = num_wa_regs;
+
+	out = base = gem_mmap__cpu(fd, obj[1].handle, 0, batch_sz, PROT_WRITE);
+	for (int i = 0; i < num_wa_regs; i++) {
+		*out++ = MI_STORE_REGISTER_MEM | ((gen >= 8 ? 4 : 2) - 2);
+		*out++ = wa_regs[i].addr;
+		reloc[i].target_handle = obj[0].handle;
+		reloc[i].offset = (out - base) * sizeof(*out);
+		reloc[i].delta = i * sizeof(uint32_t);
+		reloc[i].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+		reloc[i].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
+		*out++ = reloc[i].delta;
+		if (gen >= 8)
+			*out++ = 0;
+	}
+	*out++ = MI_BATCH_BUFFER_END;
+	munmap(base, batch_sz);
 
-	for (i = 0; i < num_wa_regs; ++i) {
-		const uint32_t val = intel_register_read(wa_regs[i].addr);
-		const bool ok = (wa_regs[i].value & wa_regs[i].mask) ==
-			(val & wa_regs[i].mask);
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(obj);
+	execbuf.buffer_count = 2;
+	gem_execbuf(fd, &execbuf);
 
-		igt_debug("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
-			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
-			  val, ok ? "OK" : "FAIL");
+	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
 
-		if (write_only(wa_regs[i].addr))
-			continue;
+	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
 
-		if (!ok) {
-			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
-				 wa_regs[i].addr, wa_regs[i].value,
-				 wa_regs[i].mask,
-				 val, ok ? "OK" : "FAIL");
+	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
+	for (int i = 0; i < num_wa_regs; i++) {
+		const bool ok =
+			(wa_regs[i].value & wa_regs[i].mask) ==
+			(out[i] & wa_regs[i].mask);
+		char buf[80];
+
+		snprintf(buf, sizeof(buf),
+			 "0x%05X\t0x%08X\t0x%08X\t0x%08X",
+			 wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
+			 out[i]);
+
+		if (ok) {
+			igt_debug("%s\tOK\n", buf);
+		} else if (write_only(wa_regs[i].addr)) {
+			igt_debug("%s\tIGNORED (w/o)\n", buf);
+		} else {
+			igt_warn("%s\tFAIL\n", buf);
 			fail_count++;
 		}
 	}
+	munmap(out, result_sz);
+
+	gem_close(fd, obj[1].handle);
+	gem_close(fd, obj[0].handle);
+	free(reloc);
 
 	return fail_count;
 }
 
-static void check_workarounds(enum operation op)
+static void check_workarounds(int fd, enum operation op)
 {
-	igt_assert_eq(workaround_fail_count(), 0);
+	igt_assert_eq(workaround_fail_count(fd), 0);
 
 	switch (op) {
 	case GPU_RESET:
-		test_hang_gpu();
+		igt_force_gpu_reset(fd);
 		break;
 
 	case SUSPEND_RESUME:
@@ -152,40 +184,30 @@ static void check_workarounds(enum operation op)
 		igt_assert(0);
 	}
 
-	igt_assert_eq(workaround_fail_count(), 0);
+	igt_assert_eq(workaround_fail_count(fd), 0);
 }
 
 igt_main
 {
+	int device = -1;
+
 	igt_fixture {
-		int device = drm_open_driver_master(DRIVER_INTEL);
-		struct pci_device *pci_dev;
 		FILE *file;
 		char *line = NULL;
 		size_t line_size;
 		int i, fd;
 
+		device = drm_open_driver(DRIVER_INTEL);
 		igt_require_gem(device);
 
 		gen = intel_gen(intel_get_drm_devid(device));
 
-		pci_dev = intel_get_pci_device();
-		igt_require(pci_dev);
-
-		intel_register_access_init(pci_dev, 0, device);
-
 		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
 		file = fdopen(fd, "r");
 		igt_assert(getline(&line, &line_size, file) > 0);
 		igt_debug("i915_wa_registers: %s", line);
 		sscanf(line, "Workarounds applied: %d", &num_wa_regs);
-
-		/* For newer gens, the lri wa list has always something.
-		 * If it doesn't, go and add one. */
-		if (gen >= 8)
-			igt_assert_lt(0, num_wa_regs);
-		else
-			igt_assert_lte(0, num_wa_regs);
+		igt_require(num_wa_regs > 0);
 
 		wa_regs = malloc(num_wa_regs * sizeof(*wa_regs));
 		igt_assert(wa_regs);
@@ -205,21 +227,14 @@ igt_main
 		free(line);
 		fclose(file);
 		close(fd);
-		close(device);
 	}
 
 	igt_subtest("basic-read")
-		check_workarounds(SIMPLE_READ);
+		check_workarounds(device, SIMPLE_READ);
 
 	igt_subtest("reset")
-		check_workarounds(GPU_RESET);
+		check_workarounds(device, GPU_RESET);
 
 	igt_subtest("suspend-resume")
-		check_workarounds(SUSPEND_RESUME);
-
-	igt_fixture {
-		free(wa_regs);
-		intel_register_access_fini();
-	}
-
+		check_workarounds(device, SUSPEND_RESUME);
 }
-- 
2.14.2

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

[CI 2/5] igt/gem_workarounds: Also exercise fresh contexts not the persistent default

[Chris Wilson]

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

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 95ec250a..c252133f 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -164,8 +164,14 @@ static int workaround_fail_count(int fd)
 	return fail_count;
 }
 
-static void check_workarounds(int fd, enum operation op)
+#define CONTEXT 0x1
+static void check_workarounds(int fd, enum operation op, unsigned int flags)
 {
+	uint32_t ctx = 0;
+
+	if (flags & CONTEXT)
+		ctx = gem_context_create(fd);
+
 	igt_assert_eq(workaround_fail_count(fd), 0);
 
 	switch (op) {
@@ -178,13 +184,16 @@ static void check_workarounds(int fd, enum operation op)
 		break;
 
 	case SIMPLE_READ:
-		return;
+		break;
 
 	default:
 		igt_assert(0);
 	}
 
 	igt_assert_eq(workaround_fail_count(fd), 0);
+
+	if (ctx)
+		gem_context_destroy(fd, ctx);
 }
 
 igt_main
@@ -230,11 +239,20 @@ igt_main
 	}
 
 	igt_subtest("basic-read")
-		check_workarounds(device, SIMPLE_READ);
+		check_workarounds(device, SIMPLE_READ, 0);
+
+	igt_subtest("basic-read-context")
+		check_workarounds(device, SIMPLE_READ, CONTEXT);
 
 	igt_subtest("reset")
-		check_workarounds(device, GPU_RESET);
+		check_workarounds(device, GPU_RESET, 0);
+
+	igt_subtest("reset-context")
+		check_workarounds(device, GPU_RESET, CONTEXT);
 
 	igt_subtest("suspend-resume")
-		check_workarounds(device, SUSPEND_RESUME);
+		check_workarounds(device, SUSPEND_RESUME, 0);
+
+	igt_subtest("suspend-resume-context")
+		check_workarounds(device, SUSPEND_RESUME, CONTEXT);
 }
-- 
2.14.2

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

[CI 3/5] igt/gem_workarounds: Also test new fd (implicit default context)

[Chris Wilson]

To complete the picture also test a new fd with its implicit default
context. Now we have a test for a longstanding fd, new client, new
context.

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

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index c252133f..1c514a62 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -164,11 +164,25 @@ static int workaround_fail_count(int fd)
 	return fail_count;
 }
 
+static int reopen(int fd)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+	fd = open(path, O_RDWR);
+	igt_assert_lte(0, fd);
+
+	return fd;
+}
+
 #define CONTEXT 0x1
+#define FDS 0x2
 static void check_workarounds(int fd, enum operation op, unsigned int flags)
 {
 	uint32_t ctx = 0;
 
+	if (flags & FDS)
+		fd = reopen(fd);
 	if (flags & CONTEXT)
 		ctx = gem_context_create(fd);
 
@@ -194,6 +208,8 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 
 	if (ctx)
 		gem_context_destroy(fd, ctx);
+	if (flags & FDS)
+		close(fd);
 }
 
 igt_main
@@ -244,15 +260,24 @@ igt_main
 	igt_subtest("basic-read-context")
 		check_workarounds(device, SIMPLE_READ, CONTEXT);
 
+	igt_subtest("basic-read-fd")
+		check_workarounds(device, SIMPLE_READ, FDS);
+
 	igt_subtest("reset")
 		check_workarounds(device, GPU_RESET, 0);
 
 	igt_subtest("reset-context")
 		check_workarounds(device, GPU_RESET, CONTEXT);
 
+	igt_subtest("reset-fd")
+		check_workarounds(device, GPU_RESET, FDS);
+
 	igt_subtest("suspend-resume")
 		check_workarounds(device, SUSPEND_RESUME, 0);
 
 	igt_subtest("suspend-resume-context")
 		check_workarounds(device, SUSPEND_RESUME, CONTEXT);
+
+	igt_subtest("suspend-resume-fd")
+		check_workarounds(device, SUSPEND_RESUME, FDS);
 }
-- 
2.14.2

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

[CI 4/5] igt/gem_workarounds: Reduce manual list to combinatorial loops

[Chris Wilson]

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

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 1c514a62..74bb11d2 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -215,6 +215,24 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 igt_main
 {
 	int device = -1;
+	const struct {
+		const char *name;
+		enum operation op;
+	} ops[] =   {
+		{ "basic-read", SIMPLE_READ },
+		{ "reset", GPU_RESET },
+		{ "suspend-resume", SUSPEND_RESUME },
+		{ }
+	}, *op;
+	const struct {
+		const char *name;
+		unsigned int flags;
+	} modes[] =   {
+		{ "", 0 },
+		{ "-context", CONTEXT },
+		{ "-fd", FDS },
+		{ }
+	}, *m;
 
 	igt_fixture {
 		FILE *file;
@@ -254,30 +272,10 @@ igt_main
 		close(fd);
 	}
 
-	igt_subtest("basic-read")
-		check_workarounds(device, SIMPLE_READ, 0);
-
-	igt_subtest("basic-read-context")
-		check_workarounds(device, SIMPLE_READ, CONTEXT);
-
-	igt_subtest("basic-read-fd")
-		check_workarounds(device, SIMPLE_READ, FDS);
-
-	igt_subtest("reset")
-		check_workarounds(device, GPU_RESET, 0);
-
-	igt_subtest("reset-context")
-		check_workarounds(device, GPU_RESET, CONTEXT);
-
-	igt_subtest("reset-fd")
-		check_workarounds(device, GPU_RESET, FDS);
-
-	igt_subtest("suspend-resume")
-		check_workarounds(device, SUSPEND_RESUME, 0);
-
-	igt_subtest("suspend-resume-context")
-		check_workarounds(device, SUSPEND_RESUME, CONTEXT);
-
-	igt_subtest("suspend-resume-fd")
-		check_workarounds(device, SUSPEND_RESUME, FDS);
+	for (op = ops; op->name; op++) {
+		for (m = modes; m->name; m++) {
+			igt_subtest_f("%s%s", op->name, m->name)
+				check_workarounds(device, op->op, m->flags);
+		}
+	}
 }
-- 
2.14.2

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

[CI 5/5] igt/gem_workarounds: Add hibernation coverage

[Chris Wilson]

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

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 74bb11d2..dc39a76b 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -34,6 +34,7 @@ static int gen;
 enum operation {
 	GPU_RESET,
 	SUSPEND_RESUME,
+	HIBERNATE_RESUME,
 	SIMPLE_READ,
 };
 
@@ -61,12 +62,6 @@ static struct write_only_list {
 static struct intel_wa_reg *wa_regs;
 static int num_wa_regs;
 
-static void test_suspend_resume(void)
-{
-	igt_info("Suspending the device ...\n");
-	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
-}
-
 static bool write_only(const uint32_t addr)
 {
 	int i;
@@ -194,7 +189,13 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 		break;
 
 	case SUSPEND_RESUME:
-		test_suspend_resume();
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
+					      SUSPEND_TEST_NONE);
+		break;
+
+	case HIBERNATE_RESUME:
+		igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
+					      SUSPEND_TEST_NONE);
 		break;
 
 	case SIMPLE_READ:
@@ -222,6 +223,7 @@ igt_main
 		{ "basic-read", SIMPLE_READ },
 		{ "reset", GPU_RESET },
 		{ "suspend-resume", SUSPEND_RESUME },
+		{ "hibernate-resume", HIBERNATE_RESUME },
 		{ }
 	}, *op;
 	const struct {
-- 
2.14.2

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

Re: [CI 1/5] igt/gem_workarounds: Read the workaround registers from the active context

[Mika Kuoppala]

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

> The workarounds are only valid whilst the GPU is active. To be sure we
> are reading the registers in the right state, issue the reads from the GPU.
>

Yay, this is the right way :)

Some comments and findings below...

> v2: Show ignored write-only failures as debug.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/gem_workarounds.c | 147 ++++++++++++++++++++++++++----------------------
>  1 file changed, 81 insertions(+), 66 deletions(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index 5e30a7b8..95ec250a 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -61,20 +61,6 @@ static struct write_only_list {
>  static struct intel_wa_reg *wa_regs;
>  static int num_wa_regs;
>  
> -static void wait_gpu(void)
> -{
> -	int fd = drm_open_driver(DRIVER_INTEL);
> -	gem_quiescent_gpu(fd);
> -	close(fd);
> -}
> -
> -static void test_hang_gpu(void)
> -{
> -	int fd = drm_open_driver(DRIVER_INTEL);
> -	igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT));
> -	close(fd);
> -}
> -
>  static void test_suspend_resume(void)
>  {
>  	igt_info("Suspending the device ...\n");
> @@ -96,49 +82,95 @@ static bool write_only(const uint32_t addr)
>  	return false;
>  }
>  
> -static int workaround_fail_count(void)
> -{
> -	int i, fail_count = 0;
> -
> -	/* There is a small delay after coming ot of rc6 to the correct
> -	   render context values will get loaded by hardware (bdw,chv).
> -	   This here ensures that we have the correct context loaded before
> -	   we start to read values */
> -	wait_gpu();
> +#define MI_STORE_REGISTER_MEM (0x24 << 23)
>  
> -	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
> +static int workaround_fail_count(int fd)
> +{
> +	struct drm_i915_gem_exec_object2 obj[2];
> +	struct drm_i915_gem_relocation_entry *reloc;
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	uint32_t result_sz, batch_sz;
> +	uint32_t *base, *out;
> +	int fail_count = 0;
> +
> +	reloc = calloc(num_wa_regs, sizeof(*reloc));
> +	igt_assert(reloc);
> +
> +	result_sz = 4 * num_wa_regs;
> +	result_sz = (result_sz + 4095) & -4096;

Macro for align? Further, why do even need it. For
what I can gather, the mapping should work for smaller
objects also.

> +
> +	batch_sz = 16 * num_wa_regs;
> +	batch_sz = (batch_sz + 4 + 4095) & -4096;
> +
> +	memset(obj, 0, sizeof(obj));
> +	obj[0].handle = gem_create(fd, result_sz);
> +	gem_set_caching(fd, obj[0].handle, 1);

s/1/I915_CACHING_CACHED

> +	obj[1].handle = gem_create(fd, batch_sz);
> +	obj[1].relocs_ptr = to_user_pointer(reloc);
> +	obj[1].relocation_count = num_wa_regs;
> +
> +	out = base = gem_mmap__cpu(fd, obj[1].handle, 0, batch_sz, PROT_WRITE);
> +	for (int i = 0; i < num_wa_regs; i++) {
> +		*out++ = MI_STORE_REGISTER_MEM | ((gen >= 8 ? 4 : 2) - 2);
> +		*out++ = wa_regs[i].addr;
> +		reloc[i].target_handle = obj[0].handle;
> +		reloc[i].offset = (out - base) * sizeof(*out);
> +		reloc[i].delta = i * sizeof(uint32_t);
> +		reloc[i].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
> +		reloc[i].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
> +		*out++ = reloc[i].delta;
> +		if (gen >= 8)
> +			*out++ = 0;
> +	}
> +	*out++ = MI_BATCH_BUFFER_END;
> +	munmap(base, batch_sz);
>  
> -	for (i = 0; i < num_wa_regs; ++i) {
> -		const uint32_t val = intel_register_read(wa_regs[i].addr);
> -		const bool ok = (wa_regs[i].value & wa_regs[i].mask) ==
> -			(val & wa_regs[i].mask);
> +	memset(&execbuf, 0, sizeof(execbuf));
> +	execbuf.buffers_ptr = to_user_pointer(obj);
> +	execbuf.buffer_count = 2;
> +	gem_execbuf(fd, &execbuf);
>  
> -		igt_debug("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
> -			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
> -			  val, ok ? "OK" : "FAIL");
> +	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
>  
> -		if (write_only(wa_regs[i].addr))
> -			continue;
> +	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
>  
> -		if (!ok) {
> -			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
> -				 wa_regs[i].addr, wa_regs[i].value,
> -				 wa_regs[i].mask,
> -				 val, ok ? "OK" : "FAIL");
> +	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
> +	for (int i = 0; i < num_wa_regs; i++) {
> +		const bool ok =
> +			(wa_regs[i].value & wa_regs[i].mask) ==
> +			(out[i] & wa_regs[i].mask);
> +		char buf[80];
> +
> +		snprintf(buf, sizeof(buf),
> +			 "0x%05X\t0x%08X\t0x%08X\t0x%08X",
> +			 wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
> +			 out[i]);
> +
> +		if (ok) {
> +			igt_debug("%s\tOK\n", buf);
> +		} else if (write_only(wa_regs[i].addr)) {
> +			igt_debug("%s\tIGNORED (w/o)\n", buf);
> +		} else {
> +			igt_warn("%s\tFAIL\n", buf);
>  			fail_count++;
>  		}
>  	}
> +	munmap(out, result_sz);
> +
> +	gem_close(fd, obj[1].handle);
> +	gem_close(fd, obj[0].handle);
> +	free(reloc);
>  
>  	return fail_count;
>  }
>  
> -static void check_workarounds(enum operation op)
> +static void check_workarounds(int fd, enum operation op)
>  {
> -	igt_assert_eq(workaround_fail_count(), 0);
> +	igt_assert_eq(workaround_fail_count(fd), 0);
>  
>  	switch (op) {
>  	case GPU_RESET:
> -		test_hang_gpu();
> +		igt_force_gpu_reset(fd);

My kbl fails with the tests as you need some mechanism
to wait that the reset really did happen?

Hmm the kernel should ensure that the next reading batch
is post reset and everything should be fine.

(gem_workarounds:7286) WARNING: 0x024D0 0x00002248      0xFFFFFFFF      0x00002094      FAIL
(gem_workarounds:7286) WARNING: 0x024D4 0x00002580      0xFFFFFFFF      0x00002094      FAIL
(gem_workarounds:7286) WARNING: 0x024D8 0x00007304      0xFFFFFFFF      0x00002094      FAIL

which are fine pre reset...

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

Re: [CI 1/5] igt/gem_workarounds: Read the workaround registers from the active context

[Chris Wilson]

Quoting Mika Kuoppala (2017-10-03 16:19:10)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > The workarounds are only valid whilst the GPU is active. To be sure we
> > are reading the registers in the right state, issue the reads from the GPU.
> >
> 
> Yay, this is the right way :)
> 
> Some comments and findings below...
> 
> > v2: Show ignored write-only failures as debug.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  tests/gem_workarounds.c | 147 ++++++++++++++++++++++++++----------------------
> >  1 file changed, 81 insertions(+), 66 deletions(-)
> >
> > diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> > index 5e30a7b8..95ec250a 100644
> > --- a/tests/gem_workarounds.c
> > +++ b/tests/gem_workarounds.c
> > @@ -61,20 +61,6 @@ static struct write_only_list {
> >  static struct intel_wa_reg *wa_regs;
> >  static int num_wa_regs;
> >  
> > -static void wait_gpu(void)
> > -{
> > -     int fd = drm_open_driver(DRIVER_INTEL);
> > -     gem_quiescent_gpu(fd);
> > -     close(fd);
> > -}
> > -
> > -static void test_hang_gpu(void)
> > -{
> > -     int fd = drm_open_driver(DRIVER_INTEL);
> > -     igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT));
> > -     close(fd);
> > -}
> > -
> >  static void test_suspend_resume(void)
> >  {
> >       igt_info("Suspending the device ...\n");
> > @@ -96,49 +82,95 @@ static bool write_only(const uint32_t addr)
> >       return false;
> >  }
> >  
> > -static int workaround_fail_count(void)
> > -{
> > -     int i, fail_count = 0;
> > -
> > -     /* There is a small delay after coming ot of rc6 to the correct
> > -        render context values will get loaded by hardware (bdw,chv).
> > -        This here ensures that we have the correct context loaded before
> > -        we start to read values */
> > -     wait_gpu();
> > +#define MI_STORE_REGISTER_MEM (0x24 << 23)
> >  
> > -     igt_debug("Address    val        mask        read        result\n");
> > +static int workaround_fail_count(int fd)
> > +{
> > +     struct drm_i915_gem_exec_object2 obj[2];
> > +     struct drm_i915_gem_relocation_entry *reloc;
> > +     struct drm_i915_gem_execbuffer2 execbuf;
> > +     uint32_t result_sz, batch_sz;
> > +     uint32_t *base, *out;
> > +     int fail_count = 0;
> > +
> > +     reloc = calloc(num_wa_regs, sizeof(*reloc));
> > +     igt_assert(reloc);
> > +
> > +     result_sz = 4 * num_wa_regs;
> > +     result_sz = (result_sz + 4095) & -4096;
> 
> Macro for align?

I never remember if we have PAGE_ALIGN() or not. Quicker to write than
grep.

> Further, why do even need it. For
> what I can gather, the mapping should work for smaller
> objects also.

Our mmap interfaces we like to operate on pages and tend to complain for
some interfaces if not, or not whole object. So it is just simpler to
think in pages and not worry about which work on less.

> > -static void check_workarounds(enum operation op)
> > +static void check_workarounds(int fd, enum operation op)
> >  {
> > -     igt_assert_eq(workaround_fail_count(), 0);
> > +     igt_assert_eq(workaround_fail_count(fd), 0);
> >  
> >       switch (op) {
> >       case GPU_RESET:
> > -             test_hang_gpu();
> > +             igt_force_gpu_reset(fd);
> 
> My kbl fails with the tests as you need some mechanism
> to wait that the reset really did happen?

It waits for the reset to complete (double checked, otherwise we have a
number of nasty races around).

> Hmm the kernel should ensure that the next reading batch
> is post reset and everything should be fine.
> 
> (gem_workarounds:7286) WARNING: 0x024D0 0x00002248      0xFFFFFFFF      0x00002094      FAIL
> (gem_workarounds:7286) WARNING: 0x024D4 0x00002580      0xFFFFFFFF      0x00002094      FAIL
> (gem_workarounds:7286) WARNING: 0x024D8 0x00007304      0xFFFFFFFF      0x00002094      FAIL

These are RING_FORCE_TO_NONPRIV, hence the thread about moving them from
the WA_WRITE to I915_WRITE as they are not part of the context image.

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

Re: [CI 2/5] igt/gem_workarounds: Also exercise fresh contexts not the persistent default

[Mika Kuoppala]

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

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/gem_workarounds.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index 95ec250a..c252133f 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -164,8 +164,14 @@ static int workaround_fail_count(int fd)
>  	return fail_count;
>  }
>  
> -static void check_workarounds(int fd, enum operation op)
> +#define CONTEXT 0x1
> +static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  {
> +	uint32_t ctx = 0;
> +
> +	if (flags & CONTEXT)
> +		ctx = gem_context_create(fd);
> +
>  	igt_assert_eq(workaround_fail_count(fd), 0);

The batch to read them will still use the default context. I would
have assumed that you want to read the results throught your
newly created context?

-Mika


>  
>  	switch (op) {
> @@ -178,13 +184,16 @@ static void check_workarounds(int fd, enum operation op)
>  		break;
>  
>  	case SIMPLE_READ:
> -		return;
> +		break;
>  
>  	default:
>  		igt_assert(0);
>  	}
>  
>  	igt_assert_eq(workaround_fail_count(fd), 0);
> +
> +	if (ctx)
> +		gem_context_destroy(fd, ctx);
>  }
>  
>  igt_main
> @@ -230,11 +239,20 @@ igt_main
>  	}
>  
>  	igt_subtest("basic-read")
> -		check_workarounds(device, SIMPLE_READ);
> +		check_workarounds(device, SIMPLE_READ, 0);
> +
> +	igt_subtest("basic-read-context")
> +		check_workarounds(device, SIMPLE_READ, CONTEXT);
>  
>  	igt_subtest("reset")
> -		check_workarounds(device, GPU_RESET);
> +		check_workarounds(device, GPU_RESET, 0);
> +
> +	igt_subtest("reset-context")
> +		check_workarounds(device, GPU_RESET, CONTEXT);
>  
>  	igt_subtest("suspend-resume")
> -		check_workarounds(device, SUSPEND_RESUME);
> +		check_workarounds(device, SUSPEND_RESUME, 0);
> +
> +	igt_subtest("suspend-resume-context")
> +		check_workarounds(device, SUSPEND_RESUME, CONTEXT);
>  }
> -- 
> 2.14.2
>
> _______________________________________________
> 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: [CI 2/5] igt/gem_workarounds: Also exercise fresh contexts not the persistent default

[Chris Wilson]

Quoting Mika Kuoppala (2017-10-04 14:13:59)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  tests/gem_workarounds.c | 28 +++++++++++++++++++++++-----
> >  1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> > index 95ec250a..c252133f 100644
> > --- a/tests/gem_workarounds.c
> > +++ b/tests/gem_workarounds.c
> > @@ -164,8 +164,14 @@ static int workaround_fail_count(int fd)
> >       return fail_count;
> >  }
> >  
> > -static void check_workarounds(int fd, enum operation op)
> > +#define CONTEXT 0x1
> > +static void check_workarounds(int fd, enum operation op, unsigned int flags)
> >  {
> > +     uint32_t ctx = 0;
> > +
> > +     if (flags & CONTEXT)
> > +             ctx = gem_context_create(fd);
> > +
> >       igt_assert_eq(workaround_fail_count(fd), 0);
> 
> The batch to read them will still use the default context. I would
> have assumed that you want to read the results throught your
> newly created context?

Yes, that was stupid!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 3/5] igt/gem_workarounds: Also test new fd (implicit default context)

[Mika Kuoppala]

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

> To complete the picture also test a new fd with its implicit default
> context. Now we have a test for a longstanding fd, new client, new
> context.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

> ---
>  tests/gem_workarounds.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index c252133f..1c514a62 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -164,11 +164,25 @@ static int workaround_fail_count(int fd)
>  	return fail_count;
>  }
>  
> +static int reopen(int fd)
> +{
> +	char path[256];
> +
> +	snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
> +	fd = open(path, O_RDWR);
> +	igt_assert_lte(0, fd);
> +
> +	return fd;
> +}
> +
>  #define CONTEXT 0x1
> +#define FDS 0x2
>  static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  {
>  	uint32_t ctx = 0;
>  
> +	if (flags & FDS)
> +		fd = reopen(fd);
>  	if (flags & CONTEXT)
>  		ctx = gem_context_create(fd);
>  
> @@ -194,6 +208,8 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  
>  	if (ctx)
>  		gem_context_destroy(fd, ctx);
> +	if (flags & FDS)
> +		close(fd);
>  }
>  
>  igt_main
> @@ -244,15 +260,24 @@ igt_main
>  	igt_subtest("basic-read-context")
>  		check_workarounds(device, SIMPLE_READ, CONTEXT);
>  
> +	igt_subtest("basic-read-fd")
> +		check_workarounds(device, SIMPLE_READ, FDS);
> +
>  	igt_subtest("reset")
>  		check_workarounds(device, GPU_RESET, 0);
>  
>  	igt_subtest("reset-context")
>  		check_workarounds(device, GPU_RESET, CONTEXT);
>  
> +	igt_subtest("reset-fd")
> +		check_workarounds(device, GPU_RESET, FDS);
> +
>  	igt_subtest("suspend-resume")
>  		check_workarounds(device, SUSPEND_RESUME, 0);
>  
>  	igt_subtest("suspend-resume-context")
>  		check_workarounds(device, SUSPEND_RESUME, CONTEXT);
> +
> +	igt_subtest("suspend-resume-fd")
> +		check_workarounds(device, SUSPEND_RESUME, FDS);
>  }
> -- 
> 2.14.2
>
> _______________________________________________
> 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: [CI 4/5] igt/gem_workarounds: Reduce manual list to combinatorial loops

[Mika Kuoppala]

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

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

> ---
>  tests/gem_workarounds.c | 50 ++++++++++++++++++++++++-------------------------
>  1 file changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index 1c514a62..74bb11d2 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -215,6 +215,24 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  igt_main
>  {
>  	int device = -1;
> +	const struct {
> +		const char *name;
> +		enum operation op;
> +	} ops[] =   {
> +		{ "basic-read", SIMPLE_READ },
> +		{ "reset", GPU_RESET },
> +		{ "suspend-resume", SUSPEND_RESUME },
> +		{ }
> +	}, *op;
> +	const struct {
> +		const char *name;
> +		unsigned int flags;
> +	} modes[] =   {
> +		{ "", 0 },
> +		{ "-context", CONTEXT },
> +		{ "-fd", FDS },
> +		{ }
> +	}, *m;
>  
>  	igt_fixture {
>  		FILE *file;
> @@ -254,30 +272,10 @@ igt_main
>  		close(fd);
>  	}
>  
> -	igt_subtest("basic-read")
> -		check_workarounds(device, SIMPLE_READ, 0);
> -
> -	igt_subtest("basic-read-context")
> -		check_workarounds(device, SIMPLE_READ, CONTEXT);
> -
> -	igt_subtest("basic-read-fd")
> -		check_workarounds(device, SIMPLE_READ, FDS);
> -
> -	igt_subtest("reset")
> -		check_workarounds(device, GPU_RESET, 0);
> -
> -	igt_subtest("reset-context")
> -		check_workarounds(device, GPU_RESET, CONTEXT);
> -
> -	igt_subtest("reset-fd")
> -		check_workarounds(device, GPU_RESET, FDS);
> -
> -	igt_subtest("suspend-resume")
> -		check_workarounds(device, SUSPEND_RESUME, 0);
> -
> -	igt_subtest("suspend-resume-context")
> -		check_workarounds(device, SUSPEND_RESUME, CONTEXT);
> -
> -	igt_subtest("suspend-resume-fd")
> -		check_workarounds(device, SUSPEND_RESUME, FDS);
> +	for (op = ops; op->name; op++) {
> +		for (m = modes; m->name; m++) {
> +			igt_subtest_f("%s%s", op->name, m->name)
> +				check_workarounds(device, op->op, m->flags);
> +		}
> +	}
>  }
> -- 
> 2.14.2
>
> _______________________________________________
> 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: [CI 5/5] igt/gem_workarounds: Add hibernation coverage

[Mika Kuoppala]

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

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

> ---
>  tests/gem_workarounds.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index 74bb11d2..dc39a76b 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -34,6 +34,7 @@ static int gen;
>  enum operation {
>  	GPU_RESET,
>  	SUSPEND_RESUME,
> +	HIBERNATE_RESUME,
>  	SIMPLE_READ,
>  };
>  
> @@ -61,12 +62,6 @@ static struct write_only_list {
>  static struct intel_wa_reg *wa_regs;
>  static int num_wa_regs;
>  
> -static void test_suspend_resume(void)
> -{
> -	igt_info("Suspending the device ...\n");
> -	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
> -}
> -
>  static bool write_only(const uint32_t addr)
>  {
>  	int i;
> @@ -194,7 +189,13 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  		break;
>  
>  	case SUSPEND_RESUME:
> -		test_suspend_resume();
> +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> +					      SUSPEND_TEST_NONE);
> +		break;
> +
> +	case HIBERNATE_RESUME:
> +		igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
> +					      SUSPEND_TEST_NONE);
>  		break;
>  
>  	case SIMPLE_READ:
> @@ -222,6 +223,7 @@ igt_main
>  		{ "basic-read", SIMPLE_READ },
>  		{ "reset", GPU_RESET },
>  		{ "suspend-resume", SUSPEND_RESUME },
> +		{ "hibernate-resume", HIBERNATE_RESUME },
>  		{ }
>  	}, *op;
>  	const struct {
> -- 
> 2.14.2
>
> _______________________________________________
> 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: [CI 1/5] igt/gem_workarounds: Read the workaround registers from the active context

[Mika Kuoppala]

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

> Quoting Mika Kuoppala (2017-10-03 16:19:10)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > The workarounds are only valid whilst the GPU is active. To be sure we
>> > are reading the registers in the right state, issue the reads from the GPU.
>> >
>> 
>> Yay, this is the right way :)
>> 
>> Some comments and findings below...
>> 
>> > v2: Show ignored write-only failures as debug.
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  tests/gem_workarounds.c | 147 ++++++++++++++++++++++++++----------------------
>> >  1 file changed, 81 insertions(+), 66 deletions(-)
>> >
>> > diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
>> > index 5e30a7b8..95ec250a 100644
>> > --- a/tests/gem_workarounds.c
>> > +++ b/tests/gem_workarounds.c
>> > @@ -61,20 +61,6 @@ static struct write_only_list {
>> >  static struct intel_wa_reg *wa_regs;
>> >  static int num_wa_regs;
>> >  
>> > -static void wait_gpu(void)
>> > -{
>> > -     int fd = drm_open_driver(DRIVER_INTEL);
>> > -     gem_quiescent_gpu(fd);
>> > -     close(fd);
>> > -}
>> > -
>> > -static void test_hang_gpu(void)
>> > -{
>> > -     int fd = drm_open_driver(DRIVER_INTEL);
>> > -     igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT));
>> > -     close(fd);
>> > -}
>> > -
>> >  static void test_suspend_resume(void)
>> >  {
>> >       igt_info("Suspending the device ...\n");
>> > @@ -96,49 +82,95 @@ static bool write_only(const uint32_t addr)
>> >       return false;
>> >  }
>> >  
>> > -static int workaround_fail_count(void)
>> > -{
>> > -     int i, fail_count = 0;
>> > -
>> > -     /* There is a small delay after coming ot of rc6 to the correct
>> > -        render context values will get loaded by hardware (bdw,chv).
>> > -        This here ensures that we have the correct context loaded before
>> > -        we start to read values */
>> > -     wait_gpu();
>> > +#define MI_STORE_REGISTER_MEM (0x24 << 23)
>> >  
>> > -     igt_debug("Address    val        mask        read        result\n");
>> > +static int workaround_fail_count(int fd)
>> > +{
>> > +     struct drm_i915_gem_exec_object2 obj[2];
>> > +     struct drm_i915_gem_relocation_entry *reloc;
>> > +     struct drm_i915_gem_execbuffer2 execbuf;
>> > +     uint32_t result_sz, batch_sz;
>> > +     uint32_t *base, *out;
>> > +     int fail_count = 0;
>> > +
>> > +     reloc = calloc(num_wa_regs, sizeof(*reloc));
>> > +     igt_assert(reloc);
>> > +
>> > +     result_sz = 4 * num_wa_regs;
>> > +     result_sz = (result_sz + 4095) & -4096;
>> 
>> Macro for align?
>
> I never remember if we have PAGE_ALIGN() or not. Quicker to write than
> grep.
>
>> Further, why do even need it. For
>> what I can gather, the mapping should work for smaller
>> objects also.
>
> Our mmap interfaces we like to operate on pages and tend to complain for
> some interfaces if not, or not whole object. So it is just simpler to
> think in pages and not worry about which work on less.
>
>> > -static void check_workarounds(enum operation op)
>> > +static void check_workarounds(int fd, enum operation op)
>> >  {
>> > -     igt_assert_eq(workaround_fail_count(), 0);
>> > +     igt_assert_eq(workaround_fail_count(fd), 0);
>> >  
>> >       switch (op) {
>> >       case GPU_RESET:
>> > -             test_hang_gpu();
>> > +             igt_force_gpu_reset(fd);
>> 
>> My kbl fails with the tests as you need some mechanism
>> to wait that the reset really did happen?
>
> It waits for the reset to complete (double checked, otherwise we have a
> number of nasty races around).
>
>> Hmm the kernel should ensure that the next reading batch
>> is post reset and everything should be fine.
>> 
>> (gem_workarounds:7286) WARNING: 0x024D0 0x00002248      0xFFFFFFFF      0x00002094      FAIL
>> (gem_workarounds:7286) WARNING: 0x024D4 0x00002580      0xFFFFFFFF      0x00002094      FAIL
>> (gem_workarounds:7286) WARNING: 0x024D8 0x00007304      0xFFFFFFFF      0x00002094      FAIL
>
> These are RING_FORCE_TO_NONPRIV, hence the thread about moving them from
> the WA_WRITE to I915_WRITE as they are not part of the context image.
>
> https://patchwork.freedesktop.org/series/31099/

Bikescheds aside, NONPRIV and the MMCD_MISC_CTRL changes are indeed
needed, which this more precise method found out.

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