[Intel-gfx] [PATCH igt 08/10] igt/pm_rc6_residency: Measure residency after checking for applicability

[Ewelina Musial]

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 at chris-wilson.co.uk>
> ---
>  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx