On Mon, Dec 04, 2017 at 09:27:29AM +0000, Chris Wilson wrote:
> CI doesn't run in whole-test mode, but runs each subtest individually.
> Tests that are designed to do a block of work to be shared between many
> subtests end up running that work multiple times (once per subtest) and
> worse, that work is wasted if the subtest will be skipped.
>
> pm_rc6_residency is one such example that measured all the residencies
> up front before skipping, each skip was therefore taking in excess of
> 10s.
Looks good for me.
-Ewelina
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> tests/pm_rc6_residency.c | 68 ++++++++++++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c
> index ad05cca4..3d46fbd1 100644
> --- a/tests/pm_rc6_residency.c
> +++ b/tests/pm_rc6_residency.c
> @@ -53,11 +53,11 @@ struct residencies {
>
> static unsigned long get_rc6_enabled_mask(void)
> {
> - unsigned long rc6_mask;
> + unsigned long enabled;
>
> - rc6_mask = 0;
> - igt_sysfs_scanf(sysfs, "power/rc6_enable", "%lu", &rc6_mask);
> - return rc6_mask;
> + enabled = 0;
> + igt_sysfs_scanf(sysfs, "power/rc6_enable", "%lu", &enabled);
> + return enabled;
> }
>
> static unsigned long read_rc6_residency(const char *name)
> @@ -85,20 +85,20 @@ static void residency_accuracy(unsigned int diff,
> "Sysfs RC6 residency counter is inaccurate.\n");
> }
>
> -static void read_residencies(int devid, unsigned int rc6_mask,
> +static void read_residencies(int devid, unsigned int mask,
> struct residencies *res)
> {
> - if (rc6_mask & RC6_ENABLED)
> + if (mask & RC6_ENABLED)
> res->rc6 = read_rc6_residency("rc6");
>
> - if ((rc6_mask & RC6_ENABLED) &&
> + if ((mask & RC6_ENABLED) &&
> (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)))
> res->media_rc6 = read_rc6_residency("media_rc6");
>
> - if (rc6_mask & RC6P_ENABLED)
> + if (mask & RC6P_ENABLED)
> res->rc6p = read_rc6_residency("rc6p");
>
> - if (rc6_mask & RC6PP_ENABLED)
> + if (mask & RC6PP_ENABLED)
> res->rc6pp = read_rc6_residency("rc6pp");
> }
>
> @@ -111,7 +111,7 @@ static unsigned long gettime_ms(void)
> return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
> }
>
> -static void measure_residencies(int devid, unsigned int rc6_mask,
> +static void measure_residencies(int devid, unsigned int mask,
> struct residencies *res)
> {
> struct residencies start = { };
> @@ -119,15 +119,9 @@ static void measure_residencies(int devid, unsigned int rc6_mask,
> int retry;
> unsigned long t;
>
> - if (!rc6_mask)
> + if (!mask)
> return;
>
> - /*
> - * For some reason my ivb isn't idle even after syncing up with the gpu.
> - * Let's add a sleep just to make it happy.
> - */
> - sleep(8);
> -
> /*
> * Retry in case of counter wrap-around. We simply re-run the
> * measurement, since the valid counter range is different on
> @@ -135,9 +129,9 @@ static void measure_residencies(int devid, unsigned int rc6_mask,
> */
> for (retry = 0; retry < 2; retry++) {
> t = gettime_ms();
> - read_residencies(devid, rc6_mask, &start);
> + read_residencies(devid, mask, &start);
> sleep(SLEEP_DURATION);
> - read_residencies(devid, rc6_mask, &end);
> + read_residencies(devid, mask, &end);
> t = gettime_ms() - t;
>
> if (end.rc6 >= start.rc6 && end.media_rc6 >= start.media_rc6 &&
> @@ -166,9 +160,8 @@ static void measure_residencies(int devid, unsigned int rc6_mask,
>
> igt_main
> {
> - unsigned int rc6_mask;
> - int devid = 0;
> - struct residencies res;
> + unsigned int rc6_enabled = 0;
> + unsigned int devid = 0;
>
> igt_skip_on_simulation();
>
> @@ -181,31 +174,44 @@ igt_main
> sysfs = igt_sysfs_open(fd, NULL);
> close(fd);
>
> - rc6_mask = get_rc6_enabled_mask();
> - igt_require(rc6_mask);
> -
> - measure_residencies(devid, rc6_mask, &res);
> + rc6_enabled = get_rc6_enabled_mask();
> + igt_require(rc6_enabled);
> }
>
> igt_subtest("rc6-accuracy") {
> - igt_skip_on(!(rc6_mask & RC6_ENABLED));
> + struct residencies res;
> +
> + igt_require(rc6_enabled & RC6_ENABLED);
>
> + measure_residencies(devid, RC6_ENABLED, &res);
> residency_accuracy(res.rc6, res.duration, "rc6");
> }
> +
> igt_subtest("media-rc6-accuracy") {
> - igt_skip_on(!((rc6_mask & RC6_ENABLED) &&
> - (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))));
> + struct residencies res;
>
> + igt_require((rc6_enabled & RC6_ENABLED) &&
> + (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)));
> +
> + measure_residencies(devid, RC6_ENABLED, &res);
> residency_accuracy(res.media_rc6, res.duration, "media_rc6");
> }
> +
> igt_subtest("rc6p-accuracy") {
> - igt_skip_on(!(rc6_mask & RC6P_ENABLED));
> + struct residencies res;
> +
> + igt_require(rc6_enabled & RC6P_ENABLED);
>
> + measure_residencies(devid, RC6P_ENABLED, &res);
> residency_accuracy(res.rc6p, res.duration, "rc6p");
> }
> +
> igt_subtest("rc6pp-accuracy") {
> - igt_skip_on(!(rc6_mask & RC6PP_ENABLED));
> + struct residencies res;
> +
> + igt_require(rc6_enabled & RC6PP_ENABLED);
>
> + measure_residencies(devid, RC6PP_ENABLED, &res);
> residency_accuracy(res.rc6pp, res.duration, "rc6pp");
> }
> }
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
