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@xxxxxxxxxxxxxxxxxx>
> Cc: Jeff Mcgee <jeff.mcgee@xxxxxxxxx>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@xxxxxxxxx>
>
> Signed-off-by: Katarzyna Dec <katarzyna.dec@xxxxxxxxx>
> ---
> 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();
??
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
