| Message ID | 1447277879-4106-1-git-send-email-bob.j.paauwe@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ke, 2015-11-11 at 13:37 -0800, Bob Paauwe wrote: > Since commit > > commit aed242ff7ebb697e4dff912bd4dc7ec7192f7581 > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Wed Mar 18 09:48:21 2015 +0000 > > drm/i915: Relax RPS contraints to allows setting minfreq on > idle > > it is now possible that the current frequency will drop below the > user > specified minimum frequency. Update the pm_rps tests to reflect that > this is no longer considered a failure. Yes, in case the GPU is idle the frequency will drop now to the idle fr equency regardless of the minimum limit set by the user. We should still check though if the frequency drops to the idle value not something higher. You can use the i915_frequency_info in debugfs for that. > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > --- > tests/pm_rps.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > index 74f08f4..e92ca3b 100644 > --- a/tests/pm_rps.c > +++ b/tests/pm_rps.c > @@ -146,7 +146,6 @@ static void checkit(const int *freqs) > { > igt_assert_lte(freqs[MIN], freqs[MAX]); > igt_assert_lte(freqs[CUR], freqs[MAX]); > - igt_assert_lte(freqs[MIN], freqs[CUR]); > igt_assert_lte(freqs[RPn], freqs[MIN]); > igt_assert_lte(freqs[MAX], freqs[RP0]); > igt_assert_lte(freqs[RP1], freqs[RP0]); > @@ -472,14 +471,14 @@ static void idle_check(void) > read_freqs(freqs); > dump(freqs); > checkit(freqs); > - if (freqs[CUR] == freqs[MIN]) > + if (freqs[CUR] <= freqs[MIN]) > break; > usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC); > wait += IDLE_WAIT_TIMESTEP_MSEC; > } while (wait < IDLE_WAIT_TIMEOUT_MSEC); > > - igt_assert_eq(freqs[CUR], freqs[MIN]); > - igt_debug("Required %d msec to reach cur=min\n", wait); > + igt_assert_lte(freqs[CUR], freqs[MIN]); > + igt_debug("Required %d msec to reach cur<=min\n", wait); > } > > #define LOADED_WAIT_TIMESTEP_MSEC 100
On Thu, 12 Nov 2015 11:18:11 +0200 Imre Deak <imre.deak@intel.com> wrote: > On ke, 2015-11-11 at 13:37 -0800, Bob Paauwe wrote: > > Since commit > > > > commit aed242ff7ebb697e4dff912bd4dc7ec7192f7581 > > Author: Chris Wilson <chris@chris-wilson.co.uk> > > Date: Wed Mar 18 09:48:21 2015 +0000 > > > > drm/i915: Relax RPS contraints to allows setting minfreq on > > idle > > > > it is now possible that the current frequency will drop below the > > user > > specified minimum frequency. Update the pm_rps tests to reflect that > > this is no longer considered a failure. > > Yes, in case the GPU is idle the frequency will drop now to the idle fr > equency regardless of the minimum limit set by the user. We should > still check though if the frequency drops to the idle value not > something higher. You can use the i915_frequency_info in debugfs for > that. > Yeah, good idea. Right now, the idle freq is the same as the hardware minimum. So I could have it check against the hardware min. Adding support to read the info out i915_frequency_info would be a larger change to the test. But that might be a better long term solution. > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > > --- > > tests/pm_rps.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > > index 74f08f4..e92ca3b 100644 > > --- a/tests/pm_rps.c > > +++ b/tests/pm_rps.c > > @@ -146,7 +146,6 @@ static void checkit(const int *freqs) > > { > > igt_assert_lte(freqs[MIN], freqs[MAX]); > > igt_assert_lte(freqs[CUR], freqs[MAX]); > > - igt_assert_lte(freqs[MIN], freqs[CUR]); > > igt_assert_lte(freqs[RPn], freqs[MIN]); > > igt_assert_lte(freqs[MAX], freqs[RP0]); > > igt_assert_lte(freqs[RP1], freqs[RP0]); > > @@ -472,14 +471,14 @@ static void idle_check(void) > > read_freqs(freqs); > > dump(freqs); > > checkit(freqs); > > - if (freqs[CUR] == freqs[MIN]) > > + if (freqs[CUR] <= freqs[MIN]) > > break; > > usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC); > > wait += IDLE_WAIT_TIMESTEP_MSEC; > > } while (wait < IDLE_WAIT_TIMEOUT_MSEC); > > > > - igt_assert_eq(freqs[CUR], freqs[MIN]); > > - igt_debug("Required %d msec to reach cur=min\n", wait); > > + igt_assert_lte(freqs[CUR], freqs[MIN]); > > + igt_debug("Required %d msec to reach cur<=min\n", wait); > > } > > > > #define LOADED_WAIT_TIMESTEP_MSEC 100
On ke, 2015-11-11 at 13:37 -0800, Bob Paauwe wrote: > Since commit > > commit aed242ff7ebb697e4dff912bd4dc7ec7192f7581 > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Wed Mar 18 09:48:21 2015 +0000 > > drm/i915: Relax RPS contraints to allows setting minfreq on > idle > > it is now possible that the current frequency will drop below the > user > specified minimum frequency. Update the pm_rps tests to reflect that > this is no longer considered a failure. > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89728 Bob could you send an updated version of this patch? > --- > tests/pm_rps.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > index 74f08f4..e92ca3b 100644 > --- a/tests/pm_rps.c > +++ b/tests/pm_rps.c > @@ -146,7 +146,6 @@ static void checkit(const int *freqs) > { > igt_assert_lte(freqs[MIN], freqs[MAX]); > igt_assert_lte(freqs[CUR], freqs[MAX]); > - igt_assert_lte(freqs[MIN], freqs[CUR]); > igt_assert_lte(freqs[RPn], freqs[MIN]); > igt_assert_lte(freqs[MAX], freqs[RP0]); > igt_assert_lte(freqs[RP1], freqs[RP0]); > @@ -472,14 +471,14 @@ static void idle_check(void) > read_freqs(freqs); > dump(freqs); > checkit(freqs); > - if (freqs[CUR] == freqs[MIN]) > + if (freqs[CUR] <= freqs[MIN]) > break; > usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC); > wait += IDLE_WAIT_TIMESTEP_MSEC; > } while (wait < IDLE_WAIT_TIMEOUT_MSEC); > > - igt_assert_eq(freqs[CUR], freqs[MIN]); > - igt_debug("Required %d msec to reach cur=min\n", wait); > + igt_assert_lte(freqs[CUR], freqs[MIN]); > + igt_debug("Required %d msec to reach cur<=min\n", wait); > } > > #define LOADED_WAIT_TIMESTEP_MSEC 100
The first patch updates the test to work now that the frequency can drop below the user specified minimum to the idle frequency. The second patch attempts to improve the checking and actually checks that the frequency has dropped to RPn in some cases. I'm not real happy with this patch but didn't want to re-write the entire test program to make handle these cases more elegantly. Other ideas welcome. Bob Paauwe (2): igt/pm_rps: current freq < user specified min is not a fail (v2) igt/pm_rps: Add checks for freq = idle (RPn) in specific cases. tests/pm_rps.c | 55 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 17 deletions(-)
diff --git a/tests/pm_rps.c b/tests/pm_rps.c index 74f08f4..e92ca3b 100644 --- a/tests/pm_rps.c +++ b/tests/pm_rps.c @@ -146,7 +146,6 @@ static void checkit(const int *freqs) { igt_assert_lte(freqs[MIN], freqs[MAX]); igt_assert_lte(freqs[CUR], freqs[MAX]); - igt_assert_lte(freqs[MIN], freqs[CUR]); igt_assert_lte(freqs[RPn], freqs[MIN]); igt_assert_lte(freqs[MAX], freqs[RP0]); igt_assert_lte(freqs[RP1], freqs[RP0]); @@ -472,14 +471,14 @@ static void idle_check(void) read_freqs(freqs); dump(freqs); checkit(freqs); - if (freqs[CUR] == freqs[MIN]) + if (freqs[CUR] <= freqs[MIN]) break; usleep(1000 * IDLE_WAIT_TIMESTEP_MSEC); wait += IDLE_WAIT_TIMESTEP_MSEC; } while (wait < IDLE_WAIT_TIMEOUT_MSEC); - igt_assert_eq(freqs[CUR], freqs[MIN]); - igt_debug("Required %d msec to reach cur=min\n", wait); + igt_assert_lte(freqs[CUR], freqs[MIN]); + igt_debug("Required %d msec to reach cur<=min\n", wait); } #define LOADED_WAIT_TIMESTEP_MSEC 100
Since commit commit aed242ff7ebb697e4dff912bd4dc7ec7192f7581 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed Mar 18 09:48:21 2015 +0000 drm/i915: Relax RPS contraints to allows setting minfreq on idle it is now possible that the current frequency will drop below the user specified minimum frequency. Update the pm_rps tests to reflect that this is no longer considered a failure. Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> --- tests/pm_rps.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)