| Message ID | 20191204132051.3740419-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] i915/perf_pmu: Measure how many batches can fit into the ring | expand |
On Wed, 2019-12-04 at 13:20 +0000, Chris Wilson wrote: > Do not blindly assume 30 spin batches will always fit into the ring, > but > use our measurement tool instead. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/perf_pmu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c > index de4c231dd..8e50ac9a0 100644 > --- a/tests/perf_pmu.c > +++ b/tests/perf_pmu.c > @@ -42,6 +42,7 @@ > #include "igt_perf.h" > #include "igt_sysfs.h" > #include "igt_pm.h" > +#include "i915/gem_ring.h" > #include "sw_sync.h" > > IGT_TEST_DESCRIPTION("Test the i915 pmu perf interface"); > @@ -1276,8 +1277,9 @@ static void cpu_hotplug(int gem_fd) > static void > test_interrupts(int gem_fd) > { > + const int target = > + gem_measure_ring_inflight(gem_fd, I915_EXEC_DEFAULT, > 0); In case we ever want to change this engine, should we make I915_EXEC_DEFAULT a macro within this test? Looks a lot better. My only question here is can we make gem_measure_ring_inflight a generic routine instead of something i915- specific, since we're using this in one of the cross-arch tests? Thanks, Stuart > const unsigned int test_duration_ms = 1000; > - const int target = 30; > igt_spin_t *spin[target]; > struct pollfd pfd; > uint64_t idle, busy;
Quoting Summers, Stuart (2019-12-04 19:13:16) > On Wed, 2019-12-04 at 13:20 +0000, Chris Wilson wrote: > > Do not blindly assume 30 spin batches will always fit into the ring, > > but > > use our measurement tool instead. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > tests/perf_pmu.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c > > index de4c231dd..8e50ac9a0 100644 > > --- a/tests/perf_pmu.c > > +++ b/tests/perf_pmu.c > > @@ -42,6 +42,7 @@ > > #include "igt_perf.h" > > #include "igt_sysfs.h" > > #include "igt_pm.h" > > +#include "i915/gem_ring.h" > > #include "sw_sync.h" > > > > IGT_TEST_DESCRIPTION("Test the i915 pmu perf interface"); > > @@ -1276,8 +1277,9 @@ static void cpu_hotplug(int gem_fd) > > static void > > test_interrupts(int gem_fd) > > { > > + const int target = > > + gem_measure_ring_inflight(gem_fd, I915_EXEC_DEFAULT, > > 0); > > In case we ever want to change this engine, should we make > I915_EXEC_DEFAULT a macro within this test? Not really, EXEC_DEFAULT itself is the placeholder for first engine on the system... I really should land my ffs() before it makes a difference. > > Looks a lot better. My only question here is can we make > gem_measure_ring_inflight a generic routine instead of something i915- > specific, since we're using this in one of the cross-arch tests? This is not a generic test. Simply has not been moved yet. -Chris
On Wed, 2019-12-04 at 19:21 +0000, Chris Wilson wrote: > Quoting Summers, Stuart (2019-12-04 19:13:16) > > On Wed, 2019-12-04 at 13:20 +0000, Chris Wilson wrote: > > > Do not blindly assume 30 spin batches will always fit into the > > > ring, > > > but > > > use our measurement tool instead. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > --- > > > tests/perf_pmu.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c > > > index de4c231dd..8e50ac9a0 100644 > > > --- a/tests/perf_pmu.c > > > +++ b/tests/perf_pmu.c > > > @@ -42,6 +42,7 @@ > > > #include "igt_perf.h" > > > #include "igt_sysfs.h" > > > #include "igt_pm.h" > > > +#include "i915/gem_ring.h" > > > #include "sw_sync.h" > > > > > > IGT_TEST_DESCRIPTION("Test the i915 pmu perf interface"); > > > @@ -1276,8 +1277,9 @@ static void cpu_hotplug(int gem_fd) > > > static void > > > test_interrupts(int gem_fd) > > > { > > > + const int target = > > > + gem_measure_ring_inflight(gem_fd, > > > I915_EXEC_DEFAULT, > > > 0); > > > > In case we ever want to change this engine, should we make > > I915_EXEC_DEFAULT a macro within this test? > > Not really, EXEC_DEFAULT itself is the placeholder for first engine > on > the system... I really should land my ffs() before it makes a > difference. > > > > Looks a lot better. My only question here is can we make > > gem_measure_ring_inflight a generic routine instead of something > > i915- > > specific, since we're using this in one of the cross-arch tests? > > This is not a generic test. Simply has not been moved yet. Makes sense then. Reviewed-by: Stuart Summers <stuart.summers@intel.com> > -Chris
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c index de4c231dd..8e50ac9a0 100644 --- a/tests/perf_pmu.c +++ b/tests/perf_pmu.c @@ -42,6 +42,7 @@ #include "igt_perf.h" #include "igt_sysfs.h" #include "igt_pm.h" +#include "i915/gem_ring.h" #include "sw_sync.h" IGT_TEST_DESCRIPTION("Test the i915 pmu perf interface"); @@ -1276,8 +1277,9 @@ static void cpu_hotplug(int gem_fd) static void test_interrupts(int gem_fd) { + const int target = + gem_measure_ring_inflight(gem_fd, I915_EXEC_DEFAULT, 0); const unsigned int test_duration_ms = 1000; - const int target = 30; igt_spin_t *spin[target]; struct pollfd pfd; uint64_t idle, busy;
Do not blindly assume 30 spin batches will always fit into the ring, but use our measurement tool instead. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- tests/perf_pmu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)