[I-G-T 0/3] igt/gem_mocs_settings: Update MOCS tests

[I-G-T 0/3] igt/gem_mocs_settings: Update MOCS tests

[Peter Antoine]

These changes fix several problems with the MOCS tests.
The RC6 save/restore is not the same on all platforms and does not save restore
the same registers on all platforms. The testing relied on the fact that it
always restored all the MOCS registers. This patchset removes that assumption.

Also as some of the platforms don't support "rtcwake" the tests that will fail
have had some changes that will allow for following tests to not break. It also
reduces the number of cascade failures in the other tests.

Peter Antoine (3):
  igt/gem_mocs_settings: Remove direct register tests
  igt/gem_mocs_settings: adding RC6 tests
  igt/gem_mocs_settings: Reduce the amount of cascading failures

 lib/igt_aux.c             |  67 +++++++++++++++++
 lib/igt_aux.h             |   2 +
 tests/gem_mocs_settings.c | 186 +++++++++++++++++++++++-----------------------
 3 files changed, 160 insertions(+), 95 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[I-G-T 1/3] igt/gem_mocs_settings: Remove direct register tests

[Peter Antoine]

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.

So this change removes the direct testing of the values.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 tests/gem_mocs_settings.c | 80 +++++------------------------------------------
 1 file changed, 8 insertions(+), 72 deletions(-)

diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
index 3e6101d..4fb3a02 100644
--- a/tests/gem_mocs_settings.c
+++ b/tests/gem_mocs_settings.c
@@ -132,65 +132,6 @@ static uint32_t get_engine_base(uint32_t engine)
 	}
 }
 
-static uint32_t get_mocs_register_value(int fd, uint64_t offset, uint32_t index)
-{
-	igt_assert(index < MAX_NUMBER_MOCS_REGISTERS);
-	return intel_register_read(offset + index * 4);
-}
-
-static void test_mocs_control_values(int fd, uint32_t engine)
-{
-	const uint32_t engine_base = get_engine_base(engine);
-	struct mocs_table table;
-	int local_fd;
-	int i;
-
-	local_fd = fd;
-	if (local_fd == -1)
-		local_fd = drm_open_driver_master(DRIVER_INTEL);
-
-	igt_assert(get_mocs_settings(local_fd, &table, false));
-
-	for (i = 0; i < table.size; i++)
-		igt_assert_eq_u32(get_mocs_register_value(local_fd,
-							  engine_base, i),
-				  table.table[i].control_value);
-
-	if (local_fd != fd)
-		close(local_fd);
-}
-
-static void test_mocs_l3cc_values(int fd)
-{
-	uint32_t reg_values[MAX_NUMBER_MOCS_REGISTERS/2];
-	struct mocs_table table;
-	int local_fd;
-	int i;
-
-	local_fd = fd;
-	if (local_fd == -1)
-		local_fd = drm_open_driver_master(DRIVER_INTEL);
-
-	for (i = 0; i < MAX_NUMBER_MOCS_REGISTERS / 2; i++)
-		reg_values[i] = intel_register_read(GEN9_LNCFCMOCS0 + (i * 4));
-
-	igt_assert(get_mocs_settings(local_fd, &table, false));
-
-	for (i = 0; i < table.size / 2; i++) {
-		igt_assert_eq_u32((reg_values[i] & 0xffff),
-				  table.table[i * 2].l3cc_value);
-		igt_assert_eq_u32((reg_values[i] >> 16),
-				  table.table[i * 2 + 1].l3cc_value);
-	}
-
-	if (table.size & 1)
-		igt_assert_eq_u32((reg_values[i] & 0xffff),
-				  table.table[i * 2].l3cc_value);
-
-	if (local_fd != fd)
-		close(local_fd);
-}
-
 #define MI_STORE_REGISTER_MEM_64_BIT_ADDR	((0x24 << 23) | 2)
 
 static int create_read_batch(struct drm_i915_gem_relocation_entry *reloc,
@@ -428,11 +369,8 @@ static void test_mocs_values(int fd)
 			continue;
 
 		igt_debug("Testing %s\n", e->name);
-		test_mocs_control_values(fd, engine);
 		test_context_mocs_values(fd, engine);
 	}
-
-	test_mocs_l3cc_values(fd);
 }
 
 static void default_context_tests(unsigned mode)
@@ -566,10 +504,18 @@ static void context_dirty_test(unsigned mode)
 
 static void run_tests(unsigned mode)
 {
+	struct pci_device *pci_dev;
+
+	pci_dev = intel_get_pci_device();
+	igt_require(pci_dev);
+	intel_register_access_init(pci_dev, 0);
+
 	default_context_tests(mode);
 	default_dirty_tests(mode);
 	context_save_restore_test(mode);
 	context_dirty_test(mode);
+
+	intel_register_access_fini();
 }
 
 static void test_requirements(void)
@@ -584,14 +530,8 @@ static void test_requirements(void)
 
 igt_main
 {
-	struct pci_device *pci_dev;
-
 	igt_fixture {
 		test_requirements();
-
-		pci_dev = intel_get_pci_device();
-		igt_require(pci_dev);
-		intel_register_access_init(pci_dev, 0);
 	}
 
 	igt_subtest("mocs-settings")
@@ -605,8 +545,4 @@ igt_main
 
 	igt_subtest("mocs-hibernate")
 		run_tests(HIBERNATE);
-
-	igt_fixture {
-		intel_register_access_fini();
-	}
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[I-G-T 2/3] igt/gem_mocs_settings: adding RC6 testings

[Peter Antoine]

This change adds a RC6 test for the MOCS. The MOCS registers are loaded
and saved as part of the RC6 cycle but not all the registers are
saved/restored. This tests that those registers are correctly restored.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
index 4fb3a02..66d02d9 100644
--- a/tests/gem_mocs_settings.c
+++ b/tests/gem_mocs_settings.c
@@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
 	intel_register_access_fini();
 }
 
+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);
+
+	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);
+	igt_assert_neq(res_ms, read_rc6_residency());
+
+	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	close(fd);
+}
+
+
 static void test_requirements(void)
 {
 	int fd = drm_open_driver_master(DRIVER_INTEL);
@@ -537,6 +590,9 @@ igt_main
 	igt_subtest("mocs-settings")
 		run_tests(NONE);
 
+	igt_subtest("mocs-rc6")
+		context_rc6_test();
+
 	igt_subtest("mocs-reset")
 		run_tests(RESET);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 testings

[Antoine, Peter]

Please Ignore this patch.
Finger trouble.

Peter.

-----Original Message-----
From: Antoine, Peter 
Sent: Friday, July 29, 2016 10:35 AM
To: intel-gfx@lists.freedesktop.org
Cc: chris@chris-wilson.co.uk; Antoine, Peter <peter.antoine@intel.com>
Subject: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 testings

This change adds a RC6 test for the MOCS. The MOCS registers are loaded
and saved as part of the RC6 cycle but not all the registers are
saved/restored. This tests that those registers are correctly restored.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
index 4fb3a02..66d02d9 100644
--- a/tests/gem_mocs_settings.c
+++ b/tests/gem_mocs_settings.c
@@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
 	intel_register_access_fini();
 }
 
+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);
+
+	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);
+	igt_assert_neq(res_ms, read_rc6_residency());
+
+	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	close(fd);
+}
+
+
 static void test_requirements(void)
 {
 	int fd = drm_open_driver_master(DRIVER_INTEL);
@@ -537,6 +590,9 @@ igt_main
 	igt_subtest("mocs-settings")
 		run_tests(NONE);
 
+	igt_subtest("mocs-rc6")
+		context_rc6_test();
+
 	igt_subtest("mocs-reset")
 		run_tests(RESET);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[I-G-T 2/3] igt/gem_mocs_settings: adding RC6 tests

[Peter Antoine]

This change adds a RC6 test for the MOCS. The MOCS registers are loaded
and saved as part of the RC6 cycle but not all the registers are
saved/restored. This tests that those registers are correctly restored.

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
index 4fb3a02..66d02d9 100644
--- a/tests/gem_mocs_settings.c
+++ b/tests/gem_mocs_settings.c
@@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
 	intel_register_access_fini();
 }
 
+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);
+
+	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);
+	igt_assert_neq(res_ms, read_rc6_residency());
+
+	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
+	close(fd);
+}
+
+
 static void test_requirements(void)
 {
 	int fd = drm_open_driver_master(DRIVER_INTEL);
@@ -537,6 +590,9 @@ igt_main
 	igt_subtest("mocs-settings")
 		run_tests(NONE);
 
+	igt_subtest("mocs-rc6")
+		context_rc6_test();
+
 	igt_subtest("mocs-reset")
 		run_tests(RESET);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 tests

[Chris Wilson]

On Fri, Jul 29, 2016 at 10:34:35AM +0100, Peter Antoine wrote:
> This change adds a RC6 test for the MOCS. The MOCS registers are loaded
> and saved as part of the RC6 cycle but not all the registers are
> saved/restored. This tests that those registers are correctly restored.
> 
> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> ---
>  tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
> index 4fb3a02..66d02d9 100644
> --- a/tests/gem_mocs_settings.c
> +++ b/tests/gem_mocs_settings.c
> @@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
>  	intel_register_access_fini();
>  }
>  
> +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 is duplicating code from igt_sysfs.c

> +
> +	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);

Still a large and arbitrary delay.

> +	igt_assert_neq(res_ms, read_rc6_residency());

Still an assert rather than skipping when the user has disabled rc6.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 tests

[Peter Antoine]

On Mon, 1 Aug 2016, Chris Wilson wrote:

> On Fri, Jul 29, 2016 at 10:34:35AM +0100, Peter Antoine wrote:
>> This change adds a RC6 test for the MOCS. The MOCS registers are loaded
>> and saved as part of the RC6 cycle but not all the registers are
>> saved/restored. This tests that those registers are correctly restored.
>>
>> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>> ---
>>  tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
>> index 4fb3a02..66d02d9 100644
>> --- a/tests/gem_mocs_settings.c
>> +++ b/tests/gem_mocs_settings.c
>> @@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
>>  	intel_register_access_fini();
>>  }
>>
>> +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 is duplicating code from igt_sysfs.c
Ok. Will fix.
>
>> +
>> +	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);
Distracted. Will fix.
>
> Still a large and arbitrary delay.
>
>> +	igt_assert_neq(res_ms, read_rc6_residency());
Will change.
>
> Still an assert rather than skipping when the user has disabled rc6.
> -Chris
>
>

--
    Peter Antoine (Android Graphics Driver Software Engineer)
    ---------------------------------------------------------------------
    Intel Corporation (UK) Limited
    Registered No. 1134945 (England)
    Registered Office: Pipers Way, Swindon SN3 1RJ
    VAT No: 860 2173 47
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 tests

[Peter Antoine]

On Mon, 1 Aug 2016, Chris Wilson wrote:

> On Fri, Jul 29, 2016 at 10:34:35AM +0100, Peter Antoine wrote:
>> This change adds a RC6 test for the MOCS. The MOCS registers are loaded
>> and saved as part of the RC6 cycle but not all the registers are
>> saved/restored. This tests that those registers are correctly restored.
>>
>> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>> ---
>>  tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
>> index 4fb3a02..66d02d9 100644
>> --- a/tests/gem_mocs_settings.c
>> +++ b/tests/gem_mocs_settings.c
>> @@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
>>  	intel_register_access_fini();
>>  }
>>
>> +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 is duplicating code from igt_sysfs.c

PS: Not in the current tree. The code is based on code from 
pm_rc6_residency.

>
>> +
>> +	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);
>
> Still a large and arbitrary delay.
>
>> +	igt_assert_neq(res_ms, read_rc6_residency());
>
> Still an assert rather than skipping when the user has disabled rc6.
> -Chris
>
>

--
    Peter Antoine (Android Graphics Driver Software Engineer)
    ---------------------------------------------------------------------
    Intel Corporation (UK) Limited
    Registered No. 1134945 (England)
    Registered Office: Pipers Way, Swindon SN3 1RJ
    VAT No: 860 2173 47
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 tests

[Chris Wilson]

On Mon, Aug 01, 2016 at 11:03:48AM +0100, Peter Antoine wrote:
> On Mon, 1 Aug 2016, Chris Wilson wrote:
> 
> >On Fri, Jul 29, 2016 at 10:34:35AM +0100, Peter Antoine wrote:
> >>This change adds a RC6 test for the MOCS. The MOCS registers are loaded
> >>and saved as part of the RC6 cycle but not all the registers are
> >>saved/restored. This tests that those registers are correctly restored.
> >>
> >>Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> >>---
> >> tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 56 insertions(+)
> >>
> >>diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
> >>index 4fb3a02..66d02d9 100644
> >>--- a/tests/gem_mocs_settings.c
> >>+++ b/tests/gem_mocs_settings.c
> >>@@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
> >> 	intel_register_access_fini();
> >> }
> >>
> >>+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 is duplicating code from igt_sysfs.c
> 
> PS: Not in the current tree. The code is based on code from
> pm_rc6_residency.

Your tree is a couple of months out of date.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 2/3] igt/gem_mocs_settings: adding RC6 tests

[Peter Antoine]

On Mon, 1 Aug 2016, Chris Wilson wrote:

> On Mon, Aug 01, 2016 at 11:03:48AM +0100, Peter Antoine wrote:
>> On Mon, 1 Aug 2016, Chris Wilson wrote:
>>
>>> On Fri, Jul 29, 2016 at 10:34:35AM +0100, Peter Antoine wrote:
>>>> This change adds a RC6 test for the MOCS. The MOCS registers are loaded
>>>> and saved as part of the RC6 cycle but not all the registers are
>>>> saved/restored. This tests that those registers are correctly restored.
>>>>
>>>> Signed-off-by: Peter Antoine <peter.antoine@intel.com>
>>>> ---
>>>> tests/gem_mocs_settings.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 56 insertions(+)
>>>>
>>>> diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
>>>> index 4fb3a02..66d02d9 100644
>>>> --- a/tests/gem_mocs_settings.c
>>>> +++ b/tests/gem_mocs_settings.c
>>>> @@ -518,6 +518,59 @@ static void run_tests(unsigned mode)
>>>> 	intel_register_access_fini();
>>>> }
>>>>
>>>> +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 is duplicating code from igt_sysfs.c
>>
>> PS: Not in the current tree. The code is based on code from
>> pm_rc6_residency.
>
> Your tree is a couple of months out of date.
> -Chris

Am I pulling from the wrong repo?

git clone git://anongit.freedesktop.org/xorg/app/intel-gpu-tools re_pull
Cloning into 're_pull'...
remote: Counting objects: 25319, done.
remote: Compressing objects: 100% (5802/5802), done.
remote: Total 25319 (delta 19840), reused 24789 (delta 19458)
Receiving objects: 100% (25319/25319), 16.67 MiB | 1.35 MiB/s, done.
Resolving deltas: 100% (19840/19840), done.
Checking connectivity... done.

Top 10 commits:
53b4dfd tests: Skip if we cannot reserve infinite fd for ourselves
5c10362 igt/gem_busy: Don't run hang tests under the hangchecker
b2e8d2c igt/gem_concurrent_blit: More subtest grouping
d199ad8 igt/gem_concurrent_all: Fix up copy sizes
9315399 docs: minimal docs for igt_vgem.c

I get 62 references to the following search:
:vimgrep _residen ./**/*.c

All in:
overlay/rc6.c
tests/pm_rc6_residency.c
tests/pm_rpm.c
tools/intel_reg_decode.c
tools/intel_residency.c

Only the first 3 yielding code. What am I missing?

Peter.

>
>

--
    Peter Antoine (Android Graphics Driver Software Engineer)
    ---------------------------------------------------------------------
    Intel Corporation (UK) Limited
    Registered No. 1134945 (England)
    Registered Office: Pipers Way, Swindon SN3 1RJ
    VAT No: 860 2173 47
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Peter Antoine]

If one of the previous tests fails then the following tests fail.
This patch means that the following tests do not fail when the previous
test fails (for some cases).

Signed-off-by: Peter Antoine <peter.antoine@intel.com>
---
 lib/igt_aux.c             | 67 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h             |  2 ++
 tests/gem_mocs_settings.c | 56 +++++++++++++++++++++------------------
 3 files changed, 99 insertions(+), 26 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 1cb9398..cc3ce26 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -629,6 +629,41 @@ void igt_cleanup_aperture_trashers(void)
 #define SQUELCH ">/dev/null 2>&1"
 
 /**
+ * igt_system_suspend_autoresume_no_assert:
+ *
+ * Execute a system suspend-to-mem cycle and automatically wake up again using
+ * the firmware's resume timer.
+ *
+ * This is very handy for implementing any kind of suspend/resume test.
+ *
+ * This version does not cause an "exception" as the test will need to tidy-up
+ * to allow for subsequent tests to run.
+ */
+bool igt_system_suspend_autoresume_no_assert(void)
+{
+	/* FIXME: Simulation doesn't like suspend/resume, and not even a lighter
+	 * approach using /sys/power/pm_test to just test our driver's callbacks
+	 * seems to fare better. We need to investigate what's going on. */
+	if (igt_run_in_simulation()) {
+		igt_debug("autoresume cannot be used in simulation\n");
+		return false;
+
+	} else if (system("rtcwake -n -s 15 -m mem" SQUELCH) != 0) {
+		igt_debug("rtcwake -n -s 15 -m mem is not supported\n");
+		return false;
+
+	} else if (system("rtcwake -s 15 -m mem") != 0) {
+		igt_debug(
+		     "This failure means that something is wrong with the "
+		     "rtcwake tool or how your distro is set up. This is not "
+		     "a i915.ko or i-g-t bug.\n");
+		return false;
+	}
+
+	return true;
+}
+
+/**
  * igt_system_suspend_autoresume:
  *
  * Execute a system suspend-to-mem cycle and automatically wake up again using
@@ -653,6 +688,38 @@ void igt_system_suspend_autoresume(void)
 }
 
 /**
+ * igt_system_hibernate_autoresume_no_assert:
+ *
+ * Execute a system suspend-to-disk cycle and automatically wake up again using
+ * the firmware's resume timer.
+ *
+ * This is very handy for implementing any kind of hibernate/resume test.
+ *
+ * This version does not cause an "exception" as the test will need to tidy-up
+ * to allow for subsequent tests to run.
+ */
+bool igt_system_hibernate_autoresume_no_assert(void)
+{
+	if (igt_run_in_simulation()) {
+		igt_debug("autoresume cannot be used in simulation\n");
+		return false;
+
+	} else if (system("rtcwake -n -s 30 -m disk" SQUELCH) != 0) {
+		igt_debug("rtcwake -n -s 30 -m disk is not supported\n");
+		return false;
+
+	} else if (system("rtcwake -s 30 -m disk") != 0) {
+		igt_debug(
+		     "This failure means that something is wrong with the "
+		     "rtcwake tool or how your distro is set up. This is not "
+		     "a i915.ko or i-g-t bug.\n");
+		return false;
+	}
+
+	return true;
+}
+
+/**
  * igt_system_hibernate_autoresume:
  *
  * Execute a system suspend-to-disk cycle and automatically wake up again using
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index be0d2d6..483c444 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -103,7 +103,9 @@ void igt_cleanup_aperture_trashers(void);
 
 /* suspend/hibernate and auto-resume system */
 void igt_system_suspend_autoresume(void);
+bool igt_system_suspend_autoresume_no_assert(void);
 void igt_system_hibernate_autoresume(void);
+bool igt_system_hibernate_autoresume_no_assert(void);
 
 /* dropping priviledges */
 void igt_drop_root(void);
diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
index 66d02d9..1da7473 100644
--- a/tests/gem_mocs_settings.c
+++ b/tests/gem_mocs_settings.c
@@ -373,6 +373,27 @@ static void test_mocs_values(int fd)
 	}
 }
 
+static void action_test(int fd, unsigned int mode)
+{
+	switch (mode) {
+	case RESET:
+			igt_force_gpu_reset();
+			break;
+	case SUSPEND:
+			if (!igt_system_suspend_autoresume_no_assert()) {
+				close(fd);
+				igt_fail(1);
+			}
+			break;
+	case HIBERNATE:
+			if (!igt_system_hibernate_autoresume_no_assert()) {
+				close(fd);
+				igt_fail(1);
+			}
+			break;
+	}
+}
+
 static void default_context_tests(unsigned mode)
 {
 	int fd = drm_open_driver_master(DRIVER_INTEL);
@@ -380,12 +401,7 @@ static void default_context_tests(unsigned mode)
 	igt_debug("Testing Non/Default Context Engines\n");
 	test_mocs_values(fd);
 
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset();	break;
-	case SUSPEND:	igt_system_suspend_autoresume(); break;
-	case HIBERNATE:	igt_system_hibernate_autoresume(); break;
-	}
+	action_test(fd, mode);
 
 	test_mocs_values(fd);
 	close(fd);
@@ -419,12 +435,7 @@ static void default_dirty_tests(unsigned mode)
 				engine);
 	}
 
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset();	break;
-	case SUSPEND:	igt_system_suspend_autoresume(); break;
-	case HIBERNATE:	igt_system_hibernate_autoresume(); break;
-	}
+	action_test(fd, mode);
 
 	close(fd);
 
@@ -442,12 +453,7 @@ static void context_save_restore_test(unsigned mode)
 	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
 	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
 
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset();	break;
-	case SUSPEND:	igt_system_suspend_autoresume(); break;
-	case HIBERNATE:	igt_system_hibernate_autoresume(); break;
-	}
+	action_test(fd, mode);
 
 	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
 	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
@@ -485,12 +491,7 @@ static void context_dirty_test(unsigned mode)
 	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, true);
 	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, true);
 
-	switch (mode) {
-	case NONE:	break;
-	case RESET:	igt_force_gpu_reset();	break;
-	case SUSPEND:	igt_system_suspend_autoresume(); break;
-	case HIBERNATE:	igt_system_hibernate_autoresume(); break;
-	}
+	action_test(fd, mode);
 
 	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, true);
 	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, true);
@@ -562,8 +563,11 @@ static void context_rc6_test(void)
 	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
 
 	res_ms = read_rc6_residency();
-	sleep(3);
-	igt_assert_neq(res_ms, read_rc6_residency());
+	sleep(6);
+	if (res_ms == read_rc6_residency()) {
+		close(fd);
+		igt_assert_neq(res_ms, read_rc6_residency());
+	}
 
 	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
 	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Chris Wilson]

On Fri, Jul 29, 2016 at 10:34:36AM +0100, Peter Antoine wrote:
> If one of the previous tests fails then the following tests fail.
> This patch means that the following tests do not fail when the previous
> test fails (for some cases).

The problem is just gem_mocs_settings hasn't split its tests up into
subtests.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Peter Antoine]

On Mon, 1 Aug 2016, Chris Wilson wrote:

> On Fri, Jul 29, 2016 at 10:34:36AM +0100, Peter Antoine wrote:
>> If one of the previous tests fails then the following tests fail.
>> This patch means that the following tests do not fail when the previous
>> test fails (for some cases).
>
> The problem is just gem_mocs_settings hasn't split its tests up into
> subtests.
Chris,

Can you expand? The tests are at the minimal size for sensible results (I 
think). The problem is opening the driver for master when the test fails 
then the following tests will fail as master is not closed.

Is there a mechanism in the igt framework for doing this close on 
failure/skip?

If I move the master open code in the fixtures will this get called on all 
exit cases?

Peter.

> -Chris
>
>

--
    Peter Antoine (Android Graphics Driver Software Engineer)
    ---------------------------------------------------------------------
    Intel Corporation (UK) Limited
    Registered No. 1134945 (England)
    Registered Office: Pipers Way, Swindon SN3 1RJ
    VAT No: 860 2173 47
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Daniel Vetter]

On Mon, Aug 01, 2016 at 10:33:17AM +0100, Peter Antoine wrote:
> On Mon, 1 Aug 2016, Chris Wilson wrote:
> 
> > On Fri, Jul 29, 2016 at 10:34:36AM +0100, Peter Antoine wrote:
> > > If one of the previous tests fails then the following tests fail.
> > > This patch means that the following tests do not fail when the previous
> > > test fails (for some cases).
> > 
> > The problem is just gem_mocs_settings hasn't split its tests up into
> > subtests.
> Chris,
> 
> Can you expand? The tests are at the minimal size for sensible results (I
> think). The problem is opening the driver for master when the test fails
> then the following tests will fail as master is not closed.
> 
> Is there a mechanism in the igt framework for doing this close on
> failure/skip?
> 
> If I move the master open code in the fixtures will this get called on all
> exit cases?

For a real test runner you need to run each individual subtest separate
(to avoid contamination). Then process exit will take care of any cleanup
needed with file descriptors. For anything else there's exit handlers, but
they're not 100% reliable

Adding hacks to make subtest runs differently isn't really how igt tests
are meant to be. Hence I concure on Chris' objection here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Antoine, Peter]

They do run separately. It's just that when one fails it does not close the fd and the next cannot open it as master.
Is there a mechanism for closing the fd (or any other tidy up on close.failure).

As the test runner implements a psudo exception handler it should really have a "final"/"skip" handler so that the tidyup can be done cleanly.

That was the question that was asked and not answered.

Peter. 

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, August 2, 2016 3:37 PM
To: Antoine, Peter <peter.antoine@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

On Mon, Aug 01, 2016 at 10:33:17AM +0100, Peter Antoine wrote:
> On Mon, 1 Aug 2016, Chris Wilson wrote:
> 
> > On Fri, Jul 29, 2016 at 10:34:36AM +0100, Peter Antoine wrote:
> > > If one of the previous tests fails then the following tests fail.
> > > This patch means that the following tests do not fail when the 
> > > previous test fails (for some cases).
> > 
> > The problem is just gem_mocs_settings hasn't split its tests up into 
> > subtests.
> Chris,
> 
> Can you expand? The tests are at the minimal size for sensible results 
> (I think). The problem is opening the driver for master when the test 
> fails then the following tests will fail as master is not closed.
> 
> Is there a mechanism in the igt framework for doing this close on 
> failure/skip?
> 
> If I move the master open code in the fixtures will this get called on 
> all exit cases?

For a real test runner you need to run each individual subtest separate (to avoid contamination). Then process exit will take care of any cleanup needed with file descriptors. For anything else there's exit handlers, but they're not 100% reliable

Adding hacks to make subtest runs differently isn't really how igt tests are meant to be. Hence I concure on Chris' objection here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Daniel Vetter]

On Tue, Aug 02, 2016 at 02:47:13PM +0000, Antoine, Peter wrote:
> They do run separately. It's just that when one fails it does not close the fd and the next cannot open it as master.
> Is there a mechanism for closing the fd (or any other tidy up on close.failure).
> 
> As the test runner implements a psudo exception handler it should really
> have a "final"/"skip" handler so that the tidyup can be done cleanly.
> 
> That was the question that was asked and not answered.

#define igt_finally igt_fixture

At least that's the best thing we came up with when last discussing this.
And I did answer your question by claiming that it's not really a problem
when you run tests (in CI) like they should be run. And for manual testing
I'm not sure it's all that valuable really. Hence why I didn't end up
merging the above with a bit of documentation to explain it.
-Daniel

> 
> Peter. 
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Tuesday, August 2, 2016 3:37 PM
> To: Antoine, Peter <peter.antoine@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures
> 
> On Mon, Aug 01, 2016 at 10:33:17AM +0100, Peter Antoine wrote:
> > On Mon, 1 Aug 2016, Chris Wilson wrote:
> > 
> > > On Fri, Jul 29, 2016 at 10:34:36AM +0100, Peter Antoine wrote:
> > > > If one of the previous tests fails then the following tests fail.
> > > > This patch means that the following tests do not fail when the 
> > > > previous test fails (for some cases).
> > > 
> > > The problem is just gem_mocs_settings hasn't split its tests up into 
> > > subtests.
> > Chris,
> > 
> > Can you expand? The tests are at the minimal size for sensible results 
> > (I think). The problem is opening the driver for master when the test 
> > fails then the following tests will fail as master is not closed.
> > 
> > Is there a mechanism in the igt framework for doing this close on 
> > failure/skip?
> > 
> > If I move the master open code in the fixtures will this get called on 
> > all exit cases?
> 
> For a real test runner you need to run each individual subtest separate (to avoid contamination). Then process exit will take care of any cleanup needed with file descriptors. For anything else there's exit handlers, but they're not 100% reliable
> 
> Adding hacks to make subtest runs differently isn't really how igt tests are meant to be. Hence I concure on Chris' objection here.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

[Antoine, Peter]

Drop this patch.

Peter.

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, August 3, 2016 8:37 AM
To: Antoine, Peter <peter.antoine@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>; Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [I-G-T 3/3] igt/gem_mocs_settings: Reduce the amount of cascading failures

On Tue, Aug 02, 2016 at 02:47:13PM +0000, Antoine, Peter wrote:
> They do run separately. It's just that when one fails it does not close the fd and the next cannot open it as master.
> Is there a mechanism for closing the fd (or any other tidy up on close.failure).
> 
> As the test runner implements a psudo exception handler it should 
> really have a "final"/"skip" handler so that the tidyup can be done cleanly.
> 
> That was the question that was asked and not answered.

#define igt_finally igt_fixture

At least that's the best thing we came up with when last discussing this.
And I did answer your question by claiming that it's not really a problem when you run tests (in CI) like they should be run. And for manual testing I'm not sure it's all that valuable really. Hence why I didn't end up merging the above with a bit of documentation to explain it.
-Daniel

> 
> Peter. 
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of 
> Daniel Vetter
> Sent: Tuesday, August 2, 2016 3:37 PM
> To: Antoine, Peter <peter.antoine@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>; 
> intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [I-G-T 3/3] igt/gem_mocs_settings: Reduce the 
> amount of cascading failures
> 
> On Mon, Aug 01, 2016 at 10:33:17AM +0100, Peter Antoine wrote:
> > On Mon, 1 Aug 2016, Chris Wilson wrote:
> > 
> > > On Fri, Jul 29, 2016 at 10:34:36AM +0100, Peter Antoine wrote:
> > > > If one of the previous tests fails then the following tests fail.
> > > > This patch means that the following tests do not fail when the 
> > > > previous test fails (for some cases).
> > > 
> > > The problem is just gem_mocs_settings hasn't split its tests up 
> > > into subtests.
> > Chris,
> > 
> > Can you expand? The tests are at the minimal size for sensible 
> > results (I think). The problem is opening the driver for master when 
> > the test fails then the following tests will fail as master is not closed.
> > 
> > Is there a mechanism in the igt framework for doing this close on 
> > failure/skip?
> > 
> > If I move the master open code in the fixtures will this get called 
> > on all exit cases?
> 
> For a real test runner you need to run each individual subtest 
> separate (to avoid contamination). Then process exit will take care of 
> any cleanup needed with file descriptors. For anything else there's 
> exit handlers, but they're not 100% reliable
> 
> Adding hacks to make subtest runs differently isn't really how igt tests are meant to be. Hence I concure on Chris' objection here.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx