| Message ID | 20190131161717.1359-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,1/3] lib/i915: Pretty print HW semaphores | expand |
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) { ^~~~~~~~~~~~~~~~~~~~~~~~
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
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);
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(-)