Re: [Intel-gfx] [PATCH i-g-t 2/2] i915/query: Directly check query results against GETPARAM
Quoting Tvrtko Ursulin (2020-12-08 11:18:56) > > On 08/12/2020 11:04, Petri Latvala wrote: > > On Mon, Dec 07, 2020 at 04:11:50PM +, Chris Wilson wrote: > >> Simplify the cross-check by asserting that the existence of an engine in > >> the list matches the existence of the engine as reported by GETPARAM. > >> By using the comparison, we check both directions at once. > >> > >> Signed-off-by: Chris Wilson > >> Cc: Petri Latvala > > > > > > For the series, > > Reviewed-by: Petri Latvala > > Yeah it's a yes from me as well. Either test was merged with or before > the engine map feature so it had to be a bit more backward compatible. As a sanity check, drm/i915: Allow a context to define its set of engines CommitDate: Wed May 22 08:40:31 2019 +0100 drm/i915: Engine discovery query CommitDate: Wed May 22 14:17:55 2019 +0100 So they are paired. If the kernel supports the engine query, it will support the engine map. > I wonder at which point we re-implement gem_has_xcs family to use the > query and move the get_param based tests to a single legacy test. gem_has_xcs() is a quirk of igt, and we are very very close to completely removing it. The only place where it remains relevant is verifying that we do not break the existing GETPARAM (so this test and gem_exec_param). That seems like an afternoon task to move the GETPARAM into a dungeon and throw away the key. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/2] i915/query: Directly check query results against GETPARAM
On 08/12/2020 11:04, Petri Latvala wrote: On Mon, Dec 07, 2020 at 04:11:50PM +, Chris Wilson wrote: Simplify the cross-check by asserting that the existence of an engine in the list matches the existence of the engine as reported by GETPARAM. By using the comparison, we check both directions at once. Signed-off-by: Chris Wilson Cc: Petri Latvala For the series, Reviewed-by: Petri Latvala Yeah it's a yes from me as well. Either test was merged with or before the engine map feature so it had to be a bit more backward compatible. I wonder at which point we re-implement gem_has_xcs family to use the query and move the get_param based tests to a single legacy test. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/2] i915/query: Directly check query results against GETPARAM
On Mon, Dec 07, 2020 at 04:11:50PM +, Chris Wilson wrote: > Simplify the cross-check by asserting that the existence of an engine in > the list matches the existence of the engine as reported by GETPARAM. > By using the comparison, we check both directions at once. > > Signed-off-by: Chris Wilson > Cc: Petri Latvala For the series, Reviewed-by: Petri Latvala Thanks! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] i915/query: Directly check query results against GETPARAM
Simplify the cross-check by asserting that the existence of an engine in the list matches the existence of the engine as reported by GETPARAM. By using the comparison, we check both directions at once. Signed-off-by: Chris Wilson Cc: Petri Latvala --- tests/i915/i915_query.c | 49 - 1 file changed, 9 insertions(+), 40 deletions(-) diff --git a/tests/i915/i915_query.c b/tests/i915/i915_query.c index cdf2d3403..b415650ae 100644 --- a/tests/i915/i915_query.c +++ b/tests/i915/i915_query.c @@ -719,46 +719,15 @@ static void engines(int fd) gem_context_reset_engines(fd, 0); /* Check results match the legacy GET_PARAM (where we can). */ - for (i = 0; i < engines->num_engines; i++) { - struct drm_i915_engine_info *engine = - (struct drm_i915_engine_info *)>engines[i]; - - switch (engine->engine.engine_class) { - case I915_ENGINE_CLASS_RENDER: - /* Will be tested later. */ - break; - case I915_ENGINE_CLASS_COPY: - igt_assert(gem_has_blt(fd)); - break; - case I915_ENGINE_CLASS_VIDEO: - switch (engine->engine.engine_instance) { - case 0: - igt_assert(gem_has_bsd(fd)); - break; - case 1: - igt_assert(gem_has_bsd2(fd)); - break; - } - break; - case I915_ENGINE_CLASS_VIDEO_ENHANCE: - igt_assert(gem_has_vebox(fd)); - break; - default: - igt_assert(0); - } - } - - /* Reverse check to the above - all GET_PARAM engines are present. */ - igt_assert(has_engine(engines, I915_ENGINE_CLASS_RENDER, 0)); - if (gem_has_blt(fd)) - igt_assert(has_engine(engines, I915_ENGINE_CLASS_COPY, 0)); - if (gem_has_bsd(fd)) - igt_assert(has_engine(engines, I915_ENGINE_CLASS_VIDEO, 0)); - if (gem_has_bsd2(fd)) - igt_assert(has_engine(engines, I915_ENGINE_CLASS_VIDEO, 1)); - if (gem_has_vebox(fd)) - igt_assert(has_engine(engines, I915_ENGINE_CLASS_VIDEO_ENHANCE, - 0)); + igt_assert_eq(has_engine(engines, I915_ENGINE_CLASS_RENDER, 0), 1); + igt_assert_eq(has_engine(engines, I915_ENGINE_CLASS_COPY, 0), + gem_has_blt(fd)); + igt_assert_eq(has_engine(engines, I915_ENGINE_CLASS_VIDEO, 0), + gem_has_bsd(fd)); + igt_assert_eq(has_engine(engines, I915_ENGINE_CLASS_VIDEO, 1), + gem_has_bsd2(fd)); + igt_assert_eq(has_engine(engines, I915_ENGINE_CLASS_VIDEO_ENHANCE, 0), + gem_has_vebox(fd)); free(engines); } -- 2.29.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx