diff mbox

[i-g-t,v2] tests/gem_mocs_settings: Fix LNCFCMOCS testing and extract the subtests

Message ID 20170704144308.27805-1-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski July 4, 2017, 2:43 p.m. UTC 
Testing LNCFCMOCS values on non-render engines is tricky. The values in
those registers are lost on RC6, which means that if users of non-render
engines want to see the proper values, they need to obtain a forcewake
and execute something on render (relying on it to restore the values)
before using non-render engine.
Previous version of the test did exactly that - we were relying on the
fact that we're taking forcewake (hidden by intel_register_access_init,
even though the test is not doing any mmio accesses) before iterating
through engines (and render is before other engines, so job done).
I really hope that this is not an ABI and those registers are not used
on non-render in any way. Let's limit testing LNCFCMOCS to render
engine only.
The other non-render issue is that when we're using I915_EXEC_BSD, we
can't be sure which BSD ring we'll end up executing on. Let's
explicitly select BSD1 and BSD2 in our tests.
While we're here, let's also remove the duplicated code and add some
structure by extracting moving more content into subtests.
We're only doing tests that involve "dirtying" the registers for the
render engine - since it's the only one that has those registers in its
context.

v2: Do not skip all BSD engines, test non-default contexts on render
only, change names in CI extended.testlist

Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Łukasz Kałamarz <lukasz.kalamarz@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Łukasz Kałamarz <lukasz.kalamarz@intel.com>
---
 tests/gem_mocs_settings.c        | 355 ++++++++++++---------------------------
 tests/intel-ci/extended.testlist |  27 ++-
 2 files changed, 134 insertions(+), 248 deletions(-)

Comments

Arkadiusz Hiler July 6, 2017, 11:03 a.m. UTC | #1 
On Tue, Jul 04, 2017 at 04:43:08PM +0200, Michał Winiarski wrote:
> Testing LNCFCMOCS values on non-render engines is tricky. The values in
> those registers are lost on RC6, which means that if users of non-render
> engines want to see the proper values, they need to obtain a forcewake
> and execute something on render (relying on it to restore the values)
> before using non-render engine.
> Previous version of the test did exactly that - we were relying on the
> fact that we're taking forcewake (hidden by intel_register_access_init,
> even though the test is not doing any mmio accesses) before iterating
> through engines (and render is before other engines, so job done).
> I really hope that this is not an ABI and those registers are not used
> on non-render in any way. Let's limit testing LNCFCMOCS to render
> engine only.
> The other non-render issue is that when we're using I915_EXEC_BSD, we
> can't be sure which BSD ring we'll end up executing on. Let's
> explicitly select BSD1 and BSD2 in our tests.
> While we're here, let's also remove the duplicated code and add some
> structure by extracting moving more content into subtests.
> We're only doing tests that involve "dirtying" the registers for the
> render engine - since it's the only one that has those registers in its
> context.
> 
> v2: Do not skip all BSD engines, test non-default contexts on render
> only, change names in CI extended.testlist
> 
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Łukasz Kałamarz <lukasz.kalamarz@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Reviewed-by: Łukasz Kałamarz <lukasz.kalamarz@intel.com>

pushed

thanks for the patch and the review
diff mbox

Patch

diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
index a96aa66..05dfc63 100644
--- a/tests/gem_mocs_settings.c
+++ b/tests/gem_mocs_settings.c
@@ -32,14 +32,28 @@ 
 #include "igt_sysfs.h"
 
 #define MAX_NUMBER_MOCS_REGISTERS	(64)
-
 enum {
 	NONE,
 	RESET,
+	RC6,
 	SUSPEND,
-	HIBERNATE
+	HIBERNATE,
+	MAX_MOCS_TEST_MODES
+};
+
+static const char * const test_modes[] = {
+	[NONE] = "settings",
+	[RESET] = "reset",
+	[RC6] = "rc6",
+	[SUSPEND] = "suspend",
+	[HIBERNATE] = "hibernate"
 };
 
+#define MOCS_NON_DEFAULT_CTX	(1<<0)
+#define MOCS_DIRTY_VALUES	(1<<1)
+#define ALL_MOCS_FLAGS		(MOCS_NON_DEFAULT_CTX | \
+				 MOCS_DIRTY_VALUES)
+
 #define GEN9_LNCFCMOCS0		(0xB020)	/* L3 Cache Control base */
 #define GEN9_GFX_MOCS_0		(0xc800)	/* Graphics MOCS base register*/
 #define GEN9_MFX0_MOCS_0	(0xc900)	/* Media 0 MOCS base register*/
@@ -117,19 +131,18 @@  static bool get_mocs_settings(int fd, struct mocs_table *table, bool dirty)
 	return result;
 }
 
+#define LOCAL_I915_EXEC_BSD1 (I915_EXEC_BSD | (1<<13))
+#define LOCAL_I915_EXEC_BSD2 (I915_EXEC_BSD | (2<<13))
+
 static uint32_t get_engine_base(uint32_t engine)
 {
-	/* Note we cannot test BSD1 or BSD2 due to limitations of current ANI */
 	switch (engine) {
-	case I915_EXEC_BSD:	return GEN9_MFX0_MOCS_0;
-/*
-	case I915_EXEC_BSD1:	return GEN9_MFX0_MOCS_0;
-	case I915_EXEC_BSD2:	return GEN9_MFX1_MOCS_0;
-*/
-	case I915_EXEC_RENDER:	return GEN9_GFX_MOCS_0;
-	case I915_EXEC_BLT:	return GEN9_BLT_MOCS_0;
-	case I915_EXEC_VEBOX:	return GEN9_VEBOX_MOCS_0;
-	default:		return 0;
+	case LOCAL_I915_EXEC_BSD1:	return GEN9_MFX0_MOCS_0;
+	case LOCAL_I915_EXEC_BSD2:	return GEN9_MFX1_MOCS_0;
+	case I915_EXEC_RENDER:		return GEN9_GFX_MOCS_0;
+	case I915_EXEC_BLT:		return GEN9_BLT_MOCS_0;
+	case I915_EXEC_VEBOX:		return GEN9_VEBOX_MOCS_0;
+	default:			return 0;
 	}
 }
 
@@ -252,7 +265,7 @@  static void check_control_registers(int fd,
 				    uint32_t ctx_id,
 				    bool dirty)
 {
-	const uint32_t reg_base  = get_engine_base(engine);
+	const uint32_t reg_base = get_engine_base(engine);
 	uint32_t dst_handle = gem_create(fd, 4096);
 	uint32_t *read_regs;
 	struct mocs_table table;
@@ -299,6 +312,7 @@  static void check_l3cc_registers(int fd,
 	read_regs = gem_mmap__cpu(fd, dst_handle, 0, 4096, PROT_READ);
 
 	gem_set_domain(fd, dst_handle, I915_GEM_DOMAIN_CPU, 0);
+
 	for (index = 0; index < table.size / 2; index++) {
 		igt_assert_eq_u32(read_regs[index] & 0xffff,
 				  table.table[index * 2].l3cc_value);
@@ -314,183 +328,65 @@  static void check_l3cc_registers(int fd,
 	gem_close(fd, dst_handle);
 }
 
-static void test_context_mocs_values(int fd, unsigned engine)
-{
-	int local_fd;
-	uint32_t ctx_id = 0;
-
-	local_fd = fd;
-	if (local_fd == -1)
-		local_fd = drm_open_driver_master(DRIVER_INTEL);
-
-	check_control_registers(local_fd, engine, ctx_id, false);
-	check_l3cc_registers(local_fd, engine, ctx_id, false);
-
-	if (engine == I915_EXEC_RENDER) {
-		ctx_id = gem_context_create(local_fd);
-
-		check_control_registers(local_fd, engine, ctx_id, false);
-		check_l3cc_registers(local_fd, engine, ctx_id, false);
-
-		gem_context_destroy(local_fd, ctx_id);
-	}
-
-	if (local_fd != fd)
-		close(local_fd);
-}
 
-static bool local_has_ring(int fd, unsigned engine)
+static uint32_t rc6_residency(int dir)
 {
-	bool has_ring;
-	int local_fd;
-
-	if (get_engine_base(engine) == 0)
-		return false;
-
-	if (fd == -1)
-		local_fd = drm_open_driver_master(DRIVER_INTEL);
-	else
-		local_fd = fd;
-
-	has_ring = gem_has_ring(local_fd, engine);
-	if (local_fd != fd)
-		close(local_fd);
-
-	return has_ring;
+	return igt_sysfs_get_u32(dir, "power/rc6_residency_ms");
 }
 
-static void test_mocs_values(int fd)
+static void rc6_wait(int fd)
 {
-	const struct intel_execution_engine *e;
+	int sysfs;
+	uint32_t residency;
 
-	for (e = intel_execution_engines; e->name; e++) {
-		unsigned engine = e->exec_id | e->flags;
+	sysfs = igt_sysfs_open(fd, NULL);
+	igt_assert_lte(0, sysfs);
 
-		if (!local_has_ring(fd, engine))
-			continue;
+	residency = rc6_residency(sysfs);
+	igt_require(igt_wait(rc6_residency(sysfs) != residency, 10000, 2));
 
-		igt_debug("Testing %s\n", e->name);
-		test_context_mocs_values(fd, engine);
-	}
+	close(sysfs);
 }
 
-static void default_context_tests(unsigned mode)
+static void check_mocs_values(int fd, unsigned engine, uint32_t ctx_id, bool dirty)
 {
-	int fd = drm_open_driver_master(DRIVER_INTEL);
+	check_control_registers(fd, engine, ctx_id, dirty);
 
-	igt_debug("Testing Non/Default Context Engines\n");
-	test_mocs_values(fd);
-
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset(fd);	break;
-	case SUSPEND:	igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
-						      SUSPEND_TEST_NONE); break;
-	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
-						      SUSPEND_TEST_NONE); break;
-	}
-
-	test_mocs_values(fd);
-	close(fd);
-
-	igt_debug("Testing Pristine Defaults\n");
-	test_mocs_values(-1);
+	if (engine == I915_EXEC_RENDER)
+		check_l3cc_registers(fd, engine, ctx_id, dirty);
 }
 
-static void default_dirty_tests(unsigned mode)
+static void write_dirty_mocs(int fd, unsigned engine, uint32_t ctx_id)
 {
-	const struct intel_execution_engine *e;
-	int fd = drm_open_driver_master(DRIVER_INTEL);
-
-	igt_debug("Testing Dirty Default Context Engines\n");
-	test_mocs_values(fd);
-
-	for (e = intel_execution_engines; e->name; e++) {
-		unsigned engine = e->exec_id | e->flags;
+	write_registers(fd, ctx_id, get_engine_base(engine),
+			write_values, ARRAY_SIZE(write_values),
+			engine);
 
-		if (!local_has_ring(fd, engine))
-			continue;
-
-		write_registers(fd, 0,
-				GEN9_GFX_MOCS_0,
+	if (engine == I915_EXEC_RENDER)
+		write_registers(fd, ctx_id, GEN9_LNCFCMOCS0,
 				write_values, ARRAY_SIZE(write_values),
 				engine);
-
-		write_registers(fd, 0,
-				GEN9_LNCFCMOCS0,
-				write_values, ARRAY_SIZE(write_values),
-				engine);
-	}
-
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset(fd);	break;
-	case SUSPEND:	igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
-						      SUSPEND_TEST_NONE); break;
-	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
-						      SUSPEND_TEST_NONE); break;
-	}
-
-	close(fd);
-
-	igt_debug("Testing Pristine after Dirty Defaults\n");
-	test_mocs_values(-1);
 }
 
-static void context_save_restore_test(unsigned mode)
+static void run_test(int fd, unsigned engine, unsigned flags, unsigned mode)
 {
-	int fd = drm_open_driver_master(DRIVER_INTEL);
-	uint32_t ctx_id = gem_context_create(fd);
+	uint32_t ctx_id = 0;
+	uint32_t ctx_clean_id;
+	uint32_t ctx_dirty_id;
 
-	igt_debug("Testing Save Restore\n");
+	/* Skip if we don't know where the registers are for this engine */
+	igt_require(get_engine_base(engine));
 
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	if (flags & MOCS_NON_DEFAULT_CTX)
+		ctx_id = gem_context_create(fd);
 
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset(fd);	break;
-	case SUSPEND:	igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
-						      SUSPEND_TEST_NONE); break;
-	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
-						      SUSPEND_TEST_NONE); break;
+	if (flags & MOCS_DIRTY_VALUES) {
+		ctx_dirty_id = gem_context_create(fd);
+		write_dirty_mocs(fd, engine, ctx_dirty_id);
+		check_mocs_values(fd, engine, ctx_dirty_id, true);
 	}
 
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-
-	close(fd);
-}
-
-static void context_dirty_test(unsigned mode)
-{
-	int fd = drm_open_driver_master(DRIVER_INTEL);
-	uint32_t ctx_id = gem_context_create(fd);
-
-	igt_debug("Testing Dirty Context\n");
-	test_mocs_values(fd);
-
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-
-	/* XXX !RCS as well */
-
-	write_registers(fd,
-			ctx_id,
-			GEN9_GFX_MOCS_0,
-			write_values,
-			ARRAY_SIZE(write_values),
-			I915_EXEC_RENDER);
-
-	write_registers(fd,
-			ctx_id,
-			GEN9_LNCFCMOCS0,
-			write_values,
-			ARRAY_SIZE(write_values),
-			I915_EXEC_RENDER);
-
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, true);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, true);
+	check_mocs_values(fd, engine, ctx_id, false);
 
 	switch (mode) {
 	case NONE:	break;
@@ -499,99 +395,68 @@  static void context_dirty_test(unsigned mode)
 						      SUSPEND_TEST_NONE); break;
 	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND_STATE_DISK,
 						      SUSPEND_TEST_NONE); break;
+	case RC6:	rc6_wait(fd);			break;
 	}
 
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, true);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, true);
-
-	close(fd);
-
-	/* Check that unmodified contexts are pristine */
-	igt_debug("Testing Prestine Context (after dirty)\n");
-	test_mocs_values(-1);
-}
-
-static void run_tests(unsigned mode)
-{
-	struct pci_device *pci_dev;
-	int fd;
-
-	pci_dev = intel_get_pci_device();
-	igt_require(pci_dev);
-
-	fd = drm_open_driver_master(DRIVER_INTEL);
-	intel_register_access_init(pci_dev, 0, fd);
-	close(fd);
-
-	default_context_tests(mode);
-	default_dirty_tests(mode);
-	context_save_restore_test(mode);
-	context_dirty_test(mode);
+	check_mocs_values(fd, engine, ctx_id, false);
 
-	intel_register_access_fini();
-}
-
-static int rc6_residency(int dir)
-{
-	return igt_sysfs_get_u32(dir, "power/rc6_residency_ms");
-}
-
-static void context_rc6_test(void)
-{
-	int fd = drm_open_driver(DRIVER_INTEL);
-	uint32_t ctx_id = gem_context_create(fd);
-	int residency;
-	int timeout;
-	int sysfs;
-
-	igt_debug("RC6 Context Test\n");
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-
-	sysfs = igt_sysfs_open(fd, NULL);
-
-	timeout = 3000 / 2;
-	residency = rc6_residency(sysfs);
-	while (rc6_residency(sysfs) == residency && --timeout)
-		usleep(2000);
-	igt_require(timeout);
-
-	close(sysfs);
+	if (flags & MOCS_DIRTY_VALUES) {
+		ctx_clean_id = gem_context_create(fd);
+		check_mocs_values(fd, engine, ctx_dirty_id, true);
+		check_mocs_values(fd, engine, ctx_clean_id, false);
+		gem_context_destroy(fd, ctx_dirty_id);
+		gem_context_destroy(fd, ctx_clean_id);
+	}
 
-	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-	close(fd);
+	if (ctx_id)
+		gem_context_destroy(fd, ctx_id);
 }
 
-
-static void test_requirements(void)
+igt_main
 {
-	int fd = drm_open_driver_master(DRIVER_INTEL);
+	const struct intel_execution_engine *e;
 	struct mocs_table table;
+	int fd = -1;
 
-	gem_require_mocs_registers(fd);
-	igt_require(get_mocs_settings(fd, &table, false));
-	close(fd);
-}
-
-igt_main
-{
 	igt_fixture {
-		test_requirements();
+		fd = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(fd);
+		gem_require_mocs_registers(fd);
+		igt_require(get_mocs_settings(fd, &table, false));
 	}
 
-	igt_subtest("mocs-settings")
-		run_tests(NONE);
-
-	igt_subtest("mocs-rc6")
-		context_rc6_test();
-
-	igt_subtest("mocs-reset")
-		run_tests(RESET);
+	for (e = intel_execution_engines; e->name; e++) {
+		/* We don't know which engine will be assigned to us if we're
+		 * using plain I915_EXEC_BSD, I915_EXEC_DEFAULT is just
+		 * duplicating render
+		 */
+		if ((e->exec_id == I915_EXEC_BSD && !e->flags) ||
+		    e->exec_id == I915_EXEC_DEFAULT)
+			continue;
 
-	igt_subtest("mocs-suspend")
-		run_tests(SUSPEND);
+		for (unsigned mode = NONE; mode < MAX_MOCS_TEST_MODES; mode++) {
+			for (unsigned flags = 0; flags < ALL_MOCS_FLAGS + 1; flags++) {
+				/* Trying to test non-render engines for dirtying MOCS
+				 * values from one context having effect on different
+				 * context is bound to fail - only render engine is
+				 * doing context save/restore of MOCS registers.
+				 * Let's also limit testing values on non-default
+				 * contexts to render-only.
+				 */
+				if (flags && e->exec_id != I915_EXEC_RENDER)
+					continue;
+
+				igt_subtest_f("mocs-%s%s%s-%s",
+					      test_modes[mode],
+					      flags & MOCS_NON_DEFAULT_CTX ? "-ctx": "",
+					      flags & MOCS_DIRTY_VALUES ? "-dirty" : "",
+					      e->name) {
+					run_test(fd, e->exec_id | e->flags, flags, mode);
+				}
+			}
+		}
+	}
 
-	igt_subtest("mocs-hibernate")
-		run_tests(HIBERNATE);
+	igt_fixture
+		close(fd);
 }
diff --git a/tests/intel-ci/extended.testlist b/tests/intel-ci/extended.testlist
index f96043b..17eed01 100644
--- a/tests/intel-ci/extended.testlist
+++ b/tests/intel-ci/extended.testlist
@@ -1505,9 +1505,30 @@  igt@gem_mmap_wc@write-gtt-read-wc
 igt@gem_mmap_wc@write-no-prefault
 igt@gem_mmap_wc@write-read
 igt@gem_mmap_wc@write-read-distinct
-igt@gem_mocs_settings@mocs-rc6
-igt@gem_mocs_settings@mocs-reset
-igt@gem_mocs_settings@mocs-settings
+igt@gem_mocs_settings@mocs-rc6-blt
+igt@gem_mocs_settings@mocs-rc6-bsd1
+igt@gem_mocs_settings@mocs-rc6-bsd2
+igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render
+igt@gem_mocs_settings@mocs-rc6-ctx-render
+igt@gem_mocs_settings@mocs-rc6-dirty-render
+igt@gem_mocs_settings@mocs-rc6-render
+igt@gem_mocs_settings@mocs-rc6-vebox
+igt@gem_mocs_settings@mocs-reset-blt
+igt@gem_mocs_settings@mocs-reset-bsd1
+igt@gem_mocs_settings@mocs-reset-bsd2
+igt@gem_mocs_settings@mocs-reset-ctx-dirty-render
+igt@gem_mocs_settings@mocs-reset-ctx-render
+igt@gem_mocs_settings@mocs-reset-dirty-render
+igt@gem_mocs_settings@mocs-reset-render
+igt@gem_mocs_settings@mocs-reset-vebox
+igt@gem_mocs_settings@mocs-settings-blt
+igt@gem_mocs_settings@mocs-settings-bsd1
+igt@gem_mocs_settings@mocs-settings-bsd2
+igt@gem_mocs_settings@mocs-settings-ctx-dirty-render
+igt@gem_mocs_settings@mocs-settings-ctx-render
+igt@gem_mocs_settings@mocs-settings-dirty-render
+igt@gem_mocs_settings@mocs-settings-render
+igt@gem_mocs_settings@mocs-settings-vebox
 igt@gem_partial_pwrite_pread@reads
 igt@gem_partial_pwrite_pread@reads-display
 igt@gem_partial_pwrite_pread@reads-snoop