diff mbox series

[i-g-t,2/2] tests/i915_query: Engine discovery tests

Message ID 20190501114259.16158-3-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Engine discovery tests | expand

Commit Message

Tvrtko Ursulin May 1, 2019, 11:42 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Test the new engine discovery query.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/i915_query.c | 247 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 247 insertions(+)

Comments

Chris Wilson May 1, 2019, 11:52 a.m. UTC | #1
Quoting Tvrtko Ursulin (2019-05-01 12:42:59)
> +               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;


Is that relationship a given?

One could argue that gem_has_blt() means that I915_EXEC_BLT works, but
without !gem_has_blt() we could still access CLASS_COPY:0 via
ctx->engines[].

> +                       }
> +                       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));

Whereas this should always make sense, given the legacy interface and
the modern interface, the modern interface should be a superset of the
legacy.

Just thinking aloud.
-Chris
Tvrtko Ursulin May 1, 2019, 3:56 p.m. UTC | #2
On 01/05/2019 12:52, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-01 12:42:59)
>> +               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;
> 
> 
> Is that relationship a given?
> 
> One could argue that gem_has_blt() means that I915_EXEC_BLT works, but
> without !gem_has_blt() we could still access CLASS_COPY:0 via
> ctx->engines[].

"without !gem_has_blt()".. hmmm what do you mean? If you mean 
gem_has_blt() is false but bcs:0 is still accessible via ctx->engines[] 
then how?

>> +                       }
>> +                       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));
> 
> Whereas this should always make sense, given the legacy interface and
> the modern interface, the modern interface should be a superset of the
> legacy.
> 
> Just thinking aloud.

Sure, no harm in looking at it again. The test was written long time ago.

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/tests/i915/i915_query.c b/tests/i915/i915_query.c
index 7d0c0e3a061c..ecbec3ae141d 100644
--- a/tests/i915/i915_query.c
+++ b/tests/i915/i915_query.c
@@ -483,6 +483,241 @@  test_query_topology_known_pci_ids(int fd, int devid)
 	free(topo_info);
 }
 
+static bool query_engine_info_supported(int fd)
+{
+	struct drm_i915_query_item item = {
+		.query_id = DRM_I915_QUERY_ENGINE_INFO,
+	};
+
+	return __i915_query_items(fd, &item, 1) == 0 && item.length > 0;
+}
+
+static void engines_invalid(int fd)
+{
+	struct drm_i915_query_engine_info *engines;
+	struct drm_i915_query_item item;
+	unsigned int len;
+
+	/* Flags is MBZ. */
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.flags = 1;
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EINVAL);
+
+	/* Length not zero and not greater or equal required size. */
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = 1;
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EINVAL);
+
+	/* Query correct length. */
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	i915_query_items(fd, &item, 1);
+	igt_assert(item.length >= 0);
+	len = item.length;
+
+	engines = malloc(len);
+	igt_assert(engines);
+
+	/* Ivalid pointer. */
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EFAULT);
+
+	/* All fields in engines query are MBZ and only filled by the kernel. */
+
+	memset(engines, 0, len);
+	engines->num_engines = 1;
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EINVAL);
+
+	memset(engines, 0, len);
+	engines->rsvd[0] = 1;
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EINVAL);
+
+	memset(engines, 0, len);
+	engines->rsvd[1] = 1;
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EINVAL);
+
+	memset(engines, 0, len);
+	engines->rsvd[2] = 1;
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EINVAL);
+
+	free(engines);
+
+	igt_assert(len <= 4096);
+	engines = mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON,
+		       -1, 0);
+	igt_assert(engines != MAP_FAILED);
+
+	/* PROT_NONE is similar to unmapped area. */
+	memset(engines, 0, len);
+	igt_assert_eq(mprotect(engines, len, PROT_NONE), 0);
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EFAULT);
+	igt_assert_eq(mprotect(engines, len, PROT_WRITE), 0);
+
+	/* Read-only so kernel cannot fill the data back. */
+	memset(engines, 0, len);
+	igt_assert_eq(mprotect(engines, len, PROT_READ), 0);
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -EFAULT);
+
+	munmap(engines, 4096);
+}
+
+static bool
+has_engine(struct drm_i915_query_engine_info *engines,
+	   unsigned class, unsigned instance)
+{
+	unsigned int i;
+
+	for (i = 0; i < engines->num_engines; i++) {
+		struct drm_i915_engine_info *engine =
+			(struct drm_i915_engine_info *)&engines->engines[i];
+
+		if (engine->engine.engine_class == class &&
+		    engine->engine.engine_instance == instance)
+			return true;
+	}
+
+	return false;
+}
+
+static void engines(int fd)
+{
+	struct drm_i915_query_engine_info *engines;
+	struct drm_i915_query_item item;
+	unsigned int len, i;
+
+	engines = malloc(4096);
+	igt_assert(engines);
+
+	/* Query required buffer length. */
+	memset(engines, 0, 4096);
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert(item.length >= 0);
+	igt_assert(item.length <= 4096);
+	len = item.length;
+
+	/* Check length larger than required works and reports same length. */
+	memset(engines, 0, 4096);
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = 4096;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, len);
+
+	/* Actual query. */
+	memset(engines, 0, 4096);
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_ENGINE_INFO;
+	item.length = len;
+	item.data_ptr = to_user_pointer(engines);
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, len);
+
+	/* Every GPU has at least one engine. */
+	igt_assert(engines->num_engines > 0);
+
+	/* MBZ fields. */
+	igt_assert_eq(engines->rsvd[0], 0);
+	igt_assert_eq(engines->rsvd[1], 0);
+	igt_assert_eq(engines->rsvd[2], 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->engines[i];
+
+		igt_debug("%u: class=%u instance=%u flags=%llx capabilities=%llx\n",
+			  i,
+			  engine->engine.engine_class,
+			  engine->engine.engine_instance,
+			  engine->flags,
+			  engine->capabilities);
+
+		/* MBZ fields. */
+		igt_assert_eq(engine->rsvd0, 0);
+		igt_assert_eq(engine->rsvd1[0], 0);
+		igt_assert_eq(engine->rsvd1[1], 0);
+
+		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));
+
+	free(engines);
+}
+
 igt_main
 {
 	int fd = -1;
@@ -530,6 +765,18 @@  igt_main
 		test_query_topology_known_pci_ids(fd, devid);
 	}
 
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(query_engine_info_supported(fd));
+		}
+
+		igt_subtest("engine-info-invalid")
+			engines_invalid(fd);
+
+		igt_subtest("engine-info")
+			engines(fd);
+	}
+
 	igt_fixture {
 		close(fd);
 	}