| Message ID | 20191027171152.31128-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] i915/gem_exec_suspend: Measure power consumption during suspend | expand |
Chris Wilson <chris@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@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@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@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
Quoting Mika Kuoppala (2019-10-30 12:30:47) > Chris Wilson <chris@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@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@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. /sys/devices/power_supply/BAT0/ gives mWh and voltage iirc. I went with mWh/mW as that seems the easiest with less modelling required. > There are lot of potential in here to automate against pm > regressions. Plugging this into the apc and gathering > baseline consumptions. Yup, fingers crossed that may happen. :) > 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@linux.intel.com> Ta. -Chris
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); + + 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 */ + + 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)); +} + 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);
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@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- tests/i915/gem_exec_suspend.c | 67 ++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-)