diff mbox

[i-g-t,v3] igt/gem_workarounds: Test all types of workarounds

Message ID 1507928159-7144-1-git-send-email-oscar.mateo@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

oscar.mateo@intel.com Oct. 13, 2017, 8:55 p.m. UTC
Apart from context based workarounds, we can now also test for global
MMIO and whitelisting ones.

Do take into account that this test does not guarantee that all known
WAs for a given platform are applied. It only checks that the WAs the
kernel does know about are correctly applied (e.g. they didn't get
lost on a GPU reset or a suspend/resume).

v2:
  - Do not wait for the GPU unnecessarily (Chris)
  - Make a comment that this tests only looks for regressions (Chris)

v3:
  - GT instead of MMIO (Chris, Mika)
  - Limit register access to just the mmio tests (Chris)
  - Add the possibility to read the WAs from a local file (Chris)
  - Display workarounds (Ville)

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/gem_workarounds.c | 204 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 165 insertions(+), 39 deletions(-)

Comments

Chris Wilson Oct. 15, 2017, 8:59 p.m. UTC | #1
Quoting Oscar Mateo (2017-10-13 21:55:59)
> @@ -134,20 +147,20 @@ static int workaround_fail_count(int fd, uint32_t ctx)
>         igt_debug("Address    val        mask        read        result\n");
>  
>         out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
> -       for (int i = 0; i < num_wa_regs; i++) {
> +       for (int i = 0; i < num_ctx_wa_regs; i++) {
>                 const bool ok =
> -                       (wa_regs[i].value & wa_regs[i].mask) ==
> -                       (out[i] & wa_regs[i].mask);
> +                       (ctx_wa_regs[i].value & ctx_wa_regs[i].mask) ==
> +                       (out[i] & ctx_wa_regs[i].mask);
>                 char buf[80];
>  
>                 snprintf(buf, sizeof(buf),
>                          "0x%05X    0x%08X    0x%08X    0x%08X",
> -                        wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
> +                        ctx_wa_regs[i].addr, ctx_wa_regs[i].value, ctx_wa_regs[i].mask,
>                          out[i]);
>  
>                 if (ok) {
>                         igt_debug("%s    OK\n", buf);
> -               } else if (write_only(wa_regs[i].addr)) {
> +               } else if (write_only(ctx_wa_regs[i].addr)) {
>                         igt_debug("%s    IGNORED (w/o)\n", buf);
>                 } else {
>                         igt_warn("%s    FAIL\n", buf);
> @@ -163,6 +176,58 @@ static int workaround_fail_count(int fd, uint32_t ctx)
>         return fail_count;
>  }
>  
> +static int mmio_workarounds_fail_count(struct intel_wa_reg *wa_regs, int num_wa_regs)
> +{
> +       int i, fail_count = 0;
> +
> +       if (!num_wa_regs)
> +               return 0;
> +
> +       igt_debug("Address    val        mask        read        result\n");
> +
> +       for (i = 0; i < num_wa_regs; ++i) {
> +               const uint32_t val = intel_register_read(wa_regs[i].addr);
> +               const bool ok = (wa_regs[i].value & wa_regs[i].mask) ==
> +                       (val & wa_regs[i].mask);
> +
> +               igt_debug("0x%05X    0x%08X    0x%08X    0x%08X    %s\n",
> +                         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    0x%08X    0x%08X    0x%08X    %s\n",
> +                                wa_regs[i].addr, wa_regs[i].value,
> +                                wa_regs[i].mask, val, ok ? "OK" : "FAIL");
> +                       fail_count++;

Ugh, not the debug then duplicate warn again (and not telling us about
the ignored). My vote is that we feed the result into a single pretty
printer (based on the current ctx w/a checker)  for all loops and
remember to include the type (whether ctx, gt, display etc).
-Chris
diff mbox

Patch

diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index 7b99961..d7624fd 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -28,6 +28,7 @@ 
 #include "igt.h"
 
 #include <fcntl.h>
+#include <ctype.h>
 
 #define PAGE_SIZE 4096
 #define PAGE_ALIGN(x) ALIGN(x, PAGE_SIZE)
@@ -62,8 +63,17 @@  static struct write_only_list {
 	 */
 };
 
-static struct intel_wa_reg *wa_regs;
-static int num_wa_regs;
+static struct intel_wa_reg *ctx_wa_regs;
+static int num_ctx_wa_regs;
+
+static struct intel_wa_reg *gt_wa_regs;
+static int num_gt_wa_regs;
+
+static struct intel_wa_reg *display_wa_regs;
+static int num_display_wa_regs;
+
+static struct intel_wa_reg *whitelist_wa_regs;
+static int num_whitelist_wa_regs;
 
 static bool write_only(const uint32_t addr)
 {
@@ -82,7 +92,7 @@  static bool write_only(const uint32_t addr)
 
 #define MI_STORE_REGISTER_MEM (0x24 << 23)
 
-static int workaround_fail_count(int fd, uint32_t ctx)
+static int ctx_workarounds_fail_count(int fd, uint32_t ctx)
 {
 	struct drm_i915_gem_exec_object2 obj[2];
 	struct drm_i915_gem_relocation_entry *reloc;
@@ -91,13 +101,16 @@  static int workaround_fail_count(int fd, uint32_t ctx)
 	uint32_t *base, *out;
 	int fail_count = 0;
 
-	reloc = calloc(num_wa_regs, sizeof(*reloc));
+	if (!num_ctx_wa_regs)
+		return 0;
+
+	reloc = calloc(num_ctx_wa_regs, sizeof(*reloc));
 	igt_assert(reloc);
 
-	result_sz = 4 * num_wa_regs;
+	result_sz = 4 * num_ctx_wa_regs;
 	result_sz = PAGE_ALIGN(result_sz);
 
-	batch_sz = 16 * num_wa_regs + 4;
+	batch_sz = 16 * num_ctx_wa_regs + 4;
 	batch_sz = PAGE_ALIGN(batch_sz);
 
 	memset(obj, 0, sizeof(obj));
@@ -105,12 +118,12 @@  static int workaround_fail_count(int fd, uint32_t ctx)
 	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;
+	obj[1].relocation_count = num_ctx_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++) {
+	for (int i = 0; i < num_ctx_wa_regs; i++) {
 		*out++ = MI_STORE_REGISTER_MEM | ((gen >= 8 ? 4 : 2) - 2);
-		*out++ = wa_regs[i].addr;
+		*out++ = ctx_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);
@@ -134,20 +147,20 @@  static int workaround_fail_count(int fd, uint32_t ctx)
 	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
 
 	out = gem_mmap__cpu(fd, obj[0].handle, 0, result_sz, PROT_READ);
-	for (int i = 0; i < num_wa_regs; i++) {
+	for (int i = 0; i < num_ctx_wa_regs; i++) {
 		const bool ok =
-			(wa_regs[i].value & wa_regs[i].mask) ==
-			(out[i] & wa_regs[i].mask);
+			(ctx_wa_regs[i].value & ctx_wa_regs[i].mask) ==
+			(out[i] & ctx_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,
+			 ctx_wa_regs[i].addr, ctx_wa_regs[i].value, ctx_wa_regs[i].mask,
 			 out[i]);
 
 		if (ok) {
 			igt_debug("%s\tOK\n", buf);
-		} else if (write_only(wa_regs[i].addr)) {
+		} else if (write_only(ctx_wa_regs[i].addr)) {
 			igt_debug("%s\tIGNORED (w/o)\n", buf);
 		} else {
 			igt_warn("%s\tFAIL\n", buf);
@@ -163,6 +176,58 @@  static int workaround_fail_count(int fd, uint32_t ctx)
 	return fail_count;
 }
 
+static int mmio_workarounds_fail_count(struct intel_wa_reg *wa_regs, int num_wa_regs)
+{
+	int i, fail_count = 0;
+
+	if (!num_wa_regs)
+		return 0;
+
+	igt_debug("Address\tval\t\tmask\t\tread\t\tresult\n");
+
+	for (i = 0; i < num_wa_regs; ++i) {
+		const uint32_t val = intel_register_read(wa_regs[i].addr);
+		const bool ok = (wa_regs[i].value & wa_regs[i].mask) ==
+			(val & wa_regs[i].mask);
+
+		igt_debug("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
+			  wa_regs[i].addr, wa_regs[i].value,
+			  wa_regs[i].mask, val, ok ? "OK" : "FAIL");
+
+		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,
+				 wa_regs[i].mask, val, ok ? "OK" : "FAIL");
+			fail_count++;
+		}
+	}
+
+	return fail_count;
+}
+
+static int workarounds_fail_count(int fd, uint32_t ctx)
+{
+	int fail_count = 0;
+
+	fail_count += ctx_workarounds_fail_count(fd, ctx);
+
+	intel_register_access_init(intel_get_pci_device(), 0, fd);
+
+	fail_count += mmio_workarounds_fail_count(gt_wa_regs,
+						  num_gt_wa_regs);
+	fail_count += mmio_workarounds_fail_count(display_wa_regs,
+						  num_display_wa_regs);
+	fail_count += mmio_workarounds_fail_count(whitelist_wa_regs,
+						  num_whitelist_wa_regs);
+
+	intel_register_access_fini();
+
+	return fail_count;
+}
+
 static int reopen(int fd)
 {
 	char path[256];
@@ -185,7 +250,7 @@  static void check_workarounds(int fd, enum operation op, unsigned int flags)
 	if (flags & CONTEXT)
 		ctx = gem_context_create(fd);
 
-	igt_assert_eq(workaround_fail_count(fd, ctx), 0);
+	igt_assert_eq(workarounds_fail_count(fd, ctx), 0);
 
 	switch (op) {
 	case GPU_RESET:
@@ -209,7 +274,7 @@  static void check_workarounds(int fd, enum operation op, unsigned int flags)
 		igt_assert(0);
 	}
 
-	igt_assert_eq(workaround_fail_count(fd, ctx), 0);
+	igt_assert_eq(workarounds_fail_count(fd, ctx), 0);
 
 	if (flags & CONTEXT)
 		gem_context_destroy(fd, ctx);
@@ -217,6 +282,62 @@  static void check_workarounds(int fd, enum operation op, unsigned int flags)
 		close(fd);
 }
 
+static bool is_empty(const char *s)
+{
+	while (*s != '\0') {
+		if (!isspace(*s))
+			return false;
+		s++;
+	}
+
+	return true;
+}
+
+static char *skip_preamble(char *s, const char *preamble)
+{
+	while (*s == *preamble) {
+		s++;
+		preamble++;
+	}
+
+	return s;
+}
+
+static void read_workarounds(FILE *file, const char *preamble, struct intel_wa_reg **regs, int *num)
+{
+	char *header, *line = NULL;
+	size_t line_size;
+	struct intel_wa_reg *wa_regs;
+	int num_wa_regs;
+	int i = 0;
+
+	igt_assert(getline(&line, &line_size, file) > 0);
+	igt_debug("i915_wa_registers: %s", line);
+	header = skip_preamble(line, preamble);
+	sscanf(header, " workarounds applied: %d", &num_wa_regs);
+
+	wa_regs = malloc(num_wa_regs * sizeof(*wa_regs));
+
+	while (getline(&line, &line_size, file) > 0) {
+		if (is_empty(line))
+			break;
+
+		igt_debug("%s", line);
+		if (sscanf(line, "0x%X: 0x%08X, mask: 0x%08X",
+			   &wa_regs[i].addr,
+			   &wa_regs[i].value,
+			   &wa_regs[i].mask) == 3)
+			i++;
+	}
+
+	igt_assert_lte(i, num_wa_regs);
+
+	*regs = wa_regs;
+	*num = num_wa_regs;
+
+	free(line);
+}
+
 igt_main
 {
 	int device = -1;
@@ -242,38 +363,38 @@  igt_main
 
 	igt_fixture {
 		FILE *file;
-		char *line = NULL;
-		size_t line_size;
-		int i, fd;
+		int fd;
 
 		device = drm_open_driver(DRIVER_INTEL);
 		igt_require_gem(device);
 
 		gen = intel_gen(intel_get_drm_devid(device));
 
-		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
-		file = fdopen(fd, "r");
-		igt_assert(getline(&line, &line_size, file) > 0);
-		igt_debug("i915_wa_registers: %s", line);
-		sscanf(line, "Workarounds applied: %d", &num_wa_regs);
-		igt_require(num_wa_regs > 0);
-
-		wa_regs = malloc(num_wa_regs * sizeof(*wa_regs));
-		igt_assert(wa_regs);
-
-		i = 0;
-		while (getline(&line, &line_size, file) > 0) {
-			igt_debug("%s", line);
-			if (sscanf(line, "0x%X: 0x%08X, mask: 0x%08X",
-				   &wa_regs[i].addr,
-				   &wa_regs[i].value,
-				   &wa_regs[i].mask) == 3)
-				i++;
+		if ((file = fopen("wa_registers.txt", "r"))) {
+			igt_info("Using a local workarounds file\n");
+		} else {
+			fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
+			file = fdopen(fd, "r");
 		}
 
-		igt_assert_lte(i, num_wa_regs);
+		/*
+		 * This test relies on the list of workarounds the kernel says
+		 * have been applied and it only checks that those are (indeed)
+		 * correctly applied. It does not report whether the system has
+		 * applied all known workarounds for a fiven platform.
+		 */
+		read_workarounds(file, "Context",
+				 &ctx_wa_regs, &num_ctx_wa_regs);
+		read_workarounds(file,
+				 "GT", &gt_wa_regs, &num_gt_wa_regs);
+		read_workarounds(file,
+				 "Display", &display_wa_regs, &num_display_wa_regs);
+		read_workarounds(file,
+				 "Whitelist", &whitelist_wa_regs, &num_whitelist_wa_regs);
+		igt_require(num_ctx_wa_regs +
+			    num_gt_wa_regs +
+			    num_whitelist_wa_regs > 0);
 
-		free(line);
 		fclose(file);
 		close(fd);
 	}
@@ -284,4 +405,9 @@  igt_main
 				check_workarounds(device, op->op, m->flags);
 		}
 	}
+
+	igt_fixture {
+		free(ctx_wa_regs);
+		free(gt_wa_regs);
+	}
 }