| Message ID | 20190701075205.26410-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] tests/i915/gem_ctx_switch: Fix I915_EXEC_DEFAULT testing | expand |
On 2019-07-01 at 08:52:05 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Commit ad129d2a583689765eaef31ff57e8cdd219f1d05 > ("tests/i915/gem_ctx_switch: Update with engine discovery") broke testing > of I915_EXEC_DEFAULT. Bring it back. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Reported-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Andi Shyti <andi.shyti@intel.com> > Cc: Ramalingam C <ramalingam.c@intel.com> > --- > lib/i915/gem_engine_topology.c | 26 +++++++++++++++++++------- > lib/i915/gem_engine_topology.h | 3 +-- > tests/i915/gem_ctx_switch.c | 8 ++++++-- > 3 files changed, 26 insertions(+), 11 deletions(-) > > diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c > index 6cfe3468e3d8..cae5a0292b02 100644 > --- a/lib/i915/gem_engine_topology.c > +++ b/lib/i915/gem_engine_topology.c > @@ -290,17 +290,29 @@ bool gem_has_engine_topology(int fd) > return !__gem_context_get_param(fd, ¶m); > } > > -const struct intel_execution_engine2 * > -gem_eb_flags_to_engine(unsigned int flags) > +struct intel_execution_engine2 gem_eb_flags_to_engine(unsigned int flags) > { > - const struct intel_execution_engine2 *e2; > + const unsigned int ring = flags & (I915_EXEC_RING_MASK | 3 << 13); > + struct intel_execution_engine2 e2__ = { > + .class = -1, > + .instance = -1, > + .flags = -1, > + .name = "invalid" > + }; > + > + if (ring == I915_EXEC_DEFAULT) { > + e2__.flags = I915_EXEC_DEFAULT; > + e2__.name = "default"; > + } else { > + const struct intel_execution_engine2 *e2; > > - __for_each_static_engine(e2) { > - if (e2->flags == flags) > - return e2; > + __for_each_static_engine(e2) { > + if (e2->flags == ring) > + return *e2; > + } > } > > - return NULL; > + return e2__; > } > > bool gem_context_has_engine_map(int fd, uint32_t ctx) > diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h > index b175483fac1c..9b6e2d4f9cd8 100644 > --- a/lib/i915/gem_engine_topology.h > +++ b/lib/i915/gem_engine_topology.h > @@ -55,8 +55,7 @@ void gem_context_set_all_engines(int fd, uint32_t ctx); > > bool gem_context_has_engine_map(int fd, uint32_t ctx); > > -const struct intel_execution_engine2 * > -gem_eb_flags_to_engine(unsigned int flags); > +struct intel_execution_engine2 gem_eb_flags_to_engine(unsigned int flags); > > #define __for_each_static_engine(e__) \ > for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) > diff --git a/tests/i915/gem_ctx_switch.c b/tests/i915/gem_ctx_switch.c > index 407905de2d34..c071def7825e 100644 > --- a/tests/i915/gem_ctx_switch.c > +++ b/tests/i915/gem_ctx_switch.c > @@ -347,10 +347,14 @@ igt_main > > /* Legacy testing must be first. */ > for (e = intel_execution_engines; e->name; e++) { > - e2 = gem_eb_flags_to_engine(e->exec_id | e->flags); Could we have something like gem_eb_flags_to_engine(e->exec_id | e->flags, e2); if (e2->flags == -1) continue; this way extra e2__ declaration at caller can be avoided. Apart from that looks good to me Reviewed-by: Ramalingam C <ramalingam.c@intel.com> -Ram > - if (!e2) > + struct intel_execution_engine2 e2__; > + > + e2__ = gem_eb_flags_to_engine(e->exec_id | e->flags); > + if (e2__.flags == -1) > continue; /* I915_EXEC_BSD with no ring selectors */ > > + e2 = &e2__; > + > for (typeof(*phases) *p = phases; p->name; p++) { > igt_subtest_group { > igt_fixture { > -- > 2.20.1 >
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c index 6cfe3468e3d8..cae5a0292b02 100644 --- a/lib/i915/gem_engine_topology.c +++ b/lib/i915/gem_engine_topology.c @@ -290,17 +290,29 @@ bool gem_has_engine_topology(int fd) return !__gem_context_get_param(fd, ¶m); } -const struct intel_execution_engine2 * -gem_eb_flags_to_engine(unsigned int flags) +struct intel_execution_engine2 gem_eb_flags_to_engine(unsigned int flags) { - const struct intel_execution_engine2 *e2; + const unsigned int ring = flags & (I915_EXEC_RING_MASK | 3 << 13); + struct intel_execution_engine2 e2__ = { + .class = -1, + .instance = -1, + .flags = -1, + .name = "invalid" + }; + + if (ring == I915_EXEC_DEFAULT) { + e2__.flags = I915_EXEC_DEFAULT; + e2__.name = "default"; + } else { + const struct intel_execution_engine2 *e2; - __for_each_static_engine(e2) { - if (e2->flags == flags) - return e2; + __for_each_static_engine(e2) { + if (e2->flags == ring) + return *e2; + } } - return NULL; + return e2__; } bool gem_context_has_engine_map(int fd, uint32_t ctx) diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h index b175483fac1c..9b6e2d4f9cd8 100644 --- a/lib/i915/gem_engine_topology.h +++ b/lib/i915/gem_engine_topology.h @@ -55,8 +55,7 @@ void gem_context_set_all_engines(int fd, uint32_t ctx); bool gem_context_has_engine_map(int fd, uint32_t ctx); -const struct intel_execution_engine2 * -gem_eb_flags_to_engine(unsigned int flags); +struct intel_execution_engine2 gem_eb_flags_to_engine(unsigned int flags); #define __for_each_static_engine(e__) \ for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) diff --git a/tests/i915/gem_ctx_switch.c b/tests/i915/gem_ctx_switch.c index 407905de2d34..c071def7825e 100644 --- a/tests/i915/gem_ctx_switch.c +++ b/tests/i915/gem_ctx_switch.c @@ -347,10 +347,14 @@ igt_main /* Legacy testing must be first. */ for (e = intel_execution_engines; e->name; e++) { - e2 = gem_eb_flags_to_engine(e->exec_id | e->flags); - if (!e2) + struct intel_execution_engine2 e2__; + + e2__ = gem_eb_flags_to_engine(e->exec_id | e->flags); + if (e2__.flags == -1) continue; /* I915_EXEC_BSD with no ring selectors */ + e2 = &e2__; + for (typeof(*phases) *p = phases; p->name; p++) { igt_subtest_group { igt_fixture {