All of lore.kernel.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 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.