[PATCH 0/3] igt/pm_rps: fix some subcases for vlv

[PATCH 0/3] igt/pm_rps: fix some subcases for vlv

[Imre Deak]

While checking my latest RPS patchset, this test was failing on VLV, so
I went ahead and fixed a couple of issues I found. With these fixes and
a related kernel change to fix the VLV minimum frequency value the
basic-api and min-max-config subtests are passing for me on VLV.

The reset and blocking subtests are still failing, but for some
unrelated reasons (GPU reset/ring flushing failing), so I didn't check
them closer.

Imre Deak (3):
  tests/pm_rps: vlv: wait for freq to settle
  tests/pm_rps: vlv: load gpu for idle min/max tests
  tests/pm_rps: vlv: round middle point to freq supported by HW

 tests/pm_rps.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 10 deletions(-)

-- 
1.8.4

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

[PATCH 1/3] tests/pm_rps: vlv: wait for freq to settle

[Imre Deak]

At least on VLV when forcing a new GT frequency by writing to the
min/max freq sysfs entries the kernel doesn't wait until the new
frequency settles, so the subsequent readback check might fail. To fix
this wait until the current frequency is between the min/max values
using a 10ms timeout.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 tests/pm_rps.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 101f65d..9ed2125 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -34,6 +34,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <errno.h>
+#include <time.h>
 #include <sys/wait.h>
 
 #include "drmtest.h"
@@ -87,6 +88,38 @@ static void read_freqs(int *freqs)
 		freqs[i] = readval(stuff[i].filp);
 }
 
+static void nsleep(unsigned long ns)
+{
+	struct timespec ts;
+	int ret;
+
+	ts.tv_sec = 0;
+	ts.tv_nsec = ns;
+	do {
+		struct timespec rem;
+
+		ret = nanosleep(&ts, &rem);
+		igt_assert(ret == 0 || errno == EINTR);
+		ts = rem;
+	} while (ret && errno == -EINTR);
+}
+
+static void wait_freq_settle(void)
+{
+	int timeout = 10;
+
+	while (1) {
+		int freqs[NUMFREQ];
+
+		read_freqs(freqs);
+		if (freqs[CUR] >= freqs[MIN] && freqs[CUR] <= freqs[MAX])
+			break;
+		nsleep(1000000);
+		if (!timeout--)
+			break;
+	}
+}
+
 static int do_writeval(FILE *filp, int val, int lerrno)
 {
 	int ret, orig;
@@ -102,6 +135,7 @@ static int do_writeval(FILE *filp, int val, int lerrno)
 	} else {
 		/* Expecting no error */
 		igt_assert_neq(ret, 0);
+		wait_freq_settle();
 		igt_assert(readval(filp) == val);
 	}
 
-- 
1.8.4

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

[PATCH 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests

[Imre Deak]

When changing the sysfs GT min/max frequencies, the kernel won't
explicitly change the current frequency, unless it becomes out of bound
based on the new min/max values. The test happens to work on non-VLV
platforms because on those the kernel resets the current frequency
unconditionally (to adjust the RPS interrupt mask as a side-effect) and
that will lead to an RPS interrupt setting the minimum frequency.

To fix this load the GPU after decreasing the min frequency and before
checking the current frequency. This should set the current frequency to
the minimum.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 tests/pm_rps.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 9ed2125..1fd5995 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -335,7 +335,14 @@ static void load_helper_deinit(void)
 		drm_intel_bufmgr_destroy(lh.bufmgr);
 }
 
-static void min_max_config(void (*check)(void))
+static void do_load_gpu(void)
+{
+	load_helper_run(LOW);
+	nsleep(10000000);
+	load_helper_stop();
+}
+
+static void min_max_config(void (*check)(void), bool load_gpu)
 {
 	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
 
@@ -343,11 +350,15 @@ static void min_max_config(void (*check)(void))
 	fmid = fmid / 50 * 50;
 
 	igt_debug("\nCheck original min and max...\n");
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nSet min=RPn and max=RP0...\n");
 	writeval(stuff[MIN].filp, origfreqs[RPn]);
 	writeval(stuff[MAX].filp, origfreqs[RP0]);
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nIncrease min to midpoint...\n");
@@ -368,10 +379,14 @@ static void min_max_config(void (*check)(void))
 
 	igt_debug("\nDecrease min to midpoint...\n");
 	writeval(stuff[MIN].filp, fmid);
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nDecrease min to RPn...\n");
 	writeval(stuff[MIN].filp, origfreqs[RPn]);
+	if (load_gpu)
+		do_load_gpu();
 	check();
 
 	igt_debug("\nDecrease min below RPn (invalid)...\n");
@@ -605,14 +620,14 @@ igt_main
 	}
 
 	igt_subtest("basic-api")
-		min_max_config(basic_check);
+		min_max_config(basic_check, false);
 
 	igt_subtest("min-max-config-idle")
-		min_max_config(idle_check);
+		min_max_config(idle_check, true);
 
 	igt_subtest("min-max-config-loaded") {
 		load_helper_run(HIGH);
-		min_max_config(loaded_check);
+		min_max_config(loaded_check, false);
 		load_helper_stop();
 	}
 
-- 
1.8.4

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

[PATCH 3/3] tests/pm_rps: vlv: round middle point to freq supported by HW

[Imre Deak]

Atm the test assumes that the calculated middle frequency point is
supported by the HW, but it's not so at least on VLV. On my B0
BYT-M there is a 22MHz step between the allowed values, so the test will
fail trying to set the calculated middle freq that isn't aligned to
this.

To fix this get the nearest supported value by setting the target
frequency as a min or max frequency and read it back. The kernel will
round the returned value to the nearest supported.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 tests/pm_rps.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index 1fd5995..8777a73 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -120,7 +120,7 @@ static void wait_freq_settle(void)
 	}
 }
 
-static int do_writeval(FILE *filp, int val, int lerrno)
+static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
 {
 	int ret, orig;
 
@@ -131,18 +131,21 @@ static int do_writeval(FILE *filp, int val, int lerrno)
 	if (lerrno) {
 		/* Expecting specific error */
 		igt_assert(ret == EOF && errno == lerrno);
-		igt_assert(readval(filp) == orig);
+		if (readback_check)
+			igt_assert_eq(readval(filp), orig);
 	} else {
 		/* Expecting no error */
 		igt_assert_neq(ret, 0);
 		wait_freq_settle();
-		igt_assert(readval(filp) == val);
+		if (readback_check)
+			igt_assert_eq(readval(filp), val);
 	}
 
 	return ret;
 }
-#define writeval(filp, val) do_writeval(filp, val, 0)
-#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL)
+#define writeval(filp, val) do_writeval(filp, val, 0, true)
+#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
+#define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
 
 static void checkit(const int *freqs)
 {
@@ -342,12 +345,36 @@ static void do_load_gpu(void)
 	load_helper_stop();
 }
 
+/* Return a frequency rounded by HW to the nearest supported value */
+static int get_hw_rounded_freq(int target)
+{
+	int freqs[NUMFREQ];
+	int old_freq;
+	int idx;
+	int ret;
+
+	read_freqs(freqs);
+
+	if (freqs[MIN] > target)
+		idx = MIN;
+	else
+		idx = MAX;
+
+	old_freq = freqs[idx];
+	writeval_nocheck(stuff[idx].filp, target);
+	read_freqs(freqs);
+	ret = freqs[idx];
+	writeval_nocheck(stuff[idx].filp, old_freq);
+
+	return ret;
+}
+
 static void min_max_config(void (*check)(void), bool load_gpu)
 {
 	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
 
 	/* hw (and so kernel) currently rounds to 50 MHz ... */
-	fmid = fmid / 50 * 50;
+	fmid = get_hw_rounded_freq(fmid / 50 * 50);
 
 	igt_debug("\nCheck original min and max...\n");
 	if (load_gpu)
-- 
1.8.4

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

Re: [PATCH 3/3] tests/pm_rps: vlv: round middle point to freq supported by HW

[Ville Syrjälä]

On Thu, Dec 04, 2014 at 06:09:40PM +0200, Imre Deak wrote:
> Atm the test assumes that the calculated middle frequency point is
> supported by the HW, but it's not so at least on VLV. On my B0
> BYT-M there is a 22MHz step between the allowed values, so the test will
> fail trying to set the calculated middle freq that isn't aligned to
> this.
> 
> To fix this get the nearest supported value by setting the target
> frequency as a min or max frequency and read it back. The kernel will
> round the returned value to the nearest supported.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  tests/pm_rps.c | 39 +++++++++++++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> index 1fd5995..8777a73 100644
> --- a/tests/pm_rps.c
> +++ b/tests/pm_rps.c
> @@ -120,7 +120,7 @@ static void wait_freq_settle(void)
>  	}
>  }
>  
> -static int do_writeval(FILE *filp, int val, int lerrno)
> +static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
>  {
>  	int ret, orig;
>  
> @@ -131,18 +131,21 @@ static int do_writeval(FILE *filp, int val, int lerrno)
>  	if (lerrno) {
>  		/* Expecting specific error */
>  		igt_assert(ret == EOF && errno == lerrno);
> -		igt_assert(readval(filp) == orig);
> +		if (readback_check)
> +			igt_assert_eq(readval(filp), orig);
>  	} else {
>  		/* Expecting no error */
>  		igt_assert_neq(ret, 0);
>  		wait_freq_settle();
> -		igt_assert(readval(filp) == val);
> +		if (readback_check)
> +			igt_assert_eq(readval(filp), val);
>  	}
>  
>  	return ret;
>  }
> -#define writeval(filp, val) do_writeval(filp, val, 0)
> -#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL)
> +#define writeval(filp, val) do_writeval(filp, val, 0, true)
> +#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
> +#define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
>  
>  static void checkit(const int *freqs)
>  {
> @@ -342,12 +345,36 @@ static void do_load_gpu(void)
>  	load_helper_stop();
>  }
>  
> +/* Return a frequency rounded by HW to the nearest supported value */
> +static int get_hw_rounded_freq(int target)
> +{
> +	int freqs[NUMFREQ];
> +	int old_freq;
> +	int idx;
> +	int ret;
> +
> +	read_freqs(freqs);
> +
> +	if (freqs[MIN] > target)
> +		idx = MIN;
> +	else
> +		idx = MAX;
> +
> +	old_freq = freqs[idx];
> +	writeval_nocheck(stuff[idx].filp, target);
> +	read_freqs(freqs);
> +	ret = freqs[idx];
> +	writeval_nocheck(stuff[idx].filp, old_freq);
> +
> +	return ret;
> +}
> +
>  static void min_max_config(void (*check)(void), bool load_gpu)
>  {
>  	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
>  
>  	/* hw (and so kernel) currently rounds to 50 MHz ... */

This comment could use a bit of updating as well.

> -	fmid = fmid / 50 * 50;
> +	fmid = get_hw_rounded_freq(fmid / 50 * 50);
>  
>  	igt_debug("\nCheck original min and max...\n");
>  	if (load_gpu)
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/3] tests/pm_rps: vlv: round middle point to freq supported by HW

[Imre Deak]

When setting the calculated middle frequency value the test assumes that
the HW/kernel rounds this value according to a 50MHz step value. This is
not so at least on VLV/CHV, on my B0 BYT-M for example this step value
is 22MHz, so there the test will fail.

To fix this get the nearest supported value by setting the target
frequency as a min or max frequency and read it back. The kernel will
round the returned value to the nearest supported.

v2:
- remove the 50MHz rounding that was done for non-VLV platforms, the new
  way of rounding should provide the correct value for all platforms
  (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 tests/pm_rps.c | 44 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index fa6c014..180a147 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -120,7 +120,7 @@ static void wait_freq_settle(void)
 	}
 }
 
-static int do_writeval(FILE *filp, int val, int lerrno)
+static int do_writeval(FILE *filp, int val, int lerrno, bool readback_check)
 {
 	int ret, orig;
 
@@ -131,18 +131,21 @@ static int do_writeval(FILE *filp, int val, int lerrno)
 	if (lerrno) {
 		/* Expecting specific error */
 		igt_assert(ret == EOF && errno == lerrno);
-		igt_assert(readval(filp) == orig);
+		if (readback_check)
+			igt_assert_eq(readval(filp), orig);
 	} else {
 		/* Expecting no error */
 		igt_assert_neq(ret, 0);
 		wait_freq_settle();
-		igt_assert(readval(filp) == val);
+		if (readback_check)
+			igt_assert_eq(readval(filp), val);
 	}
 
 	return ret;
 }
-#define writeval(filp, val) do_writeval(filp, val, 0)
-#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL)
+#define writeval(filp, val) do_writeval(filp, val, 0, true)
+#define writeval_inval(filp, val) do_writeval(filp, val, EINVAL, true)
+#define writeval_nocheck(filp, val) do_writeval(filp, val, 0, false)
 
 static void checkit(const int *freqs)
 {
@@ -342,12 +345,39 @@ static void do_load_gpu(void)
 	load_helper_stop();
 }
 
+/* Return a frequency rounded by HW to the nearest supported value */
+static int get_hw_rounded_freq(int target)
+{
+	int freqs[NUMFREQ];
+	int old_freq;
+	int idx;
+	int ret;
+
+	read_freqs(freqs);
+
+	if (freqs[MIN] > target)
+		idx = MIN;
+	else
+		idx = MAX;
+
+	old_freq = freqs[idx];
+	writeval_nocheck(stuff[idx].filp, target);
+	read_freqs(freqs);
+	ret = freqs[idx];
+	writeval_nocheck(stuff[idx].filp, old_freq);
+
+	return ret;
+}
+
 static void min_max_config(void (*check)(void), bool load_gpu)
 {
 	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
 
-	/* hw (and so kernel) currently rounds to 50 MHz ... */
-	fmid = fmid / 50 * 50;
+	/*
+	 * hw (and so kernel) rounds to the nearest value supported by
+	 * the given platform.
+	 */
+	fmid = get_hw_rounded_freq(fmid);
 
 	igt_debug("\nCheck original min and max...\n");
 	if (load_gpu)
-- 
1.8.4

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

Re: [PATCH 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests

[Daniel Vetter]

On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote:
> When changing the sysfs GT min/max frequencies, the kernel won't
> explicitly change the current frequency, unless it becomes out of bound
> based on the new min/max values. The test happens to work on non-VLV
> platforms because on those the kernel resets the current frequency
> unconditionally (to adjust the RPS interrupt mask as a side-effect) and
> that will lead to an RPS interrupt setting the minimum frequency.
> 
> To fix this load the GPU after decreasing the min frequency and before
> checking the current frequency. This should set the current frequency to
> the minimum.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Isn't this a kernel bug which should be fixed in the kernel? If userspace
changes the requested freq, the kernel should obey right away. Not
somewhen later when something happens ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests

[Chris Wilson]

On Fri, Dec 05, 2014 at 09:56:18PM +0100, Daniel Vetter wrote:
> On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote:
> > When changing the sysfs GT min/max frequencies, the kernel won't
> > explicitly change the current frequency, unless it becomes out of bound
> > based on the new min/max values. The test happens to work on non-VLV
> > platforms because on those the kernel resets the current frequency
> > unconditionally (to adjust the RPS interrupt mask as a side-effect) and
> > that will lead to an RPS interrupt setting the minimum frequency.
> > 
> > To fix this load the GPU after decreasing the min frequency and before
> > checking the current frequency. This should set the current frequency to
> > the minimum.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Isn't this a kernel bug which should be fixed in the kernel? If userspace
> changes the requested freq, the kernel should obey right away. Not
> somewhen later when something happens ...

Only if busy. If the GPU is idle, we should ignore the softlimits and
program the most efficient idle frequency - as part of i915.powersave.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests

[Imre Deak]

On Fri, 2014-12-05 at 21:56 +0100, Daniel Vetter wrote:
> On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote:
> > When changing the sysfs GT min/max frequencies, the kernel won't
> > explicitly change the current frequency, unless it becomes out of bound
> > based on the new min/max values. The test happens to work on non-VLV
> > platforms because on those the kernel resets the current frequency
> > unconditionally (to adjust the RPS interrupt mask as a side-effect) and
> > that will lead to an RPS interrupt setting the minimum frequency.
> > 
> > To fix this load the GPU after decreasing the min frequency and before
> > checking the current frequency. This should set the current frequency to
> > the minimum.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Isn't this a kernel bug which should be fixed in the kernel? If userspace
> changes the requested freq, the kernel should obey right away. Not
> somewhen later when something happens ...

It isn't the current frequency that's changed but the min/max limits. If
by changing those the current frequency gets out of bound the kernel
properly changes the current frequency too. 


> -Daniel


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

Re: [PATCH 2/3] tests/pm_rps: vlv: load gpu for idle min/max tests

[Daniel Vetter]

On Fri, Dec 05, 2014 at 08:59:42PM +0000, Chris Wilson wrote:
> On Fri, Dec 05, 2014 at 09:56:18PM +0100, Daniel Vetter wrote:
> > On Thu, Dec 04, 2014 at 06:09:39PM +0200, Imre Deak wrote:
> > > When changing the sysfs GT min/max frequencies, the kernel won't
> > > explicitly change the current frequency, unless it becomes out of bound
> > > based on the new min/max values. The test happens to work on non-VLV
> > > platforms because on those the kernel resets the current frequency
> > > unconditionally (to adjust the RPS interrupt mask as a side-effect) and
> > > that will lead to an RPS interrupt setting the minimum frequency.
> > > 
> > > To fix this load the GPU after decreasing the min frequency and before
> > > checking the current frequency. This should set the current frequency to
> > > the minimum.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > 
> > Isn't this a kernel bug which should be fixed in the kernel? If userspace
> > changes the requested freq, the kernel should obey right away. Not
> > somewhen later when something happens ...
> 
> Only if busy. If the GPU is idle, we should ignore the softlimits and
> program the most efficient idle frequency - as part of i915.powersave.

Hm yeah that slight reinterpretation of the limits makes them actually
more useful. So ack from me on all 3 patches.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx