[i-g-t] tests/gen7_forcewake_mt: Fix test

Commit Message

Tvrtko Ursulin March 1, 2018, 2:32 p.m. UTC

From: Tvrtko Ursulin <[e-mail]>

1.
We need to tell the compiler mmio access cannot be optimized away
(volatile).

2.
We need to ensure we don't exit with forcewake left on. Signal threads
to exit in a controlled fashion and install atexit handler just in case.

Signed-off-by: Tvrtko Ursulin <[e-mail]>
---
50% speculation, compile tested only!
---
 tests/gen7_forcewake_mt.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

Comments

Chris Wilson March 1, 2018, 2:41 p.m. UTC | #1

Quoting Tvrtko Ursulin (2018-03-01 14:32:17)
> +static void *mmio_base;
> +
> +static void cleanup(int sig)
> +{
> +       volatile uint32_t *forcewake_mt =
> +               (uint32_t *)((char *)mmio_base + FORCEWAKE_MT);
> +       unsigned int bit;
> +
> +       for (bit = 2; bit < 16; bit++) {
> +               *forcewake_mt = (1 << bit) << 16;
> +               if (*forcewake_mt & (1 << bit))
> +                       igt_warn("Failed to restore bit %u!\n", bit);
> +       }

Is the exit handler called after threads are terminated... I don't think
so, my understanding are the threads are terminated by process shutdown
not libc.
-Chris

Tvrtko Ursulin March 1, 2018, 2:47 p.m. UTC | #2

On 01/03/2018 14:41, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-03-01 14:32:17)
>> +static void *mmio_base;
>> +
>> +static void cleanup(int sig)
>> +{
>> +       volatile uint32_t *forcewake_mt =
>> +               (uint32_t *)((char *)mmio_base + FORCEWAKE_MT);
>> +       unsigned int bit;
>> +
>> +       for (bit = 2; bit < 16; bit++) {
>> +               *forcewake_mt = (1 << bit) << 16;
>> +               if (*forcewake_mt & (1 << bit))
>> +                       igt_warn("Failed to restore bit %u!\n", bit);
>> +       }
> 
> Is the exit handler called after threads are terminated... I don't think
> so, my understanding are the threads are terminated by process shutdown
> not libc.

Yes, my bad. At least we'll see if orderly thread exit makes any difference.

Regards,

Tvrtko

Patch

diff --git a/tests/gen7_forcewake_mt.c b/tests/gen7_forcewake_mt.c
index 07320ef9e8ac..a2dc33bbb9dd 100644
--- a/tests/gen7_forcewake_mt.c
+++ b/tests/gen7_forcewake_mt.c
@@ -44,6 +44,7 @@  IGT_TEST_DESCRIPTION("Exercise a suspect workaround required for"
 
 struct thread {
 	pthread_t thread;
+	bool run;
 	void *mmio;
 	int fd;
 	int bit;
@@ -106,10 +107,11 @@  static void *igfx_get_mmio(void)
 static void *thread(void *arg)
 {
 	struct thread *t = arg;
-	uint32_t *forcewake_mt = (uint32_t *)((char *)t->mmio + FORCEWAKE_MT);
+	volatile uint32_t *forcewake_mt =
+		(uint32_t *)((char *)t->mmio + FORCEWAKE_MT);
 	uint32_t bit = 1 << t->bit;
 
-	while (1) {
+	while (t->run) {
 		*forcewake_mt = bit << 16 | bit;
 		igt_assert(*forcewake_mt & bit);
 		*forcewake_mt = bit << 16;
@@ -121,13 +123,33 @@  static void *thread(void *arg)
 
 #define MI_STORE_REGISTER_MEM                   (0x24<<23)
 
+static void *mmio_base;
+
+static void cleanup(int sig)
+{
+	volatile uint32_t *forcewake_mt =
+		(uint32_t *)((char *)mmio_base + FORCEWAKE_MT);
+	unsigned int bit;
+
+	for (bit = 2; bit < 16; bit++) {
+		*forcewake_mt = (1 << bit) << 16;
+		if (*forcewake_mt & (1 << bit))
+			igt_warn("Failed to restore bit %u!\n", bit);
+	}
+}
+
 igt_simple_main
 {
 	struct thread t[16];
 	int i;
 
+	mmio_base = igfx_get_mmio();
+
 	t[0].fd = drm_open_driver(DRIVER_INTEL);
-	t[0].mmio = igfx_get_mmio();
+	t[0].run = true;
+	t[0].mmio = mmio_base;
+
+	igt_install_exit_handler(cleanup);
 
 	for (i = 2; i < 16; i++) {
 		t[i] = t[0];
@@ -201,4 +223,10 @@  igt_simple_main
 
 		usleep(1000);
 	}
+
+	for (i = 2; i < 16; i++)
+		t[i].run = false;
+
+	for (i = 2; i < 16; i++)
+		pthread_join(t[i].thread, NULL);
 }