| Message ID | 20180329091128.32341-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Tvrtko Ursulin (2018-03-29 10:11:28) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Wrong file descriptor was passed to the iterator. This had currently no > effect, since it wasn't used in the macro, but needs to be fixed. > > At the same time make the macro consistent by checking for engine presence > like the other iterators do. > > Added __for_each_engine_class_instance which does not check for engine > presence and so is useful for enumerating all possible engines - like for > instance for subtest enumeration. > > And another 'wrong fd used' fixlet in the render node subtests. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Reported-by: Michel Thierry <michel.thierry@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > --- > lib/igt_gt.h | 12 +++++++----- > tests/perf_pmu.c | 30 ++++++++++-------------------- > 2 files changed, 17 insertions(+), 25 deletions(-) > > diff --git a/lib/igt_gt.h b/lib/igt_gt.h > index a517ed7b29a0..d44b7552f3c4 100644 > --- a/lib/igt_gt.h > +++ b/lib/igt_gt.h > @@ -100,11 +100,6 @@ extern const struct intel_execution_engine2 { > int instance; > } intel_execution_engines2[]; > > -#define for_each_engine_class_instance(fd__, e__) \ > - for ((e__) = intel_execution_engines2;\ > - (e__)->name; \ > - (e__)++) > - > unsigned int > gem_class_instance_to_eb_flags(int gem_fd, > enum drm_i915_gem_engine_class class, > @@ -122,4 +117,11 @@ void gem_require_engine(int gem_fd, > igt_require(gem_has_engine(gem_fd, class, instance)); > } > > +#define __for_each_engine_class_instance(fd__, e__) \ > + for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) > + > +#define for_each_engine_class_instance(fd__, e__) \ > + for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \ > + for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance)) Ok. Conversions made sense. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On 29/03/18 02:11, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Wrong file descriptor was passed to the iterator. This had currently no > effect, since it wasn't used in the macro, but needs to be fixed. > > At the same time make the macro consistent by checking for engine presence > like the other iterators do. > > Added __for_each_engine_class_instance which does not check for engine > presence and so is useful for enumerating all possible engines - like for > instance for subtest enumeration. > > And another 'wrong fd used' fixlet in the render node subtests. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Reported-by: Michel Thierry <michel.thierry@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> Thanks, Reviewed-by: Michel Thierry <michel.thierry@intel.com> > --- > lib/igt_gt.h | 12 +++++++----- > tests/perf_pmu.c | 30 ++++++++++-------------------- > 2 files changed, 17 insertions(+), 25 deletions(-) > > diff --git a/lib/igt_gt.h b/lib/igt_gt.h > index a517ed7b29a0..d44b7552f3c4 100644 > --- a/lib/igt_gt.h > +++ b/lib/igt_gt.h > @@ -100,11 +100,6 @@ extern const struct intel_execution_engine2 { > int instance; > } intel_execution_engines2[]; > > -#define for_each_engine_class_instance(fd__, e__) \ > - for ((e__) = intel_execution_engines2;\ > - (e__)->name; \ > - (e__)++) > - > unsigned int > gem_class_instance_to_eb_flags(int gem_fd, > enum drm_i915_gem_engine_class class, > @@ -122,4 +117,11 @@ void gem_require_engine(int gem_fd, > igt_require(gem_has_engine(gem_fd, class, instance)); > } > > +#define __for_each_engine_class_instance(fd__, e__) \ > + for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) > + > +#define for_each_engine_class_instance(fd__, e__) \ > + for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \ > + for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance)) > + > #endif /* IGT_GT_H */ > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c > index b59af81819c7..2273ddb9e684 100644 > --- a/tests/perf_pmu.c > +++ b/tests/perf_pmu.c > @@ -434,10 +434,8 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e, > > i = 0; > fd[0] = -1; > - for_each_engine_class_instance(fd, e_) { > - if (!gem_has_engine(gem_fd, e_->class, e_->instance)) > - continue; > - else if (e == e_) > + for_each_engine_class_instance(gem_fd, e_) { > + if (e == e_) > busy_idx = i; > > fd[i++] = open_group(I915_PMU_ENGINE_BUSY(e_->class, > @@ -499,10 +497,7 @@ most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e, > unsigned int idle_idx, i; > > i = 0; > - for_each_engine_class_instance(fd, e_) { > - if (!gem_has_engine(gem_fd, e_->class, e_->instance)) > - continue; > - > + for_each_engine_class_instance(gem_fd, e_) { > if (e == e_) > idle_idx = i; > else if (spin) > @@ -559,10 +554,7 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines, > unsigned int i; > > i = 0; > - for_each_engine_class_instance(fd, e) { > - if (!gem_has_engine(gem_fd, e->class, e->instance)) > - continue; > - > + for_each_engine_class_instance(gem_fd, e) { > if (spin) > __submit_spin_batch(gem_fd, spin, e, 64); > else > @@ -1677,10 +1669,8 @@ igt_main > igt_require_gem(fd); > igt_require(i915_type_id() > 0); > > - for_each_engine_class_instance(fd, e) { > - if (gem_has_engine(fd, e->class, e->instance)) > - num_engines++; > - } > + for_each_engine_class_instance(fd, e) > + num_engines++; > } > > /** > @@ -1689,7 +1679,7 @@ igt_main > igt_subtest("invalid-init") > invalid_init(); > > - for_each_engine_class_instance(fd, e) { > + __for_each_engine_class_instance(fd, e) { > const unsigned int pct[] = { 2, 50, 98 }; > > /** > @@ -1888,7 +1878,7 @@ igt_main > gem_quiescent_gpu(fd); > } > > - for_each_engine_class_instance(fd, e) { > + __for_each_engine_class_instance(render_fd, e) { > igt_subtest_group { > igt_fixture { > gem_require_engine(render_fd, > @@ -1897,10 +1887,10 @@ igt_main > } > > igt_subtest_f("render-node-busy-%s", e->name) > - single(fd, e, TEST_BUSY); > + single(render_fd, e, TEST_BUSY); > igt_subtest_f("render-node-busy-idle-%s", > e->name) > - single(fd, e, > + single(render_fd, e, > TEST_BUSY | TEST_TRAILING_IDLE); > } > } >
diff --git a/lib/igt_gt.h b/lib/igt_gt.h index a517ed7b29a0..d44b7552f3c4 100644 --- a/lib/igt_gt.h +++ b/lib/igt_gt.h @@ -100,11 +100,6 @@ extern const struct intel_execution_engine2 { int instance; } intel_execution_engines2[]; -#define for_each_engine_class_instance(fd__, e__) \ - for ((e__) = intel_execution_engines2;\ - (e__)->name; \ - (e__)++) - unsigned int gem_class_instance_to_eb_flags(int gem_fd, enum drm_i915_gem_engine_class class, @@ -122,4 +117,11 @@ void gem_require_engine(int gem_fd, igt_require(gem_has_engine(gem_fd, class, instance)); } +#define __for_each_engine_class_instance(fd__, e__) \ + for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) + +#define for_each_engine_class_instance(fd__, e__) \ + for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \ + for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance)) + #endif /* IGT_GT_H */ diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c index b59af81819c7..2273ddb9e684 100644 --- a/tests/perf_pmu.c +++ b/tests/perf_pmu.c @@ -434,10 +434,8 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e, i = 0; fd[0] = -1; - for_each_engine_class_instance(fd, e_) { - if (!gem_has_engine(gem_fd, e_->class, e_->instance)) - continue; - else if (e == e_) + for_each_engine_class_instance(gem_fd, e_) { + if (e == e_) busy_idx = i; fd[i++] = open_group(I915_PMU_ENGINE_BUSY(e_->class, @@ -499,10 +497,7 @@ most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e, unsigned int idle_idx, i; i = 0; - for_each_engine_class_instance(fd, e_) { - if (!gem_has_engine(gem_fd, e_->class, e_->instance)) - continue; - + for_each_engine_class_instance(gem_fd, e_) { if (e == e_) idle_idx = i; else if (spin) @@ -559,10 +554,7 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines, unsigned int i; i = 0; - for_each_engine_class_instance(fd, e) { - if (!gem_has_engine(gem_fd, e->class, e->instance)) - continue; - + for_each_engine_class_instance(gem_fd, e) { if (spin) __submit_spin_batch(gem_fd, spin, e, 64); else @@ -1677,10 +1669,8 @@ igt_main igt_require_gem(fd); igt_require(i915_type_id() > 0); - for_each_engine_class_instance(fd, e) { - if (gem_has_engine(fd, e->class, e->instance)) - num_engines++; - } + for_each_engine_class_instance(fd, e) + num_engines++; } /** @@ -1689,7 +1679,7 @@ igt_main igt_subtest("invalid-init") invalid_init(); - for_each_engine_class_instance(fd, e) { + __for_each_engine_class_instance(fd, e) { const unsigned int pct[] = { 2, 50, 98 }; /** @@ -1888,7 +1878,7 @@ igt_main gem_quiescent_gpu(fd); } - for_each_engine_class_instance(fd, e) { + __for_each_engine_class_instance(render_fd, e) { igt_subtest_group { igt_fixture { gem_require_engine(render_fd, @@ -1897,10 +1887,10 @@ igt_main } igt_subtest_f("render-node-busy-%s", e->name) - single(fd, e, TEST_BUSY); + single(render_fd, e, TEST_BUSY); igt_subtest_f("render-node-busy-idle-%s", e->name) - single(fd, e, + single(render_fd, e, TEST_BUSY | TEST_TRAILING_IDLE); } }