Quoting Mika Kuoppala (2019-10-30 12:30:47) > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> > > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxx> Ta. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
