Re: [Intel-gfx] [PATCH i-g-t 2/2] i915/query: Directly check query results against GETPARAM

2020-12-08 Thread Chris Wilson
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

2020-12-08 Thread Tvrtko Ursulin



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

2020-12-08 Thread Petri Latvala
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

2020-12-07 Thread Chris Wilson
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