[igt,v2,2/6] igt/gem_exec_parse: remove oacontrol checks

Commit Message

Robert Bragg Nov. 7, 2016, 10:26 p.m. UTC

The command parser no longer whitelists or does anything special for the
OACONTROL register which is now considered owned by i915-perf.

As a follow up the plan is to at least check that attempting to write to
OACONTROL from userspace must not fail with an EINVAL error, otherwise
Mesa's graceful fallback path for not being able to write to OACONTROL
via LRI commands will cause Mesa applications to abort().

Signed-off-by: Robert Bragg <[e-mail]>
---
 tests/gem_exec_parse.c | 88 --------------------------------------------------
 1 file changed, 88 deletions(-)

Comments

Chris Wilson Nov. 8, 2016, 10:29 a.m. UTC | #1

On Mon, Nov 07, 2016 at 10:26:59PM +0000, Robert Bragg wrote:
> The command parser no longer whitelists or does anything special for the
> OACONTROL register which is now considered owned by i915-perf.
> 
> As a follow up the plan is to at least check that attempting to write to
> OACONTROL from userspace must not fail with an EINVAL error, otherwise
> Mesa's graceful fallback path for not being able to write to OACONTROL
> via LRI commands will cause Mesa applications to abort().

This was part of the earlier ABI, and will remain ABI for those kernels.
If the cmdparser advertises that it expects OACONTROL to be paired, we
should keep testing that the cmdparser does as advertised.
-Chris

Patch

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index a39db3e..36bf57d 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -34,7 +34,6 @@ 
 #define I915_PARAM_CMD_PARSER_VERSION       28
 #endif
 
-#define OACONTROL 0x2360
 #define DERRMR 0x44050
 
 static int command_parser_version(int fd)
@@ -133,10 +132,6 @@  static void hsw_load_register_reg(void)
 	gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
 	igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
 
-	buf[2] = OACONTROL; /* filtered */
-	gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
-	igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
-
 	buf[2] = DERRMR; /* master only */
 	gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
 	igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
@@ -385,29 +380,6 @@  static void exec_batch_chained(int fd, uint32_t cmd_bo, uint32_t *cmds,
 	gem_close(fd, target_bo);
 }
 
-static void stray_lri(int fd, uint32_t handle)
-{
-	/* Ideally this would test all once whitelisted registers */
-	uint32_t lri[] = {
-		MI_LOAD_REGISTER_IMM,
-		OACONTROL,
-		0x31337000,
-		MI_BATCH_BUFFER_END,
-	};
-	int err;
-
-	igt_assert_eq_u32(intel_register_read(OACONTROL), 0xdeadbeef);
-
-	err = __exec_batch(fd, handle, lri, sizeof(lri), I915_EXEC_RENDER);
-	if (err == -EINVAL)
-		return;
-
-	igt_assert_eq(err, 0);
-	gem_sync(fd, handle);
-
-	igt_assert_eq_u32(intel_register_read(OACONTROL), 0xdeadbeef);
-}
-
 uint32_t handle;
 int fd;
 
@@ -486,23 +458,6 @@  igt_main
 			   -EINVAL);
 	}
 
-	igt_subtest_group {
-		igt_fixture {
-			intel_register_access_init(intel_get_pci_device(), 0);
-
-			intel_register_write(OACONTROL, 0xdeadbeef);
-			igt_assert_eq_u32(intel_register_read(OACONTROL), 0xdeadbeef);
-		}
-
-		igt_subtest("basic-stray-lri")
-			stray_lri(fd, handle);
-
-		igt_fixture {
-			intel_register_write(OACONTROL, 0);
-			intel_register_access_fini();
-		}
-	}
-
 	igt_subtest("registers") {
 		uint32_t lri_bad[] = {
 			MI_LOAD_REGISTER_IMM,
@@ -563,49 +518,6 @@  igt_main
 				 0);
 	}
 
-	igt_subtest("oacontrol-tracking") {
-		uint32_t lri_ok[] = {
-			MI_LOAD_REGISTER_IMM,
-			OACONTROL,
-			0x31337000,
-			MI_LOAD_REGISTER_IMM,
-			OACONTROL,
-			0x0,
-			MI_BATCH_BUFFER_END,
-			0
-		};
-		uint32_t lri_bad[] = {
-			MI_LOAD_REGISTER_IMM,
-			OACONTROL,
-			0x31337000,
-			MI_BATCH_BUFFER_END,
-		};
-		uint32_t lri_extra_bad[] = {
-			MI_LOAD_REGISTER_IMM,
-			OACONTROL,
-			0x31337000,
-			MI_LOAD_REGISTER_IMM,
-			OACONTROL,
-			0x0,
-			MI_LOAD_REGISTER_IMM,
-			OACONTROL,
-			0x31337000,
-			MI_BATCH_BUFFER_END,
-		};
-		exec_batch(fd, handle,
-			   lri_ok, sizeof(lri_ok),
-			   I915_EXEC_RENDER,
-			   0);
-		exec_batch(fd, handle,
-			   lri_bad, sizeof(lri_bad),
-			   I915_EXEC_RENDER,
-			   -EINVAL);
-		exec_batch(fd, handle,
-			   lri_extra_bad, sizeof(lri_extra_bad),
-			   I915_EXEC_RENDER,
-			   -EINVAL);
-	}
-
 	igt_subtest("chained-batch") {
 		uint32_t pc[] = {
 			GFX_OP_PIPE_CONTROL,