| Message ID | 20170818073311.8924-1-katarzyna.dec@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Katarzyna Dec (2017-08-18 08:33:11) > Waitboost and reset test cases were failing because maximum > frequency is not always boost frequency (sometimes overcloking > occurs). > Moreover more time is needed for boost to be finished. > Changed boost_freq function so boost occurred on the same engine > as low load. > Added function waiting for boost to be finished. > Made test DRM master to make sure that the test isn't ran along Xorg > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Jeff Mcgee <jeff.mcgee@intel.com> > Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com> > > Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com> > --- > tests/pm_rps.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 50 insertions(+), 8 deletions(-) > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > index f0455e78..08a73f5d 100644 > --- a/tests/pm_rps.c > +++ b/tests/pm_rps.c > @@ -50,6 +50,7 @@ enum { > RP0, > RP1, > RPn, > + BOOST, > NUMFREQ > }; > > @@ -60,7 +61,7 @@ struct junk { > const char *mode; > FILE *filp; > } stuff[] = { > - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { NULL, NULL, NULL } > + { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL } > }; > > static int readval(FILE *filp) > @@ -556,25 +557,61 @@ static void stabilize_check(int *out) > static void reset_gpu(void) > { > int fd = drm_open_driver(DRIVER_INTEL); > + > igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT)); > close(fd); > } > > +static bool is_in_boost(void) > +{ > + char buf[1024]; > + > + igt_debugfs_read(drm_fd, "i915_rps_boost_info", buf); > + return strstr(buf, "Boosts outstanding? 1"); Might as well make this generic and return the count. Then you can use it to return -1 if either i915_rps_boost_info doesn't exist (oops, buf used unterminated!) or doesn't contain the count. > +} > + > static void boost_freq(int fd, int *boost_freqs) > { > int64_t timeout = 1; > - int ring = -1; > igt_spin_t *load; > + unsigned int engine; > > - load = igt_spin_batch_new(fd, ring, 0); > - > + /* put boost on the same engine as low load */ > + engine = I915_EXEC_RENDER; > + if (intel_gen(lh.devid) >= 6) > + engine = I915_EXEC_BLT; > + load = igt_spin_batch_new(fd, engine, 0); Something to note is that spin-batch will also force the GPU to maximum. You could set the boost freq > max freq to differentiate > /* Waiting will grant us a boost to maximum */ > gem_wait(fd, load->handle, &timeout); > > read_freqs(boost_freqs); > dump(boost_freqs); > + igt_assert_eq(is_in_boost(), 1); Will fail on older kernels. > + /* Avoid downlocking till boost request is pending */ > + igt_spin_batch_end(load); > + gem_sync(fd, load->handle); > igt_spin_batch_free(fd, load); > + Misplaced whitespace. > +} > + > +#define BOOST_WAIT_TIMESTEP_MSEC 250 > +#define BOOST_WAIT_TIMEOUT_MSEC 15000 > +static void fall_from_boost_wait(int *freqs) > +{ > + int wait = 0; > + > + do { > + read_freqs(freqs); > + dump(freqs); Why pass in freqs? After this function you overwrite them, and by its nature the value of those freqs whilst in this function is either boosted or some other value. > + if (!is_in_boost()) > + break; > + usleep(1000 * BOOST_WAIT_TIMESTEP_MSEC); > + wait += BOOST_WAIT_TIMESTEP_MSEC; > + > + } while (wait < BOOST_WAIT_TIMEOUT_MSEC); > + > + igt_debug("Waited %d till falling from boost\n", wait); Units! "Waited %dms" > } > > static void waitboost(bool reset) > @@ -601,6 +638,7 @@ static void waitboost(bool reset) > * to maximum. > */ > boost_freq(fd, boost_freqs); > + fall_from_boost_wait(post_freqs); wait_for_fall_from_grace() ? > > igt_debug("Apply low load again...\n"); > sleep(1); > @@ -611,7 +649,7 @@ static void waitboost(bool reset) > idle_check(); > > igt_assert_lt(pre_freqs[CUR], pre_freqs[MAX]); > - igt_assert_eq(boost_freqs[CUR], boost_freqs[MAX]); > + igt_assert_eq(boost_freqs[CUR], boost_freqs[BOOST]); > igt_assert_lt(post_freqs[CUR], post_freqs[MAX]); > > close(fd); > @@ -640,14 +678,15 @@ igt_main > struct junk *junk = stuff; > int ret; > > - /* Use drm_open_driver to verify device existence */ > - drm_fd = drm_open_driver(DRIVER_INTEL); > + /* Use drm_open_driver to to force running tests as drm master */ > + drm_fd = drm_open_driver_master(DRIVER_INTEL); ?? This interface / policy does not require the display. > igt_require_gem(drm_fd); > igt_require(gem_can_store_dword(drm_fd, 0)); > > do { > int val = -1; > char *path; > + > ret = asprintf(&path, sysfs_base_path, device, junk->name); > igt_assert(ret != -1); > junk->filp = fopen(path, junk->mode); > @@ -657,7 +696,7 @@ igt_main > val = readval(junk->filp); > igt_assert(val >= 0); > junk++; > - } while(junk->name != NULL); > + } while (junk->name != NULL); > > read_freqs(origfreqs); > > @@ -684,4 +723,7 @@ igt_main > igt_subtest("reset") > waitboost(true); > > + igt_fixture { > + intel_register_access_fini(); ??
On Thu, Aug 31, 2017 at 01:03:24PM +0000, Patchwork wrote: > == Series Details == > > Series: pm_rps: Changes in waitboost scenario (rev10) > URL : https://patchwork.freedesktop.org/series/28966/ > State : failure > > == Summary == > > IGT patchset tested on top of latest successful build > c2159678d283fea5615ec8e846a51cf4954ac82d tests/perf: add Geminilake support > > with latest DRM-Tip kernel build CI_DRM_3021 > c399d43adc55 drm-tip: 2017y-08m-31d-07h-25m-28s UTC integration manifest > > Test kms_busy: > Subgroup basic-flip-a: > pass -> FAIL (fi-bwr-2160) That's not caused by the series (PW's rev10 == PATCH v8). v7 and v8 do not change anything in the code, only comments and commit messages are amended. Filtering the CI noise from all the runs the patch had - it healthy and it also gets rid of a flipflop we had. Merged with minor commit message reformatting. Thanks for the patch and reviews!
diff --git a/tests/pm_rps.c b/tests/pm_rps.c index f0455e78..08a73f5d 100644 --- a/tests/pm_rps.c +++ b/tests/pm_rps.c @@ -50,6 +50,7 @@ enum { RP0, RP1, RPn, + BOOST, NUMFREQ }; @@ -60,7 +61,7 @@ struct junk { const char *mode; FILE *filp; } stuff[] = { - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { NULL, NULL, NULL } + { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL } }; static int readval(FILE *filp) @@ -556,25 +557,61 @@ static void stabilize_check(int *out) static void reset_gpu(void) { int fd = drm_open_driver(DRIVER_INTEL); + igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT)); close(fd); } +static bool is_in_boost(void) +{ + char buf[1024]; + + igt_debugfs_read(drm_fd, "i915_rps_boost_info", buf); + return strstr(buf, "Boosts outstanding? 1"); +} + static void boost_freq(int fd, int *boost_freqs) { int64_t timeout = 1; - int ring = -1; igt_spin_t *load; + unsigned int engine; - load = igt_spin_batch_new(fd, ring, 0); - + /* put boost on the same engine as low load */ + engine = I915_EXEC_RENDER; + if (intel_gen(lh.devid) >= 6) + engine = I915_EXEC_BLT; + load = igt_spin_batch_new(fd, engine, 0); /* Waiting will grant us a boost to maximum */ gem_wait(fd, load->handle, &timeout); read_freqs(boost_freqs); dump(boost_freqs); + igt_assert_eq(is_in_boost(), 1); + /* Avoid downlocking till boost request is pending */ + igt_spin_batch_end(load); + gem_sync(fd, load->handle); igt_spin_batch_free(fd, load); + +} + +#define BOOST_WAIT_TIMESTEP_MSEC 250 +#define BOOST_WAIT_TIMEOUT_MSEC 15000 +static void fall_from_boost_wait(int *freqs) +{ + int wait = 0; + + do { + read_freqs(freqs); + dump(freqs); + if (!is_in_boost()) + break; + usleep(1000 * BOOST_WAIT_TIMESTEP_MSEC); + wait += BOOST_WAIT_TIMESTEP_MSEC; + + } while (wait < BOOST_WAIT_TIMEOUT_MSEC); + + igt_debug("Waited %d till falling from boost\n", wait); } static void waitboost(bool reset) @@ -601,6 +638,7 @@ static void waitboost(bool reset) * to maximum. */ boost_freq(fd, boost_freqs); + fall_from_boost_wait(post_freqs); igt_debug("Apply low load again...\n"); sleep(1); @@ -611,7 +649,7 @@ static void waitboost(bool reset) idle_check(); igt_assert_lt(pre_freqs[CUR], pre_freqs[MAX]); - igt_assert_eq(boost_freqs[CUR], boost_freqs[MAX]); + igt_assert_eq(boost_freqs[CUR], boost_freqs[BOOST]); igt_assert_lt(post_freqs[CUR], post_freqs[MAX]); close(fd); @@ -640,14 +678,15 @@ igt_main struct junk *junk = stuff; int ret; - /* Use drm_open_driver to verify device existence */ - drm_fd = drm_open_driver(DRIVER_INTEL); + /* Use drm_open_driver to to force running tests as drm master */ + drm_fd = drm_open_driver_master(DRIVER_INTEL); igt_require_gem(drm_fd); igt_require(gem_can_store_dword(drm_fd, 0)); do { int val = -1; char *path; + ret = asprintf(&path, sysfs_base_path, device, junk->name); igt_assert(ret != -1); junk->filp = fopen(path, junk->mode); @@ -657,7 +696,7 @@ igt_main val = readval(junk->filp); igt_assert(val >= 0); junk++; - } while(junk->name != NULL); + } while (junk->name != NULL); read_freqs(origfreqs); @@ -684,4 +723,7 @@ igt_main igt_subtest("reset") waitboost(true); + igt_fixture { + intel_register_access_fini(); + } }
Waitboost and reset test cases were failing because maximum frequency is not always boost frequency (sometimes overcloking occurs). Moreover more time is needed for boost to be finished. Changed boost_freq function so boost occurred on the same engine as low load. Added function waiting for boost to be finished. Made test DRM master to make sure that the test isn't ran along Xorg Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Jeff Mcgee <jeff.mcgee@intel.com> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com> --- tests/pm_rps.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 8 deletions(-)