Re: [PATCH i-g-t] i915: Improve static engine map for legacy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 23/05/2019 09:06, Chris Wilson wrote:
We need to keep igt working on linus and dif, or Joonas gets very upset.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Andi Shyti <andi.shyti@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
  lib/i915/gem_context.c         |  2 +-
  lib/i915/gem_engine_topology.c | 15 +++-------
  lib/igt_gt.c                   | 54 ++++------------------------------
  lib/igt_gt.h                   | 17 -----------
  tests/i915/gem_ctx_isolation.c | 21 +++++--------
  tools/intel_reg.c              |  4 +--
  6 files changed, 19 insertions(+), 94 deletions(-)

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index f94d89cb4..07ab78174 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -290,7 +290,7 @@ bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
  	 * wouldn't produce any result.
  	 */
  	if ((engine & ~(3<<13)) == I915_EXEC_BSD) {
-		if (engine & (3 << 13) && !gem_has_bsd2(fd))
+		if (engine & (2 << 13) && !gem_has_bsd2(fd))
  			return false;
  	}
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index d0c8bd5aa..fdd1b9516 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c
@@ -223,22 +223,15 @@ struct intel_engine_data intel_init_engine_list(int fd, uint32_t ctx_id)
  			struct intel_execution_engine2 *__e2 =
  				&engine_data.engines[engine_data.nengines];
- if (!igt_only_list_subtests()) {
-				__e2->flags = gem_class_instance_to_eb_flags(fd,
-						e2->class, e2->instance);
-
-				if (!gem_has_ring(fd, __e2->flags))
-					continue;
-			} else {
-				__e2->flags = -1; /* 0xfff... */
-			}
-
  			__e2->name       = e2->name;
  			__e2->instance   = e2->instance;
  			__e2->class      = e2->class;
+			__e2->flags      = e2->flags;
  			__e2->is_virtual = false;
- engine_data.nengines++;
+			if (igt_only_list_subtests() ||
+			    gem_has_ring(fd, e2->flags))
+				engine_data.nengines++;
  		}
  		return engine_data;
  	}
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 6b7c037e6..78e3cd089 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -585,12 +585,12 @@ bool gem_can_store_dword(int fd, unsigned int engine)
  }
const struct intel_execution_engine2 intel_execution_engines2[] = {
-	{ "rcs0", I915_ENGINE_CLASS_RENDER, 0 },
-	{ "bcs0", I915_ENGINE_CLASS_COPY, 0 },
-	{ "vcs0", I915_ENGINE_CLASS_VIDEO, 0 },
-	{ "vcs1", I915_ENGINE_CLASS_VIDEO, 1 },
-	{ "vcs2", I915_ENGINE_CLASS_VIDEO, 2 },
-	{ "vecs0", I915_ENGINE_CLASS_VIDEO_ENHANCE, 0 },
+	{ "rcs0", I915_ENGINE_CLASS_RENDER, 0, I915_EXEC_RENDER },
+	{ "bcs0", I915_ENGINE_CLASS_COPY, 0, I915_EXEC_BLT },
+	{ "vcs0", I915_ENGINE_CLASS_VIDEO, 0, I915_EXEC_BSD | I915_EXEC_BSD_RING1 },
+	{ "vcs1", I915_ENGINE_CLASS_VIDEO, 1, I915_EXEC_BSD | I915_EXEC_BSD_RING2 },
+	{ "vcs2", I915_ENGINE_CLASS_VIDEO, 2, -1 },
+	{ "vecs0", I915_ENGINE_CLASS_VIDEO_ENHANCE, 0, I915_EXEC_VEBOX },
  	{ }
  };
@@ -611,48 +611,6 @@ int gem_execbuf_flags_to_engine_class(unsigned int flags)
  	}
  }
-unsigned int
-gem_class_instance_to_eb_flags(int gem_fd,
-			       enum drm_i915_gem_engine_class class,
-			       unsigned int instance)
-{
-	if (class != I915_ENGINE_CLASS_VIDEO)
-		igt_assert(instance == 0);
-	else
-		igt_assert(instance >= 0 && instance <= 1);
-
-	switch (class) {
-	case I915_ENGINE_CLASS_RENDER:
-		return I915_EXEC_RENDER;
-	case I915_ENGINE_CLASS_COPY:
-		return I915_EXEC_BLT;
-	case I915_ENGINE_CLASS_VIDEO:
-		if (instance == 0) {
-			if (gem_has_bsd2(gem_fd))
-				return I915_EXEC_BSD | I915_EXEC_BSD_RING1;
-			else
-				return I915_EXEC_BSD;
-
-		} else {
-			return I915_EXEC_BSD | I915_EXEC_BSD_RING2;
-		}
-	case I915_ENGINE_CLASS_VIDEO_ENHANCE:
-		return I915_EXEC_VEBOX;
-	case I915_ENGINE_CLASS_INVALID:
-	default:
-		igt_assert(0);
-	};
-}
-
-bool gem_has_engine(int gem_fd,
-		    enum drm_i915_gem_engine_class class,
-		    unsigned int instance)
-{
-	return gem_has_ring(gem_fd,
-			    gem_class_instance_to_eb_flags(gem_fd, class,
-							   instance));
-}
-
  bool gem_ring_is_physical_engine(int fd, unsigned ring)
  {
  	if (ring == I915_EXEC_DEFAULT)
diff --git a/lib/igt_gt.h b/lib/igt_gt.h
index 77318e2a8..73b5002a0 100644
--- a/lib/igt_gt.h
+++ b/lib/igt_gt.h
@@ -102,21 +102,4 @@ extern const struct intel_execution_engine2 {
int gem_execbuf_flags_to_engine_class(unsigned int flags); -unsigned int
-gem_class_instance_to_eb_flags(int gem_fd,
-			       enum drm_i915_gem_engine_class class,
-			       unsigned int instance);
-
-bool gem_has_engine(int gem_fd,
-		    enum drm_i915_gem_engine_class class,
-		    unsigned int instance);
-
-static inline
-void gem_require_engine(int gem_fd,
-			enum drm_i915_gem_engine_class class,
-			unsigned int instance)
-{
-	igt_require(gem_has_engine(gem_fd, class, instance));
-}
-
  #endif /* IGT_GT_H */
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index bcd0f4812..5b054c81d 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -321,8 +321,7 @@ static uint32_t read_regs(int fd,
  	memset(&execbuf, 0, sizeof(execbuf));
  	execbuf.buffers_ptr = to_user_pointer(obj);
  	execbuf.buffer_count = 2;
-	execbuf.flags =
-		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.flags = e->flags;
  	execbuf.rsvd1 = ctx;
  	gem_execbuf(fd, &execbuf);
  	gem_close(fd, obj[1].handle);
@@ -377,8 +376,7 @@ static void write_regs(int fd,
  	memset(&execbuf, 0, sizeof(execbuf));
  	execbuf.buffers_ptr = to_user_pointer(&obj);
  	execbuf.buffer_count = 1;
-	execbuf.flags =
-		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.flags = e->flags;
  	execbuf.rsvd1 = ctx;
  	gem_execbuf(fd, &execbuf);
  	gem_close(fd, obj.handle);
@@ -448,8 +446,7 @@ static void restore_regs(int fd,
  	memset(&execbuf, 0, sizeof(execbuf));
  	execbuf.buffers_ptr = to_user_pointer(obj);
  	execbuf.buffer_count = 2;
-	execbuf.flags =
-		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	execbuf.flags = e->flags;
  	execbuf.rsvd1 = ctx;
  	gem_execbuf(fd, &execbuf);
  	gem_close(fd, obj[1].handle);
@@ -559,8 +556,7 @@ static void nonpriv(int fd,
  		0x0505c0c0,
  		0xdeadbeef
  	};
-	unsigned int engine =
-		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	unsigned int engine = e->flags;
  	unsigned int num_values = ARRAY_SIZE(values);
/* Sigh -- hsw: we need cmdparser access to our own registers! */
@@ -616,9 +612,7 @@ static void isolation(int fd,
  		0xaaaaaaaa,
  		0xdeadbeef
  	};
-	unsigned int engine = gem_class_instance_to_eb_flags(fd,
-							     e->class,
-							     e->instance);
+	unsigned int engine = e->flags;
  	unsigned int num_values =
  		flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
@@ -729,8 +723,7 @@ static void preservation(int fd,
  		0xdeadbeef
  	};
  	const unsigned int num_values = ARRAY_SIZE(values);
-	unsigned int engine =
-		gem_class_instance_to_eb_flags(fd, e->class, e->instance);
+	unsigned int engine = e->flags;
  	uint32_t ctx[num_values +1 ];
  	uint32_t regs[num_values + 1][2];
  	igt_spin_t *spin;
@@ -840,7 +833,7 @@ igt_main
  		igt_subtest_group {
  			igt_fixture {
  				igt_require(has_context_isolation & (1 << e->class));
-				gem_require_engine(fd, e->class, e->instance);
+				gem_require_ring(fd, e->flags);
  				igt_fork_hang_detector(fd);
  			}
diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 1247b70b0..e517956b8 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -329,9 +329,7 @@ static int register_srm(struct config *config, struct reg *reg,
  	memset(&execbuf, 0, sizeof(execbuf));
  	execbuf.buffers_ptr = to_user_pointer(obj);
  	execbuf.buffer_count = 2;
-	execbuf.flags = gem_class_instance_to_eb_flags(fd,
-						       engine->class,
-						       engine->instance);
+	execbuf.flags = engine->flags;
  	if (secure)
  		execbuf.flags |= I915_EXEC_SECURE;

It still puzzles me at which point was I915_EXEC_BSD_RING1 not allowed on single vcs parts. Was it just a fail in IGT?!? The first hunk in this patch. Digging in history points to that..

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux