[PATCH 1/2] lib/igt_aux: Extract runtime pm helpers from pm_pc8

[PATCH 1/2] lib/igt_aux: Extract runtime pm helpers from pm_pc8

[Daniel Vetter]

I want to use them elsewhere ...

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 lib/igt_aux.c  | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.h  |  11 ++++++
 tests/pm_pc8.c |  96 ++----------------------------------------------
 3 files changed, 128 insertions(+), 93 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 5ae2e42716b6..182e1c04d1fb 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -369,3 +369,117 @@ void igt_wait_for_keypress(void)
 	getchar();
 	tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
 }
+
+#define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
+/* We just leak this on exit ... */
+int pm_status_fd = -1;
+
+/**
+ * igt_setup_runtime_pm:
+ *
+ * Sets up the runtime PM helper functions and enables runtime PM. To speed up
+ * tests the autosuspend delay is set to 0.
+ *
+ * Returns:
+ * True if runtime pm is available, false otherwise.
+ */
+bool igt_setup_runtime_pm(void)
+{
+	int fd;
+	ssize_t size;
+	char buf[6];
+
+	if (pm_status_fd >= 0)
+		return true;
+
+	/* Our implementation uses autosuspend. Try to set it to 0ms so the test
+	 * suite goes faster and we have a higher probability of triggering race
+	 * conditions. */
+	fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
+	igt_assert_f(fd >= 0,
+		     "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
+
+	/* If we fail to write to the file, it means this system doesn't support
+	 * runtime PM. */
+	size = write(fd, "0\n", 2);
+
+	close(fd);
+
+	if (size != 2)
+		return false;
+
+	/* We know we support runtime PM, let's try to enable it now. */
+	fd = open(POWER_DIR "/control", O_RDWR);
+	igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n");
+
+	size = write(fd, "auto\n", 5);
+	igt_assert(size == 5);
+
+	lseek(fd, 0, SEEK_SET);
+	size = read(fd, buf, ARRAY_SIZE(buf));
+	igt_assert(size == 5);
+	igt_assert(strncmp(buf, "auto\n", 5) == 0);
+
+	close(fd);
+
+	pm_status_fd = open(POWER_DIR "/runtime_status", O_RDONLY);
+	igt_assert_f(pm_status_fd >= 0,
+		     "Can't open " POWER_DIR "/runtime_status\n");
+
+	return true;
+}
+
+/**
+ * igt_runtime_pm_status:
+ *
+ * Returns:
+ * The current runtime PM status.
+ */
+enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
+{
+	ssize_t n_read;
+	char buf[32];
+
+	lseek(pm_status_fd, 0, SEEK_SET);
+	n_read = read(pm_status_fd, buf, ARRAY_SIZE(buf));
+	igt_assert(n_read >= 0);
+	buf[n_read] = '\0';
+
+	if (strncmp(buf, "suspended\n", n_read) == 0)
+		return IGT_RUNTIME_PM_STATUS_SUSPENDED;
+	else if (strncmp(buf, "active\n", n_read) == 0)
+		return IGT_RUNTIME_PM_STATUS_ACTIVE;
+	else if (strncmp(buf, "suspending\n", n_read) == 0)
+		return IGT_RUNTIME_PM_STATUS_SUSPENDING;
+	else if (strncmp(buf, "resuming\n", n_read) == 0)
+		return IGT_RUNTIME_PM_STATUS_RESUMING;
+
+	igt_assert_f(false, "Unknown status %s\n", buf);
+	return IGT_RUNTIME_PM_STATUS_UNKNOWN;
+}
+
+/**
+ * igt_wait_for_pm_status:
+ * @status: desired runtime PM status
+ *
+ * Waits until for the driver to switch to into the desired runtime PM status,
+ * with a 10 second timeout.
+ *
+ * Returns:
+ * True if the desired runtime PM status was attained, false if the operation
+ * timed out.
+ */
+bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
+{
+	int i;
+	int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
+
+	for (i = 0; i < ten_s; i += hundred_ms) {
+		if (igt_get_runtime_pm_status() == status)
+			return true;
+
+		usleep(hundred_ms);
+	}
+
+	return false;
+}
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index aa8a3878a62d..597580da0b4e 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -57,6 +57,17 @@ void igt_drop_root(void);
 
 void igt_wait_for_keypress(void);
 
+enum igt_runtime_pm_status {
+	IGT_RUNTIME_PM_STATUS_ACTIVE,
+	IGT_RUNTIME_PM_STATUS_SUSPENDED,
+	IGT_RUNTIME_PM_STATUS_SUSPENDING,
+	IGT_RUNTIME_PM_STATUS_RESUMING,
+	IGT_RUNTIME_PM_STATUS_UNKNOWN,
+};
+bool igt_setup_runtime_pm(void);
+enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
+bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
+
 /* sysinfo cross-arch wrappers from intel_os.c */
 
 /* These are separate to allow easier testing when porting, see the comment at
diff --git a/tests/pm_pc8.c b/tests/pm_pc8.c
index 67583fe278d5..d36350370ba2 100644
--- a/tests/pm_pc8.c
+++ b/tests/pm_pc8.c
@@ -61,14 +61,6 @@
 
 #define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
 
-enum runtime_pm_status {
-	RUNTIME_PM_STATUS_ACTIVE,
-	RUNTIME_PM_STATUS_SUSPENDED,
-	RUNTIME_PM_STATUS_SUSPENDING,
-	RUNTIME_PM_STATUS_RESUMING,
-	RUNTIME_PM_STATUS_UNKNOWN,
-};
-
 enum pc8_status {
 	PC8_ENABLED,
 	PC8_DISABLED
@@ -202,50 +194,12 @@ static bool wait_for_pc8_status(enum pc8_status status)
 	return false;
 }
 
-static enum runtime_pm_status get_runtime_pm_status(void)
-{
-	ssize_t n_read;
-	char buf[32];
-
-	lseek(pm_status_fd, 0, SEEK_SET);
-	n_read = read(pm_status_fd, buf, ARRAY_SIZE(buf));
-	igt_assert(n_read >= 0);
-	buf[n_read] = '\0';
-
-	if (strncmp(buf, "suspended\n", n_read) == 0)
-		return RUNTIME_PM_STATUS_SUSPENDED;
-	else if (strncmp(buf, "active\n", n_read) == 0)
-		return RUNTIME_PM_STATUS_ACTIVE;
-	else if (strncmp(buf, "suspending\n", n_read) == 0)
-		return RUNTIME_PM_STATUS_SUSPENDING;
-	else if (strncmp(buf, "resuming\n", n_read) == 0)
-		return RUNTIME_PM_STATUS_RESUMING;
-
-	igt_assert_f(false, "Unknown status %s\n", buf);
-	return RUNTIME_PM_STATUS_UNKNOWN;
-}
-
-static bool wait_for_pm_status(enum runtime_pm_status status)
-{
-	int i;
-	int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
-
-	for (i = 0; i < ten_s; i += hundred_ms) {
-		if (get_runtime_pm_status() == status)
-			return true;
-
-		usleep(hundred_ms);
-	}
-
-	return false;
-}
-
 static bool wait_for_suspended(void)
 {
 	if (has_pc8 && !has_runtime_pm)
 		return wait_for_pc8_status(PC8_ENABLED);
 	else
-		return wait_for_pm_status(RUNTIME_PM_STATUS_SUSPENDED);
+		return igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED);
 }
 
 static bool wait_for_active(void)
@@ -253,7 +207,7 @@ static bool wait_for_active(void)
 	if (has_pc8 && !has_runtime_pm)
 		return wait_for_pc8_status(PC8_DISABLED);
 	else
-		return wait_for_pm_status(RUNTIME_PM_STATUS_ACTIVE);
+		return igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_ACTIVE);
 }
 
 static void disable_all_screens_dpms(struct mode_set_data *data)
@@ -655,48 +609,6 @@ static void test_i2c(struct mode_set_data *data)
 		     drm_edids);
 }
 
-static void setup_runtime_pm(void)
-{
-	int fd;
-	ssize_t size;
-	char buf[6];
-
-	/* Our implementation uses autosuspend. Try to set it to 0ms so the test
-	 * suite goes faster and we have a higher probability of triggering race
-	 * conditions. */
-	fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
-	igt_assert_f(fd >= 0,
-		     "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
-
-	/* If we fail to write to the file, it means this system doesn't support
-	 * runtime PM. */
-	size = write(fd, "0\n", 2);
-	has_runtime_pm = (size == 2);
-
-	close(fd);
-
-	if (!has_runtime_pm)
-		return;
-
-	/* We know we support runtime PM, let's try to enable it now. */
-	fd = open(POWER_DIR "/control", O_RDWR);
-	igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n");
-
-	size = write(fd, "auto\n", 5);
-	igt_assert(size == 5);
-
-	lseek(fd, 0, SEEK_SET);
-	size = read(fd, buf, ARRAY_SIZE(buf));
-	igt_assert(size == 5);
-	igt_assert(strncmp(buf, "auto\n", 5) == 0);
-
-	close(fd);
-
-	pm_status_fd = open(POWER_DIR "/runtime_status", O_RDONLY);
-	igt_assert_f(pm_status_fd >= 0,
-		     "Can't open " POWER_DIR "/runtime_status\n");
-}
-
 static void setup_pc8(void)
 {
 	has_pc8 = false;
@@ -776,7 +688,7 @@ static void setup_environment(void)
 
 	setup_non_graphics_runtime_pm();
 
-	setup_runtime_pm();
+	has_runtime_pm = igt_setup_runtime_pm();
 	setup_pc8();
 
 	igt_info("Runtime PM support: %d\n", has_runtime_pm);
@@ -799,8 +711,6 @@ static void teardown_environment(void)
 	fini_mode_set_data(&ms_data);
 	drmClose(drm_fd);
 	close(msr_fd);
-	if (has_runtime_pm)
-		close(pm_status_fd);
 	if (has_pc8)
 		close(pc8_status_fd);
 }
-- 
1.8.1.4

[PATCH 2/2] tests/kms_flip: nasty power management tests

[Daniel Vetter]

These check whether everything is still ok wrt vblank handling after
runtime pm and system suspend-resume.

In addition to the usual checks they also ensure that the vblank frame
counter isn't totally ridiculous, something Keith complained about
aeons ago. With Ville's drm_vblank_on/off rework this should now be
fixed and solid.

v2:
- Ignore seq_step, vblanks completely immediately when the crtc goes off
- Only run system suspend/resume tests once.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 tests/kms_flip.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 9783c7ed8ad5..bb4f71d19939 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -71,6 +71,9 @@
 #define TEST_ENOENT		(1 << 22)
 #define TEST_FENCE_STRESS	(1 << 23)
 #define TEST_VBLANK_RACE	(1 << 24)
+#define TEST_RPM		(1 << 25)
+#define TEST_SUSPEND		(1 << 26)
+#define TEST_TS_CONT		(1 << 27)
 
 #define EVENT_FLIP		(1 << 0)
 #define EVENT_VBLANK		(1 << 1)
@@ -551,14 +554,24 @@ static void check_state(struct test_output *o, struct event_state *es)
 		     es->name, es->name,
 		     (int) diff.tv_sec, (int) diff.tv_usec);
 
-	/* This bounding matches the one in DRM_IOCTL_WAIT_VBLANK. */
-	if (!(o->flags & (TEST_DPMS | TEST_MODESET))) {
-		/* check only valid if no modeset happens in between, that
-		 * increments by (1 << 23) on each step. */
-
+	/* check only valid if no modeset happens in between, that increments by
+	 * (1 << 23) on each step. This bounding matches the one in
+	 * DRM_IOCTL_WAIT_VBLANK. */
+	if (!(o->flags & (TEST_DPMS | TEST_MODESET)))
 		igt_assert_f(es->current_seq - (es->last_seq + o->seq_step) <= 1UL << 23,
 			     "unexpected %s seq %u, should be >= %u\n",
 			     es->name, es->current_seq, es->last_seq + o->seq_step);
+
+	/* Check that the vblank frame didn't wrap unexpectedly. */
+	if (o->flags & TEST_TS_CONT) {
+		/* Ignore seq_step here since vblank waits time out immediately
+		 * when we kill the crtc. */
+		igt_assert_f(es->current_seq - es->last_seq >= 0,
+			     "unexpected %s seq %u, should be >= %u\n",
+			     es->name, es->current_seq, es->last_seq);
+		igt_assert_f(es->current_seq - es->last_seq <= 100,
+			     "unexpected %s seq %u, should be < %u\n",
+			     es->name, es->current_seq, es->last_seq + 100);
 	}
 
 	if ((o->flags & TEST_CHECK_TS) && (!analog_tv_connector(o))) {
@@ -919,6 +932,12 @@ static unsigned int run_test_step(struct test_output *o)
 			     "failed to disable output: %s\n",
 			     strerror(errno));
 
+	if (o->flags & TEST_RPM)
+		igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+
+	if (o->flags & TEST_SUSPEND)
+		igt_system_suspend_autoresume();
+
 	if (do_vblank && (o->flags & TEST_EINVAL) && o->vblank_state.count > 0)
 		igt_assert(do_wait_for_vblank(o, o->pipe, target_seq, &vbl_reply)
 			   == -EINVAL);
@@ -1312,6 +1331,10 @@ static int run_test(int duration, int flags)
 
 	igt_require((flags & TEST_HANG) == 0 || !is_hung(drm_fd));
 
+
+	if (flags & TEST_RPM)
+		igt_require(igt_setup_runtime_pm());
+
 	resources = drmModeGetResources(drm_fd);
 	igt_assert(resources);
 
@@ -1516,6 +1539,10 @@ int main(int argc, char **argv)
 		{ 0, TEST_ENOENT | TEST_NOEVENT, "nonexisting-fb" },
 		{ 10, TEST_DPMS_OFF | TEST_DPMS | TEST_VBLANK_RACE, "dpms-vs-vblank-race" },
 		{ 10, TEST_MODESET | TEST_VBLANK_RACE, "modeset-vs-vblank-race" },
+		{ 10, TEST_VBLANK | TEST_DPMS | TEST_RPM | TEST_TS_CONT, "dpms-vs-rpm" },
+		{ 10, TEST_VBLANK | TEST_DPMS | TEST_SUSPEND | TEST_TS_CONT, "dpms-vs-suspend" },
+		{ 0, TEST_VBLANK | TEST_MODESET | TEST_RPM | TEST_TS_CONT, "modeset-vs-rpm" },
+		{ 0, TEST_VBLANK | TEST_MODESET | TEST_SUSPEND | TEST_TS_CONT, "modeset-vs-suspend" },
 	};
 	int i;
 
@@ -1541,6 +1568,10 @@ int main(int argc, char **argv)
 		if (tests[i].flags & TEST_NO_2X_OUTPUT)
 			continue;
 
+		/* code doesn't disable all crtcs, so skip rpm tests */
+		if (tests[i].flags & TEST_RPM)
+			continue;
+
 		igt_subtest_f( "2x-%s", tests[i].name)
 			run_pair(tests[i].duration, tests[i].flags);
 	}
@@ -1559,6 +1590,10 @@ int main(int argc, char **argv)
 		if (tests[i].flags & TEST_NO_2X_OUTPUT)
 			continue;
 
+		/* code doesn't disable all crtcs, so skip rpm tests */
+		if (tests[i].flags & TEST_RPM)
+			continue;
+
 		igt_subtest_f( "2x-%s-interruptible", tests[i].name)
 			run_pair(tests[i].duration, tests[i].flags);
 	}
-- 
1.8.1.4

Re: [PATCH 1/2] lib/igt_aux: Extract runtime pm helpers from pm_pc8

[Damien Lespiau]

On Wed, May 14, 2014 at 06:00:46PM +0200, Daniel Vetter wrote:
> I want to use them elsewhere ...
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  lib/igt_aux.c  | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_aux.h  |  11 ++++++
>  tests/pm_pc8.c |  96 ++----------------------------------------------
>  3 files changed, 128 insertions(+), 93 deletions(-)

Just a small thing, let's not abuse _aux.c to put random stuff. How about
adding a new _pm.c?

-- 
Damien

Re: [PATCH 1/2] lib/igt_aux: Extract runtime pm helpers from pm_pc8

[Daniel Vetter]

On Wed, May 14, 2014 at 06:50:27PM +0100, Damien Lespiau wrote:
> On Wed, May 14, 2014 at 06:00:46PM +0200, Daniel Vetter wrote:
> > I want to use them elsewhere ...
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  lib/igt_aux.c  | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_aux.h  |  11 ++++++
> >  tests/pm_pc8.c |  96 ++----------------------------------------------
> >  3 files changed, 128 insertions(+), 93 deletions(-)
> 
> Just a small thing, let's not abuse _aux.c to put random stuff. How about
> adding a new _pm.c?

Well it's just three functions, the suspend helper is also already in
there and igt_aux is fairly small. We can split once it's too big.

igt_aux is very much just the bin for everything that doesn't fit anywhere
else ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/2] lib/igt_aux: Extract runtime pm helpers from pm_pc8

[Damien Lespiau]

On Wed, May 14, 2014 at 08:34:03PM +0200, Daniel Vetter wrote:
> On Wed, May 14, 2014 at 06:50:27PM +0100, Damien Lespiau wrote:
> > On Wed, May 14, 2014 at 06:00:46PM +0200, Daniel Vetter wrote:
> > > I want to use them elsewhere ...
> > > 
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  lib/igt_aux.c  | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/igt_aux.h  |  11 ++++++
> > >  tests/pm_pc8.c |  96 ++----------------------------------------------
> > >  3 files changed, 128 insertions(+), 93 deletions(-)
> > 
> > Just a small thing, let's not abuse _aux.c to put random stuff. How about
> > adding a new _pm.c?
> 
> Well it's just three functions, the suspend helper is also already in
> there and igt_aux is fairly small. We can split once it's too big.
> 
> igt_aux is very much just the bin for everything that doesn't fit anywhere
> else ;-)

Yeah, I can't see a kitchen-sink file going wrong, that has never
happened before :)

-- 
Damien