Various fixes and updates to igt/pm_rps

Various fixes and updates to igt/pm_rps

[jeff.mcgee]

From: Jeff McGee <jeff.mcgee@intel.com>

Jeff McGee (4):
  pm_rps: Use unbuffered I/O on sysfs files
  pm_rps: Assert that valid sysfs writes return success
  pm_rps: Fix test to target original min and max
  pm_rps: Use igt exit handler for restore

 tests/pm_rps.c | 85 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 36 deletions(-)

-- 
1.8.5.2

[PATCH 1/4] pm_rps: Use unbuffered I/O on sysfs files

[jeff.mcgee]

From: Jeff McGee <jeff.mcgee@intel.com>

Bionic C library may not re-read a buffered, read-only file which
results in failure to monitor changes in gt_cur_freq_mhz.

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 tests/pm_rps.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index e8affdb..9123451 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -22,6 +22,7 @@
  *
  * Authors:
  *    Ben Widawsky <ben@bwidawsk.net>
+ *    Jeff McGee <jeff.mcgee@intel.com>
  *
  */
 
@@ -66,7 +67,6 @@ static int readval(FILE *filp)
 	int val;
 	int scanned;
 
-	fflush(filp);
 	rewind(filp);
 	scanned = fscanf(filp, "%d", &val);
 	igt_assert(scanned == 1);
@@ -76,15 +76,11 @@ static int readval(FILE *filp)
 
 static int do_writeval(FILE *filp, int val, int lerrno)
 {
-	/* Must write twice to sysfs since the first one simply calculates the size and won't return the error */
 	int ret;
 	rewind(filp);
 	ret = fprintf(filp, "%d", val);
-	rewind(filp);
-	ret = fprintf(filp, "%d", val);
 	if (ret && lerrno)
 		igt_assert(errno = lerrno);
-	fflush(filp);
 	return ret;
 }
 #define writeval(filp, val) do_writeval(filp, val, 0)
@@ -146,6 +142,7 @@ igt_simple_main
 		igt_assert(ret != -1);
 		junk->filp = fopen(path, junk->mode);
 		igt_require(junk->filp);
+		setbuf(junk->filp, NULL);
 
 		val = readval(junk->filp);
 		igt_assert(val >= 0);
-- 
1.8.5.2

[PATCH 2/4] pm_rps: Assert that valid sysfs writes return success

[jeff.mcgee]

From: Jeff McGee <jeff.mcgee@intel.com>

do_writeval now always checks the return value, whether we expect
success or a specific error. Also add new macro writeval_inval to
simplify repeated use of do_writeval to test for EINVAL return code.

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 tests/pm_rps.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 9123451..69eeb81 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -79,11 +79,18 @@ static int do_writeval(FILE *filp, int val, int lerrno)
 	int ret;
 	rewind(filp);
 	ret = fprintf(filp, "%d", val);
-	if (ret && lerrno)
-		igt_assert(errno = lerrno);
+	if (lerrno) {
+		/* Expecting specific error */
+		igt_assert(ret == EOF && errno == lerrno);
+	} else {
+		/* Expecting no error */
+		igt_assert(ret != EOF);
+	}
+
 	return ret;
 }
 #define writeval(filp, val) do_writeval(filp, val, 0)
+#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL)
 
 #define fcur (readval(stuff[CUR].filp))
 #define fmin (readval(stuff[MIN].filp))
@@ -170,16 +177,16 @@ igt_simple_main
 	checkit();
 
 	/* And some errors */
-	writeval(stuff[MIN].filp, frpn - 1);
-	writeval(stuff[MAX].filp, frp0 + 1000);
+	writeval_inval(stuff[MIN].filp, frpn - 1);
+	writeval_inval(stuff[MAX].filp, frp0 + 1000);
 	checkit();
 
-	writeval(stuff[MIN].filp, fmax + 1000);
-	writeval(stuff[MAX].filp, fmin - 1);
+	writeval_inval(stuff[MIN].filp, fmax + 1000);
+	writeval_inval(stuff[MAX].filp, fmin - 1);
 	checkit();
 
-	do_writeval(stuff[MIN].filp, 0x11111110, EINVAL);
-	do_writeval(stuff[MAX].filp, 0, EINVAL);
+	writeval_inval(stuff[MIN].filp, 0x11111110);
+	writeval_inval(stuff[MAX].filp, 0);
 
 	writeval(stuff[MIN].filp, origmin);
 	writeval(stuff[MAX].filp, origmax);
-- 
1.8.5.2

[PATCH 3/4] pm_rps: Fix test to target original min and max

[jeff.mcgee]

From: Jeff McGee <jeff.mcgee@intel.com>

The goal of the test is to confirm that gt_cur_freq_mhz can be forced
to the boundaries of the frequency range by collapsing gt_min_freq_mhz
and gt_max_freq_mhz to the target value. But we miss testing the upper
end of the range by targetting the current value of max after it has
been set equal to min. So fix by targetting orginal max instead of
current max.

This correction exposes a problem in setfreq where min is always set
to target before max, which should fail if the target value is greater
than max. So fix that too.

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 tests/pm_rps.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 69eeb81..c968ecb 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -101,8 +101,13 @@ static int do_writeval(FILE *filp, int val, int lerrno)
 
 static void setfreq(int val)
 {
-	writeval(stuff[MIN].filp, val);
-	writeval(stuff[MAX].filp, val);
+	if (val > fmax) {
+		writeval(stuff[MAX].filp, val);
+		writeval(stuff[MIN].filp, val);
+	} else {
+		writeval(stuff[MIN].filp, val);
+		writeval(stuff[MAX].filp, val);
+	}
 }
 
 static void checkit(void)
@@ -166,11 +171,11 @@ igt_simple_main
 		dumpit();
 
 	checkit();
-	setfreq(fmin);
+	setfreq(origmin);
 	if (verbose)
 		dumpit();
 	restore_assert(fcur == fmin);
-	setfreq(fmax);
+	setfreq(origmax);
 	if (verbose)
 		dumpit();
 	restore_assert(fcur == fmax);
-- 
1.8.5.2

[PATCH 4/4] pm_rps: Use igt exit handler for restore

[jeff.mcgee]

From: Jeff McGee <jeff.mcgee@intel.com>

Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
---
 tests/pm_rps.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index c968ecb..7c739b6 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -36,14 +36,6 @@
 static bool verbose = false;
 static int origmin, origmax;
 
-#define restore_assert(COND) do { \
-	if (!(COND)) { \
-		writeval(stuff[MIN].filp, origmin); \
-		writeval(stuff[MAX].filp, origmax); \
-		igt_assert(0); \
-	} \
-} while (0);
-
 static const char sysfs_base_path[] = "/sys/class/drm/card%d/gt_%s_freq_mhz";
 enum {
 	CUR,
@@ -112,15 +104,15 @@ static void setfreq(int val)
 
 static void checkit(void)
 {
-	restore_assert(fmin <= fmax);
-	restore_assert(fcur <= fmax);
-	restore_assert(fmin <= fcur);
-	restore_assert(frpn <= fmin);
-	restore_assert(fmax <= frp0);
-	restore_assert(frp1 <= frp0);
-	restore_assert(frpn <= frp1);
-	restore_assert(frp0 != 0);
-	restore_assert(frp1 != 0);
+	igt_assert(fmin <= fmax);
+	igt_assert(fcur <= fmax);
+	igt_assert(fmin <= fcur);
+	igt_assert(frpn <= fmin);
+	igt_assert(fmax <= frp0);
+	igt_assert(frp1 <= frp0);
+	igt_assert(frpn <= frp1);
+	igt_assert(frp0 != 0);
+	igt_assert(frp1 != 0);
 }
 
 static void dumpit(void)
@@ -134,6 +126,16 @@ static void dumpit(void)
 	printf("\n");
 }
 
+static void pm_rps_exit_handler(int sig)
+{
+	if (origmin > fmax) {
+		writeval(stuff[MAX].filp, origmax);
+		writeval(stuff[MIN].filp, origmin);
+	} else {
+		writeval(stuff[MIN].filp, origmin);
+		writeval(stuff[MAX].filp, origmax);
+	}
+}
 
 igt_simple_main
 {
@@ -164,6 +166,8 @@ igt_simple_main
 	origmin = fmin;
 	origmax = fmax;
 
+	igt_install_exit_handler(pm_rps_exit_handler);
+
 	if (verbose)
 		printf("Original min = %d\nOriginal max = %d\n", origmin, origmax);
 
@@ -174,11 +178,11 @@ igt_simple_main
 	setfreq(origmin);
 	if (verbose)
 		dumpit();
-	restore_assert(fcur == fmin);
+	igt_assert(fcur == fmin);
 	setfreq(origmax);
 	if (verbose)
 		dumpit();
-	restore_assert(fcur == fmax);
+	igt_assert(fcur == fmax);
 	checkit();
 
 	/* And some errors */
-- 
1.8.5.2

Re: Various fixes and updates to igt/pm_rps

[Daniel Vetter]

On Fri, Jan 10, 2014 at 03:12:29PM -0600, jeff.mcgee@intel.com wrote:
> From: Jeff McGee <jeff.mcgee@intel.com>
> 
> Jeff McGee (4):
>   pm_rps: Use unbuffered I/O on sysfs files
>   pm_rps: Assert that valid sysfs writes return success
>   pm_rps: Fix test to target original min and max
>   pm_rps: Use igt exit handler for restore

Thanks for the patches, all merged.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch