[igt-dev] [PATCH i-g-t] i915/gem_exec_suspend: Measure power consumption during suspend

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> For this test, we need a laptop running on battery power so that we can
> read the battery charge level before and after suspend. And then wait
> long enough for a reliable measure.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=111909
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>  tests/i915/gem_exec_suspend.c | 67 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/tests/i915/gem_exec_suspend.c b/tests/i915/gem_exec_suspend.c
> index af6190ddd..ee0d0719f 100644
> --- a/tests/i915/gem_exec_suspend.c
> +++ b/tests/i915/gem_exec_suspend.c
> @@ -27,9 +27,13 @@
>   * Exercise executing batches across suspend before checking the results.
>   */
>  
> +#include <fcntl.h>
> +#include <unistd.h>
> +
>  #include "igt.h"
> -#include "igt_gt.h"
>  #include "igt_dummyload.h"
> +#include "igt_gt.h"
> +#include "igt_sysfs.h"
>  
>  #define NOSLEEP 0
>  #define IDLE 1
> @@ -232,6 +236,62 @@ static void run_test(int fd, unsigned engine, unsigned flags)
>  		test_all(fd, flags);
>  }
>  
> +struct battery_sample {
> +	struct timespec tv;
> +	uint64_t charge;
> +};
> +
> +static bool get_power(int dir, struct battery_sample *s)
> +{
> +	return (clock_gettime(CLOCK_REALTIME, &s->tv) == 0 &&
> +		igt_sysfs_scanf(dir, "charge_now", "%"PRIu64, &s->charge) == 1);
> +}
> +
> +static double d_charge(const struct battery_sample *after,
> +		       const struct battery_sample *before)
> +{
> +	return (before->charge - after->charge) * 1e-3; /* mWh */
> +}
> +
> +static double d_time(const struct battery_sample *after,
> +		     const struct battery_sample *before)
> +{
> +	return ((after->tv.tv_sec - before->tv.tv_sec) +
> +		(after->tv.tv_nsec - before->tv.tv_nsec) * 1e-9);
> +}
> +
> +static void power_test(int i915, unsigned engine, unsigned flags)
> +{
> +	struct battery_sample before, after;
> +	char *status;
> +	int dir;
> +
> +	dir = open("/sys/class/power_supply/BAT0", O_RDONLY);
> +	igt_require_f(dir != -1, "/sys/class/power_supply/BAT0 not available\n");
> +
> +	igt_require_f(get_power(dir, &before),
> +		      "power test needs reported energy level\n");
> +	free(status);
This looks bogus.

> +
> +	status = igt_sysfs_get(dir, "status");
> +	igt_require_f(status && strcmp(status, "Discharging") == 0,
> +		      "power test needs to be on battery, not mains, power\n");
> +	free(status);
> +
> +	igt_set_autoresume_delay(30 * 60); /* 30 minutes */

As you said, this is quite a long time. Even with the suspend and modern
laptop it should show meaningful data quicker. But we need moar
data on multiple specimens to find a good spot.

> +
> +	igt_assert(get_power(dir, &before));
> +	run_test(i915, engine, flags);
> +	igt_assert(get_power(dir, &after));
> +
> +	igt_set_autoresume_delay(0);
> +
> +	igt_info("Power consumed while suspended: %.3fmWh\n",
> +		 d_charge(&after, &before));
> +	igt_info("Discharge rate while suspended: %.3fmW\n",
> +		 d_charge(&after, &before) * 3600 / d_time(&after, &before));

I dunno what is the go-to unit in here, but mA would be
as comparable and time independant. Perhaps show both.

There are lot of potential in here to automate against pm
regressions. Plugging this into the apc and gathering
baseline consumptions.


This is a neat start and I aimed at t-b too.
Didn't happen so I can offer only,

Acked-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

> +}
> +
>  igt_main
>  {
>  	const struct {
> @@ -289,6 +349,11 @@ igt_main
>  	igt_subtest("hang-S4")
>  		run_test(fd, 0, HIBERNATE | HANG);
>  
> +	igt_subtest("power-S0")
> +		power_test(fd, 0, IDLE);
> +	igt_subtest("power-S3")
> +		power_test(fd, 0, SUSPEND);
> +
>  	igt_fixture {
>  		igt_disallow_hang(fd, hang);
>  		close(fd);
> -- 
> 2.24.0.rc1