[PATCH igt] igt/pm_rc6_residency: Measure residency after checking for applicability

* [PATCH igt] igt/pm_rc6_residency: Measure residency after checking for applicability
@ 2017-10-17  8:49 Chris Wilson
  2017-10-17  9:43 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2017-10-17  8:49 UTC (permalink / raw)
  To: intel-gfx

CI doesn't run in whole-test mode, but runs each subtest individually.
Tests that are designed to do a block of work to be shared between many
subtests end up running that work multiple times (once per subtest) and
worse, that work is wasted if the subtest will be skipped.

pm_rc6_residency is one such example that measured all the residencies
up front before skipping, each skip was therefore taking in excess of
10s.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/pm_rc6_residency.c | 68 ++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c
index 7c87302d..d6fc4701 100644
--- a/tests/pm_rc6_residency.c
+++ b/tests/pm_rc6_residency.c
@@ -53,11 +53,11 @@ struct residencies {
 
 static unsigned long get_rc6_enabled_mask(void)
 {
-	unsigned long rc6_mask;
+	unsigned long enabled;
 
-	rc6_mask = 0;
-	igt_sysfs_scanf(sysfs, "power/rc6_enable", "%lu", &rc6_mask);
-	return rc6_mask;
+	enabled = 0;
+	igt_sysfs_scanf(sysfs, "power/rc6_enable", "%lu", &enabled);
+	return enabled;
 }
 
 static unsigned long read_rc6_residency(const char *name)
@@ -85,20 +85,20 @@ static void residency_accuracy(unsigned int diff,
 		     "Sysfs RC6 residency counter is inaccurate.\n");
 }
 
-static void read_residencies(int devid, unsigned int rc6_mask,
+static void read_residencies(int devid, unsigned int mask,
 			     struct residencies *res)
 {
-	if (rc6_mask & RC6_ENABLED)
+	if (mask & RC6_ENABLED)
 		res->rc6 = read_rc6_residency("rc6");
 
-	if ((rc6_mask & RC6_ENABLED) &&
+	if ((mask & RC6_ENABLED) &&
 	    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
 		res->media_rc6 = read_rc6_residency("media_rc6");
 
-	if (rc6_mask & RC6P_ENABLED)
+	if (mask & RC6P_ENABLED)
 		res->rc6p = read_rc6_residency("rc6p");
 
-	if (rc6_mask & RC6PP_ENABLED)
+	if (mask & RC6PP_ENABLED)
 		res->rc6pp = read_rc6_residency("rc6pp");
 }
 
@@ -111,7 +111,7 @@ static unsigned long gettime_ms(void)
 	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
 }
 
-static void measure_residencies(int devid, unsigned int rc6_mask,
+static void measure_residencies(int devid, unsigned int mask,
 				struct residencies *res)
 {
 	struct residencies start = { };
@@ -119,15 +119,9 @@ static void measure_residencies(int devid, unsigned int rc6_mask,
 	int retry;
 	unsigned long t;
 
-	if (!rc6_mask)
+	if (!mask)
 		return;
 
-	/*
-	 * For some reason my ivb isn't idle even after syncing up with the gpu.
-	 * Let's add a sleep just to make it happy.
-	 */
-	sleep(8);
-
 	/*
 	 * Retry in case of counter wrap-around. We simply re-run the
 	 * measurement, since the valid counter range is different on
@@ -135,9 +129,9 @@ static void measure_residencies(int devid, unsigned int rc6_mask,
 	 */
 	for (retry = 0; retry < 2; retry++) {
 		t = gettime_ms();
-		read_residencies(devid, rc6_mask, &start);
+		read_residencies(devid, mask, &start);
 		sleep(SLEEP_DURATION);
-		read_residencies(devid, rc6_mask, &end);
+		read_residencies(devid, mask, &end);
 		t = gettime_ms() - t;
 
 		if (end.rc6 >= start.rc6 && end.media_rc6 >= start.media_rc6 &&
@@ -166,9 +160,8 @@ static void measure_residencies(int devid, unsigned int rc6_mask,
 
 igt_main
 {
-	unsigned int rc6_mask;
-	int devid = 0;
-	struct residencies res;
+	unsigned int rc6_enabled = 0;
+	unsigned int devid = 0;
 
 	igt_skip_on_simulation();
 
@@ -181,31 +174,44 @@ igt_main
 		sysfs = igt_sysfs_open(fd, NULL);
 		close(fd);
 
-		rc6_mask = get_rc6_enabled_mask();
-		igt_require(rc6_mask);
-
-		measure_residencies(devid, rc6_mask, &res);
+		rc6_enabled = get_rc6_enabled_mask();
+		igt_require(rc6_enabled);
 	}
 
 	igt_subtest("rc6-accuracy") {
-		igt_skip_on(!(rc6_mask & RC6_ENABLED));
+		struct residencies res;
+
+		igt_require(rc6_enabled & RC6_ENABLED);
 
+		measure_residencies(devid, RC6_ENABLED, &res);
 		residency_accuracy(res.rc6, res.duration, "rc6");
 	}
+
 	igt_subtest("media-rc6-accuracy") {
-		igt_skip_on(!((rc6_mask & RC6_ENABLED) &&
-			      (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))));
+		struct residencies res;
 
+		igt_require((rc6_enabled & RC6_ENABLED) &&
+			    (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)));
+
+		measure_residencies(devid, RC6_ENABLED, &res);
 		residency_accuracy(res.media_rc6, res.duration, "media_rc6");
 	}
+
 	igt_subtest("rc6p-accuracy") {
-		igt_skip_on(!(rc6_mask & RC6P_ENABLED));
+		struct residencies res;
+
+		igt_require(rc6_enabled & RC6P_ENABLED);
 
+		measure_residencies(devid, RC6P_ENABLED, &res);
 		residency_accuracy(res.rc6p, res.duration, "rc6p");
 	}
+
 	igt_subtest("rc6pp-accuracy") {
-		igt_skip_on(!(rc6_mask & RC6PP_ENABLED));
+		struct residencies res;
+
+		igt_require(rc6_enabled & RC6PP_ENABLED);
 
+		measure_residencies(devid, RC6PP_ENABLED, &res);
 		residency_accuracy(res.rc6pp, res.duration, "rc6pp");
 	}
 }
-- 
2.15.0.rc0

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread