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

["Kalamarz, Lukasz" <lukasz.kalamarz@intel.com>]

On Wed, 2017-06-28 at 17:41 +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.
> 
> 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: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

-- 
Lukasz

> ---
>  tests/gem_mocs_settings.c | 353 ++++++++++++++--------------------
> ------------
>  1 file changed, 108 insertions(+), 245 deletions(-)
> 
> diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
> index a96aa66..9760c0e 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_S
> TATE_MEM,
> -						      SUSPEND_TEST_N
> ONE); break;
> -	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND
> _STATE_DISK,
> -						      SUSPEND_TEST_N
> ONE); 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_S
> TATE_MEM,
> -						      SUSPEND_TEST_N
> ONE); break;
> -	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND
> _STATE_DISK,
> -						      SUSPEND_TEST_N
> ONE); 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_S
> TATE_MEM,
> -						      SUSPEND_TEST_N
> ONE); break;
> -	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND
> _STATE_DISK,
> -						      SUSPEND_TEST_N
> ONE); 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,66 @@ static void context_dirty_test(unsigned mode)
>  						      SUSPEND_TEST_N
> ONE); break;
>  	case HIBERNATE:	igt_system_suspend_autoresume(SUSPEND
> _STATE_DISK,
>  						      SUSPEND_TEST_N
> ONE); 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->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
> +				 */
> +				if (flags & MOCS_DIRTY_VALUES && 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);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx