public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH igt] tests/gem_workarounds: Skip write only registers
@ 2017-09-28 13:45 Mika Kuoppala
  2017-09-28 13:57 ` Chris Wilson
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Mika Kuoppala @ 2017-09-28 13:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

We have no means to check write only registers as
this would need access through context image. For now we
know that cnl has a one such register, 0xe5f0 which is used
to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
the context image without and with workaround applied:

0x0000a840: 0x0000e5f0 0xffff0800
0x0000a840: 0x0000e5f0 0xffff0820

we can conclude that the workaround setup is working right
in this particular case.

Add a write only list and add register 0xe5f0 into this list.
This is a temporary solution until a more capable context image
checker emerges.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 tests/gem_workarounds.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index d6641bd5..03b4dc6c 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -29,6 +29,8 @@
 
 #include <fcntl.h>
 
+static int gen;
+
 enum operation {
 	GPU_RESET,
 	SUSPEND_RESUME,
@@ -41,6 +43,13 @@ struct intel_wa_reg {
 	uint32_t mask;
 };
 
+static struct write_only_list {
+	unsigned int gen;
+	uint32_t addr;
+} wo_list[] = {
+	{ 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
+};
+
 static struct intel_wa_reg *wa_regs;
 static int num_wa_regs;
 
@@ -64,6 +73,21 @@ static void test_suspend_resume(void)
 	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
 }
 
+static bool write_only(const uint32_t addr)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
+		if (gen == wo_list[i].gen &&
+		    addr == wo_list[i].addr) {
+			igt_info("Skipping check for 0x%x due to write only\n", addr);
+			return true;
+		}
+	}
+
+	return false;
+}
+
 static int workaround_fail_count(void)
 {
 	int i, fail_count = 0;
@@ -85,6 +109,9 @@ static int workaround_fail_count(void)
 			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
 			  val, ok ? "OK" : "FAIL");
 
+		if (write_only(wa_regs[i].addr))
+			continue;
+
 		if (!ok) {
 			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
 				 wa_regs[i].addr, wa_regs[i].value,
@@ -124,7 +151,6 @@ igt_main
 {
 	igt_fixture {
 		int device = drm_open_driver_master(DRIVER_INTEL);
-		const int gen = intel_gen(intel_get_drm_devid(device));
 		struct pci_device *pci_dev;
 		FILE *file;
 		char *line = NULL;
@@ -133,6 +159,8 @@ igt_main
 
 		igt_require_gem(device);
 
+		gen = intel_gen(intel_get_drm_devid(device));
+
 		pci_dev = intel_get_pci_device();
 		igt_require(pci_dev);
 
-- 
2.11.0

_______________________________________________
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

* Re: [PATCH igt] tests/gem_workarounds: Skip write only registers
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
@ 2017-09-28 13:57 ` Chris Wilson
  2017-09-28 14:13 ` Petri Latvala
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-09-28 13:57 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx; +Cc: Rodrigo Vivi

Quoting Mika Kuoppala (2017-09-28 14:45:06)
> We have no means to check write only registers as
> this would need access through context image. For now we
> know that cnl has a one such register, 0xe5f0 which is used
> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
> the context image without and with workaround applied:
> 
> 0x0000a840: 0x0000e5f0 0xffff0800
> 0x0000a840: 0x0000e5f0 0xffff0820
> 
> we can conclude that the workaround setup is working right
> in this particular case.
> 
> Add a write only list and add register 0xe5f0 into this list.
> This is a temporary solution until a more capable context image
> checker emerges.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

It makes sense given the discovery of the w/o register. I was thinking
of how we could communicate these through the debugfs, but I think any
changes in direction involve pulling this test into the kernel. So, this
appears to be the pragmatic solution.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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: [PATCH igt] tests/gem_workarounds: Skip write only registers
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
  2017-09-28 13:57 ` Chris Wilson
@ 2017-09-28 14:13 ` Petri Latvala
  2017-09-28 14:27 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2017-09-28 14:13 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx, Rodrigo Vivi

On Thu, Sep 28, 2017 at 04:45:06PM +0300, Mika Kuoppala wrote:
> We have no means to check write only registers as
> this would need access through context image. For now we
> know that cnl has a one such register, 0xe5f0 which is used
> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
> the context image without and with workaround applied:
> 
> 0x0000a840: 0x0000e5f0 0xffff0800
> 0x0000a840: 0x0000e5f0 0xffff0820
> 
> we can conclude that the workaround setup is working right
> in this particular case.
> 
> Add a write only list and add register 0xe5f0 into this list.
> This is a temporary solution until a more capable context image
> checker emerges.


According to old wisdom, nothing is as permanent as a temporary
solution. I'd like this temporary-ness noted in a comment in the code
as well to ease future generations who finally get to fix this
properly.



-- 
Petri Latvala



> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  tests/gem_workarounds.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index d6641bd5..03b4dc6c 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -29,6 +29,8 @@
>  
>  #include <fcntl.h>
>  
> +static int gen;
> +
>  enum operation {
>  	GPU_RESET,
>  	SUSPEND_RESUME,
> @@ -41,6 +43,13 @@ struct intel_wa_reg {
>  	uint32_t mask;
>  };
>  
> +static struct write_only_list {
> +	unsigned int gen;
> +	uint32_t addr;
> +} wo_list[] = {
> +	{ 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
> +};
> +
>  static struct intel_wa_reg *wa_regs;
>  static int num_wa_regs;
>  
> @@ -64,6 +73,21 @@ static void test_suspend_resume(void)
>  	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>  }
>  
> +static bool write_only(const uint32_t addr)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
> +		if (gen == wo_list[i].gen &&
> +		    addr == wo_list[i].addr) {
> +			igt_info("Skipping check for 0x%x due to write only\n", addr);
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static int workaround_fail_count(void)
>  {
>  	int i, fail_count = 0;
> @@ -85,6 +109,9 @@ static int workaround_fail_count(void)
>  			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
>  			  val, ok ? "OK" : "FAIL");
>  
> +		if (write_only(wa_regs[i].addr))
> +			continue;
> +
>  		if (!ok) {
>  			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
>  				 wa_regs[i].addr, wa_regs[i].value,
> @@ -124,7 +151,6 @@ igt_main
>  {
>  	igt_fixture {
>  		int device = drm_open_driver_master(DRIVER_INTEL);
> -		const int gen = intel_gen(intel_get_drm_devid(device));
>  		struct pci_device *pci_dev;
>  		FILE *file;
>  		char *line = NULL;
> @@ -133,6 +159,8 @@ igt_main
>  
>  		igt_require_gem(device);
>  
> +		gen = intel_gen(intel_get_drm_devid(device));
> +
>  		pci_dev = intel_get_pci_device();
>  		igt_require(pci_dev);
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> 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

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

* ✓ Fi.CI.BAT: success for tests/gem_workarounds: Skip write only registers
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
  2017-09-28 13:57 ` Chris Wilson
  2017-09-28 14:13 ` Petri Latvala
@ 2017-09-28 14:27 ` Patchwork
  2017-09-28 15:00 ` [PATCH igt] " Mika Kuoppala
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-09-28 14:27 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: tests/gem_workarounds: Skip write only registers
URL   : https://patchwork.freedesktop.org/series/31073/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
2885b10f99b4beeb046e75af8b8488c229f629d3 igt/gem_exec_schedule: Ignore set-priority failures on old kernels

with latest DRM-Tip kernel build CI_DRM_3150
f4bd0d12dbf2 drm-tip: 2017y-09m-28d-13h-39m-47s UTC integration manifest

Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-warn -> PASS       (fi-glk-1) fdo#102777

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:471s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:420s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:524s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:507s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:494s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:557s
fi-cnl-y         total:289  pass:259  dwarn:0   dfail:0   fail:3   skip:27  time:667s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:417s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:572s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:428s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:406s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:444s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:488s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:470s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:468s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:579s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:595s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:553s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:753s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:479s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:570s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:416s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_267/
_______________________________________________
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

* [PATCH igt] tests/gem_workarounds: Skip write only registers
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
                   ` (2 preceding siblings ...)
  2017-09-28 14:27 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-09-28 15:00 ` Mika Kuoppala
  2017-09-28 15:05   ` Petri Latvala
  2017-09-28 21:51   ` Oscar Mateo
  2017-09-28 16:12 ` ✓ Fi.CI.BAT: success for tests/gem_workarounds: Skip write only registers (rev2) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 11+ messages in thread
From: Mika Kuoppala @ 2017-09-28 15:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

We have no means to check write only registers as
this would need access through context image. For now we
know that cnl has a one such register, 0xe5f0 which is used
to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
the context image without and with workaround applied:

0x0000a840: 0x0000e5f0 0xffff0800
0x0000a840: 0x0000e5f0 0xffff0820

we can conclude that the workaround setup is working right
in this particular case.

Add a write only list and add register 0xe5f0 into this list.
This is a temporary solution until a more capable context image
checker emerges.

v2: add code comment about adhocness (Petri)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_workarounds.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index d6641bd5..5e30a7b8 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -29,6 +29,8 @@
 
 #include <fcntl.h>
 
+static int gen;
+
 enum operation {
 	GPU_RESET,
 	SUSPEND_RESUME,
@@ -41,6 +43,21 @@ struct intel_wa_reg {
 	uint32_t mask;
 };
 
+static struct write_only_list {
+	unsigned int gen;
+	uint32_t addr;
+} wo_list[] = {
+	{ 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
+
+	/*
+	 * FIXME: If you are contemplating adding stuff here
+	 * consider this as a temporary solution. You need to
+	 * manually check from context image that your workaround
+	 * is having an effect. Consider creating a context image
+	 * validator to act as a superior solution.
+	 */
+};
+
 static struct intel_wa_reg *wa_regs;
 static int num_wa_regs;
 
@@ -64,6 +81,21 @@ static void test_suspend_resume(void)
 	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
 }
 
+static bool write_only(const uint32_t addr)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
+		if (gen == wo_list[i].gen &&
+		    addr == wo_list[i].addr) {
+			igt_info("Skipping check for 0x%x due to write only\n", addr);
+			return true;
+		}
+	}
+
+	return false;
+}
+
 static int workaround_fail_count(void)
 {
 	int i, fail_count = 0;
@@ -85,6 +117,9 @@ static int workaround_fail_count(void)
 			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
 			  val, ok ? "OK" : "FAIL");
 
+		if (write_only(wa_regs[i].addr))
+			continue;
+
 		if (!ok) {
 			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
 				 wa_regs[i].addr, wa_regs[i].value,
@@ -124,7 +159,6 @@ igt_main
 {
 	igt_fixture {
 		int device = drm_open_driver_master(DRIVER_INTEL);
-		const int gen = intel_gen(intel_get_drm_devid(device));
 		struct pci_device *pci_dev;
 		FILE *file;
 		char *line = NULL;
@@ -133,6 +167,8 @@ igt_main
 
 		igt_require_gem(device);
 
+		gen = intel_gen(intel_get_drm_devid(device));
+
 		pci_dev = intel_get_pci_device();
 		igt_require(pci_dev);
 
-- 
2.11.0

_______________________________________________
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

* Re: [PATCH igt] tests/gem_workarounds: Skip write only registers
  2017-09-28 15:00 ` [PATCH igt] " Mika Kuoppala
@ 2017-09-28 15:05   ` Petri Latvala
  2017-09-28 21:51   ` Oscar Mateo
  1 sibling, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2017-09-28 15:05 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx, Rodrigo Vivi

On Thu, Sep 28, 2017 at 06:00:03PM +0300, Mika Kuoppala wrote:
> We have no means to check write only registers as
> this would need access through context image. For now we
> know that cnl has a one such register, 0xe5f0 which is used
> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
> the context image without and with workaround applied:
> 
> 0x0000a840: 0x0000e5f0 0xffff0800
> 0x0000a840: 0x0000e5f0 0xffff0820
> 
> we can conclude that the workaround setup is working right
> in this particular case.
> 
> Add a write only list and add register 0xe5f0 into this list.
> This is a temporary solution until a more capable context image
> checker emerges.
> 
> v2: add code comment about adhocness (Petri)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/gem_workarounds.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index d6641bd5..5e30a7b8 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -29,6 +29,8 @@
>  
>  #include <fcntl.h>
>  
> +static int gen;
> +
>  enum operation {
>  	GPU_RESET,
>  	SUSPEND_RESUME,
> @@ -41,6 +43,21 @@ struct intel_wa_reg {
>  	uint32_t mask;
>  };
>  
> +static struct write_only_list {
> +	unsigned int gen;
> +	uint32_t addr;
> +} wo_list[] = {
> +	{ 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
> +
> +	/*
> +	 * FIXME: If you are contemplating adding stuff here
> +	 * consider this as a temporary solution. You need to
> +	 * manually check from context image that your workaround
> +	 * is having an effect. Consider creating a context image
> +	 * validator to act as a superior solution.
> +	 */
> +};

Excellent.

Reviewed-by: Petri Latvala <petri.latvala@intel.com>




>  static struct intel_wa_reg *wa_regs;
>  static int num_wa_regs;
>  
> @@ -64,6 +81,21 @@ static void test_suspend_resume(void)
>  	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>  }
>  
> +static bool write_only(const uint32_t addr)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
> +		if (gen == wo_list[i].gen &&
> +		    addr == wo_list[i].addr) {
> +			igt_info("Skipping check for 0x%x due to write only\n", addr);
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  static int workaround_fail_count(void)
>  {
>  	int i, fail_count = 0;
> @@ -85,6 +117,9 @@ static int workaround_fail_count(void)
>  			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
>  			  val, ok ? "OK" : "FAIL");
>  
> +		if (write_only(wa_regs[i].addr))
> +			continue;
> +
>  		if (!ok) {
>  			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
>  				 wa_regs[i].addr, wa_regs[i].value,
> @@ -124,7 +159,6 @@ igt_main
>  {
>  	igt_fixture {
>  		int device = drm_open_driver_master(DRIVER_INTEL);
> -		const int gen = intel_gen(intel_get_drm_devid(device));
>  		struct pci_device *pci_dev;
>  		FILE *file;
>  		char *line = NULL;
> @@ -133,6 +167,8 @@ igt_main
>  
>  		igt_require_gem(device);
>  
> +		gen = intel_gen(intel_get_drm_devid(device));
> +
>  		pci_dev = intel_get_pci_device();
>  		igt_require(pci_dev);
>  
> -- 
> 2.11.0
> 
_______________________________________________
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

* ✓ Fi.CI.BAT: success for tests/gem_workarounds: Skip write only registers (rev2)
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
                   ` (3 preceding siblings ...)
  2017-09-28 15:00 ` [PATCH igt] " Mika Kuoppala
@ 2017-09-28 16:12 ` Patchwork
  2017-09-28 16:19 ` ✗ Fi.CI.IGT: warning for tests/gem_workarounds: Skip write only registers Patchwork
  2017-09-28 17:27 ` ✗ Fi.CI.IGT: failure for tests/gem_workarounds: Skip write only registers (rev2) Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-09-28 16:12 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: tests/gem_workarounds: Skip write only registers (rev2)
URL   : https://patchwork.freedesktop.org/series/31073/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
3df22e0d2f8934311c62e4fd84bee24b32addb58 benchmarks/gem_exec_fault: Update for tryhard kernels.

with latest DRM-Tip kernel build CI_DRM_3150
f4bd0d12dbf2 drm-tip: 2017y-09m-28d-13h-39m-47s UTC integration manifest

Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-warn -> PASS       (fi-glk-1) fdo#102777

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:449s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:476s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:421s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:521s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:499s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:522s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:500s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:493s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:559s
fi-cnl-y         total:289  pass:259  dwarn:0   dfail:0   fail:3   skip:27  time:670s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:420s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:567s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:427s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:408s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:437s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:468s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:579s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:595s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:464s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:746s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:472s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:573s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:419s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_268/
_______________________________________________
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

* ✗ Fi.CI.IGT: warning for tests/gem_workarounds: Skip write only registers
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
                   ` (4 preceding siblings ...)
  2017-09-28 16:12 ` ✓ Fi.CI.BAT: success for tests/gem_workarounds: Skip write only registers (rev2) Patchwork
@ 2017-09-28 16:19 ` Patchwork
  2017-09-28 17:27 ` ✗ Fi.CI.IGT: failure for tests/gem_workarounds: Skip write only registers (rev2) Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-09-28 16:19 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: tests/gem_workarounds: Skip write only registers
URL   : https://patchwork.freedesktop.org/series/31073/
State : warning

== Summary ==

Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252
Test gem_eio:
        Subgroup wait:
                dmesg-warn -> PASS       (shard-hsw) fdo#102886
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-C-planes:
                pass       -> SKIP       (shard-hsw)

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102886 https://bugs.freedesktop.org/show_bug.cgi?id=102886
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2429 pass:1332 dwarn:3   dfail:0   fail:10  skip:1084 time:9894s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_267/shards.html
_______________________________________________
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

* ✗ Fi.CI.IGT: failure for tests/gem_workarounds: Skip write only registers (rev2)
  2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
                   ` (5 preceding siblings ...)
  2017-09-28 16:19 ` ✗ Fi.CI.IGT: warning for tests/gem_workarounds: Skip write only registers Patchwork
@ 2017-09-28 17:27 ` Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-09-28 17:27 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: tests/gem_workarounds: Skip write only registers (rev2)
URL   : https://patchwork.freedesktop.org/series/31073/
State : failure

== Summary ==

Test prime_mmap:
        Subgroup test_userptr:
                pass       -> DMESG-WARN (shard-hsw) fdo#102939
Test gem_eio:
        Subgroup in-flight:
                pass       -> DMESG-WARN (shard-hsw) fdo#102886 +1
Test gem_sync:
        Subgroup basic-each:
                pass       -> FAIL       (shard-hsw)

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

shard-hsw        total:2429 pass:1330 dwarn:5   dfail:0   fail:11  skip:1083 time:10045s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_268/shards.html
_______________________________________________
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: [PATCH igt] tests/gem_workarounds: Skip write only registers
  2017-09-28 15:00 ` [PATCH igt] " Mika Kuoppala
  2017-09-28 15:05   ` Petri Latvala
@ 2017-09-28 21:51   ` Oscar Mateo
  2017-09-29  6:30     ` Mika Kuoppala
  1 sibling, 1 reply; 11+ messages in thread
From: Oscar Mateo @ 2017-09-28 21:51 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx; +Cc: Rodrigo Vivi



On 09/28/2017 08:00 AM, Mika Kuoppala wrote:
> We have no means to check write only registers as
> this would need access through context image. For now we
> know that cnl has a one such register, 0xe5f0 which is used
> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
> the context image without and with workaround applied:
>
> 0x0000a840: 0x0000e5f0 0xffff0800
> 0x0000a840: 0x0000e5f0 0xffff0820
>
> we can conclude that the workaround setup is working right
> in this particular case.
>
> Add a write only list and add register 0xe5f0 into this list.
> This is a temporary solution until a more capable context image
> checker emerges.
>
> v2: add code comment about adhocness (Petri)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Acked-by: Oscar Mateo <oscar.mateo@intel.com>

>   tests/gem_workarounds.c | 38 +++++++++++++++++++++++++++++++++++++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index d6641bd5..5e30a7b8 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -29,6 +29,8 @@
>   
>   #include <fcntl.h>
>   
> +static int gen;
> +
>   enum operation {
>   	GPU_RESET,
>   	SUSPEND_RESUME,
> @@ -41,6 +43,21 @@ struct intel_wa_reg {
>   	uint32_t mask;
>   };
>   
> +static struct write_only_list {
> +	unsigned int gen;
> +	uint32_t addr;
> +} wo_list[] = {
> +	{ 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
> +
> +	/*
> +	 * FIXME: If you are contemplating adding stuff here
> +	 * consider this as a temporary solution. You need to
> +	 * manually check from context image that your workaround
> +	 * is having an effect. Consider creating a context image
> +	 * validator to act as a superior solution.
> +	 */
> +};
> +
>   static struct intel_wa_reg *wa_regs;
>   static int num_wa_regs;
>   
> @@ -64,6 +81,21 @@ static void test_suspend_resume(void)
>   	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>   }
>   
> +static bool write_only(const uint32_t addr)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
> +		if (gen == wo_list[i].gen &&
> +		    addr == wo_list[i].addr) {
> +			igt_info("Skipping check for 0x%x due to write only\n", addr);
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>   static int workaround_fail_count(void)
>   {
>   	int i, fail_count = 0;
> @@ -85,6 +117,9 @@ static int workaround_fail_count(void)
>   			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
>   			  val, ok ? "OK" : "FAIL");
>   
> +		if (write_only(wa_regs[i].addr))
> +			continue;
> +
>   		if (!ok) {
>   			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
>   				 wa_regs[i].addr, wa_regs[i].value,
> @@ -124,7 +159,6 @@ igt_main
>   {
>   	igt_fixture {
>   		int device = drm_open_driver_master(DRIVER_INTEL);
> -		const int gen = intel_gen(intel_get_drm_devid(device));
>   		struct pci_device *pci_dev;
>   		FILE *file;
>   		char *line = NULL;
> @@ -133,6 +167,8 @@ igt_main
>   
>   		igt_require_gem(device);
>   
> +		gen = intel_gen(intel_get_drm_devid(device));
> +
>   		pci_dev = intel_get_pci_device();
>   		igt_require(pci_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: [PATCH igt] tests/gem_workarounds: Skip write only registers
  2017-09-28 21:51   ` Oscar Mateo
@ 2017-09-29  6:30     ` Mika Kuoppala
  0 siblings, 0 replies; 11+ messages in thread
From: Mika Kuoppala @ 2017-09-29  6:30 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx; +Cc: Rodrigo Vivi

Oscar Mateo <oscar.mateo@intel.com> writes:

> On 09/28/2017 08:00 AM, Mika Kuoppala wrote:
>> We have no means to check write only registers as
>> this would need access through context image. For now we
>> know that cnl has a one such register, 0xe5f0 which is used
>> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
>> the context image without and with workaround applied:
>>
>> 0x0000a840: 0x0000e5f0 0xffff0800
>> 0x0000a840: 0x0000e5f0 0xffff0820
>>
>> we can conclude that the workaround setup is working right
>> in this particular case.
>>
>> Add a write only list and add register 0xe5f0 into this list.
>> This is a temporary solution until a more capable context image
>> checker emerges.
>>
>> v2: add code comment about adhocness (Petri)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>
> Acked-by: Oscar Mateo <oscar.mateo@intel.com>

Pushed, thanks for reviews and ack.
-Mika

>
>>   tests/gem_workarounds.c | 38 +++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
>> index d6641bd5..5e30a7b8 100644
>> --- a/tests/gem_workarounds.c
>> +++ b/tests/gem_workarounds.c
>> @@ -29,6 +29,8 @@
>>   
>>   #include <fcntl.h>
>>   
>> +static int gen;
>> +
>>   enum operation {
>>   	GPU_RESET,
>>   	SUSPEND_RESUME,
>> @@ -41,6 +43,21 @@ struct intel_wa_reg {
>>   	uint32_t mask;
>>   };
>>   
>> +static struct write_only_list {
>> +	unsigned int gen;
>> +	uint32_t addr;
>> +} wo_list[] = {
>> +	{ 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
>> +
>> +	/*
>> +	 * FIXME: If you are contemplating adding stuff here
>> +	 * consider this as a temporary solution. You need to
>> +	 * manually check from context image that your workaround
>> +	 * is having an effect. Consider creating a context image
>> +	 * validator to act as a superior solution.
>> +	 */
>> +};
>> +
>>   static struct intel_wa_reg *wa_regs;
>>   static int num_wa_regs;
>>   
>> @@ -64,6 +81,21 @@ static void test_suspend_resume(void)
>>   	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>>   }
>>   
>> +static bool write_only(const uint32_t addr)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
>> +		if (gen == wo_list[i].gen &&
>> +		    addr == wo_list[i].addr) {
>> +			igt_info("Skipping check for 0x%x due to write only\n", addr);
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>   static int workaround_fail_count(void)
>>   {
>>   	int i, fail_count = 0;
>> @@ -85,6 +117,9 @@ static int workaround_fail_count(void)
>>   			  wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
>>   			  val, ok ? "OK" : "FAIL");
>>   
>> +		if (write_only(wa_regs[i].addr))
>> +			continue;
>> +
>>   		if (!ok) {
>>   			igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
>>   				 wa_regs[i].addr, wa_regs[i].value,
>> @@ -124,7 +159,6 @@ igt_main
>>   {
>>   	igt_fixture {
>>   		int device = drm_open_driver_master(DRIVER_INTEL);
>> -		const int gen = intel_gen(intel_get_drm_devid(device));
>>   		struct pci_device *pci_dev;
>>   		FILE *file;
>>   		char *line = NULL;
>> @@ -133,6 +167,8 @@ igt_main
>>   
>>   		igt_require_gem(device);
>>   
>> +		gen = intel_gen(intel_get_drm_devid(device));
>> +
>>   		pci_dev = intel_get_pci_device();
>>   		igt_require(pci_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

end of thread, other threads:[~2017-09-29  6:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-28 13:45 [PATCH igt] tests/gem_workarounds: Skip write only registers Mika Kuoppala
2017-09-28 13:57 ` Chris Wilson
2017-09-28 14:13 ` Petri Latvala
2017-09-28 14:27 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-09-28 15:00 ` [PATCH igt] " Mika Kuoppala
2017-09-28 15:05   ` Petri Latvala
2017-09-28 21:51   ` Oscar Mateo
2017-09-29  6:30     ` Mika Kuoppala
2017-09-28 16:12 ` ✓ Fi.CI.BAT: success for tests/gem_workarounds: Skip write only registers (rev2) Patchwork
2017-09-28 16:19 ` ✗ Fi.CI.IGT: warning for tests/gem_workarounds: Skip write only registers Patchwork
2017-09-28 17:27 ` ✗ Fi.CI.IGT: failure for tests/gem_workarounds: Skip write only registers (rev2) Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox