All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
@ 2019-05-29  9:58 ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-29  9:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

It seems like the HW validator is getting better at preventing our
snooping of system registers from non-privileged batches! If we can't
use SRM, let's probe the register directly through mmio, making sure we
have the context spinning on the GPU first.

v2: Hold forcewake just in case the spinning batch isn't enough to
justify our register access.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 tests/i915/gem_workarounds.c | 88 +++++++-----------------------------
 1 file changed, 17 insertions(+), 71 deletions(-)

diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
index 44e3dce8a..2767b04d7 100644
--- a/tests/i915/gem_workarounds.c
+++ b/tests/i915/gem_workarounds.c
@@ -80,70 +80,27 @@ static bool write_only(const uint32_t addr)
 	return false;
 }
 
-#define MI_STORE_REGISTER_MEM (0x24 << 23)
-
-static int workaround_fail_count(int fd, uint32_t ctx)
+static int workaround_fail_count(int i915, uint32_t ctx)
 {
-	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 = PAGE_ALIGN(result_sz);
-
-	batch_sz = 16 * num_wa_regs + 4;
-	batch_sz = PAGE_ALIGN(batch_sz);
-
-	memset(obj, 0, sizeof(obj));
-	obj[0].handle = gem_create(fd, result_sz);
-	gem_set_caching(fd, obj[0].handle, 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);
+	igt_spin_t *spin;
+	int fw, fail = 0;
 
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(obj);
-	execbuf.buffer_count = 2;
-	execbuf.rsvd1 = ctx;
-	gem_execbuf(fd, &execbuf);
+	spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
+	igt_spin_busywait_until_started(spin);
 
-	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
-
-	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
-
-	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
+	fw = igt_open_forcewake_handle(i915);
 	for (int i = 0; i < num_wa_regs; i++) {
+		uint32_t value =
+			*(uint32_t *)(igt_global_mmio + wa_regs[i].addr);
 		const bool ok =
 			(wa_regs[i].value & wa_regs[i].mask) ==
-			(out[i] & wa_regs[i].mask);
+			(value & 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]);
+			 value);
 
 		if (ok) {
 			igt_debug("%s\tOK\n", buf);
@@ -151,27 +108,14 @@ static int workaround_fail_count(int fd, uint32_t ctx)
 			igt_debug("%s\tIGNORED (w/o)\n", buf);
 		} else {
 			igt_warn("%s\tFAIL\n", buf);
-			fail_count++;
+			fail++;
 		}
 	}
-	munmap(out, result_sz);
+	close(fw);
 
-	gem_close(fd, obj[1].handle);
-	gem_close(fd, obj[0].handle);
-	free(reloc);
+	igt_spin_free(i915, spin);
 
-	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;
+	return fail;
 }
 
 #define CONTEXT 0x1
@@ -181,7 +125,7 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 	uint32_t ctx = 0;
 
 	if (flags & FD)
-		fd = reopen(fd);
+		fd = gem_reopen_driver(fd);
 
 	if (flags & CONTEXT) {
 		gem_require_contexts(fd);
@@ -252,6 +196,8 @@ igt_main
 		device = drm_open_driver(DRIVER_INTEL);
 		igt_require_gem(device);
 
+		intel_mmio_use_pci_bar(intel_get_pci_device());
+
 		gen = intel_gen(intel_get_drm_devid(device));
 
 		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
@ 2019-05-29  9:58 ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-29  9:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

It seems like the HW validator is getting better at preventing our
snooping of system registers from non-privileged batches! If we can't
use SRM, let's probe the register directly through mmio, making sure we
have the context spinning on the GPU first.

v2: Hold forcewake just in case the spinning batch isn't enough to
justify our register access.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 tests/i915/gem_workarounds.c | 88 +++++++-----------------------------
 1 file changed, 17 insertions(+), 71 deletions(-)

diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
index 44e3dce8a..2767b04d7 100644
--- a/tests/i915/gem_workarounds.c
+++ b/tests/i915/gem_workarounds.c
@@ -80,70 +80,27 @@ static bool write_only(const uint32_t addr)
 	return false;
 }
 
-#define MI_STORE_REGISTER_MEM (0x24 << 23)
-
-static int workaround_fail_count(int fd, uint32_t ctx)
+static int workaround_fail_count(int i915, uint32_t ctx)
 {
-	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 = PAGE_ALIGN(result_sz);
-
-	batch_sz = 16 * num_wa_regs + 4;
-	batch_sz = PAGE_ALIGN(batch_sz);
-
-	memset(obj, 0, sizeof(obj));
-	obj[0].handle = gem_create(fd, result_sz);
-	gem_set_caching(fd, obj[0].handle, 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);
+	igt_spin_t *spin;
+	int fw, fail = 0;
 
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(obj);
-	execbuf.buffer_count = 2;
-	execbuf.rsvd1 = ctx;
-	gem_execbuf(fd, &execbuf);
+	spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
+	igt_spin_busywait_until_started(spin);
 
-	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
-
-	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
-
-	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
+	fw = igt_open_forcewake_handle(i915);
 	for (int i = 0; i < num_wa_regs; i++) {
+		uint32_t value =
+			*(uint32_t *)(igt_global_mmio + wa_regs[i].addr);
 		const bool ok =
 			(wa_regs[i].value & wa_regs[i].mask) ==
-			(out[i] & wa_regs[i].mask);
+			(value & 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]);
+			 value);
 
 		if (ok) {
 			igt_debug("%s\tOK\n", buf);
@@ -151,27 +108,14 @@ static int workaround_fail_count(int fd, uint32_t ctx)
 			igt_debug("%s\tIGNORED (w/o)\n", buf);
 		} else {
 			igt_warn("%s\tFAIL\n", buf);
-			fail_count++;
+			fail++;
 		}
 	}
-	munmap(out, result_sz);
+	close(fw);
 
-	gem_close(fd, obj[1].handle);
-	gem_close(fd, obj[0].handle);
-	free(reloc);
+	igt_spin_free(i915, spin);
 
-	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;
+	return fail;
 }
 
 #define CONTEXT 0x1
@@ -181,7 +125,7 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 	uint32_t ctx = 0;
 
 	if (flags & FD)
-		fd = reopen(fd);
+		fd = gem_reopen_driver(fd);
 
 	if (flags & CONTEXT) {
 		gem_require_contexts(fd);
@@ -252,6 +196,8 @@ igt_main
 		device = drm_open_driver(DRIVER_INTEL);
 		igt_require_gem(device);
 
+		intel_mmio_use_pci_bar(intel_get_pci_device());
+
 		gen = intel_gen(intel_get_drm_devid(device));
 
 		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
-- 
2.20.1

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

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

* Re: [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
  2019-05-29  9:58 ` [igt-dev] " Chris Wilson
@ 2019-05-29 10:00   ` Chris Wilson
  -1 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-29 10:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

Quoting Chris Wilson (2019-05-29 10:58:05)
> It seems like the HW validator is getting better at preventing our
> snooping of system registers from non-privileged batches! If we can't
> use SRM, let's probe the register directly through mmio, making sure we
> have the context spinning on the GPU first.
> 
> v2: Hold forcewake just in case the spinning batch isn't enough to
> justify our register access.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>

Carried over from v1,
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
@ 2019-05-29 10:00   ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-29 10:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

Quoting Chris Wilson (2019-05-29 10:58:05)
> It seems like the HW validator is getting better at preventing our
> snooping of system registers from non-privileged batches! If we can't
> use SRM, let's probe the register directly through mmio, making sure we
> have the context spinning on the GPU first.
> 
> v2: Hold forcewake just in case the spinning batch isn't enough to
> justify our register access.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>

Carried over from v1,
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
  2019-05-29  9:58 ` [igt-dev] " Chris Wilson
@ 2019-05-29 10:15   ` Mika Kuoppala
  -1 siblings, 0 replies; 11+ messages in thread
From: Mika Kuoppala @ 2019-05-29 10:15 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev

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

> It seems like the HW validator is getting better at preventing our
> snooping of system registers from non-privileged batches! If we can't
> use SRM, let's probe the register directly through mmio, making sure we
> have the context spinning on the GPU first.
>
> v2: Hold forcewake just in case the spinning batch isn't enough to
> justify our register access.
>

If I recall correctly, either of them separately didn't
work. And there was delay after grabbing the fw before
the register contents appeared. Don't remember the gen tho.

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> ---
>  tests/i915/gem_workarounds.c | 88 +++++++-----------------------------
>  1 file changed, 17 insertions(+), 71 deletions(-)
>
> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> index 44e3dce8a..2767b04d7 100644
> --- a/tests/i915/gem_workarounds.c
> +++ b/tests/i915/gem_workarounds.c
> @@ -80,70 +80,27 @@ static bool write_only(const uint32_t addr)
>  	return false;
>  }
>  
> -#define MI_STORE_REGISTER_MEM (0x24 << 23)
> -
> -static int workaround_fail_count(int fd, uint32_t ctx)
> +static int workaround_fail_count(int i915, uint32_t ctx)
>  {
> -	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 = PAGE_ALIGN(result_sz);
> -
> -	batch_sz = 16 * num_wa_regs + 4;
> -	batch_sz = PAGE_ALIGN(batch_sz);
> -
> -	memset(obj, 0, sizeof(obj));
> -	obj[0].handle = gem_create(fd, result_sz);
> -	gem_set_caching(fd, obj[0].handle, 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);
> +	igt_spin_t *spin;
> +	int fw, fail = 0;
>  
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(obj);
> -	execbuf.buffer_count = 2;
> -	execbuf.rsvd1 = ctx;
> -	gem_execbuf(fd, &execbuf);
> +	spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
> +	igt_spin_busywait_until_started(spin);
>  
> -	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
> -
> -	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
> -
> -	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
> +	fw = igt_open_forcewake_handle(i915);

assert that it went fine?

Perhaps both will now do the trick. But if it fails
get the forcewake before spinner so you get more delay.

>  	for (int i = 0; i < num_wa_regs; i++) {
> +		uint32_t value =
> +			*(uint32_t *)(igt_global_mmio + wa_regs[i].addr);

const might have been warranted.

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

>  		const bool ok =
>  			(wa_regs[i].value & wa_regs[i].mask) ==
> -			(out[i] & wa_regs[i].mask);
> +			(value & 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]);
> +			 value);
>  
>  		if (ok) {
>  			igt_debug("%s\tOK\n", buf);
> @@ -151,27 +108,14 @@ static int workaround_fail_count(int fd, uint32_t ctx)
>  			igt_debug("%s\tIGNORED (w/o)\n", buf);
>  		} else {
>  			igt_warn("%s\tFAIL\n", buf);
> -			fail_count++;
> +			fail++;
>  		}
>  	}
> -	munmap(out, result_sz);
> +	close(fw);
>  
> -	gem_close(fd, obj[1].handle);
> -	gem_close(fd, obj[0].handle);
> -	free(reloc);
> +	igt_spin_free(i915, spin);
>  
> -	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;
> +	return fail;
>  }
>  
>  #define CONTEXT 0x1
> @@ -181,7 +125,7 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  	uint32_t ctx = 0;
>  
>  	if (flags & FD)
> -		fd = reopen(fd);
> +		fd = gem_reopen_driver(fd);
>  
>  	if (flags & CONTEXT) {
>  		gem_require_contexts(fd);
> @@ -252,6 +196,8 @@ igt_main
>  		device = drm_open_driver(DRIVER_INTEL);
>  		igt_require_gem(device);
>  
> +		intel_mmio_use_pci_bar(intel_get_pci_device());
> +
>  		gen = intel_gen(intel_get_drm_devid(device));
>  
>  		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
> -- 
> 2.20.1
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
@ 2019-05-29 10:15   ` Mika Kuoppala
  0 siblings, 0 replies; 11+ messages in thread
From: Mika Kuoppala @ 2019-05-29 10:15 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev

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

> It seems like the HW validator is getting better at preventing our
> snooping of system registers from non-privileged batches! If we can't
> use SRM, let's probe the register directly through mmio, making sure we
> have the context spinning on the GPU first.
>
> v2: Hold forcewake just in case the spinning batch isn't enough to
> justify our register access.
>

If I recall correctly, either of them separately didn't
work. And there was delay after grabbing the fw before
the register contents appeared. Don't remember the gen tho.

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> ---
>  tests/i915/gem_workarounds.c | 88 +++++++-----------------------------
>  1 file changed, 17 insertions(+), 71 deletions(-)
>
> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> index 44e3dce8a..2767b04d7 100644
> --- a/tests/i915/gem_workarounds.c
> +++ b/tests/i915/gem_workarounds.c
> @@ -80,70 +80,27 @@ static bool write_only(const uint32_t addr)
>  	return false;
>  }
>  
> -#define MI_STORE_REGISTER_MEM (0x24 << 23)
> -
> -static int workaround_fail_count(int fd, uint32_t ctx)
> +static int workaround_fail_count(int i915, uint32_t ctx)
>  {
> -	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 = PAGE_ALIGN(result_sz);
> -
> -	batch_sz = 16 * num_wa_regs + 4;
> -	batch_sz = PAGE_ALIGN(batch_sz);
> -
> -	memset(obj, 0, sizeof(obj));
> -	obj[0].handle = gem_create(fd, result_sz);
> -	gem_set_caching(fd, obj[0].handle, 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);
> +	igt_spin_t *spin;
> +	int fw, fail = 0;
>  
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(obj);
> -	execbuf.buffer_count = 2;
> -	execbuf.rsvd1 = ctx;
> -	gem_execbuf(fd, &execbuf);
> +	spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
> +	igt_spin_busywait_until_started(spin);
>  
> -	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
> -
> -	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
> -
> -	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
> +	fw = igt_open_forcewake_handle(i915);

assert that it went fine?

Perhaps both will now do the trick. But if it fails
get the forcewake before spinner so you get more delay.

>  	for (int i = 0; i < num_wa_regs; i++) {
> +		uint32_t value =
> +			*(uint32_t *)(igt_global_mmio + wa_regs[i].addr);

const might have been warranted.

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

>  		const bool ok =
>  			(wa_regs[i].value & wa_regs[i].mask) ==
> -			(out[i] & wa_regs[i].mask);
> +			(value & 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]);
> +			 value);
>  
>  		if (ok) {
>  			igt_debug("%s\tOK\n", buf);
> @@ -151,27 +108,14 @@ static int workaround_fail_count(int fd, uint32_t ctx)
>  			igt_debug("%s\tIGNORED (w/o)\n", buf);
>  		} else {
>  			igt_warn("%s\tFAIL\n", buf);
> -			fail_count++;
> +			fail++;
>  		}
>  	}
> -	munmap(out, result_sz);
> +	close(fw);
>  
> -	gem_close(fd, obj[1].handle);
> -	gem_close(fd, obj[0].handle);
> -	free(reloc);
> +	igt_spin_free(i915, spin);
>  
> -	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;
> +	return fail;
>  }
>  
>  #define CONTEXT 0x1
> @@ -181,7 +125,7 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
>  	uint32_t ctx = 0;
>  
>  	if (flags & FD)
> -		fd = reopen(fd);
> +		fd = gem_reopen_driver(fd);
>  
>  	if (flags & CONTEXT) {
>  		gem_require_contexts(fd);
> @@ -252,6 +196,8 @@ igt_main
>  		device = drm_open_driver(DRIVER_INTEL);
>  		igt_require_gem(device);
>  
> +		intel_mmio_use_pci_bar(intel_get_pci_device());
> +
>  		gen = intel_gen(intel_get_drm_devid(device));
>  
>  		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
> -- 
> 2.20.1
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
  2019-05-29 10:15   ` Mika Kuoppala
@ 2019-05-29 10:24     ` Chris Wilson
  -1 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-29 10:24 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx; +Cc: igt-dev

Quoting Mika Kuoppala (2019-05-29 11:15:46)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > It seems like the HW validator is getting better at preventing our
> > snooping of system registers from non-privileged batches! If we can't
> > use SRM, let's probe the register directly through mmio, making sure we
> > have the context spinning on the GPU first.
> >
> > v2: Hold forcewake just in case the spinning batch isn't enough to
> > justify our register access.
> >
> 
> If I recall correctly, either of them separately didn't
> work. And there was delay after grabbing the fw before
> the register contents appeared. Don't remember the gen tho.

That would be a kernel bug / HW bug. Either we fail in our ack sequence
(maybe let the read overtake the fw ack or something equally impossible),
or the HW fails in its.

If you can think of a way of spotting it, add it to
selftests/intel_uncore -- it's exactly the type of test we need in bring
up, verifying our mmio is accurate.

> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > ---
> >  tests/i915/gem_workarounds.c | 88 +++++++-----------------------------
> >  1 file changed, 17 insertions(+), 71 deletions(-)
> >
> > diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> > index 44e3dce8a..2767b04d7 100644
> > --- a/tests/i915/gem_workarounds.c
> > +++ b/tests/i915/gem_workarounds.c
> > @@ -80,70 +80,27 @@ static bool write_only(const uint32_t addr)
> >       return false;
> >  }
> >  
> > -#define MI_STORE_REGISTER_MEM (0x24 << 23)
> > -
> > -static int workaround_fail_count(int fd, uint32_t ctx)
> > +static int workaround_fail_count(int i915, uint32_t ctx)
> >  {
> > -     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 = PAGE_ALIGN(result_sz);
> > -
> > -     batch_sz = 16 * num_wa_regs + 4;
> > -     batch_sz = PAGE_ALIGN(batch_sz);
> > -
> > -     memset(obj, 0, sizeof(obj));
> > -     obj[0].handle = gem_create(fd, result_sz);
> > -     gem_set_caching(fd, obj[0].handle, 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);
> > +     igt_spin_t *spin;
> > +     int fw, fail = 0;
> >  
> > -     memset(&execbuf, 0, sizeof(execbuf));
> > -     execbuf.buffers_ptr = to_user_pointer(obj);
> > -     execbuf.buffer_count = 2;
> > -     execbuf.rsvd1 = ctx;
> > -     gem_execbuf(fd, &execbuf);
> > +     spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
> > +     igt_spin_busywait_until_started(spin);
> >  
> > -     gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
> > -
> > -     igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
> > -
> > -     out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
> > +     fw = igt_open_forcewake_handle(i915);
> 
> assert that it went fine?

Do we always expect the debugfs to be present? Do we strictly need it?
igt_debug if it isn't present and correlate that to a fail.
 
> Perhaps both will now do the trick. But if it fails
> get the forcewake before spinner so you get more delay.

Nah, I vote that in that case forcewake is broken and needs a kernel fix,
as we don't have any such delay when using I915_READ().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
@ 2019-05-29 10:24     ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-29 10:24 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx; +Cc: igt-dev

Quoting Mika Kuoppala (2019-05-29 11:15:46)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > It seems like the HW validator is getting better at preventing our
> > snooping of system registers from non-privileged batches! If we can't
> > use SRM, let's probe the register directly through mmio, making sure we
> > have the context spinning on the GPU first.
> >
> > v2: Hold forcewake just in case the spinning batch isn't enough to
> > justify our register access.
> >
> 
> If I recall correctly, either of them separately didn't
> work. And there was delay after grabbing the fw before
> the register contents appeared. Don't remember the gen tho.

That would be a kernel bug / HW bug. Either we fail in our ack sequence
(maybe let the read overtake the fw ack or something equally impossible),
or the HW fails in its.

If you can think of a way of spotting it, add it to
selftests/intel_uncore -- it's exactly the type of test we need in bring
up, verifying our mmio is accurate.

> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110544
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > ---
> >  tests/i915/gem_workarounds.c | 88 +++++++-----------------------------
> >  1 file changed, 17 insertions(+), 71 deletions(-)
> >
> > diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> > index 44e3dce8a..2767b04d7 100644
> > --- a/tests/i915/gem_workarounds.c
> > +++ b/tests/i915/gem_workarounds.c
> > @@ -80,70 +80,27 @@ static bool write_only(const uint32_t addr)
> >       return false;
> >  }
> >  
> > -#define MI_STORE_REGISTER_MEM (0x24 << 23)
> > -
> > -static int workaround_fail_count(int fd, uint32_t ctx)
> > +static int workaround_fail_count(int i915, uint32_t ctx)
> >  {
> > -     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 = PAGE_ALIGN(result_sz);
> > -
> > -     batch_sz = 16 * num_wa_regs + 4;
> > -     batch_sz = PAGE_ALIGN(batch_sz);
> > -
> > -     memset(obj, 0, sizeof(obj));
> > -     obj[0].handle = gem_create(fd, result_sz);
> > -     gem_set_caching(fd, obj[0].handle, 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);
> > +     igt_spin_t *spin;
> > +     int fw, fail = 0;
> >  
> > -     memset(&execbuf, 0, sizeof(execbuf));
> > -     execbuf.buffers_ptr = to_user_pointer(obj);
> > -     execbuf.buffer_count = 2;
> > -     execbuf.rsvd1 = ctx;
> > -     gem_execbuf(fd, &execbuf);
> > +     spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
> > +     igt_spin_busywait_until_started(spin);
> >  
> > -     gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
> > -
> > -     igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
> > -
> > -     out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
> > +     fw = igt_open_forcewake_handle(i915);
> 
> assert that it went fine?

Do we always expect the debugfs to be present? Do we strictly need it?
igt_debug if it isn't present and correlate that to a fail.
 
> Perhaps both will now do the trick. But if it fails
> get the forcewake before spinner so you get more delay.

Nah, I vote that in that case forcewake is broken and needs a kernel fix,
as we don't have any such delay when using I915_READ().
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for i915/gem_workarounds: Verify regs directly (rev2)
  2019-05-29  9:58 ` [igt-dev] " Chris Wilson
                   ` (2 preceding siblings ...)
  (?)
@ 2019-05-29 10:27 ` Patchwork
  -1 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-29 10:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915/gem_workarounds: Verify regs directly (rev2)
URL   : https://patchwork.freedesktop.org/series/61140/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6160 -> IGTPW_3070
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61140/revisions/2/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       [PASS][1] -> [DMESG-WARN][2] ([fdo#105128] / [fdo#107139])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/fi-kbl-7500u/igt@gem_exec_suspend@basic-s4-devices.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/fi-kbl-7500u/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [PASS][3] -> [DMESG-FAIL][4] ([fdo#110235])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

  
#### Possible fixes ####

  * igt@gem_workarounds@basic-read:
    - fi-cml-u2:          [FAIL][5] ([fdo#110544]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/fi-cml-u2/igt@gem_workarounds@basic-read.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/fi-cml-u2/igt@gem_workarounds@basic-read.html
    - fi-cml-u:           [FAIL][7] ([fdo#110544]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/fi-cml-u/igt@gem_workarounds@basic-read.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/fi-cml-u/igt@gem_workarounds@basic-read.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [FAIL][9] ([fdo#110627]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

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

  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110544]: https://bugs.freedesktop.org/show_bug.cgi?id=110544
  [fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627


Participating hosts (47 -> 39)
------------------------------

  Missing    (8): fi-kbl-soraka fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-bdw-samus fi-byt-clapper fi-skl-6600u 


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

  * IGT: IGT_5023 -> IGTPW_3070

  CI_DRM_6160: 43905c26d4fd15cba890e62f271befc0eca8de4c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3070: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/
  IGT_5023: 0cb925e7f145ba2535924d9a298a62d757707e2a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* [igt-dev] ✓ Fi.CI.IGT: success for i915/gem_workarounds: Verify regs directly (rev2)
  2019-05-29  9:58 ` [igt-dev] " Chris Wilson
                   ` (3 preceding siblings ...)
  (?)
@ 2019-05-29 14:56 ` Patchwork
  -1 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-29 14:56 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915/gem_workarounds: Verify regs directly (rev2)
URL   : https://patchwork.freedesktop.org/series/61140/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6160_full -> IGTPW_3070_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61140/revisions/2/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-apl5/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-apl4/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_linear_blits@interruptible:
    - shard-apl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#103927])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-apl7/igt@gem_linear_blits@interruptible.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-apl4/igt@gem_linear_blits@interruptible.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([fdo#110544]) +7 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-apl5/igt@gem_workarounds@suspend-resume.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-apl1/igt@gem_workarounds@suspend-resume.html

  * igt@kms_busy@basic-modeset-c:
    - shard-kbl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103665]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-kbl4/igt@kms_busy@basic-modeset-c.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-kbl2/igt@kms_busy@basic-modeset-c.html
    - shard-glk:          [PASS][9] -> [INCOMPLETE][10] ([fdo#103359] / [k.org#198133])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-glk6/igt@kms_busy@basic-modeset-c.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-glk7/igt@kms_busy@basic-modeset-c.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x85-onscreen:
    - shard-apl:          [PASS][11] -> [FAIL][12] ([fdo#103232])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-apl7/igt@kms_cursor_crc@pipe-b-cursor-256x85-onscreen.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-256x85-onscreen.html
    - shard-kbl:          [PASS][13] -> [FAIL][14] ([fdo#103232])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-kbl6/igt@kms_cursor_crc@pipe-b-cursor-256x85-onscreen.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-kbl1/igt@kms_cursor_crc@pipe-b-cursor-256x85-onscreen.html

  * igt@kms_frontbuffer_tracking@fbc-2p-rte:
    - shard-hsw:          [PASS][15] -> [SKIP][16] ([fdo#109271]) +23 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-hsw5/igt@kms_frontbuffer_tracking@fbc-2p-rte.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-rte.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [PASS][17] -> [SKIP][18] ([fdo#109271])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-kbl2/igt@perf_pmu@rc6.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-kbl1/igt@perf_pmu@rc6.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-glk:          [FAIL][19] ([fdo#110667]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-glk4/igt@gem_eio@in-flight-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-glk4/igt@gem_eio@in-flight-suspend.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-snb:          [SKIP][21] ([fdo#109271]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-snb7/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-snb5/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-hsw:          [SKIP][23] ([fdo#109271]) -> [PASS][24] +34 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-hsw1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-hsw4/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
    - shard-glk:          [FAIL][25] ([fdo#100368]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-glk3/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-glk7/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-apl:          [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28] +5 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-apl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-apl1/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          [FAIL][29] ([fdo#109016]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-kbl6/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-kbl4/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][31] ([fdo#99912]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-kbl7/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-kbl2/igt@kms_setmode@basic.html

  * igt@tools_test@tools_test:
    - shard-glk:          [SKIP][33] ([fdo#109271]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-glk4/igt@tools_test@tools_test.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-glk9/igt@tools_test@tools_test.html

  
#### Warnings ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          [INCOMPLETE][35] ([fdo#103540]) -> [FAIL][36] ([fdo#108686])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6160/shard-hsw6/igt@gem_tiled_swapping@non-threaded.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/shard-hsw7/igt@gem_tiled_swapping@non-threaded.html

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110544]: https://bugs.freedesktop.org/show_bug.cgi?id=110544
  [fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
  [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 (9 -> 5)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


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

  * IGT: IGT_5023 -> IGTPW_3070
  * Piglit: piglit_4509 -> None

  CI_DRM_6160: 43905c26d4fd15cba890e62f271befc0eca8de4c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3070: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3070/
  IGT_5023: 0cb925e7f145ba2535924d9a298a62d757707e2a @ 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_3070/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [PATCH i-g-t] i915/gem_workarounds: Verify regs directly
@ 2019-05-25  7:05 Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-25  7:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

---
 tests/i915/gem_workarounds.c | 45 +++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
index 44e3dce8a..bf8b4f630 100644
--- a/tests/i915/gem_workarounds.c
+++ b/tests/i915/gem_workarounds.c
@@ -82,6 +82,7 @@ static bool write_only(const uint32_t addr)
 
 #define MI_STORE_REGISTER_MEM (0x24 << 23)
 
+#if 0
 static int workaround_fail_count(int fd, uint32_t ctx)
 {
 	struct drm_i915_gem_exec_object2 obj[2];
@@ -162,17 +163,45 @@ static int workaround_fail_count(int fd, uint32_t ctx)
 
 	return fail_count;
 }
-
-static int reopen(int fd)
+#else
+static int workaround_fail_count(int i915, uint32_t ctx)
 {
-	char path[256];
+	igt_spin_t *spin;
+	int fail = 0;
+
+	intel_mmio_use_pci_bar(intel_get_pci_device());
+
+	spin = igt_spin_new(i915, .ctx = ctx, .flags = IGT_SPIN_POLL_RUN);
+	igt_spin_busywait_until_started(spin);
+
+	for (int i = 0; i < num_wa_regs; i++) {
+		uint32_t value =
+			*(uint32_t *)(igt_global_mmio + wa_regs[i].addr);
+		const bool ok =
+			(wa_regs[i].value & wa_regs[i].mask) ==
+			(value & 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,
+			 value);
+
+		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++;
+		}
+	}
 
-	snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
-	fd = open(path, O_RDWR);
-	igt_assert_lte(0, fd);
+	igt_spin_free(i915, spin);
 
-	return fd;
+	return fail;
 }
+#endif
 
 #define CONTEXT 0x1
 #define FD 0x2
@@ -181,7 +210,7 @@ static void check_workarounds(int fd, enum operation op, unsigned int flags)
 	uint32_t ctx = 0;
 
 	if (flags & FD)
-		fd = reopen(fd);
+		fd = gem_reopen_driver(fd);
 
 	if (flags & CONTEXT) {
 		gem_require_contexts(fd);
-- 
2.20.1

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

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

end of thread, other threads:[~2019-05-29 14:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29  9:58 [PATCH i-g-t] i915/gem_workarounds: Verify regs directly Chris Wilson
2019-05-29  9:58 ` [igt-dev] " Chris Wilson
2019-05-29 10:00 ` Chris Wilson
2019-05-29 10:00   ` [Intel-gfx] " Chris Wilson
2019-05-29 10:15 ` [igt-dev] " Mika Kuoppala
2019-05-29 10:15   ` Mika Kuoppala
2019-05-29 10:24   ` Chris Wilson
2019-05-29 10:24     ` Chris Wilson
2019-05-29 10:27 ` [igt-dev] ✓ Fi.CI.BAT: success for i915/gem_workarounds: Verify regs directly (rev2) Patchwork
2019-05-29 14:56 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2019-05-25  7:05 [PATCH i-g-t] i915/gem_workarounds: Verify regs directly Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.