Re: [PATCH i-g-t v3] pm_rps: Changes in waitboost scenario

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Katarzyna Dec (2017-08-22 13:40:53)
> CI is observing sporadical failures in pm_rps subtests.
> There are a couple of reasons. One of them is the fact that
> on gen6, gen7 and gen7.5, max frequency (as in the HW limit)
> is not set to RP0, but the value obtaind from PCODE (which
> may be different from RP0). Thus the test is operating under
> wrong assumptions (SOFTMAX == RP0 == BOOST which is simply
> not the case). Let's compare current frequency with BOOST
> frequency rather than SOFTMAX to get the test behaviour under control.
> In boost_freq function I set MAX freq to midium freqency, which ensures
> that we for sure reach BOOST frequency. This could help with failures
> with boost frequency failing to drop down.
> 
> v2: Commit message, simplified waiting for boost to finish, drop
> noisy whitespace cleanup.
> 
> v3: Removed reading from i915_rps_boost_info debugfs because it not
> the same on every kernel. Removed function waiting for boost.
> Instead of that I made sure we will reach in boost by setting MAX freq to fmid.
> 
> v4: Moved proposal with making test drm master to other patch

So it still failed the reset scenario, and looking at the failure what
is notable by its absence is the background load. Without that the test
is entirely dependent upon the kernel's low priority retire worker to
ensure forward progress, which is not expected and certainly not
accounted for.  Something like the following will help and shave ~20s
off the test,

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 13c51dd5..8ea470e2 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -28,6 +28,7 @@
 
 #define _GNU_SOURCE
 #include "igt.h"
+#include "igt_gt.h"
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -167,7 +168,7 @@ static void dump(const int *freqs)
 }
 
 enum load {
-	LOW,
+	LOW = 0,
 	HIGH
 };
 
@@ -185,9 +186,10 @@ static struct load_helper {
 
 static void load_helper_signal_handler(int sig)
 {
-	if (sig == SIGUSR2)
-		lh.load = lh.load == LOW ? HIGH : LOW;
-	else
+	if (sig == SIGUSR2) {
+		lh.load = !lh.load;
+		igt_debug("Switching background load to %s\n", lh.load ? "high" : "low");
+	} else
 		lh.exit = true;
 }
 
@@ -238,6 +240,7 @@ static void load_helper_run(enum load load)
 		return;
 	}
 
+	lh.exit = false;
 	lh.load = load;
 
 	igt_fork_helper(&lh.igt_proc) {
@@ -263,6 +266,8 @@ static void load_helper_run(enum load load)
 		if (intel_gen(lh.devid) >= 6)
 			execbuf.flags = I915_EXEC_BLT;
 
+		igt_debug("Applying %s load...\n", lh.load ? "high" : "low");
+
 		while (!lh.exit) {
 			memset(&object, 0, sizeof(object));
 			object.handle = fences[val%3];
@@ -296,6 +301,8 @@ static void load_helper_run(enum load load)
 		gem_close(drm_fd, fences[0]);
 		gem_close(drm_fd, fences[1]);
 		gem_close(drm_fd, fences[2]);
+
+		igt_drop_caches_set(drm_fd, DROP_RETIRE);
 	}
 }
 
@@ -553,13 +560,6 @@ static void stabilize_check(int *out)
 	igt_debug("Waited %d msec to stabilize cur\n", wait);
 }
 
-static void reset_gpu(void)
-{
-	int fd = drm_open_driver(DRIVER_INTEL);
-	igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT));
-	close(fd);
-}
-
 static void boost_freq(int fd, int *boost_freqs)
 {
 	int64_t timeout = 1;
@@ -584,14 +584,12 @@ static void boost_freq(int fd, int *boost_freqs)
 	igt_spin_batch_free(fd, load);
 }
 
-static void waitboost(bool reset)
+static void waitboost(int fd, bool reset)
 {
 	int pre_freqs[NUMFREQ];
 	int boost_freqs[NUMFREQ];
 	int post_freqs[NUMFREQ];
 
-	int fd = drm_open_driver(DRIVER_INTEL);
-
 	load_helper_run(LOW);
 
 	igt_debug("Apply low load...\n");
@@ -600,7 +598,7 @@ static void waitboost(bool reset)
 
 	if (reset) {
 		igt_debug("Reset gpu...\n");
-		reset_gpu();
+		igt_force_gpu_reset(fd);
 		sleep(1);
 	}
 
@@ -621,8 +619,6 @@ static void waitboost(bool reset)
 	igt_assert_lt(pre_freqs[CUR], pre_freqs[MAX]);
 	igt_assert_eq(boost_freqs[CUR], boost_freqs[MAX]);
 	igt_assert_lt(post_freqs[CUR], post_freqs[MAX]);
-
-	close(fd);
 }
 
 static void pm_rps_exit_handler(int sig)
@@ -687,9 +683,14 @@ igt_main
 	}
 
 	igt_subtest("waitboost")
-		waitboost(false);
+		waitboost(drm_fd, false);
+
+	igt_subtest("reset") {
+		igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
 
-	igt_subtest("reset")
-		waitboost(true);
+		waitboost(drm_fd, true);
+
+		igt_disallow_hang(drm_fd, hang);
+	}
 
 }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx