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.
> 
> Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Ewelina Musial <[email protected]>

Forgot to add r-b :)
> ---
>  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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to