[Intel-gfx] [I-G-T] igt/gem_mocs_settings: improve RC6 testings

[Antoine, Peter]

-----Original Message-----
From: Chris Wilson [mailto:chris at chris-wilson.co.uk] 
Sent: Thursday, July 21, 2016 9:40 PM
To: Antoine, Peter <peter.antoine at intel.com>
Cc: intel-gfx at lists.freedesktop.org
Subject: Re: [I-G-T] igt/gem_mocs_settings: improve RC6 testings

On Tue, Jul 19, 2016 at 11:25:29AM +0100, Peter Antoine wrote:
> On some platforms the MOCS values are not always saved and restored on 
> RC6 enter/exit. The rational is that the context with restore these 
> values. On these platforms the test will fail as it tests the values 
> by directly reading the MOCS registers.

But there's nothing wrong with the existing tests per-se? You just want to add a new one that explicitly tests rc6 save/restore, and in doing so just need to limit the forcewake. For that you just want to limit intel_register_access to the critical sections. (You don't need to find the pci_dev afresh everytime.)

On some platforms (BXT) it does not restore L3CC registers. So that on these platforms the direct read will fail unless you hold forcewake for the whole test. It also implies that the registers are valid for the whole power lifecycle, so if you are using the tests as API documentation then it implies that the registers are always valid. This is not always true on all platforms.

So the direct memory reads are removed to give the correct API usage for the MOCS. That is that the l3CC registers are only valid while in the RCS context.
 
> +static unsigned int readit(const char *path) {
> +	unsigned int ret = 0;
> +	int scanned = 0;
> +	FILE *file;
> +
> +	file = fopen(path, "r");
> +	igt_assert(file);
> +	scanned = fscanf(file, "%u", &ret);
> +	igt_assert_eq(scanned, 1);
> +
> +	fclose(file);
> +
> +	return ret;
> +}
> +
> +static int read_rc6_residency(void)
> +{
> +	unsigned int residency;
> +	const int device = drm_get_card();
> +	static const char path_format[] =
> +				"/sys/class/drm/card%d/power/rc6_residency_ms";
> +	char path[sizeof(path_format)];
> +	int  ret;
> +
> +	ret = snprintf(path, sizeof(path)-1, path_format, device);
> +
> +	igt_assert_neq(ret, -1);
> +	residency = readit(path);

This should be using a sysfs helper. igt_sysfs.c should already be sufficient, or if not, please improve.

Ok, will look into this.

> +	return residency;
> +}
> +
> +static void context_rc6_test(void)
> +{
> +	int fd = drm_open_driver(DRIVER_INTEL);
> +	int res_ms;
> +	uint32_t ctx_id = gem_context_create(fd);
> +
> +	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);
> +
> +	res_ms = read_rc6_residency();
> +	sleep(3);

And you could spin here until rc6 residency increased.

timeout = 3000 / 2;
while (read_rc6_residency() == initial_res && --timeout)
	usleep(2000);

Ok. Decided against that as 3 seconds (I know magic value) should have been enough. I can change to do the spin with a timeout.

and importantly: igt_require(read_rc6_residency() != initial_res); Don't assert for what may be a user configuration.

Ok, that makes a lot of sense. Will check that RC6 is on in kernel and BIOS settings.

Peter.

-Chris

--
Chris Wilson, Intel Open Source Technology Centre