Re: [PATCH igt] tests/gem_workarounds: Skip write only registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---

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

  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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux