| Message ID | 20190322092155.1656-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,01/24] i915/gem_exec_latency: Measure the latency of context switching | expand |
On 22/03/2019 09:21, Chris Wilson wrote: > Read the RAPL power metrics courtesy of perf. Or your local HW > equivalent? > > v2: uselocale() > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > lib/Makefile.am | 1 + > lib/Makefile.sources | 2 + > lib/igt_gpu_power.c | 114 +++++++++++++++++++++++++++++++++++++++++++ > lib/igt_gpu_power.h | 59 ++++++++++++++++++++++ > lib/meson.build | 2 + > 5 files changed, 178 insertions(+) > create mode 100644 lib/igt_gpu_power.c > create mode 100644 lib/igt_gpu_power.h > > diff --git a/lib/Makefile.am b/lib/Makefile.am > index 3e6a7fdba..62e8bda73 100644 > --- a/lib/Makefile.am > +++ b/lib/Makefile.am > @@ -84,4 +84,5 @@ libintel_tools_la_LIBADD = \ > $(LIBUDEV_LIBS) \ > $(PIXMAN_LIBS) \ > $(GLIB_LIBS) \ > + libigt_perf.la \ > -lm > diff --git a/lib/Makefile.sources b/lib/Makefile.sources > index cf2720981..e00347f94 100644 > --- a/lib/Makefile.sources > +++ b/lib/Makefile.sources > @@ -26,6 +26,8 @@ lib_source_list = \ > igt_color_encoding.c \ > igt_color_encoding.h \ > igt_edid_template.h \ > + igt_gpu_power.c \ > + igt_gpu_power.h \ > igt_gt.c \ > igt_gt.h \ > igt_gvt.c \ > diff --git a/lib/igt_gpu_power.c b/lib/igt_gpu_power.c > new file mode 100644 > index 000000000..a4e3c1420 > --- /dev/null > +++ b/lib/igt_gpu_power.c > @@ -0,0 +1,114 @@ > +#include <ctype.h> > +#include <errno.h> > +#include <fcntl.h> > +#include <locale.h> > +#include <math.h> > +#include <unistd.h> > + > +#include "igt_gpu_power.h" > +#include "igt_perf.h" > + > +static int filename_to_buf(const char *filename, char *buf, unsigned int sz) > +{ > + int fd; > + ssize_t ret; > + > + fd = open(filename, O_RDONLY); > + if (fd < 0) > + return -1; > + > + ret = read(fd, buf, sz - 1); > + close(fd); > + if (ret < 1) > + return -1; > + > + buf[ret] = '\0'; > + > + return 0; > +} > + > +static uint64_t filename_to_u64(const char *filename, int base) > +{ > + char buf[64], *b; > + > + if (filename_to_buf(filename, buf, sizeof(buf))) > + return 0; > + > + /* > + * Handle both single integer and key=value formats by skipping > + * leading non-digits. > + */ > + b = buf; > + while (*b && !isdigit(*b)) > + b++; > + > + return strtoull(b, NULL, base); > +} > + > +static double filename_to_double(const char *filename) > +{ > + locale_t locale, oldlocale; > + char buf[80]; > + double v; > + > + if (filename_to_buf(filename, buf, sizeof(buf))) > + return 0; > + > + /* Replace user environment with plain C to match kernel format */ > + locale = newlocale(LC_ALL, "C", 0); > + oldlocale = uselocale(locale); > + > + v = strtod(buf, NULL); > + > + uselocale(oldlocale); > + freelocale(locale); > + > + return v; > +} Hey you know what our guidelines for introducing a 3rd copy of something are. ;) Maybe it would be fair game to stuff these helpers to igt_perf.la - since both overlay and intel_gpu_gpu link with it already and it is about interfacing with perf after all. There are some small differences between the functions here and in intel_gpu_top, someone is right or wrong, or at least more right? > + > +static uint64_t rapl_type_id(void) > +{ > + return filename_to_u64("/sys/devices/power/type", 10); > +} > + > +static uint64_t rapl_gpu_power(void) > +{ > + return filename_to_u64("/sys/devices/power/events/energy-gpu", 0); > +} > + > +static double rapl_gpu_power_scale(void) > +{ > + return filename_to_double("/sys/devices/power/events/energy-gpu.scale"); > +} > + > +int gpu_power_open(struct gpu_power *power) > +{ > + power->fd = igt_perf_open(rapl_type_id(), rapl_gpu_power()); > + if (power->fd < 0) { > + power->fd = -errno; > + goto err; > + } > + > + power->scale = rapl_gpu_power_scale(); > + if (isnan(power->scale) || !power->scale) { > + close(power->fd); > + goto err; > + } > + power->scale *= 1e9; Scale has no unit so I think this would go better into gpu_power_W. Would avoid * 1e9 * 1e-9 in gpu_power_J as well. > + > + return 0; > + > +err: > + errno = 0; > + return power->fd; > +} > + > +bool gpu_power_read(struct gpu_power *power, struct gpu_power_sample *s) > +{ > + return read(power->fd, s, sizeof(*s)) == sizeof(*s); > +} > + > +void gpu_power_close(struct gpu_power *power) > +{ > + close(power->fd); > +} > diff --git a/lib/igt_gpu_power.h b/lib/igt_gpu_power.h > new file mode 100644 > index 000000000..4e1b747c0 > --- /dev/null > +++ b/lib/igt_gpu_power.h > @@ -0,0 +1,59 @@ > +/* > + * Copyright © 2019 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + */ > + > +#ifndef IGT_GPU_POWER_H > +#define IGT_GPU_POWER_H > + > +#include <stdbool.h> > +#include <stdint.h> > + > +struct gpu_power { > + int fd; > + double scale; > +}; > + > +struct gpu_power_sample { > + uint64_t energy; > + uint64_t time; > +}; > + > +int gpu_power_open(struct gpu_power *power); > +bool gpu_power_read(struct gpu_power *power, struct gpu_power_sample *s); > +void gpu_power_close(struct gpu_power *power); > + > +static inline double gpu_power_J(const struct gpu_power *p, > + const struct gpu_power_sample *t0, > + const struct gpu_power_sample *t1) > +{ > + return (t1->energy - t0->energy) * p->scale * 1e-9; > +} > + > +static inline double gpu_power_W(const struct gpu_power *p, > + const struct gpu_power_sample *t0, > + const struct gpu_power_sample *t1) > +{ > + return (t1->energy - t0->energy) * p->scale / (t1->time - t0->time); > +} > + > +#endif /* IGT_GPU_POWER_H */ > diff --git a/lib/meson.build b/lib/meson.build > index 0eb5585d7..89de06e69 100644 > --- a/lib/meson.build > +++ b/lib/meson.build > @@ -9,9 +9,11 @@ lib_sources = [ > 'igt_debugfs.c', > 'igt_device.c', > 'igt_aux.c', > + 'igt_gpu_power.c', > 'igt_gt.c', > 'igt_gvt.c', > 'igt_matrix.c', > + 'igt_perf.c', > 'igt_primes.c', > 'igt_rand.c', > 'igt_stats.c', > Regards, Tvrtko
Quoting Tvrtko Ursulin (2019-03-26 08:36:18) > > On 22/03/2019 09:21, Chris Wilson wrote: > > Read the RAPL power metrics courtesy of perf. Or your local HW > > equivalent? > > > > v2: uselocale() > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > lib/Makefile.am | 1 + > > lib/Makefile.sources | 2 + > > lib/igt_gpu_power.c | 114 +++++++++++++++++++++++++++++++++++++++++++ > > lib/igt_gpu_power.h | 59 ++++++++++++++++++++++ > > lib/meson.build | 2 + > > 5 files changed, 178 insertions(+) > > create mode 100644 lib/igt_gpu_power.c > > create mode 100644 lib/igt_gpu_power.h > > > > diff --git a/lib/Makefile.am b/lib/Makefile.am > > index 3e6a7fdba..62e8bda73 100644 > > --- a/lib/Makefile.am > > +++ b/lib/Makefile.am > > @@ -84,4 +84,5 @@ libintel_tools_la_LIBADD = \ > > $(LIBUDEV_LIBS) \ > > $(PIXMAN_LIBS) \ > > $(GLIB_LIBS) \ > > + libigt_perf.la \ > > -lm > > diff --git a/lib/Makefile.sources b/lib/Makefile.sources > > index cf2720981..e00347f94 100644 > > --- a/lib/Makefile.sources > > +++ b/lib/Makefile.sources > > @@ -26,6 +26,8 @@ lib_source_list = \ > > igt_color_encoding.c \ > > igt_color_encoding.h \ > > igt_edid_template.h \ > > + igt_gpu_power.c \ > > + igt_gpu_power.h \ > > igt_gt.c \ > > igt_gt.h \ > > igt_gvt.c \ > > diff --git a/lib/igt_gpu_power.c b/lib/igt_gpu_power.c > > new file mode 100644 > > index 000000000..a4e3c1420 > > --- /dev/null > > +++ b/lib/igt_gpu_power.c > > @@ -0,0 +1,114 @@ > > +#include <ctype.h> > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <locale.h> > > +#include <math.h> > > +#include <unistd.h> > > + > > +#include "igt_gpu_power.h" > > +#include "igt_perf.h" > > + > > +static int filename_to_buf(const char *filename, char *buf, unsigned int sz) > > +{ > > + int fd; > > + ssize_t ret; > > + > > + fd = open(filename, O_RDONLY); > > + if (fd < 0) > > + return -1; > > + > > + ret = read(fd, buf, sz - 1); > > + close(fd); > > + if (ret < 1) > > + return -1; > > + > > + buf[ret] = '\0'; > > + > > + return 0; > > +} > > + > > +static uint64_t filename_to_u64(const char *filename, int base) > > +{ > > + char buf[64], *b; > > + > > + if (filename_to_buf(filename, buf, sizeof(buf))) > > + return 0; > > + > > + /* > > + * Handle both single integer and key=value formats by skipping > > + * leading non-digits. > > + */ > > + b = buf; > > + while (*b && !isdigit(*b)) > > + b++; > > + > > + return strtoull(b, NULL, base); > > +} > > + > > +static double filename_to_double(const char *filename) > > +{ > > + locale_t locale, oldlocale; > > + char buf[80]; > > + double v; > > + > > + if (filename_to_buf(filename, buf, sizeof(buf))) > > + return 0; > > + > > + /* Replace user environment with plain C to match kernel format */ > > + locale = newlocale(LC_ALL, "C", 0); > > + oldlocale = uselocale(locale); > > + > > + v = strtod(buf, NULL); > > + > > + uselocale(oldlocale); > > + freelocale(locale); > > + > > + return v; > > +} > > Hey you know what our guidelines for introducing a 3rd copy of something > are. ;) I'm not even sure this is the third copy... Except for the handling of plain C strtod(). > Maybe it would be fair game to stuff these helpers to igt_perf.la - > since both overlay and intel_gpu_gpu link with it already and it is > about interfacing with perf after all. As far as igt goes, imo we should move the strtod handling into igt_sysfs with the other scanf. > There are some small differences between the functions here and in > intel_gpu_top, someone is right or wrong, or at least more right? Only changed to use threadsafe uselocale() iirc. > > +static uint64_t rapl_type_id(void) > > +{ > > + return filename_to_u64("/sys/devices/power/type", 10); > > +} > > + > > +static uint64_t rapl_gpu_power(void) > > +{ > > + return filename_to_u64("/sys/devices/power/events/energy-gpu", 0); > > +} > > + > > +static double rapl_gpu_power_scale(void) > > +{ > > + return filename_to_double("/sys/devices/power/events/energy-gpu.scale"); > > +} > > + > > +int gpu_power_open(struct gpu_power *power) > > +{ > > + power->fd = igt_perf_open(rapl_type_id(), rapl_gpu_power()); > > + if (power->fd < 0) { > > + power->fd = -errno; > > + goto err; > > + } > > + > > + power->scale = rapl_gpu_power_scale(); > > + if (isnan(power->scale) || !power->scale) { > > + close(power->fd); > > + goto err; > > + } > > + power->scale *= 1e9; > > Scale has no unit so I think this would go better into gpu_power_W. > Would avoid * 1e9 * 1e-9 in gpu_power_J as well. It's 6 of one, half-a-dozen of the other. I was tempted to move it around, but you still end up scaling somewhere. gpu_power_J() gpu_power_s() gpu_power_W() and hope for the best. -Chris
diff --git a/lib/Makefile.am b/lib/Makefile.am index 3e6a7fdba..62e8bda73 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -84,4 +84,5 @@ libintel_tools_la_LIBADD = \ $(LIBUDEV_LIBS) \ $(PIXMAN_LIBS) \ $(GLIB_LIBS) \ + libigt_perf.la \ -lm diff --git a/lib/Makefile.sources b/lib/Makefile.sources index cf2720981..e00347f94 100644 --- a/lib/Makefile.sources +++ b/lib/Makefile.sources @@ -26,6 +26,8 @@ lib_source_list = \ igt_color_encoding.c \ igt_color_encoding.h \ igt_edid_template.h \ + igt_gpu_power.c \ + igt_gpu_power.h \ igt_gt.c \ igt_gt.h \ igt_gvt.c \ diff --git a/lib/igt_gpu_power.c b/lib/igt_gpu_power.c new file mode 100644 index 000000000..a4e3c1420 --- /dev/null +++ b/lib/igt_gpu_power.c @@ -0,0 +1,114 @@ +#include <ctype.h> +#include <errno.h> +#include <fcntl.h> +#include <locale.h> +#include <math.h> +#include <unistd.h> + +#include "igt_gpu_power.h" +#include "igt_perf.h" + +static int filename_to_buf(const char *filename, char *buf, unsigned int sz) +{ + int fd; + ssize_t ret; + + fd = open(filename, O_RDONLY); + if (fd < 0) + return -1; + + ret = read(fd, buf, sz - 1); + close(fd); + if (ret < 1) + return -1; + + buf[ret] = '\0'; + + return 0; +} + +static uint64_t filename_to_u64(const char *filename, int base) +{ + char buf[64], *b; + + if (filename_to_buf(filename, buf, sizeof(buf))) + return 0; + + /* + * Handle both single integer and key=value formats by skipping + * leading non-digits. + */ + b = buf; + while (*b && !isdigit(*b)) + b++; + + return strtoull(b, NULL, base); +} + +static double filename_to_double(const char *filename) +{ + locale_t locale, oldlocale; + char buf[80]; + double v; + + if (filename_to_buf(filename, buf, sizeof(buf))) + return 0; + + /* Replace user environment with plain C to match kernel format */ + locale = newlocale(LC_ALL, "C", 0); + oldlocale = uselocale(locale); + + v = strtod(buf, NULL); + + uselocale(oldlocale); + freelocale(locale); + + return v; +} + +static uint64_t rapl_type_id(void) +{ + return filename_to_u64("/sys/devices/power/type", 10); +} + +static uint64_t rapl_gpu_power(void) +{ + return filename_to_u64("/sys/devices/power/events/energy-gpu", 0); +} + +static double rapl_gpu_power_scale(void) +{ + return filename_to_double("/sys/devices/power/events/energy-gpu.scale"); +} + +int gpu_power_open(struct gpu_power *power) +{ + power->fd = igt_perf_open(rapl_type_id(), rapl_gpu_power()); + if (power->fd < 0) { + power->fd = -errno; + goto err; + } + + power->scale = rapl_gpu_power_scale(); + if (isnan(power->scale) || !power->scale) { + close(power->fd); + goto err; + } + power->scale *= 1e9; + + return 0; + +err: + errno = 0; + return power->fd; +} + +bool gpu_power_read(struct gpu_power *power, struct gpu_power_sample *s) +{ + return read(power->fd, s, sizeof(*s)) == sizeof(*s); +} + +void gpu_power_close(struct gpu_power *power) +{ + close(power->fd); +} diff --git a/lib/igt_gpu_power.h b/lib/igt_gpu_power.h new file mode 100644 index 000000000..4e1b747c0 --- /dev/null +++ b/lib/igt_gpu_power.h @@ -0,0 +1,59 @@ +/* + * Copyright © 2019 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#ifndef IGT_GPU_POWER_H +#define IGT_GPU_POWER_H + +#include <stdbool.h> +#include <stdint.h> + +struct gpu_power { + int fd; + double scale; +}; + +struct gpu_power_sample { + uint64_t energy; + uint64_t time; +}; + +int gpu_power_open(struct gpu_power *power); +bool gpu_power_read(struct gpu_power *power, struct gpu_power_sample *s); +void gpu_power_close(struct gpu_power *power); + +static inline double gpu_power_J(const struct gpu_power *p, + const struct gpu_power_sample *t0, + const struct gpu_power_sample *t1) +{ + return (t1->energy - t0->energy) * p->scale * 1e-9; +} + +static inline double gpu_power_W(const struct gpu_power *p, + const struct gpu_power_sample *t0, + const struct gpu_power_sample *t1) +{ + return (t1->energy - t0->energy) * p->scale / (t1->time - t0->time); +} + +#endif /* IGT_GPU_POWER_H */ diff --git a/lib/meson.build b/lib/meson.build index 0eb5585d7..89de06e69 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -9,9 +9,11 @@ lib_sources = [ 'igt_debugfs.c', 'igt_device.c', 'igt_aux.c', + 'igt_gpu_power.c', 'igt_gt.c', 'igt_gvt.c', 'igt_matrix.c', + 'igt_perf.c', 'igt_primes.c', 'igt_rand.c', 'igt_stats.c',
Read the RAPL power metrics courtesy of perf. Or your local HW equivalent? v2: uselocale() Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- lib/Makefile.am | 1 + lib/Makefile.sources | 2 + lib/igt_gpu_power.c | 114 +++++++++++++++++++++++++++++++++++++++++++ lib/igt_gpu_power.h | 59 ++++++++++++++++++++++ lib/meson.build | 2 + 5 files changed, 178 insertions(+) create mode 100644 lib/igt_gpu_power.c create mode 100644 lib/igt_gpu_power.h