[PATCH i-g-t 1/3] lib/i915: Pretty print HW semaphores

[PATCH i-g-t 1/3] lib/i915: Pretty print HW semaphores

[Chris Wilson]

Include whether the scheduler is using HW semaphore assistance in our
pretty debug strings, and make the caps known for requires.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_scheduler.c | 22 +++++++++++++++++++---
 lib/i915/gem_scheduler.h |  2 ++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/i915/gem_scheduler.c b/lib/i915/gem_scheduler.c
index ad156306f..f9e052278 100644
--- a/lib/i915/gem_scheduler.c
+++ b/lib/i915/gem_scheduler.c
@@ -67,7 +67,7 @@ unsigned gem_scheduler_capability(int fd)
 }
 
 /**
- * gem_has_scheduler:
+ * gem_scheduler_enabled:
  * @fd: open i915 drm file descriptor
  *
  * Feature test macro to query whether the driver has scheduling capability.
@@ -79,7 +79,7 @@ bool gem_scheduler_enabled(int fd)
 }
 
 /**
- * gem_has_ctx_priority:
+ * gem_scheduler_has_ctx_priority:
  * @fd: open i915 drm file descriptor
  *
  * Feature test macro to query whether the driver supports assigning custom
@@ -92,7 +92,7 @@ bool gem_scheduler_has_ctx_priority(int fd)
 }
 
 /**
- * gem_has_preemption:
+ * gem_scheduler_has_preemption:
  * @fd: open i915 drm file descriptor
  *
  * Feature test macro to query whether the driver supports preempting active
@@ -104,6 +104,20 @@ bool gem_scheduler_has_preemption(int fd)
 	       LOCAL_I915_SCHEDULER_CAP_PREEMPTION;
 }
 
+/**
+ * gem_scheduler_has_semaphores:
+ * @fd: open i915 drm file descriptor
+ *
+ * Feature test macro to query whether the driver supports using HW semaphores
+ * to schedule dependencies in parallel (using the HW to delay execution until
+ * ready to reduce latency).
+ */
+bool gem_scheduler_has_semaphores(int fd)
+{
+	return gem_scheduler_capability(fd) &
+	       LOCAL_I915_SCHEDULER_CAP_SEMAPHORES;
+}
+
 /**
  * gem_scheduler_print_capability:
  * @fd: open i915 drm file descriptor
@@ -122,4 +136,6 @@ void gem_scheduler_print_capability(int fd)
 		igt_info(" - With priority sorting\n");
 	if (caps & LOCAL_I915_SCHEDULER_CAP_PREEMPTION)
 		igt_info(" - With preemption enabled\n");
+	if (caps & LOCAL_I915_SCHEDULER_CAP_SEMAPHORES)
+		igt_info(" - With HW semaphores enabled\n");
 }
diff --git a/lib/i915/gem_scheduler.h b/lib/i915/gem_scheduler.h
index 9fcb02665..ead3eacb5 100644
--- a/lib/i915/gem_scheduler.h
+++ b/lib/i915/gem_scheduler.h
@@ -27,11 +27,13 @@
 #define LOCAL_I915_SCHEDULER_CAP_ENABLED	(1 << 0)
 #define LOCAL_I915_SCHEDULER_CAP_PRIORITY	(1 << 1)
 #define LOCAL_I915_SCHEDULER_CAP_PREEMPTION	(1 << 2)
+#define LOCAL_I915_SCHEDULER_CAP_SEMAPHORES	(1 << 3)
 
 unsigned gem_scheduler_capability(int fd);
 bool gem_scheduler_enabled(int fd);
 bool gem_scheduler_has_ctx_priority(int fd);
 bool gem_scheduler_has_preemption(int fd);
+bool gem_scheduler_has_semaphores(int fd);
 void gem_scheduler_print_capability(int fd);
 
 #endif /* GEM_SCHEDULER_H */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/3] lib: Add GPU power measurement

[Chris Wilson]

Read the RAPL power metrics courtesy of perf. Or your local HW
equivalent?

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/Makefile.sources |   2 +
 lib/igt_gpu_power.c  | 106 +++++++++++++++++++++++++++++++++++++++++++
 lib/igt_gpu_power.h  |  51 +++++++++++++++++++++
 lib/meson.build      |   2 +
 4 files changed, 161 insertions(+)
 create mode 100644 lib/igt_gpu_power.c
 create mode 100644 lib/igt_gpu_power.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 808b9617e..54d9e3e7b 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -24,6 +24,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..bf362b2f0
--- /dev/null
+++ b/lib/igt_gpu_power.c
@@ -0,0 +1,106 @@
+#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)
+{
+	char *oldlocale;
+	char buf[80];
+	double v;
+
+	if (filename_to_buf(filename, buf, sizeof(buf)))
+		return 0;
+
+	oldlocale = setlocale(LC_ALL, "C");
+	v = strtod(buf, NULL);
+	setlocale(LC_ALL, oldlocale);
+
+	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)
+		return -errno;
+
+	power->scale = rapl_gpu_power_scale();
+	if (isnan(power->scale) || !power->scale) {
+		close(power->fd);
+		return -ERANGE;
+	}
+	power->scale *= 1e9;
+
+	return 0;
+}
+
+int gpu_power_read(struct gpu_power *power, struct gpu_power_sample *s)
+{
+	if (read(power->fd, s, sizeof(*s)) != sizeof(*s))
+		return -EINVAL;
+
+	return 0;
+}
+
+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..f6e3cbb4d
--- /dev/null
+++ b/lib/igt_gpu_power.h
@@ -0,0 +1,51 @@
+/*
+ * 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 <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);
+int 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_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 dd36f8180..63279ae30 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -8,9 +8,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',
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 3/3] i915/gem_exec_schedule: Measure semaphore power consumption

[Chris Wilson]

How much energy does spinning on a semaphore consume relative to plain
old spinning?

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_schedule.c | 72 +++++++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 0462ce84f..184ceb7d6 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -29,9 +29,10 @@
 #include <signal.h>
 
 #include "igt.h"
-#include "igt_vgem.h"
+#include "igt_gpu_power.h"
 #include "igt_rand.h"
 #include "igt_sysfs.h"
+#include "igt_vgem.h"
 #include "i915/gem_ring.h"
 
 #define LO 0
@@ -1191,6 +1192,65 @@ static void test_pi_ringfull(int fd, unsigned int engine)
 	munmap(result, 4096);
 }
 
+static void measure_semaphore_power(int i915)
+{
+	struct gpu_power power;
+	unsigned int engine, signaler;
+
+	igt_require(gpu_power_open(&power) == 0);
+
+	for_each_physical_engine(i915, signaler) {
+		struct gpu_power_sample s_spin[2];
+		struct gpu_power_sample s_sema[2];
+		double baseline, total;
+		int64_t jiffie = 1;
+		igt_spin_t *spin;
+
+		spin = __igt_spin_batch_new(i915,
+					    .engine = signaler,
+					    .flags = IGT_SPIN_POLL_RUN);
+		gem_wait(i915, spin->handle, &jiffie); /* waitboost */
+		igt_assert(spin->running);
+		igt_spin_busywait_until_running(spin);
+
+		gpu_power_read(&power, &s_spin[0]);
+		usleep(100*1000);
+		gpu_power_read(&power, &s_spin[1]);
+
+		/* Add a waiter to each engine */
+		for_each_physical_engine(i915, engine) {
+			igt_spin_t *sema;
+
+			if (engine == signaler)
+				continue;
+
+			sema = __igt_spin_batch_new(i915,
+						    .engine = engine,
+						    .dependency = spin->handle);
+
+			igt_spin_batch_free(i915, sema);
+		}
+		usleep(10); /* just give the tasklets a chance to run */
+
+		gpu_power_read(&power, &s_sema[0]);
+		usleep(100*1000);
+		gpu_power_read(&power, &s_sema[1]);
+
+		igt_spin_batch_free(i915, spin);
+
+		baseline = gpu_power_W(&power, &s_spin[0], &s_spin[1]);
+		total = gpu_power_W(&power, &s_sema[0], &s_sema[1]);
+
+		igt_info("%s: %.1fmW + %.1fmW (total %1.fmW)\n",
+			 e__->name,
+			 1e3 * baseline,
+			 1e3 * (total - baseline),
+			 1e3 * total);
+	}
+
+	gpu_power_close(&power);
+}
+
 igt_main
 {
 	const struct intel_execution_engine *e;
@@ -1351,6 +1411,16 @@ igt_main
 		}
 	}
 
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(gem_scheduler_enabled(fd));
+			igt_require(gem_scheduler_has_semaphores(fd));
+		}
+
+		igt_subtest("semaphore-power")
+			measure_semaphore_power(fd);
+	}
+
 	igt_fixture {
 		igt_stop_hang_detector();
 		close(fd);
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/3] i915/gem_exec_schedule: Measure semaphore power consumption

[Ashutosh Dixit]

On Thu, 31 Jan 2019 08:17:17 -0800, Chris Wilson wrote:
> 
> How much energy does spinning on a semaphore consume relative to plain
> old spinning?
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_exec_schedule.c | 72 +++++++++++++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index 0462ce84f..184ceb7d6 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -29,9 +29,10 @@
>  #include <signal.h>
>  
>  #include "igt.h"
> -#include "igt_vgem.h"
> +#include "igt_gpu_power.h"
>  #include "igt_rand.h"
>  #include "igt_sysfs.h"
> +#include "igt_vgem.h"
>  #include "i915/gem_ring.h"
>  
>  #define LO 0
> @@ -1191,6 +1192,65 @@ static void test_pi_ringfull(int fd, unsigned int engine)
>  	munmap(result, 4096);
>  }
>  
> +static void measure_semaphore_power(int i915)
> +{
> +	struct gpu_power power;
> +	unsigned int engine, signaler;
> +
> +	igt_require(gpu_power_open(&power) == 0);
> +
> +	for_each_physical_engine(i915, signaler) {
> +		struct gpu_power_sample s_spin[2];
> +		struct gpu_power_sample s_sema[2];
> +		double baseline, total;
> +		int64_t jiffie = 1;
> +		igt_spin_t *spin;
> +
> +		spin = __igt_spin_batch_new(i915,
> +					    .engine = signaler,
> +					    .flags = IGT_SPIN_POLL_RUN);
> +		gem_wait(i915, spin->handle, &jiffie); /* waitboost */
> +		igt_assert(spin->running);
> +		igt_spin_busywait_until_running(spin);
> +
> +		gpu_power_read(&power, &s_spin[0]);
> +		usleep(100*1000);
> +		gpu_power_read(&power, &s_spin[1]);
> +
> +		/* Add a waiter to each engine */
> +		for_each_physical_engine(i915, engine) {
>
See the following warning now as a result of this patch, most likely the
code is incorrect too due to the shadowed variable:

ninja: Entering directory `build'
[182/621] Compiling C object 'tests/59830eb@@gem_exec_schedule@exe/i915_gem_exec_schedule.c.o'.
In file included from ../lib/igt.h:40,
                 from ../tests/i915/gem_exec_schedule.c:31:
../tests/i915/gem_exec_schedule.c: In function ‘measure_semaphore_power’:
../lib/igt_gt.h:84:44: warning: declaration of ‘e__’ shadows a previous local [-Wshadow]
  for (const struct intel_execution_engine *e__ = intel_execution_engines;\
                                            ^~~
../tests/i915/gem_exec_schedule.c:1316:3: note: in expansion of macro ‘for_each_physical_engine’
   for_each_physical_engine(i915, engine) {
   ^~~~~~~~~~~~~~~~~~~~~~~~
../lib/igt_gt.h:84:44: note: shadowed declaration is here
  for (const struct intel_execution_engine *e__ = intel_execution_engines;\
                                            ^~~
../tests/i915/gem_exec_schedule.c:1297:2: note: in expansion of macro ‘for_each_physical_engine’
  for_each_physical_engine(i915, signaler) {
  ^~~~~~~~~~~~~~~~~~~~~~~~
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/3] i915/gem_exec_schedule: Measure semaphore power consumption

[Chris Wilson]

Quoting Ashutosh Dixit (2019-04-03 02:28:35)
> On Thu, 31 Jan 2019 08:17:17 -0800, Chris Wilson wrote:
> > 
> > How much energy does spinning on a semaphore consume relative to plain
> > old spinning?
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  tests/i915/gem_exec_schedule.c | 72 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 71 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> > index 0462ce84f..184ceb7d6 100644
> > --- a/tests/i915/gem_exec_schedule.c
> > +++ b/tests/i915/gem_exec_schedule.c
> > @@ -29,9 +29,10 @@
> >  #include <signal.h>
> >  
> >  #include "igt.h"
> > -#include "igt_vgem.h"
> > +#include "igt_gpu_power.h"
> >  #include "igt_rand.h"
> >  #include "igt_sysfs.h"
> > +#include "igt_vgem.h"
> >  #include "i915/gem_ring.h"
> >  
> >  #define LO 0
> > @@ -1191,6 +1192,65 @@ static void test_pi_ringfull(int fd, unsigned int engine)
> >       munmap(result, 4096);
> >  }
> >  
> > +static void measure_semaphore_power(int i915)
> > +{
> > +     struct gpu_power power;
> > +     unsigned int engine, signaler;
> > +
> > +     igt_require(gpu_power_open(&power) == 0);
> > +
> > +     for_each_physical_engine(i915, signaler) {
> > +             struct gpu_power_sample s_spin[2];
> > +             struct gpu_power_sample s_sema[2];
> > +             double baseline, total;
> > +             int64_t jiffie = 1;
> > +             igt_spin_t *spin;
> > +
> > +             spin = __igt_spin_batch_new(i915,
> > +                                         .engine = signaler,
> > +                                         .flags = IGT_SPIN_POLL_RUN);
> > +             gem_wait(i915, spin->handle, &jiffie); /* waitboost */
> > +             igt_assert(spin->running);
> > +             igt_spin_busywait_until_running(spin);
> > +
> > +             gpu_power_read(&power, &s_spin[0]);
> > +             usleep(100*1000);
> > +             gpu_power_read(&power, &s_spin[1]);
> > +
> > +             /* Add a waiter to each engine */
> > +             for_each_physical_engine(i915, engine) {
> >
> See the following warning now as a result of this patch, most likely the
> code is incorrect too due to the shadowed variable:

Code is correct. Warning is just a warning that we need new iterators.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx