All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  8:06 ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  8:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

We need to keep igt working on linus and dif, or Joonas gets very upset.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 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;
 
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  8:06 ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  8:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev, Tvrtko Ursulin

We need to keep igt working on linus and dif, or Joonas gets very upset.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 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;
 
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  8:06 ` [igt-dev] " Chris Wilson
@ 2019-05-23  8:33   ` Andi Shyti
  -1 siblings, 0 replies; 22+ messages in thread
From: Andi Shyti @ 2019-05-23  8:33 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, intel-gfx

Hi Chris,

On Thu, May 23, 2019 at 09:06:49AM +0100, Chris Wilson wrote:
> We need to keep igt working on linus and dif, or Joonas gets very upset.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

looks good.

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Thanks,
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  8:33   ` Andi Shyti
  0 siblings, 0 replies; 22+ messages in thread
From: Andi Shyti @ 2019-05-23  8:33 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, intel-gfx, Tvrtko Ursulin

Hi Chris,

On Thu, May 23, 2019 at 09:06:49AM +0100, Chris Wilson wrote:
> We need to keep igt working on linus and dif, or Joonas gets very upset.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

looks good.

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Thanks,
Andi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  8:06 ` [igt-dev] " Chris Wilson
@ 2019-05-23  8:46   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  8:46 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


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@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   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 },

execbuf will reject this on single vcs parts. :( Am I not seeing some 
place where you fudge it into compliance?

Regards,

Tvrtko

> +	{ "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;
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  8:46   ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  8:46 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


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@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   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 },

execbuf will reject this on single vcs parts. :( Am I not seeing some 
place where you fudge it into compliance?

Regards,

Tvrtko

> +	{ "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;
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  8:46   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-05-23  9:09     ` Chris Wilson
  -1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  9:09 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
> 
> On 23/05/2019 09:06, Chris Wilson wrote:
> >   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 },
> 
> execbuf will reject this on single vcs parts. :( Am I not seeing some 
> place where you fudge it into compliance?


        if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
                unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;

                if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
                        bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
                } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
                           bsd_idx <= I915_EXEC_BSD_RING2) {
                        bsd_idx >>= I915_EXEC_BSD_SHIFT;
                        bsd_idx--;
                } else {
                        DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
                                  bsd_idx);
                        return NULL;
                }

                engine = dev_priv->engine[_VCS(bsd_idx)];
        } else {
                engine = dev_priv->engine[user_ring_map[user_ring_id]];
        }

Looks ok to me... I was trying to double check but dif didn't boot on
bsw. Sigh.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  9:09     ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  9:09 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
> 
> On 23/05/2019 09:06, Chris Wilson wrote:
> >   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 },
> 
> execbuf will reject this on single vcs parts. :( Am I not seeing some 
> place where you fudge it into compliance?


        if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
                unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;

                if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
                        bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
                } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
                           bsd_idx <= I915_EXEC_BSD_RING2) {
                        bsd_idx >>= I915_EXEC_BSD_SHIFT;
                        bsd_idx--;
                } else {
                        DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
                                  bsd_idx);
                        return NULL;
                }

                engine = dev_priv->engine[_VCS(bsd_idx)];
        } else {
                engine = dev_priv->engine[user_ring_map[user_ring_id]];
        }

Looks ok to me... I was trying to double check but dif didn't boot on
bsw. Sigh.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  9:09     ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-05-23  9:14       ` Chris Wilson
  -1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  9:14 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Chris Wilson (2019-05-23 10:09:48)
> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
> > 
> > On 23/05/2019 09:06, Chris Wilson wrote:
> > >   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 },
> > 
> > execbuf will reject this on single vcs parts. :( Am I not seeing some 
> > place where you fudge it into compliance?
> 
> 
>         if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
>                 unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> 
>                 if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>                         bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
>                 } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>                            bsd_idx <= I915_EXEC_BSD_RING2) {
>                         bsd_idx >>= I915_EXEC_BSD_SHIFT;
>                         bsd_idx--;
>                 } else {
>                         DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
>                                   bsd_idx);
>                         return NULL;
>                 }
> 
>                 engine = dev_priv->engine[_VCS(bsd_idx)];
>         } else {
>                 engine = dev_priv->engine[user_ring_map[user_ring_id]];
>         }
> 
> Looks ok to me... I was trying to double check but dif didn't boot on
> bsw. Sigh.

So instead I removed vcs1 from bdw, and vcs0 continues to work.

What is still incorrect though:
$ ./build/tests/gem_exec_parallel --list-subtests
...
vcs0
vcs0-contexts
vcs0-fds
vcs1
vcs1-contexts
vcs1-fds
vcs2
vcs2-contexts
vcs2-fds
...

$ sudo ./build/tests/gem_exec_parallel --run vcs1
IGT-Version: 1.23-g82137ba4 (x86_64) (Linux: 5.2.0-rc1+ x86_64)
(gem_exec_parallel:16219) igt_core-WARNING: Unknown subtest: vcs1

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  9:14       ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  9:14 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Chris Wilson (2019-05-23 10:09:48)
> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
> > 
> > On 23/05/2019 09:06, Chris Wilson wrote:
> > >   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 },
> > 
> > execbuf will reject this on single vcs parts. :( Am I not seeing some 
> > place where you fudge it into compliance?
> 
> 
>         if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
>                 unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> 
>                 if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>                         bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
>                 } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>                            bsd_idx <= I915_EXEC_BSD_RING2) {
>                         bsd_idx >>= I915_EXEC_BSD_SHIFT;
>                         bsd_idx--;
>                 } else {
>                         DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
>                                   bsd_idx);
>                         return NULL;
>                 }
> 
>                 engine = dev_priv->engine[_VCS(bsd_idx)];
>         } else {
>                 engine = dev_priv->engine[user_ring_map[user_ring_id]];
>         }
> 
> Looks ok to me... I was trying to double check but dif didn't boot on
> bsw. Sigh.

So instead I removed vcs1 from bdw, and vcs0 continues to work.

What is still incorrect though:
$ ./build/tests/gem_exec_parallel --list-subtests
...
vcs0
vcs0-contexts
vcs0-fds
vcs1
vcs1-contexts
vcs1-fds
vcs2
vcs2-contexts
vcs2-fds
...

$ sudo ./build/tests/gem_exec_parallel --run vcs1
IGT-Version: 1.23-g82137ba4 (x86_64) (Linux: 5.2.0-rc1+ x86_64)
(gem_exec_parallel:16219) igt_core-WARNING: Unknown subtest: vcs1

-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  9:09     ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-05-23  9:27       ` Tvrtko Ursulin
  -1 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:27 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 23/05/2019 10:09, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
>>
>> On 23/05/2019 09:06, Chris Wilson wrote:
>>>    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 },
>>
>> execbuf will reject this on single vcs parts. :( Am I not seeing some
>> place where you fudge it into compliance?
> 
> 
>          if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
>                  unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> 
>                  if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>                          bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
>                  } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>                             bsd_idx <= I915_EXEC_BSD_RING2) {
>                          bsd_idx >>= I915_EXEC_BSD_SHIFT;
>                          bsd_idx--;
>                  } else {
>                          DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
>                                    bsd_idx);
>                          return NULL;
>                  }
> 
>                  engine = dev_priv->engine[_VCS(bsd_idx)];
>          } else {
>                  engine = dev_priv->engine[user_ring_map[user_ring_id]];
>          }
> 
> Looks ok to me... I was trying to double check but dif didn't boot on
> bsw. Sigh.

Hmm I would have sworn we used to do that.. strange. Okay, I'll re-read 
your patch. Notice that I did not know you are working on it so have 
sent my own fix at similar time.

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  9:27       ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:27 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 23/05/2019 10:09, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
>>
>> On 23/05/2019 09:06, Chris Wilson wrote:
>>>    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 },
>>
>> execbuf will reject this on single vcs parts. :( Am I not seeing some
>> place where you fudge it into compliance?
> 
> 
>          if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
>                  unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> 
>                  if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>                          bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
>                  } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>                             bsd_idx <= I915_EXEC_BSD_RING2) {
>                          bsd_idx >>= I915_EXEC_BSD_SHIFT;
>                          bsd_idx--;
>                  } else {
>                          DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
>                                    bsd_idx);
>                          return NULL;
>                  }
> 
>                  engine = dev_priv->engine[_VCS(bsd_idx)];
>          } else {
>                  engine = dev_priv->engine[user_ring_map[user_ring_id]];
>          }
> 
> Looks ok to me... I was trying to double check but dif didn't boot on
> bsw. Sigh.

Hmm I would have sworn we used to do that.. strange. Okay, I'll re-read 
your patch. Notice that I did not know you are working on it so have 
sent my own fix at similar time.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for i915: Improve static engine map for legacy
  2019-05-23  8:06 ` [igt-dev] " Chris Wilson
                   ` (2 preceding siblings ...)
  (?)
@ 2019-05-23  9:27 ` Patchwork
  -1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-05-23  9:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915: Improve static engine map for legacy
URL   : https://patchwork.freedesktop.org/series/61015/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6127 -> IGTPW_3039
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61015/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3039 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [PASS][1] -> [DMESG-FAIL][2] ([fdo#110235])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-icl-y}:         [INCOMPLETE][3] ([fdo#107713] / [fdo#109100]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/fi-icl-y/igt@gem_ctx_create@basic-files.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/fi-icl-y/igt@gem_ctx_create@basic-files.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][5] ([fdo#108511]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  
#### Warnings ####

  * igt@i915_selftest@live_hangcheck:
    - fi-apl-guc:         [DMESG-FAIL][7] ([fdo#110620]) -> [FAIL][8] ([fdo#110623])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/fi-apl-guc/igt@i915_selftest@live_hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110620]: https://bugs.freedesktop.org/show_bug.cgi?id=110620
  [fdo#110623]: https://bugs.freedesktop.org/show_bug.cgi?id=110623


Participating hosts (51 -> 46)
------------------------------

  Additional (2): fi-skl-iommu fi-pnv-d510 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5006 -> IGTPW_3039

  CI_DRM_6127: 376943a0a929c01f9f945dcde30f58ba7e6004f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3039: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/
  IGT_5006: 95d22d5fece7af1448e3b533228c9298080d26e7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  9:14       ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-05-23  9:35         ` Tvrtko Ursulin
  -1 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:35 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 23/05/2019 10:14, Chris Wilson wrote:
> Quoting Chris Wilson (2019-05-23 10:09:48)
>> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
>>>
>>> On 23/05/2019 09:06, Chris Wilson wrote:
>>>>    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 },
>>>
>>> execbuf will reject this on single vcs parts. :( Am I not seeing some
>>> place where you fudge it into compliance?
>>
>>
>>          if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
>>                  unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
>>
>>                  if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>>                          bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
>>                  } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>>                             bsd_idx <= I915_EXEC_BSD_RING2) {
>>                          bsd_idx >>= I915_EXEC_BSD_SHIFT;
>>                          bsd_idx--;
>>                  } else {
>>                          DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
>>                                    bsd_idx);
>>                          return NULL;
>>                  }
>>
>>                  engine = dev_priv->engine[_VCS(bsd_idx)];
>>          } else {
>>                  engine = dev_priv->engine[user_ring_map[user_ring_id]];
>>          }
>>
>> Looks ok to me... I was trying to double check but dif didn't boot on
>> bsw. Sigh.
> 
> So instead I removed vcs1 from bdw, and vcs0 continues to work.
> 
> What is still incorrect though:
> $ ./build/tests/gem_exec_parallel --list-subtests
> ...
> vcs0
> vcs0-contexts
> vcs0-fds
> vcs1
> vcs1-contexts
> vcs1-fds
> vcs2
> vcs2-contexts
> vcs2-fds
> ...

As you know this part is correct since CI mandates we list subtests not 
applicable to running platform.

> $ sudo ./build/tests/gem_exec_parallel --run vcs1
> IGT-Version: 1.23-g82137ba4 (x86_64) (Linux: 5.2.0-rc1+ x86_64)
> (gem_exec_parallel:16219) igt_core-WARNING: Unknown subtest: vcs1

I guess we need to make the library function igt_skip instead of not 
enumerating the impossible and missing engines.

But it doesn't work.. skips need to be in either subtests or fixtures.

Perhaps the idea of using a single context aware iterator has therefore 
fail and we do need two separate ones. :( One just for subtest 
enumeration which *only* uses the static list, and another for use in 
tests, which skips missing/impossible engines depending on context.

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  9:35         ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:35 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 23/05/2019 10:14, Chris Wilson wrote:
> Quoting Chris Wilson (2019-05-23 10:09:48)
>> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
>>>
>>> On 23/05/2019 09:06, Chris Wilson wrote:
>>>>    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 },
>>>
>>> execbuf will reject this on single vcs parts. :( Am I not seeing some
>>> place where you fudge it into compliance?
>>
>>
>>          if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
>>                  unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
>>
>>                  if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>>                          bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
>>                  } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>>                             bsd_idx <= I915_EXEC_BSD_RING2) {
>>                          bsd_idx >>= I915_EXEC_BSD_SHIFT;
>>                          bsd_idx--;
>>                  } else {
>>                          DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
>>                                    bsd_idx);
>>                          return NULL;
>>                  }
>>
>>                  engine = dev_priv->engine[_VCS(bsd_idx)];
>>          } else {
>>                  engine = dev_priv->engine[user_ring_map[user_ring_id]];
>>          }
>>
>> Looks ok to me... I was trying to double check but dif didn't boot on
>> bsw. Sigh.
> 
> So instead I removed vcs1 from bdw, and vcs0 continues to work.
> 
> What is still incorrect though:
> $ ./build/tests/gem_exec_parallel --list-subtests
> ...
> vcs0
> vcs0-contexts
> vcs0-fds
> vcs1
> vcs1-contexts
> vcs1-fds
> vcs2
> vcs2-contexts
> vcs2-fds
> ...

As you know this part is correct since CI mandates we list subtests not 
applicable to running platform.

> $ sudo ./build/tests/gem_exec_parallel --run vcs1
> IGT-Version: 1.23-g82137ba4 (x86_64) (Linux: 5.2.0-rc1+ x86_64)
> (gem_exec_parallel:16219) igt_core-WARNING: Unknown subtest: vcs1

I guess we need to make the library function igt_skip instead of not 
enumerating the impossible and missing engines.

But it doesn't work.. skips need to be in either subtests or fixtures.

Perhaps the idea of using a single context aware iterator has therefore 
fail and we do need two separate ones. :( One just for subtest 
enumeration which *only* uses the static list, and another for use in 
tests, which skips missing/impossible engines depending on context.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  9:35         ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-05-23  9:46           ` Chris Wilson
  -1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  9:46 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-23 10:35:01)
> 
> On 23/05/2019 10:14, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-05-23 10:09:48)
> >> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
> >>>
> >>> On 23/05/2019 09:06, Chris Wilson wrote:
> >>>>    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 },
> >>>
> >>> execbuf will reject this on single vcs parts. :( Am I not seeing some
> >>> place where you fudge it into compliance?
> >>
> >>
> >>          if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
> >>                  unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> >>
> >>                  if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
> >>                          bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
> >>                  } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
> >>                             bsd_idx <= I915_EXEC_BSD_RING2) {
> >>                          bsd_idx >>= I915_EXEC_BSD_SHIFT;
> >>                          bsd_idx--;
> >>                  } else {
> >>                          DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
> >>                                    bsd_idx);
> >>                          return NULL;
> >>                  }
> >>
> >>                  engine = dev_priv->engine[_VCS(bsd_idx)];
> >>          } else {
> >>                  engine = dev_priv->engine[user_ring_map[user_ring_id]];
> >>          }
> >>
> >> Looks ok to me... I was trying to double check but dif didn't boot on
> >> bsw. Sigh.
> > 
> > So instead I removed vcs1 from bdw, and vcs0 continues to work.
> > 
> > What is still incorrect though:
> > $ ./build/tests/gem_exec_parallel --list-subtests
> > ...
> > vcs0
> > vcs0-contexts
> > vcs0-fds
> > vcs1
> > vcs1-contexts
> > vcs1-fds
> > vcs2
> > vcs2-contexts
> > vcs2-fds
> > ...
> 
> As you know this part is correct since CI mandates we list subtests not 
> applicable to running platform.
> 
> > $ sudo ./build/tests/gem_exec_parallel --run vcs1
> > IGT-Version: 1.23-g82137ba4 (x86_64) (Linux: 5.2.0-rc1+ x86_64)
> > (gem_exec_parallel:16219) igt_core-WARNING: Unknown subtest: vcs1
> 
> I guess we need to make the library function igt_skip instead of not 
> enumerating the impossible and missing engines.
> 
> But it doesn't work.. skips need to be in either subtests or fixtures.
> 
> Perhaps the idea of using a single context aware iterator has therefore 
> fail and we do need two separate ones. :( One just for subtest 
> enumeration which *only* uses the static list, and another for use in 
> tests, which skips missing/impossible engines depending on context.

I'm willing to sweep this bit under the carpet so long as it isn't
flagged as a failure to Joonas.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  9:46           ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23  9:46 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-23 10:35:01)
> 
> On 23/05/2019 10:14, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-05-23 10:09:48)
> >> Quoting Tvrtko Ursulin (2019-05-23 09:46:08)
> >>>
> >>> On 23/05/2019 09:06, Chris Wilson wrote:
> >>>>    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 },
> >>>
> >>> execbuf will reject this on single vcs parts. :( Am I not seeing some
> >>> place where you fudge it into compliance?
> >>
> >>
> >>          if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) {
> >>                  unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> >>
> >>                  if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
> >>                          bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file);
> >>                  } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
> >>                             bsd_idx <= I915_EXEC_BSD_RING2) {
> >>                          bsd_idx >>= I915_EXEC_BSD_SHIFT;
> >>                          bsd_idx--;
> >>                  } else {
> >>                          DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
> >>                                    bsd_idx);
> >>                          return NULL;
> >>                  }
> >>
> >>                  engine = dev_priv->engine[_VCS(bsd_idx)];
> >>          } else {
> >>                  engine = dev_priv->engine[user_ring_map[user_ring_id]];
> >>          }
> >>
> >> Looks ok to me... I was trying to double check but dif didn't boot on
> >> bsw. Sigh.
> > 
> > So instead I removed vcs1 from bdw, and vcs0 continues to work.
> > 
> > What is still incorrect though:
> > $ ./build/tests/gem_exec_parallel --list-subtests
> > ...
> > vcs0
> > vcs0-contexts
> > vcs0-fds
> > vcs1
> > vcs1-contexts
> > vcs1-fds
> > vcs2
> > vcs2-contexts
> > vcs2-fds
> > ...
> 
> As you know this part is correct since CI mandates we list subtests not 
> applicable to running platform.
> 
> > $ sudo ./build/tests/gem_exec_parallel --run vcs1
> > IGT-Version: 1.23-g82137ba4 (x86_64) (Linux: 5.2.0-rc1+ x86_64)
> > (gem_exec_parallel:16219) igt_core-WARNING: Unknown subtest: vcs1
> 
> I guess we need to make the library function igt_skip instead of not 
> enumerating the impossible and missing engines.
> 
> But it doesn't work.. skips need to be in either subtests or fixtures.
> 
> Perhaps the idea of using a single context aware iterator has therefore 
> fail and we do need two separate ones. :( One just for subtest 
> enumeration which *only* uses the static list, and another for use in 
> tests, which skips missing/impossible engines depending on context.

I'm willing to sweep this bit under the carpet so long as it isn't
flagged as a failure to Joonas.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  8:06 ` [igt-dev] " Chris Wilson
@ 2019-05-23  9:48   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:48 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


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@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   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@intel.com>

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23  9:48   ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:48 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


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@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   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@intel.com>

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH i-g-t] i915: Improve static engine map for legacy
  2019-05-23  9:48   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
@ 2019-05-23 11:23     ` Chris Wilson
  -1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23 11:23 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-23 10:48:27)
> 
> 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@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Andi Shyti <andi.shyti@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >   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..

I do recall making a decision to hide vcs1 if !vcs2 so that we didn't
duplicate (vcs, vcs1), and still have the plain randomly chosen vcs.
That is likely the additional source of confusion.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915: Improve static engine map for legacy
@ 2019-05-23 11:23     ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2019-05-23 11:23 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-05-23 10:48:27)
> 
> 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@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Andi Shyti <andi.shyti@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >   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..

I do recall making a decision to hide vcs1 if !vcs2 so that we didn't
duplicate (vcs, vcs1), and still have the plain randomly chosen vcs.
That is likely the additional source of confusion.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for i915: Improve static engine map for legacy
  2019-05-23  8:06 ` [igt-dev] " Chris Wilson
                   ` (4 preceding siblings ...)
  (?)
@ 2019-05-24  3:42 ` Patchwork
  -1 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-05-24  3:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915: Improve static engine map for legacy
URL   : https://patchwork.freedesktop.org/series/61015/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6127_full -> IGTPW_3039_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_3039_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3039_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61015/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_3039_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@cursor-vs-flip-legacy:
    - shard-glk:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-glk8/igt@kms_cursor_legacy@cursor-vs-flip-legacy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-glk1/igt@kms_cursor_legacy@cursor-vs-flip-legacy.html

  
#### Warnings ####

  * igt@prime_vgem@fence-wait-bsd1:
    - shard-snb:          [SKIP][3] ([fdo#109271]) -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-snb2/igt@prime_vgem@fence-wait-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-snb5/igt@prime_vgem@fence-wait-bsd1.html

  
Known issues
------------

  Here are the changes found in IGTPW_3039_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [PASS][5] -> [FAIL][6] ([fdo#105767])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-hsw:          [PASS][7] -> [SKIP][8] ([fdo#109271]) +16 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-hsw4/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-hsw5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#109349])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb7/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_flip@2x-plain-flip-ts-check-interruptible:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([fdo#100368])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-glk8/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-glk2/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +4 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-render:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#109247]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb8/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-render.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb2/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-render.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-apl:          [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-apl5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-glk:          [PASS][19] -> [INCOMPLETE][20] ([fdo#103359] / [k.org#198133])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-glk8/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-glk1/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html

  
#### Possible fixes ####

  * igt@gem_bad_reloc@negative-reloc-lut-bsd1:
    - shard-glk:          [SKIP][23] ([fdo#109271]) -> [PASS][24] +11 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-glk2/igt@gem_bad_reloc@negative-reloc-lut-bsd1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-glk5/igt@gem_bad_reloc@negative-reloc-lut-bsd1.html

  * igt@gem_ctx_isolation@bcs0-none:
    - shard-glk:          [WARN][25] ([fdo#110738]) -> [PASS][26] +26 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-glk9/igt@gem_ctx_isolation@bcs0-none.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-glk9/igt@gem_ctx_isolation@bcs0-none.html

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-kbl:          [WARN][27] ([fdo#110738]) -> [PASS][28] +34 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-kbl1/igt@gem_ctx_isolation@bcs0-s3.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-kbl4/igt@gem_ctx_isolation@bcs0-s3.html
    - shard-iclb:         [WARN][29] ([fdo#110738]) -> [PASS][30] +22 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb7/igt@gem_ctx_isolation@bcs0-s3.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb1/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_isolation@vcs0-dirty-create:
    - shard-apl:          [WARN][31] ([fdo#110738]) -> [PASS][32] +22 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-apl3/igt@gem_ctx_isolation@vcs0-dirty-create.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-apl7/igt@gem_ctx_isolation@vcs0-dirty-create.html

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          [FAIL][33] ([fdo#109661]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-snb5/igt@gem_eio@unwedge-stress.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-snb4/igt@gem_eio@unwedge-stress.html

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         [FAIL][35] ([fdo#108059]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb7/igt@i915_pm_rps@min-max-config-loaded.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb8/igt@i915_pm_rps@min-max-config-loaded.html

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-kbl:          [FAIL][37] ([fdo#104782]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-kbl1/igt@kms_color@pipe-b-legacy-gamma.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-kbl1/igt@kms_color@pipe-b-legacy-gamma.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][41] ([fdo#103166]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [SKIP][43] ([fdo#109441]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb1/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][45] ([fdo#108566]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-apl5/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-apl4/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@prime_busy@hang-bsd1:
    - shard-hsw:          [SKIP][47] ([fdo#109271]) -> [PASS][48] +23 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-hsw6/igt@prime_busy@hang-bsd1.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-hsw7/igt@prime_busy@hang-bsd1.html

  * igt@prime_vgem@fence-wait-bsd1:
    - shard-iclb:         [SKIP][49] ([fdo#109276]) -> [PASS][50] +11 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb2/igt@prime_vgem@fence-wait-bsd1.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb2/igt@prime_vgem@fence-wait-bsd1.html

  * igt@prime_vgem@sync-bsd1:
    - shard-apl:          [SKIP][51] ([fdo#109271]) -> [PASS][52] +12 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-apl8/igt@prime_vgem@sync-bsd1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-apl6/igt@prime_vgem@sync-bsd1.html

  * igt@sw_sync@sync_expired_merge:
    - shard-apl:          [INCOMPLETE][53] ([fdo#103927]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-apl1/igt@sw_sync@sync_expired_merge.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-apl1/igt@sw_sync@sync_expired_merge.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         [TIMEOUT][55] ([fdo#109673]) -> [INCOMPLETE][56] ([fdo#107713] / [fdo#109100]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-iclb8/igt@gem_mmap_gtt@forked-big-copy-xy.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-iclb1/igt@gem_mmap_gtt@forked-big-copy-xy.html

  * igt@prime_vgem@wait-bsd1:
    - shard-snb:          [SKIP][57] ([fdo#109271]) -> [INCOMPLETE][58] ([fdo#105411]) +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6127/shard-snb2/igt@prime_vgem@wait-bsd1.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/shard-snb1/igt@prime_vgem@wait-bsd1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110738]: https://bugs.freedesktop.org/show_bug.cgi?id=110738
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

  * IGT: IGT_5006 -> IGTPW_3039
  * Piglit: piglit_4509 -> None

  CI_DRM_6127: 376943a0a929c01f9f945dcde30f58ba7e6004f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3039: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/
  IGT_5006: 95d22d5fece7af1448e3b533228c9298080d26e7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3039/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-05-24  3:42 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23  8:06 [PATCH i-g-t] i915: Improve static engine map for legacy Chris Wilson
2019-05-23  8:06 ` [igt-dev] " Chris Wilson
2019-05-23  8:33 ` Andi Shyti
2019-05-23  8:33   ` [igt-dev] " Andi Shyti
2019-05-23  8:46 ` Tvrtko Ursulin
2019-05-23  8:46   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-05-23  9:09   ` Chris Wilson
2019-05-23  9:09     ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-05-23  9:14     ` Chris Wilson
2019-05-23  9:14       ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-05-23  9:35       ` Tvrtko Ursulin
2019-05-23  9:35         ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-05-23  9:46         ` Chris Wilson
2019-05-23  9:46           ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-05-23  9:27     ` Tvrtko Ursulin
2019-05-23  9:27       ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-05-23  9:27 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-05-23  9:48 ` [PATCH i-g-t] " Tvrtko Ursulin
2019-05-23  9:48   ` [igt-dev] [Intel-gfx] " Tvrtko Ursulin
2019-05-23 11:23   ` Chris Wilson
2019-05-23 11:23     ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-05-24  3:42 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.