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

[Chris Wilson]

Quoting Mika Kuoppala (2019-10-30 12:30:47)
> 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.

/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 at linux.intel.com>

Ta.
-Chris