All of lore.kernel.org
 help / color / mirror / Atom feed
* [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 17:05 ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

Since we want to do a lockless read of the current active request, and
that request is written to by process_csb also without serialisation, we
need to instruct gcc to take care in reading the pointer itself.

Otherwise, we have observed execlists_active() to report 0x40.

[ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
[ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
[ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
[ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940

which is impossible!

The answer is that as we keep the existing execlists->active pointing
into the array as we copy over that array, the unserialised read may see
a partial pointer value.

Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
 drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index bc3b72bfa9e3..01765a7ec18f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
 static inline struct i915_request *
 execlists_active(const struct intel_engine_execlists *execlists)
 {
-	GEM_BUG_ON(execlists->active - execlists->inflight >
-		   execlists_num_ports(execlists));
-	return READ_ONCE(*execlists->active);
+	return *READ_ONCE(execlists->active);
 }
 
 static inline void
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 0e2065a13f24..0d0dca3d6724 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
 		else
 			promote = gen8_csb_parse(execlists, buf + 2 * head);
 		if (promote) {
+			struct i915_request * const *old = execlists->active;
+
+			/* Point active to the new ELSP; prevent overwriting */
+			WRITE_ONCE(execlists->active, execlists->pending);
+			set_timeslice(engine);
+
 			if (!inject_preempt_hang(execlists))
 				ring_set_paused(engine, 0);
 
 			/* cancel old inflight, prepare for switch */
-			trace_ports(execlists, "preempted", execlists->active);
-			while (*execlists->active)
-				execlists_schedule_out(*execlists->active++);
+			trace_ports(execlists, "preempted", old);
+			while (*old)
+				execlists_schedule_out(*old++);
 
 			/* switch pending to inflight */
 			GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
-			execlists->active =
-				memcpy(execlists->inflight,
-				       execlists->pending,
-				       execlists_num_ports(execlists) *
-				       sizeof(*execlists->pending));
-
-			set_timeslice(engine);
+			WRITE_ONCE(execlists->active,
+				   memcpy(execlists->inflight,
+					  execlists->pending,
+					  execlists_num_ports(execlists) *
+					  sizeof(*execlists->pending)));
 
 			WRITE_ONCE(execlists->pending[0], NULL);
 		} else {
-- 
2.24.0

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

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

* [Intel-gfx] [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 17:05 ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

Since we want to do a lockless read of the current active request, and
that request is written to by process_csb also without serialisation, we
need to instruct gcc to take care in reading the pointer itself.

Otherwise, we have observed execlists_active() to report 0x40.

[ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
[ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
[ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
[ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940

which is impossible!

The answer is that as we keep the existing execlists->active pointing
into the array as we copy over that array, the unserialised read may see
a partial pointer value.

Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
 drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index bc3b72bfa9e3..01765a7ec18f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
 static inline struct i915_request *
 execlists_active(const struct intel_engine_execlists *execlists)
 {
-	GEM_BUG_ON(execlists->active - execlists->inflight >
-		   execlists_num_ports(execlists));
-	return READ_ONCE(*execlists->active);
+	return *READ_ONCE(execlists->active);
 }
 
 static inline void
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 0e2065a13f24..0d0dca3d6724 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
 		else
 			promote = gen8_csb_parse(execlists, buf + 2 * head);
 		if (promote) {
+			struct i915_request * const *old = execlists->active;
+
+			/* Point active to the new ELSP; prevent overwriting */
+			WRITE_ONCE(execlists->active, execlists->pending);
+			set_timeslice(engine);
+
 			if (!inject_preempt_hang(execlists))
 				ring_set_paused(engine, 0);
 
 			/* cancel old inflight, prepare for switch */
-			trace_ports(execlists, "preempted", execlists->active);
-			while (*execlists->active)
-				execlists_schedule_out(*execlists->active++);
+			trace_ports(execlists, "preempted", old);
+			while (*old)
+				execlists_schedule_out(*old++);
 
 			/* switch pending to inflight */
 			GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
-			execlists->active =
-				memcpy(execlists->inflight,
-				       execlists->pending,
-				       execlists_num_ports(execlists) *
-				       sizeof(*execlists->pending));
-
-			set_timeslice(engine);
+			WRITE_ONCE(execlists->active,
+				   memcpy(execlists->inflight,
+					  execlists->pending,
+					  execlists_num_ports(execlists) *
+					  sizeof(*execlists->pending)));
 
 			WRITE_ONCE(execlists->pending[0], NULL);
 		} else {
-- 
2.24.0

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

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

* [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context
@ 2019-11-24 17:05   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

As the engine->kernel_context is used within the engine-pm barrier, we
have to be careful when emitting requests outside of the barrier, as the
strict timeline locking rules do not apply. Instead, we must ensure the
engine_park() cannot be entered as we build the request, which is
simplest by taking an explicit engine-pm wakeref around the request
construction.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  3 +-
 .../i915/gem/selftests/i915_gem_client_blt.c  |  2 +
 .../i915/gem/selftests/i915_gem_coherency.c   |  3 +-
 .../drm/i915/gem/selftests/i915_gem_context.c |  7 +++-
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  3 +-
 .../i915/gem/selftests/i915_gem_object_blt.c  | 18 ++++++---
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 14 +++++--
 drivers/gpu/drm/i915/gt/intel_engine_pm.h     | 21 ++++++++++
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |  3 ++
 drivers/gpu/drm/i915/gt/selftest_context.c    |  2 +-
 drivers/gpu/drm/i915/gt/selftest_engine_cs.c  | 12 ++++++
 drivers/gpu/drm/i915/gt/selftest_lrc.c        |  6 +--
 drivers/gpu/drm/i915/gt/selftest_mocs.c       |  2 +
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  6 +--
 drivers/gpu/drm/i915/i915_perf.c              |  4 +-
 drivers/gpu/drm/i915/selftests/i915_active.c  |  2 +-
 drivers/gpu/drm/i915/selftests/i915_perf.c    |  2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c | 40 ++++++++++++++-----
 .../drm/i915/selftests/intel_memory_region.c  |  2 +
 19 files changed, 119 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 6f1e6181f67a..c94ac838401a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -70,6 +70,7 @@
 #include <drm/i915_drm.h>
 
 #include "gt/intel_engine_heartbeat.h"
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_engine_user.h"
 #include "gt/intel_lrc_reg.h"
 #include "gt/intel_ring.h"
@@ -1265,7 +1266,7 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
 	if (!intel_context_is_pinned(ce))
 		return 0;
 
-	rq = i915_request_create(ce->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(ce->engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
index da8edee4fe0a..b972be165e85 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
@@ -24,6 +24,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
 
 	prandom_seed_state(&prng, i915_selftest.random_seed);
 
+	intel_engine_pm_get(engine);
 	do {
 		const u32 max_block_size = S16_MAX * PAGE_SIZE;
 		u32 sz = min_t(u64, ce->vm->total >> 4, prandom_u32_state(&prng));
@@ -99,6 +100,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
 err_flush:
 	if (err == -ENOMEM)
 		err = 0;
+	intel_engine_pm_put(engine);
 
 	return err;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
index 2b29f6b4e1dd..9d3cd1af61f6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
@@ -6,6 +6,7 @@
 
 #include <linux/prime_numbers.h>
 
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_ring.h"
@@ -200,7 +201,7 @@ static int gpu_set(struct context *ctx, unsigned long offset, u32 v)
 	if (IS_ERR(vma))
 		return PTR_ERR(vma);
 
-	rq = i915_request_create(ctx->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(ctx->engine);
 	if (IS_ERR(rq)) {
 		i915_vma_unpin(vma);
 		return PTR_ERR(rq);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index e1d8ccd11409..2ea4790f3721 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -7,6 +7,7 @@
 #include <linux/prime_numbers.h>
 
 #include "gem/i915_gem_pm.h"
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_requests.h"
 #include "gt/intel_reset.h"
@@ -1190,9 +1191,11 @@ __sseu_test(const char *name,
 	struct igt_spinner *spin = NULL;
 	int ret;
 
+	intel_engine_pm_get(ce->engine);
+
 	ret = __sseu_prepare(name, flags, ce, &spin);
 	if (ret)
-		return ret;
+		goto out_pm;
 
 	ret = intel_context_reconfigure_sseu(ce, sseu);
 	if (ret)
@@ -1207,6 +1210,8 @@ __sseu_test(const char *name,
 		igt_spinner_fini(spin);
 		kfree(spin);
 	}
+out_pm:
+	intel_engine_pm_put(ce->engine);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..6ce9167f8c9f 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -6,6 +6,7 @@
 
 #include <linux/prime_numbers.h>
 
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "huge_gem_object.h"
@@ -536,7 +537,7 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
 		if (err)
 			return err;
 
-		rq = i915_request_create(engine->kernel_context);
+		rq = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(rq)) {
 			i915_vma_unpin(vma);
 			return PTR_ERR(rq);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
index 675c1a20a2f1..62077fe46715 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
@@ -41,6 +41,7 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
 		if (!engine)
 			return 0;
 
+		intel_engine_pm_get(engine);
 		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
 			struct intel_context *ce = engine->kernel_context;
 			ktime_t t0, t1;
@@ -49,17 +50,20 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
 
 			err = i915_gem_object_fill_blt(obj, ce, 0);
 			if (err)
-				return err;
+				break;
 
 			err = i915_gem_object_wait(obj,
 						   I915_WAIT_ALL,
 						   MAX_SCHEDULE_TIMEOUT);
 			if (err)
-				return err;
+				break;
 
 			t1 = ktime_get();
 			t[pass] = ktime_sub(t1, t0);
 		}
+		intel_engine_pm_put(engine);
+		if (err)
+			return err;
 
 		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
 		pr_info("%s: blt %zd KiB fill: %lld MiB/s\n",
@@ -109,6 +113,7 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
 		struct intel_engine_cs *engine;
 		ktime_t t[5];
 		int pass;
+		int err = 0;
 
 		engine = intel_engine_lookup_user(i915,
 						  I915_ENGINE_CLASS_COPY,
@@ -116,26 +121,29 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
 		if (!engine)
 			return 0;
 
+		intel_engine_pm_get(engine);
 		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
 			struct intel_context *ce = engine->kernel_context;
 			ktime_t t0, t1;
-			int err;
 
 			t0 = ktime_get();
 
 			err = i915_gem_object_copy_blt(src, dst, ce);
 			if (err)
-				return err;
+				break;
 
 			err = i915_gem_object_wait(dst,
 						   I915_WAIT_ALL,
 						   MAX_SCHEDULE_TIMEOUT);
 			if (err)
-				return err;
+				break;
 
 			t1 = ktime_get();
 			t[pass] = ktime_sub(t1, t0);
 		}
+		intel_engine_pm_put(engine);
+		if (err)
+			return err;
 
 		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
 		pr_info("%s: blt %zd KiB copy: %lld MiB/s\n",
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index c91fd4e4af29..742628e40201 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -215,18 +215,26 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
 int intel_engine_flush_barriers(struct intel_engine_cs *engine)
 {
 	struct i915_request *rq;
+	int err = 0;
 
 	if (llist_empty(&engine->barrier_tasks))
 		return 0;
 
+	if (!intel_engine_pm_get_if_awake(engine))
+		return 0;
+
 	rq = i915_request_create(engine->kernel_context);
-	if (IS_ERR(rq))
-		return PTR_ERR(rq);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto out_rpm;
+	}
 
 	idle_pulse(engine, rq);
 	i915_request_add(rq);
 
-	return 0;
+out_rpm:
+	intel_engine_pm_put(engine);
+	return err;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
index 24e20344dc22..e52c2b0cb245 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
@@ -7,6 +7,7 @@
 #ifndef INTEL_ENGINE_PM_H
 #define INTEL_ENGINE_PM_H
 
+#include "i915_request.h"
 #include "intel_engine_types.h"
 #include "intel_wakeref.h"
 
@@ -41,6 +42,26 @@ static inline void intel_engine_pm_flush(struct intel_engine_cs *engine)
 	intel_wakeref_unlock_wait(&engine->wakeref);
 }
 
+static inline struct i915_request *
+intel_engine_create_kernel_request(struct intel_engine_cs *engine)
+{
+	struct i915_request *rq;
+
+	/*
+	 * The engine->kernel_context is special as it is used inside
+	 * the engine-pm barrier (see __engine_park()), circumventing
+	 * the usual mutexes and relying on the engine-pm barrier
+	 * instead. So whenever we use the engine->kernel_context
+	 * outside of the barrier, we must manually handle the
+	 * engine wakeref to serialise with the use inside.
+	 */
+	intel_engine_pm_get(engine);
+	rq = i915_request_create(engine->kernel_context);
+	intel_engine_pm_put(engine);
+
+	return rq;
+}
+
 void intel_engine_init__pm(struct intel_engine_cs *engine);
 
 #endif /* INTEL_ENGINE_PM_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 226bd4cccb48..0c6d398980ba 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -6,6 +6,7 @@
 
 #include "i915_drv.h"
 #include "intel_context.h"
+#include "intel_engine_pm.h"
 #include "intel_gt.h"
 #include "intel_ring.h"
 #include "intel_workarounds.h"
@@ -1582,7 +1583,9 @@ static int engine_wa_list_verify(struct intel_context *ce,
 	if (IS_ERR(vma))
 		return PTR_ERR(vma);
 
+	intel_engine_pm_get(ce->engine);
 	rq = intel_context_create_request(ce);
+	intel_engine_pm_put(ce->engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto err_vma;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index 5bc124574170..af354ccdbf40 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -121,7 +121,7 @@ static int __live_context_size(struct intel_engine_cs *engine,
 		goto err_unpin;
 
 	/* Force the context switch */
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto err_unpin;
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
index 5981a7b71ec9..761d81f4bd68 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
@@ -132,14 +132,18 @@ static int perf_mi_bb_start(void *arg)
 		u32 cycles[COUNT];
 		int i;
 
+		intel_engine_pm_get(engine);
+
 		batch = create_empty_batch(ce);
 		if (IS_ERR(batch)) {
 			err = PTR_ERR(batch);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
 		err = i915_vma_sync(batch);
 		if (err) {
+			intel_engine_pm_put(engine);
 			i915_vma_put(batch);
 			break;
 		}
@@ -180,6 +184,7 @@ static int perf_mi_bb_start(void *arg)
 			cycles[i] = rq->hwsp_seqno[3] - rq->hwsp_seqno[2];
 		}
 		i915_vma_put(batch);
+		intel_engine_pm_put(engine);
 		if (err)
 			break;
 
@@ -251,15 +256,19 @@ static int perf_mi_noop(void *arg)
 		u32 cycles[COUNT];
 		int i;
 
+		intel_engine_pm_get(engine);
+
 		base = create_empty_batch(ce);
 		if (IS_ERR(base)) {
 			err = PTR_ERR(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
 		err = i915_vma_sync(base);
 		if (err) {
 			i915_vma_put(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
@@ -267,6 +276,7 @@ static int perf_mi_noop(void *arg)
 		if (IS_ERR(nop)) {
 			err = PTR_ERR(nop);
 			i915_vma_put(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
@@ -274,6 +284,7 @@ static int perf_mi_noop(void *arg)
 		if (err) {
 			i915_vma_put(nop);
 			i915_vma_put(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
@@ -327,6 +338,7 @@ static int perf_mi_noop(void *arg)
 		}
 		i915_vma_put(nop);
 		i915_vma_put(base);
+		intel_engine_pm_put(engine);
 		if (err)
 			break;
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index fc142dd61dd1..ac8b9116d307 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -348,7 +348,7 @@ release_queue(struct intel_engine_cs *engine,
 	struct i915_request *rq;
 	u32 *cs;
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
@@ -497,7 +497,7 @@ static struct i915_request *nop_request(struct intel_engine_cs *engine)
 {
 	struct i915_request *rq;
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		return rq;
 
@@ -3698,7 +3698,7 @@ static int gpr_make_dirty(struct intel_engine_cs *engine)
 	u32 *cs;
 	int n;
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index a34d4fb52fa1..de010f527757 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -261,7 +261,9 @@ static int live_mocs_kernel(void *arg)
 		return err;
 
 	for_each_engine(engine, gt, id) {
+		intel_engine_pm_get(engine);
 		err = check_mocs_engine(&mocs, engine->kernel_context);
+		intel_engine_pm_put(engine);
 		if (err)
 			break;
 	}
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index f04a59fe5d2c..e2d78cc22fb4 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -458,7 +458,7 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
 		goto out;
 	}
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		goto out_unpin;
 
@@ -675,9 +675,7 @@ static int live_hwsp_wrap(void *arg)
 		if (!intel_engine_can_store_dword(engine))
 			continue;
 
-		intel_engine_pm_get(engine);
-		rq = i915_request_create(engine->kernel_context);
-		intel_engine_pm_put(engine);
+		rq = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
 			goto out;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 608e6c3f3c1a..b46715b57576 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1968,7 +1968,9 @@ static int emit_oa_config(struct i915_perf_stream *stream,
 	if (err)
 		goto err_vma_put;
 
+	intel_engine_pm_get(ce->engine);
 	rq = i915_request_create(ce);
+	intel_engine_pm_put(ce->engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto err_vma_unpin;
@@ -2165,7 +2167,7 @@ static int gen8_modify_context(struct intel_context *ce,
 
 	lockdep_assert_held(&ce->pin_mutex);
 
-	rq = i915_request_create(ce->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(ce->engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index 60290f78750d..6c1db3ded446 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -99,7 +99,7 @@ __live_active_setup(struct drm_i915_private *i915)
 	for_each_uabi_engine(engine, i915) {
 		struct i915_request *rq;
 
-		rq = i915_request_create(engine->kernel_context);
+		rq = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
 			break;
diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c b/drivers/gpu/drm/i915/selftests/i915_perf.c
index aabd07f67e49..d1a1568c47ba 100644
--- a/drivers/gpu/drm/i915/selftests/i915_perf.c
+++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
@@ -132,7 +132,7 @@ static int live_noa_delay(void *arg)
 	for (i = 0; i < 4; i++)
 		intel_write_status_page(stream->engine, 0x100 + i, 0);
 
-	rq = i915_request_create(stream->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(stream->engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto out;
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
index c16d1efd2ad4..99c94b4f69fb 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -27,6 +27,7 @@
 #include "gem/i915_gem_pm.h"
 #include "gem/selftests/mock_context.h"
 
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 
 #include "i915_random.h"
@@ -541,6 +542,7 @@ static int live_nop_request(void *arg)
 		if (err)
 			return err;
 
+		intel_engine_pm_get(engine);
 		for_each_prime_number_from(prime, 1, 8192) {
 			struct i915_request *request = NULL;
 
@@ -579,6 +581,7 @@ static int live_nop_request(void *arg)
 			if (__igt_timeout(end_time, NULL))
 				break;
 		}
+		intel_engine_pm_put(engine);
 
 		err = igt_live_test_end(&t);
 		if (err)
@@ -693,10 +696,13 @@ static int live_empty_request(void *arg)
 		if (err)
 			goto out_batch;
 
+		intel_engine_pm_get(engine);
+
 		/* Warmup / preload */
 		request = empty_request(engine, batch);
 		if (IS_ERR(request)) {
 			err = PTR_ERR(request);
+			intel_engine_pm_put(engine);
 			goto out_batch;
 		}
 		i915_request_wait(request, 0, MAX_SCHEDULE_TIMEOUT);
@@ -709,6 +715,7 @@ static int live_empty_request(void *arg)
 				request = empty_request(engine, batch);
 				if (IS_ERR(request)) {
 					err = PTR_ERR(request);
+					intel_engine_pm_put(engine);
 					goto out_batch;
 				}
 			}
@@ -722,6 +729,7 @@ static int live_empty_request(void *arg)
 				break;
 		}
 		i915_request_put(request);
+		intel_engine_pm_put(engine);
 
 		err = igt_live_test_end(&t);
 		if (err)
@@ -846,7 +854,7 @@ static int live_all_engines(void *arg)
 
 	idx = 0;
 	for_each_uabi_engine(engine, i915) {
-		request[idx] = i915_request_create(engine->kernel_context);
+		request[idx] = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(request[idx])) {
 			err = PTR_ERR(request[idx]);
 			pr_err("%s: Request allocation failed with err=%d\n",
@@ -963,7 +971,7 @@ static int live_sequential_engines(void *arg)
 			goto out_free;
 		}
 
-		request[idx] = i915_request_create(engine->kernel_context);
+		request[idx] = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(request[idx])) {
 			err = PTR_ERR(request[idx]);
 			pr_err("%s: Request allocation failed for %s with err=%d\n",
@@ -1068,15 +1076,19 @@ static int __live_parallel_engine1(void *arg)
 	struct intel_engine_cs *engine = arg;
 	IGT_TIMEOUT(end_time);
 	unsigned long count;
+	int err = 0;
 
 	count = 0;
+	intel_engine_pm_get(engine);
 	do {
 		struct i915_request *rq;
-		int err;
 
 		rq = i915_request_create(engine->kernel_context);
-		if (IS_ERR(rq))
-			return PTR_ERR(rq);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			if (err)
+				break;
+		}
 
 		i915_request_get(rq);
 		i915_request_add(rq);
@@ -1086,13 +1098,14 @@ static int __live_parallel_engine1(void *arg)
 			err = -ETIME;
 		i915_request_put(rq);
 		if (err)
-			return err;
+			break;
 
 		count++;
 	} while (!__igt_timeout(end_time, NULL));
+	intel_engine_pm_put(engine);
 
 	pr_info("%s: %lu request + sync\n", engine->name, count);
-	return 0;
+	return err;
 }
 
 static int __live_parallel_engineN(void *arg)
@@ -1100,21 +1113,26 @@ static int __live_parallel_engineN(void *arg)
 	struct intel_engine_cs *engine = arg;
 	IGT_TIMEOUT(end_time);
 	unsigned long count;
+	int err = 0;
 
 	count = 0;
+	intel_engine_pm_get(engine);
 	do {
 		struct i915_request *rq;
 
 		rq = i915_request_create(engine->kernel_context);
-		if (IS_ERR(rq))
-			return PTR_ERR(rq);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			break;
+		}
 
 		i915_request_add(rq);
 		count++;
 	} while (!__igt_timeout(end_time, NULL));
+	intel_engine_pm_put(engine);
 
 	pr_info("%s: %lu requests\n", engine->name, count);
-	return 0;
+	return err;
 }
 
 static bool wake_all(struct drm_i915_private *i915)
@@ -1158,9 +1176,11 @@ static int __live_parallel_spin(void *arg)
 		return -ENOMEM;
 	}
 
+	intel_engine_pm_get(engine);
 	rq = igt_spinner_create_request(&spin,
 					engine->kernel_context,
 					MI_NOOP); /* no preemption */
+	intel_engine_pm_put(engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		if (err == -ENODEV)
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index b60916561462..04d0aa7b349e 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -506,7 +506,9 @@ static int igt_lmem_write_cpu(void *arg)
 	}
 
 	/* Put the pages into a known state -- from the gpu for added fun */
+	intel_engine_pm_get(engine);
 	err = i915_gem_object_fill_blt(obj, engine->kernel_context, 0xdeadbeaf);
+	intel_engine_pm_put(engine);
 	if (err)
 		goto out_unpin;
 
-- 
2.24.0

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

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

* [Intel-gfx] [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context
@ 2019-11-24 17:05   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

As the engine->kernel_context is used within the engine-pm barrier, we
have to be careful when emitting requests outside of the barrier, as the
strict timeline locking rules do not apply. Instead, we must ensure the
engine_park() cannot be entered as we build the request, which is
simplest by taking an explicit engine-pm wakeref around the request
construction.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  3 +-
 .../i915/gem/selftests/i915_gem_client_blt.c  |  2 +
 .../i915/gem/selftests/i915_gem_coherency.c   |  3 +-
 .../drm/i915/gem/selftests/i915_gem_context.c |  7 +++-
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  3 +-
 .../i915/gem/selftests/i915_gem_object_blt.c  | 18 ++++++---
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 14 +++++--
 drivers/gpu/drm/i915/gt/intel_engine_pm.h     | 21 ++++++++++
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |  3 ++
 drivers/gpu/drm/i915/gt/selftest_context.c    |  2 +-
 drivers/gpu/drm/i915/gt/selftest_engine_cs.c  | 12 ++++++
 drivers/gpu/drm/i915/gt/selftest_lrc.c        |  6 +--
 drivers/gpu/drm/i915/gt/selftest_mocs.c       |  2 +
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  6 +--
 drivers/gpu/drm/i915/i915_perf.c              |  4 +-
 drivers/gpu/drm/i915/selftests/i915_active.c  |  2 +-
 drivers/gpu/drm/i915/selftests/i915_perf.c    |  2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c | 40 ++++++++++++++-----
 .../drm/i915/selftests/intel_memory_region.c  |  2 +
 19 files changed, 119 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 6f1e6181f67a..c94ac838401a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -70,6 +70,7 @@
 #include <drm/i915_drm.h>
 
 #include "gt/intel_engine_heartbeat.h"
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_engine_user.h"
 #include "gt/intel_lrc_reg.h"
 #include "gt/intel_ring.h"
@@ -1265,7 +1266,7 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
 	if (!intel_context_is_pinned(ce))
 		return 0;
 
-	rq = i915_request_create(ce->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(ce->engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
index da8edee4fe0a..b972be165e85 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
@@ -24,6 +24,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
 
 	prandom_seed_state(&prng, i915_selftest.random_seed);
 
+	intel_engine_pm_get(engine);
 	do {
 		const u32 max_block_size = S16_MAX * PAGE_SIZE;
 		u32 sz = min_t(u64, ce->vm->total >> 4, prandom_u32_state(&prng));
@@ -99,6 +100,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
 err_flush:
 	if (err == -ENOMEM)
 		err = 0;
+	intel_engine_pm_put(engine);
 
 	return err;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
index 2b29f6b4e1dd..9d3cd1af61f6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
@@ -6,6 +6,7 @@
 
 #include <linux/prime_numbers.h>
 
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_ring.h"
@@ -200,7 +201,7 @@ static int gpu_set(struct context *ctx, unsigned long offset, u32 v)
 	if (IS_ERR(vma))
 		return PTR_ERR(vma);
 
-	rq = i915_request_create(ctx->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(ctx->engine);
 	if (IS_ERR(rq)) {
 		i915_vma_unpin(vma);
 		return PTR_ERR(rq);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index e1d8ccd11409..2ea4790f3721 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -7,6 +7,7 @@
 #include <linux/prime_numbers.h>
 
 #include "gem/i915_gem_pm.h"
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_requests.h"
 #include "gt/intel_reset.h"
@@ -1190,9 +1191,11 @@ __sseu_test(const char *name,
 	struct igt_spinner *spin = NULL;
 	int ret;
 
+	intel_engine_pm_get(ce->engine);
+
 	ret = __sseu_prepare(name, flags, ce, &spin);
 	if (ret)
-		return ret;
+		goto out_pm;
 
 	ret = intel_context_reconfigure_sseu(ce, sseu);
 	if (ret)
@@ -1207,6 +1210,8 @@ __sseu_test(const char *name,
 		igt_spinner_fini(spin);
 		kfree(spin);
 	}
+out_pm:
+	intel_engine_pm_put(ce->engine);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..6ce9167f8c9f 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -6,6 +6,7 @@
 
 #include <linux/prime_numbers.h>
 
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "huge_gem_object.h"
@@ -536,7 +537,7 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
 		if (err)
 			return err;
 
-		rq = i915_request_create(engine->kernel_context);
+		rq = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(rq)) {
 			i915_vma_unpin(vma);
 			return PTR_ERR(rq);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
index 675c1a20a2f1..62077fe46715 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
@@ -41,6 +41,7 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
 		if (!engine)
 			return 0;
 
+		intel_engine_pm_get(engine);
 		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
 			struct intel_context *ce = engine->kernel_context;
 			ktime_t t0, t1;
@@ -49,17 +50,20 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
 
 			err = i915_gem_object_fill_blt(obj, ce, 0);
 			if (err)
-				return err;
+				break;
 
 			err = i915_gem_object_wait(obj,
 						   I915_WAIT_ALL,
 						   MAX_SCHEDULE_TIMEOUT);
 			if (err)
-				return err;
+				break;
 
 			t1 = ktime_get();
 			t[pass] = ktime_sub(t1, t0);
 		}
+		intel_engine_pm_put(engine);
+		if (err)
+			return err;
 
 		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
 		pr_info("%s: blt %zd KiB fill: %lld MiB/s\n",
@@ -109,6 +113,7 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
 		struct intel_engine_cs *engine;
 		ktime_t t[5];
 		int pass;
+		int err = 0;
 
 		engine = intel_engine_lookup_user(i915,
 						  I915_ENGINE_CLASS_COPY,
@@ -116,26 +121,29 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
 		if (!engine)
 			return 0;
 
+		intel_engine_pm_get(engine);
 		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
 			struct intel_context *ce = engine->kernel_context;
 			ktime_t t0, t1;
-			int err;
 
 			t0 = ktime_get();
 
 			err = i915_gem_object_copy_blt(src, dst, ce);
 			if (err)
-				return err;
+				break;
 
 			err = i915_gem_object_wait(dst,
 						   I915_WAIT_ALL,
 						   MAX_SCHEDULE_TIMEOUT);
 			if (err)
-				return err;
+				break;
 
 			t1 = ktime_get();
 			t[pass] = ktime_sub(t1, t0);
 		}
+		intel_engine_pm_put(engine);
+		if (err)
+			return err;
 
 		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
 		pr_info("%s: blt %zd KiB copy: %lld MiB/s\n",
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index c91fd4e4af29..742628e40201 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -215,18 +215,26 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
 int intel_engine_flush_barriers(struct intel_engine_cs *engine)
 {
 	struct i915_request *rq;
+	int err = 0;
 
 	if (llist_empty(&engine->barrier_tasks))
 		return 0;
 
+	if (!intel_engine_pm_get_if_awake(engine))
+		return 0;
+
 	rq = i915_request_create(engine->kernel_context);
-	if (IS_ERR(rq))
-		return PTR_ERR(rq);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto out_rpm;
+	}
 
 	idle_pulse(engine, rq);
 	i915_request_add(rq);
 
-	return 0;
+out_rpm:
+	intel_engine_pm_put(engine);
+	return err;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
index 24e20344dc22..e52c2b0cb245 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
@@ -7,6 +7,7 @@
 #ifndef INTEL_ENGINE_PM_H
 #define INTEL_ENGINE_PM_H
 
+#include "i915_request.h"
 #include "intel_engine_types.h"
 #include "intel_wakeref.h"
 
@@ -41,6 +42,26 @@ static inline void intel_engine_pm_flush(struct intel_engine_cs *engine)
 	intel_wakeref_unlock_wait(&engine->wakeref);
 }
 
+static inline struct i915_request *
+intel_engine_create_kernel_request(struct intel_engine_cs *engine)
+{
+	struct i915_request *rq;
+
+	/*
+	 * The engine->kernel_context is special as it is used inside
+	 * the engine-pm barrier (see __engine_park()), circumventing
+	 * the usual mutexes and relying on the engine-pm barrier
+	 * instead. So whenever we use the engine->kernel_context
+	 * outside of the barrier, we must manually handle the
+	 * engine wakeref to serialise with the use inside.
+	 */
+	intel_engine_pm_get(engine);
+	rq = i915_request_create(engine->kernel_context);
+	intel_engine_pm_put(engine);
+
+	return rq;
+}
+
 void intel_engine_init__pm(struct intel_engine_cs *engine);
 
 #endif /* INTEL_ENGINE_PM_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 226bd4cccb48..0c6d398980ba 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -6,6 +6,7 @@
 
 #include "i915_drv.h"
 #include "intel_context.h"
+#include "intel_engine_pm.h"
 #include "intel_gt.h"
 #include "intel_ring.h"
 #include "intel_workarounds.h"
@@ -1582,7 +1583,9 @@ static int engine_wa_list_verify(struct intel_context *ce,
 	if (IS_ERR(vma))
 		return PTR_ERR(vma);
 
+	intel_engine_pm_get(ce->engine);
 	rq = intel_context_create_request(ce);
+	intel_engine_pm_put(ce->engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto err_vma;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index 5bc124574170..af354ccdbf40 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -121,7 +121,7 @@ static int __live_context_size(struct intel_engine_cs *engine,
 		goto err_unpin;
 
 	/* Force the context switch */
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto err_unpin;
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
index 5981a7b71ec9..761d81f4bd68 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
@@ -132,14 +132,18 @@ static int perf_mi_bb_start(void *arg)
 		u32 cycles[COUNT];
 		int i;
 
+		intel_engine_pm_get(engine);
+
 		batch = create_empty_batch(ce);
 		if (IS_ERR(batch)) {
 			err = PTR_ERR(batch);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
 		err = i915_vma_sync(batch);
 		if (err) {
+			intel_engine_pm_put(engine);
 			i915_vma_put(batch);
 			break;
 		}
@@ -180,6 +184,7 @@ static int perf_mi_bb_start(void *arg)
 			cycles[i] = rq->hwsp_seqno[3] - rq->hwsp_seqno[2];
 		}
 		i915_vma_put(batch);
+		intel_engine_pm_put(engine);
 		if (err)
 			break;
 
@@ -251,15 +256,19 @@ static int perf_mi_noop(void *arg)
 		u32 cycles[COUNT];
 		int i;
 
+		intel_engine_pm_get(engine);
+
 		base = create_empty_batch(ce);
 		if (IS_ERR(base)) {
 			err = PTR_ERR(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
 		err = i915_vma_sync(base);
 		if (err) {
 			i915_vma_put(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
@@ -267,6 +276,7 @@ static int perf_mi_noop(void *arg)
 		if (IS_ERR(nop)) {
 			err = PTR_ERR(nop);
 			i915_vma_put(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
@@ -274,6 +284,7 @@ static int perf_mi_noop(void *arg)
 		if (err) {
 			i915_vma_put(nop);
 			i915_vma_put(base);
+			intel_engine_pm_put(engine);
 			break;
 		}
 
@@ -327,6 +338,7 @@ static int perf_mi_noop(void *arg)
 		}
 		i915_vma_put(nop);
 		i915_vma_put(base);
+		intel_engine_pm_put(engine);
 		if (err)
 			break;
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index fc142dd61dd1..ac8b9116d307 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -348,7 +348,7 @@ release_queue(struct intel_engine_cs *engine,
 	struct i915_request *rq;
 	u32 *cs;
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
@@ -497,7 +497,7 @@ static struct i915_request *nop_request(struct intel_engine_cs *engine)
 {
 	struct i915_request *rq;
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		return rq;
 
@@ -3698,7 +3698,7 @@ static int gpr_make_dirty(struct intel_engine_cs *engine)
 	u32 *cs;
 	int n;
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index a34d4fb52fa1..de010f527757 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -261,7 +261,9 @@ static int live_mocs_kernel(void *arg)
 		return err;
 
 	for_each_engine(engine, gt, id) {
+		intel_engine_pm_get(engine);
 		err = check_mocs_engine(&mocs, engine->kernel_context);
+		intel_engine_pm_put(engine);
 		if (err)
 			break;
 	}
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index f04a59fe5d2c..e2d78cc22fb4 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -458,7 +458,7 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
 		goto out;
 	}
 
-	rq = i915_request_create(engine->kernel_context);
+	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		goto out_unpin;
 
@@ -675,9 +675,7 @@ static int live_hwsp_wrap(void *arg)
 		if (!intel_engine_can_store_dword(engine))
 			continue;
 
-		intel_engine_pm_get(engine);
-		rq = i915_request_create(engine->kernel_context);
-		intel_engine_pm_put(engine);
+		rq = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
 			goto out;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 608e6c3f3c1a..b46715b57576 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1968,7 +1968,9 @@ static int emit_oa_config(struct i915_perf_stream *stream,
 	if (err)
 		goto err_vma_put;
 
+	intel_engine_pm_get(ce->engine);
 	rq = i915_request_create(ce);
+	intel_engine_pm_put(ce->engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto err_vma_unpin;
@@ -2165,7 +2167,7 @@ static int gen8_modify_context(struct intel_context *ce,
 
 	lockdep_assert_held(&ce->pin_mutex);
 
-	rq = i915_request_create(ce->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(ce->engine);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index 60290f78750d..6c1db3ded446 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -99,7 +99,7 @@ __live_active_setup(struct drm_i915_private *i915)
 	for_each_uabi_engine(engine, i915) {
 		struct i915_request *rq;
 
-		rq = i915_request_create(engine->kernel_context);
+		rq = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(rq)) {
 			err = PTR_ERR(rq);
 			break;
diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c b/drivers/gpu/drm/i915/selftests/i915_perf.c
index aabd07f67e49..d1a1568c47ba 100644
--- a/drivers/gpu/drm/i915/selftests/i915_perf.c
+++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
@@ -132,7 +132,7 @@ static int live_noa_delay(void *arg)
 	for (i = 0; i < 4; i++)
 		intel_write_status_page(stream->engine, 0x100 + i, 0);
 
-	rq = i915_request_create(stream->engine->kernel_context);
+	rq = intel_engine_create_kernel_request(stream->engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		goto out;
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
index c16d1efd2ad4..99c94b4f69fb 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -27,6 +27,7 @@
 #include "gem/i915_gem_pm.h"
 #include "gem/selftests/mock_context.h"
 
+#include "gt/intel_engine_pm.h"
 #include "gt/intel_gt.h"
 
 #include "i915_random.h"
@@ -541,6 +542,7 @@ static int live_nop_request(void *arg)
 		if (err)
 			return err;
 
+		intel_engine_pm_get(engine);
 		for_each_prime_number_from(prime, 1, 8192) {
 			struct i915_request *request = NULL;
 
@@ -579,6 +581,7 @@ static int live_nop_request(void *arg)
 			if (__igt_timeout(end_time, NULL))
 				break;
 		}
+		intel_engine_pm_put(engine);
 
 		err = igt_live_test_end(&t);
 		if (err)
@@ -693,10 +696,13 @@ static int live_empty_request(void *arg)
 		if (err)
 			goto out_batch;
 
+		intel_engine_pm_get(engine);
+
 		/* Warmup / preload */
 		request = empty_request(engine, batch);
 		if (IS_ERR(request)) {
 			err = PTR_ERR(request);
+			intel_engine_pm_put(engine);
 			goto out_batch;
 		}
 		i915_request_wait(request, 0, MAX_SCHEDULE_TIMEOUT);
@@ -709,6 +715,7 @@ static int live_empty_request(void *arg)
 				request = empty_request(engine, batch);
 				if (IS_ERR(request)) {
 					err = PTR_ERR(request);
+					intel_engine_pm_put(engine);
 					goto out_batch;
 				}
 			}
@@ -722,6 +729,7 @@ static int live_empty_request(void *arg)
 				break;
 		}
 		i915_request_put(request);
+		intel_engine_pm_put(engine);
 
 		err = igt_live_test_end(&t);
 		if (err)
@@ -846,7 +854,7 @@ static int live_all_engines(void *arg)
 
 	idx = 0;
 	for_each_uabi_engine(engine, i915) {
-		request[idx] = i915_request_create(engine->kernel_context);
+		request[idx] = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(request[idx])) {
 			err = PTR_ERR(request[idx]);
 			pr_err("%s: Request allocation failed with err=%d\n",
@@ -963,7 +971,7 @@ static int live_sequential_engines(void *arg)
 			goto out_free;
 		}
 
-		request[idx] = i915_request_create(engine->kernel_context);
+		request[idx] = intel_engine_create_kernel_request(engine);
 		if (IS_ERR(request[idx])) {
 			err = PTR_ERR(request[idx]);
 			pr_err("%s: Request allocation failed for %s with err=%d\n",
@@ -1068,15 +1076,19 @@ static int __live_parallel_engine1(void *arg)
 	struct intel_engine_cs *engine = arg;
 	IGT_TIMEOUT(end_time);
 	unsigned long count;
+	int err = 0;
 
 	count = 0;
+	intel_engine_pm_get(engine);
 	do {
 		struct i915_request *rq;
-		int err;
 
 		rq = i915_request_create(engine->kernel_context);
-		if (IS_ERR(rq))
-			return PTR_ERR(rq);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			if (err)
+				break;
+		}
 
 		i915_request_get(rq);
 		i915_request_add(rq);
@@ -1086,13 +1098,14 @@ static int __live_parallel_engine1(void *arg)
 			err = -ETIME;
 		i915_request_put(rq);
 		if (err)
-			return err;
+			break;
 
 		count++;
 	} while (!__igt_timeout(end_time, NULL));
+	intel_engine_pm_put(engine);
 
 	pr_info("%s: %lu request + sync\n", engine->name, count);
-	return 0;
+	return err;
 }
 
 static int __live_parallel_engineN(void *arg)
@@ -1100,21 +1113,26 @@ static int __live_parallel_engineN(void *arg)
 	struct intel_engine_cs *engine = arg;
 	IGT_TIMEOUT(end_time);
 	unsigned long count;
+	int err = 0;
 
 	count = 0;
+	intel_engine_pm_get(engine);
 	do {
 		struct i915_request *rq;
 
 		rq = i915_request_create(engine->kernel_context);
-		if (IS_ERR(rq))
-			return PTR_ERR(rq);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			break;
+		}
 
 		i915_request_add(rq);
 		count++;
 	} while (!__igt_timeout(end_time, NULL));
+	intel_engine_pm_put(engine);
 
 	pr_info("%s: %lu requests\n", engine->name, count);
-	return 0;
+	return err;
 }
 
 static bool wake_all(struct drm_i915_private *i915)
@@ -1158,9 +1176,11 @@ static int __live_parallel_spin(void *arg)
 		return -ENOMEM;
 	}
 
+	intel_engine_pm_get(engine);
 	rq = igt_spinner_create_request(&spin,
 					engine->kernel_context,
 					MI_NOOP); /* no preemption */
+	intel_engine_pm_put(engine);
 	if (IS_ERR(rq)) {
 		err = PTR_ERR(rq);
 		if (err == -ENODEV)
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index b60916561462..04d0aa7b349e 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -506,7 +506,9 @@ static int igt_lmem_write_cpu(void *arg)
 	}
 
 	/* Put the pages into a known state -- from the gpu for added fun */
+	intel_engine_pm_get(engine);
 	err = i915_gem_object_fill_blt(obj, engine->kernel_context, 0xdeadbeaf);
+	intel_engine_pm_put(engine);
 	if (err)
 		goto out_unpin;
 
-- 
2.24.0

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

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

* [CI 3/4] drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire
@ 2019-11-24 17:05   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

In the next patch, we will introduce a new asynchronous retirement
worker, fed by execlists CS events. Here we may queue a retirement as
soon as a request is submitted to HW (and completes instantly), and we
also want to process that retirement as early as possible and cannot
afford to postpone (as there may not be another opportunity to retire it
for a few seconds). To allow the new async retirer to run in parallel
with our submission, pull the __i915_request_queue (that passes the
request to HW) inside the timelines spinlock so that the retirement
cannot release the timeline before we have completed the submission.

v2: Actually to play nicely with engine_retire, we have to raise the
timeline.active_lock before releasing the HW. intel_gt_retire_requsts()
is still serialised by the outer lock so they cannot see this
intermediate state, and engine_retire is serialised by HW submission.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_pm.c | 27 ++++++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 373a4b9f159c..0e1ad4a4bd97 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -74,16 +74,33 @@ static inline void __timeline_mark_unlock(struct intel_context *ce,
 #endif /* !IS_ENABLED(CONFIG_LOCKDEP) */
 
 static void
-__intel_timeline_enter_and_release_pm(struct intel_timeline *tl,
-				      struct intel_engine_cs *engine)
+__queue_and_release_pm(struct i915_request *rq,
+		       struct intel_timeline *tl,
+		       struct intel_engine_cs *engine)
 {
 	struct intel_gt_timelines *timelines = &engine->gt->timelines;
 
+	GEM_TRACE("%s\n", engine->name);
+
+	/*
+	 * We have to serialise all potential retirement paths with our
+	 * submission, as we don't want to underflow either the
+	 * engine->wakeref.counter or our timeline->active_count.
+	 *
+	 * Equally, we cannot allow a new submission to start until
+	 * after we finish queueing, nor could we allow that submitter
+	 * to retire us before we are ready!
+	 */
 	spin_lock(&timelines->lock);
 
+	/* Let intel_gt_retire_requests() retire us (acquired under lock) */
 	if (!atomic_fetch_inc(&tl->active_count))
 		list_add_tail(&tl->link, &timelines->active_list);
 
+	/* Hand the request over to HW and so engine_retire() */
+	__i915_request_queue(rq, NULL);
+
+	/* Let new submissions commence (and maybe retire this timeline) */
 	__intel_wakeref_defer_park(&engine->wakeref);
 
 	spin_unlock(&timelines->lock);
@@ -148,10 +165,8 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 	rq->sched.attr.priority = I915_PRIORITY_BARRIER;
 	__i915_request_commit(rq);
 
-	__i915_request_queue(rq, NULL);
-
-	/* Expose ourselves to intel_gt_retire_requests() and new submission */
-	__intel_timeline_enter_and_release_pm(ce->timeline, engine);
+	/* Expose ourselves to the world */
+	__queue_and_release_pm(rq, ce->timeline, engine);
 
 	result = false;
 out_unlock:
-- 
2.24.0

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

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

* [Intel-gfx] [CI 3/4] drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire
@ 2019-11-24 17:05   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

In the next patch, we will introduce a new asynchronous retirement
worker, fed by execlists CS events. Here we may queue a retirement as
soon as a request is submitted to HW (and completes instantly), and we
also want to process that retirement as early as possible and cannot
afford to postpone (as there may not be another opportunity to retire it
for a few seconds). To allow the new async retirer to run in parallel
with our submission, pull the __i915_request_queue (that passes the
request to HW) inside the timelines spinlock so that the retirement
cannot release the timeline before we have completed the submission.

v2: Actually to play nicely with engine_retire, we have to raise the
timeline.active_lock before releasing the HW. intel_gt_retire_requsts()
is still serialised by the outer lock so they cannot see this
intermediate state, and engine_retire is serialised by HW submission.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_pm.c | 27 ++++++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 373a4b9f159c..0e1ad4a4bd97 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -74,16 +74,33 @@ static inline void __timeline_mark_unlock(struct intel_context *ce,
 #endif /* !IS_ENABLED(CONFIG_LOCKDEP) */
 
 static void
-__intel_timeline_enter_and_release_pm(struct intel_timeline *tl,
-				      struct intel_engine_cs *engine)
+__queue_and_release_pm(struct i915_request *rq,
+		       struct intel_timeline *tl,
+		       struct intel_engine_cs *engine)
 {
 	struct intel_gt_timelines *timelines = &engine->gt->timelines;
 
+	GEM_TRACE("%s\n", engine->name);
+
+	/*
+	 * We have to serialise all potential retirement paths with our
+	 * submission, as we don't want to underflow either the
+	 * engine->wakeref.counter or our timeline->active_count.
+	 *
+	 * Equally, we cannot allow a new submission to start until
+	 * after we finish queueing, nor could we allow that submitter
+	 * to retire us before we are ready!
+	 */
 	spin_lock(&timelines->lock);
 
+	/* Let intel_gt_retire_requests() retire us (acquired under lock) */
 	if (!atomic_fetch_inc(&tl->active_count))
 		list_add_tail(&tl->link, &timelines->active_list);
 
+	/* Hand the request over to HW and so engine_retire() */
+	__i915_request_queue(rq, NULL);
+
+	/* Let new submissions commence (and maybe retire this timeline) */
 	__intel_wakeref_defer_park(&engine->wakeref);
 
 	spin_unlock(&timelines->lock);
@@ -148,10 +165,8 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 	rq->sched.attr.priority = I915_PRIORITY_BARRIER;
 	__i915_request_commit(rq);
 
-	__i915_request_queue(rq, NULL);
-
-	/* Expose ourselves to intel_gt_retire_requests() and new submission */
-	__intel_timeline_enter_and_release_pm(ce->timeline, engine);
+	/* Expose ourselves to the world */
+	__queue_and_release_pm(rq, ce->timeline, engine);
 
 	result = false;
 out_unlock:
-- 
2.24.0

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

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

* [CI 4/4] drm/i915/gt: Schedule request retirement when timeline idles
@ 2019-11-24 17:05   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

The major drawback of commit 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX
corruption WA") is that it disables RC6 while Skylake (and friends) is
active, and we do not consider the GPU idle until all outstanding
requests have been retired and the engine switched over to the kernel
context. If userspace is idle, this task falls onto our background idle
worker, which only runs roughly once a second, meaning that userspace has
to have been idle for a couple of seconds before we enable RC6 again.
Naturally, this causes us to consume considerably more energy than
before as powersaving is effectively disabled while a display server
(here's looking at you Xorg) is running.

As execlists will get a completion event as each context is completed,
we can use this interrupt to queue a retire worker bound to this engine
to cleanup idle timelines. We will then immediately notice the idle
engine (without userspace intervention or the aid of the background
retire worker) and start parking the GPU. Thus during light workloads,
we will do much more work to idle the GPU faster...  Hopefully with
commensurate power saving!

v2: Watch context completions and only look at those local to the engine
when retiring to reduce the amount of excess work we perform.

References: https://bugs.freedesktop.org/show_bug.cgi?id=112315
References: 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")
References: 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  8 +-
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  8 ++
 drivers/gpu/drm/i915/gt/intel_gt_requests.c   | 75 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_requests.h   |  7 ++
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  9 +++
 drivers/gpu/drm/i915/gt/intel_timeline.c      |  1 +
 .../gpu/drm/i915/gt/intel_timeline_types.h    |  3 +
 7 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index b9613d044393..8f6e353caa66 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -28,13 +28,13 @@
 
 #include "i915_drv.h"
 
-#include "gt/intel_gt.h"
-
+#include "intel_context.h"
 #include "intel_engine.h"
 #include "intel_engine_pm.h"
 #include "intel_engine_pool.h"
 #include "intel_engine_user.h"
-#include "intel_context.h"
+#include "intel_gt.h"
+#include "intel_gt_requests.h"
 #include "intel_lrc.h"
 #include "intel_reset.h"
 #include "intel_ring.h"
@@ -617,6 +617,7 @@ static int intel_engine_setup_common(struct intel_engine_cs *engine)
 	intel_engine_init_execlists(engine);
 	intel_engine_init_cmd_parser(engine);
 	intel_engine_init__pm(engine);
+	intel_engine_init_retire(engine);
 
 	intel_engine_pool_init(&engine->pool);
 
@@ -839,6 +840,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 
 	cleanup_status_page(engine);
 
+	intel_engine_fini_retire(engine);
 	intel_engine_pool_fini(&engine->pool);
 	intel_engine_fini_breadcrumbs(engine);
 	intel_engine_cleanup_cmd_parser(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 758f0e8ec672..17f1f1441efc 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -451,6 +451,14 @@ struct intel_engine_cs {
 
 	struct intel_engine_execlists execlists;
 
+	/*
+	 * Keep track of completed timelines on this engine for early
+	 * retirement with the goal of quickly enabling powersaving as
+	 * soon as the engine is idle.
+	 */
+	struct intel_timeline *retire;
+	struct work_struct retire_work;
+
 	/* status_notifier: list of callbacks for context-switch changes */
 	struct atomic_notifier_head context_status_notifier;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index f02f781b8492..8cb5421e5f0e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -4,6 +4,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include <linux/workqueue.h>
+
 #include "i915_drv.h" /* for_each_engine() */
 #include "i915_request.h"
 #include "intel_gt.h"
@@ -29,6 +31,79 @@ static void flush_submission(struct intel_gt *gt)
 		intel_engine_flush_submission(engine);
 }
 
+static void engine_retire(struct work_struct *work)
+{
+	struct intel_engine_cs *engine =
+		container_of(work, typeof(*engine), retire_work);
+	struct intel_timeline *tl = xchg(&engine->retire, NULL);
+
+	do {
+		struct intel_timeline *next = xchg(&tl->retire, NULL);
+
+		/*
+		 * Our goal here is to retire _idle_ timelines as soon as
+		 * possible (as they are idle, we do not expect userspace
+		 * to be cleaning up anytime soon).
+		 *
+		 * If the timeline is currently locked, either it is being
+		 * retired elsewhere or about to be!
+		 */
+		if (mutex_trylock(&tl->mutex)) {
+			retire_requests(tl);
+			mutex_unlock(&tl->mutex);
+		}
+		intel_timeline_put(tl);
+
+		GEM_BUG_ON(!next);
+		tl = ptr_mask_bits(next, 1);
+	} while (tl);
+}
+
+static bool add_retire(struct intel_engine_cs *engine,
+		       struct intel_timeline *tl)
+{
+	struct intel_timeline *first;
+
+	/*
+	 * We open-code a llist here to include the additional tag [BIT(0)]
+	 * so that we know when the timeline is already on a
+	 * retirement queue: either this engine or another.
+	 *
+	 * However, we rely on that a timeline can only be active on a single
+	 * engine at any one time and that add_retire() is called before the
+	 * engine releases the timeline and transferred to another to retire.
+	 */
+
+	if (READ_ONCE(tl->retire)) /* already queued */
+		return false;
+
+	intel_timeline_get(tl);
+	first = READ_ONCE(engine->retire);
+	do
+		tl->retire = ptr_pack_bits(first, 1, 1);
+	while (!try_cmpxchg(&engine->retire, &first, tl));
+
+	return !first;
+}
+
+void intel_engine_add_retire(struct intel_engine_cs *engine,
+			     struct intel_timeline *tl)
+{
+	if (add_retire(engine, tl))
+		schedule_work(&engine->retire_work);
+}
+
+void intel_engine_init_retire(struct intel_engine_cs *engine)
+{
+	INIT_WORK(&engine->retire_work, engine_retire);
+}
+
+void intel_engine_fini_retire(struct intel_engine_cs *engine)
+{
+	flush_work(&engine->retire_work);
+	GEM_BUG_ON(engine->retire);
+}
+
 long intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout)
 {
 	struct intel_gt_timelines *timelines = &gt->timelines;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.h b/drivers/gpu/drm/i915/gt/intel_gt_requests.h
index fde546424c63..dbac53baf1cb 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.h
@@ -7,7 +7,9 @@
 #ifndef INTEL_GT_REQUESTS_H
 #define INTEL_GT_REQUESTS_H
 
+struct intel_engine_cs;
 struct intel_gt;
+struct intel_timeline;
 
 long intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout);
 static inline void intel_gt_retire_requests(struct intel_gt *gt)
@@ -15,6 +17,11 @@ static inline void intel_gt_retire_requests(struct intel_gt *gt)
 	intel_gt_retire_requests_timeout(gt, 0);
 }
 
+void intel_engine_init_retire(struct intel_engine_cs *engine);
+void intel_engine_add_retire(struct intel_engine_cs *engine,
+			     struct intel_timeline *tl);
+void intel_engine_fini_retire(struct intel_engine_cs *engine);
+
 int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout);
 
 void intel_gt_init_requests(struct intel_gt *gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 0d0dca3d6724..be3c6f6aa102 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -142,6 +142,7 @@
 #include "intel_engine_pm.h"
 #include "intel_gt.h"
 #include "intel_gt_pm.h"
+#include "intel_gt_requests.h"
 #include "intel_lrc_reg.h"
 #include "intel_mocs.h"
 #include "intel_reset.h"
@@ -1170,6 +1171,14 @@ __execlists_schedule_out(struct i915_request *rq,
 	 * refrain from doing non-trivial work here.
 	 */
 
+	/*
+	 * If we have just completed this context, the engine may now be
+	 * idle and we want to re-enter powersaving.
+	 */
+	if (list_is_last(&rq->link, &ce->timeline->requests) &&
+	    i915_request_completed(rq))
+		intel_engine_add_retire(engine, ce->timeline);
+
 	intel_engine_context_out(engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 	intel_gt_pm_put_async(engine->gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index b190a5d9ab02..c1d2419444f8 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -277,6 +277,7 @@ void intel_timeline_fini(struct intel_timeline *timeline)
 {
 	GEM_BUG_ON(atomic_read(&timeline->pin_count));
 	GEM_BUG_ON(!list_empty(&timeline->requests));
+	GEM_BUG_ON(timeline->retire);
 
 	if (timeline->hwsp_cacheline)
 		cacheline_free(timeline->hwsp_cacheline);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline_types.h b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
index 5244615ed1cb..aaf15cbe1ce1 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
@@ -66,6 +66,9 @@ struct intel_timeline {
 	 */
 	struct i915_active_fence last_request;
 
+	/** A chain of completed timelines ready for early retirement. */
+	struct intel_timeline *retire;
+
 	/**
 	 * We track the most recent seqno that we wait on in every context so
 	 * that we only have to emit a new await and dependency on a more
-- 
2.24.0

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

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

* [Intel-gfx] [CI 4/4] drm/i915/gt: Schedule request retirement when timeline idles
@ 2019-11-24 17:05   ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-24 17:05 UTC (permalink / raw)
  To: intel-gfx

The major drawback of commit 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX
corruption WA") is that it disables RC6 while Skylake (and friends) is
active, and we do not consider the GPU idle until all outstanding
requests have been retired and the engine switched over to the kernel
context. If userspace is idle, this task falls onto our background idle
worker, which only runs roughly once a second, meaning that userspace has
to have been idle for a couple of seconds before we enable RC6 again.
Naturally, this causes us to consume considerably more energy than
before as powersaving is effectively disabled while a display server
(here's looking at you Xorg) is running.

As execlists will get a completion event as each context is completed,
we can use this interrupt to queue a retire worker bound to this engine
to cleanup idle timelines. We will then immediately notice the idle
engine (without userspace intervention or the aid of the background
retire worker) and start parking the GPU. Thus during light workloads,
we will do much more work to idle the GPU faster...  Hopefully with
commensurate power saving!

v2: Watch context completions and only look at those local to the engine
when retiring to reduce the amount of excess work we perform.

References: https://bugs.freedesktop.org/show_bug.cgi?id=112315
References: 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")
References: 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  8 +-
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  8 ++
 drivers/gpu/drm/i915/gt/intel_gt_requests.c   | 75 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_requests.h   |  7 ++
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  9 +++
 drivers/gpu/drm/i915/gt/intel_timeline.c      |  1 +
 .../gpu/drm/i915/gt/intel_timeline_types.h    |  3 +
 7 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index b9613d044393..8f6e353caa66 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -28,13 +28,13 @@
 
 #include "i915_drv.h"
 
-#include "gt/intel_gt.h"
-
+#include "intel_context.h"
 #include "intel_engine.h"
 #include "intel_engine_pm.h"
 #include "intel_engine_pool.h"
 #include "intel_engine_user.h"
-#include "intel_context.h"
+#include "intel_gt.h"
+#include "intel_gt_requests.h"
 #include "intel_lrc.h"
 #include "intel_reset.h"
 #include "intel_ring.h"
@@ -617,6 +617,7 @@ static int intel_engine_setup_common(struct intel_engine_cs *engine)
 	intel_engine_init_execlists(engine);
 	intel_engine_init_cmd_parser(engine);
 	intel_engine_init__pm(engine);
+	intel_engine_init_retire(engine);
 
 	intel_engine_pool_init(&engine->pool);
 
@@ -839,6 +840,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 
 	cleanup_status_page(engine);
 
+	intel_engine_fini_retire(engine);
 	intel_engine_pool_fini(&engine->pool);
 	intel_engine_fini_breadcrumbs(engine);
 	intel_engine_cleanup_cmd_parser(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 758f0e8ec672..17f1f1441efc 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -451,6 +451,14 @@ struct intel_engine_cs {
 
 	struct intel_engine_execlists execlists;
 
+	/*
+	 * Keep track of completed timelines on this engine for early
+	 * retirement with the goal of quickly enabling powersaving as
+	 * soon as the engine is idle.
+	 */
+	struct intel_timeline *retire;
+	struct work_struct retire_work;
+
 	/* status_notifier: list of callbacks for context-switch changes */
 	struct atomic_notifier_head context_status_notifier;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index f02f781b8492..8cb5421e5f0e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -4,6 +4,8 @@
  * Copyright © 2019 Intel Corporation
  */
 
+#include <linux/workqueue.h>
+
 #include "i915_drv.h" /* for_each_engine() */
 #include "i915_request.h"
 #include "intel_gt.h"
@@ -29,6 +31,79 @@ static void flush_submission(struct intel_gt *gt)
 		intel_engine_flush_submission(engine);
 }
 
+static void engine_retire(struct work_struct *work)
+{
+	struct intel_engine_cs *engine =
+		container_of(work, typeof(*engine), retire_work);
+	struct intel_timeline *tl = xchg(&engine->retire, NULL);
+
+	do {
+		struct intel_timeline *next = xchg(&tl->retire, NULL);
+
+		/*
+		 * Our goal here is to retire _idle_ timelines as soon as
+		 * possible (as they are idle, we do not expect userspace
+		 * to be cleaning up anytime soon).
+		 *
+		 * If the timeline is currently locked, either it is being
+		 * retired elsewhere or about to be!
+		 */
+		if (mutex_trylock(&tl->mutex)) {
+			retire_requests(tl);
+			mutex_unlock(&tl->mutex);
+		}
+		intel_timeline_put(tl);
+
+		GEM_BUG_ON(!next);
+		tl = ptr_mask_bits(next, 1);
+	} while (tl);
+}
+
+static bool add_retire(struct intel_engine_cs *engine,
+		       struct intel_timeline *tl)
+{
+	struct intel_timeline *first;
+
+	/*
+	 * We open-code a llist here to include the additional tag [BIT(0)]
+	 * so that we know when the timeline is already on a
+	 * retirement queue: either this engine or another.
+	 *
+	 * However, we rely on that a timeline can only be active on a single
+	 * engine at any one time and that add_retire() is called before the
+	 * engine releases the timeline and transferred to another to retire.
+	 */
+
+	if (READ_ONCE(tl->retire)) /* already queued */
+		return false;
+
+	intel_timeline_get(tl);
+	first = READ_ONCE(engine->retire);
+	do
+		tl->retire = ptr_pack_bits(first, 1, 1);
+	while (!try_cmpxchg(&engine->retire, &first, tl));
+
+	return !first;
+}
+
+void intel_engine_add_retire(struct intel_engine_cs *engine,
+			     struct intel_timeline *tl)
+{
+	if (add_retire(engine, tl))
+		schedule_work(&engine->retire_work);
+}
+
+void intel_engine_init_retire(struct intel_engine_cs *engine)
+{
+	INIT_WORK(&engine->retire_work, engine_retire);
+}
+
+void intel_engine_fini_retire(struct intel_engine_cs *engine)
+{
+	flush_work(&engine->retire_work);
+	GEM_BUG_ON(engine->retire);
+}
+
 long intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout)
 {
 	struct intel_gt_timelines *timelines = &gt->timelines;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.h b/drivers/gpu/drm/i915/gt/intel_gt_requests.h
index fde546424c63..dbac53baf1cb 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.h
@@ -7,7 +7,9 @@
 #ifndef INTEL_GT_REQUESTS_H
 #define INTEL_GT_REQUESTS_H
 
+struct intel_engine_cs;
 struct intel_gt;
+struct intel_timeline;
 
 long intel_gt_retire_requests_timeout(struct intel_gt *gt, long timeout);
 static inline void intel_gt_retire_requests(struct intel_gt *gt)
@@ -15,6 +17,11 @@ static inline void intel_gt_retire_requests(struct intel_gt *gt)
 	intel_gt_retire_requests_timeout(gt, 0);
 }
 
+void intel_engine_init_retire(struct intel_engine_cs *engine);
+void intel_engine_add_retire(struct intel_engine_cs *engine,
+			     struct intel_timeline *tl);
+void intel_engine_fini_retire(struct intel_engine_cs *engine);
+
 int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout);
 
 void intel_gt_init_requests(struct intel_gt *gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 0d0dca3d6724..be3c6f6aa102 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -142,6 +142,7 @@
 #include "intel_engine_pm.h"
 #include "intel_gt.h"
 #include "intel_gt_pm.h"
+#include "intel_gt_requests.h"
 #include "intel_lrc_reg.h"
 #include "intel_mocs.h"
 #include "intel_reset.h"
@@ -1170,6 +1171,14 @@ __execlists_schedule_out(struct i915_request *rq,
 	 * refrain from doing non-trivial work here.
 	 */
 
+	/*
+	 * If we have just completed this context, the engine may now be
+	 * idle and we want to re-enter powersaving.
+	 */
+	if (list_is_last(&rq->link, &ce->timeline->requests) &&
+	    i915_request_completed(rq))
+		intel_engine_add_retire(engine, ce->timeline);
+
 	intel_engine_context_out(engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 	intel_gt_pm_put_async(engine->gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index b190a5d9ab02..c1d2419444f8 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -277,6 +277,7 @@ void intel_timeline_fini(struct intel_timeline *timeline)
 {
 	GEM_BUG_ON(atomic_read(&timeline->pin_count));
 	GEM_BUG_ON(!list_empty(&timeline->requests));
+	GEM_BUG_ON(timeline->retire);
 
 	if (timeline->hwsp_cacheline)
 		cacheline_free(timeline->hwsp_cacheline);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline_types.h b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
index 5244615ed1cb..aaf15cbe1ce1 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
@@ -66,6 +66,9 @@ struct intel_timeline {
 	 */
 	struct i915_active_fence last_request;
 
+	/** A chain of completed timelines ready for early retirement. */
+	struct intel_timeline *retire;
+
 	/**
 	 * We track the most recent seqno that we wait on in every context so
 	 * that we only have to emit a new await and dependency on a more
-- 
2.24.0

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 17:12   ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-11-24 17:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
URL   : https://patchwork.freedesktop.org/series/69949/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
635aca2f93e1 drm/i915/gt: Mark the execlists->active as the primary volatile access
c3a0716fc1d3 drm/i915: Serialise with engine-pm around requests on the kernel_context
a1197310bd25 drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire
4af249323fd0 drm/i915/gt: Schedule request retirement when timeline idles
-:29: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")'
#29: 
References: 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")

-:30: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")'
#30: 
References: 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")

total: 2 errors, 0 warnings, 0 checks, 188 lines checked

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

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 17:12   ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-11-24 17:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
URL   : https://patchwork.freedesktop.org/series/69949/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
635aca2f93e1 drm/i915/gt: Mark the execlists->active as the primary volatile access
c3a0716fc1d3 drm/i915: Serialise with engine-pm around requests on the kernel_context
a1197310bd25 drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire
4af249323fd0 drm/i915/gt: Schedule request retirement when timeline idles
-:29: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")'
#29: 
References: 7e34f4e4aad3 ("drm/i915/gen8+: Add RC6 CTX corruption WA")

-:30: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")'
#30: 
References: 2248a28384fe ("drm/i915/gen8+: Add RC6 CTX corruption WA")

total: 2 errors, 0 warnings, 0 checks, 188 lines checked

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

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

* ✓ Fi.CI.BAT: success for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 17:36   ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-11-24 17:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
URL   : https://patchwork.freedesktop.org/series/69949/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7411 -> Patchwork_15413
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/index.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][1] -> [DMESG-WARN][2] ([fdo#112261])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_hangcheck:
    - fi-hsw-4770r:       [PASS][3] -> [DMESG-FAIL][4] ([fdo#111991])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][5] -> [FAIL][6] ([fdo#111045] / [fdo#111096])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-hsw-4770r:       [PASS][7] -> [DMESG-WARN][8] ([fdo#112353])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-hsw-4770r/igt@kms_flip@basic-flip-vs-dpms.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-hsw-4770r/igt@kms_flip@basic-flip-vs-dpms.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - {fi-kbl-7560u}:     [INCOMPLETE][9] ([fdo#109964] / [fdo#112298]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-7560u/igt@i915_module_load@reload-with-fault-injection.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-7560u/igt@i915_module_load@reload-with-fault-injection.html

  
#### Warnings ####

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][11] ([fdo#103558] / [fdo#105602] / [fdo#105763]) -> [DMESG-WARN][12] ([fdo#103558] / [fdo#105602]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([fdo#103558] / [fdo#105602]) -> [DMESG-WARN][14] ([fdo#103558] / [fdo#105602] / [fdo#105763]) +8 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-x1275/igt@kms_force_connector_basic@prune-stale-modes.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-x1275/igt@kms_force_connector_basic@prune-stale-modes.html

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

  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991
  [fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261
  [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
  [fdo#112353]: https://bugs.freedesktop.org/show_bug.cgi?id=112353


Participating hosts (46 -> 41)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7411 -> Patchwork_15413

  CI-20190529: 20190529
  CI_DRM_7411: 9e8df10785d3ae60d3b54bc45e06aa32621a472c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5303: 360b11e511d98b6370134cff6e8ec3c434a65aee @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15413: 4af249323fd0af00c3a2008987efdbd6118b41a1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4af249323fd0 drm/i915/gt: Schedule request retirement when timeline idles
a1197310bd25 drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire
c3a0716fc1d3 drm/i915: Serialise with engine-pm around requests on the kernel_context
635aca2f93e1 drm/i915/gt: Mark the execlists->active as the primary volatile access

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 17:36   ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-11-24 17:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
URL   : https://patchwork.freedesktop.org/series/69949/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7411 -> Patchwork_15413
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/index.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][1] -> [DMESG-WARN][2] ([fdo#112261])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_hangcheck:
    - fi-hsw-4770r:       [PASS][3] -> [DMESG-FAIL][4] ([fdo#111991])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][5] -> [FAIL][6] ([fdo#111045] / [fdo#111096])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-hsw-4770r:       [PASS][7] -> [DMESG-WARN][8] ([fdo#112353])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-hsw-4770r/igt@kms_flip@basic-flip-vs-dpms.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-hsw-4770r/igt@kms_flip@basic-flip-vs-dpms.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - {fi-kbl-7560u}:     [INCOMPLETE][9] ([fdo#109964] / [fdo#112298]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-7560u/igt@i915_module_load@reload-with-fault-injection.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-7560u/igt@i915_module_load@reload-with-fault-injection.html

  
#### Warnings ####

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][11] ([fdo#103558] / [fdo#105602] / [fdo#105763]) -> [DMESG-WARN][12] ([fdo#103558] / [fdo#105602]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([fdo#103558] / [fdo#105602]) -> [DMESG-WARN][14] ([fdo#103558] / [fdo#105602] / [fdo#105763]) +8 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/fi-kbl-x1275/igt@kms_force_connector_basic@prune-stale-modes.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/fi-kbl-x1275/igt@kms_force_connector_basic@prune-stale-modes.html

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

  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991
  [fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261
  [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
  [fdo#112353]: https://bugs.freedesktop.org/show_bug.cgi?id=112353


Participating hosts (46 -> 41)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7411 -> Patchwork_15413

  CI-20190529: 20190529
  CI_DRM_7411: 9e8df10785d3ae60d3b54bc45e06aa32621a472c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5303: 360b11e511d98b6370134cff6e8ec3c434a65aee @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15413: 4af249323fd0af00c3a2008987efdbd6118b41a1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4af249323fd0 drm/i915/gt: Schedule request retirement when timeline idles
a1197310bd25 drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire
c3a0716fc1d3 drm/i915: Serialise with engine-pm around requests on the kernel_context
635aca2f93e1 drm/i915/gt: Mark the execlists->active as the primary volatile access

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 22:42   ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-11-24 22:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
URL   : https://patchwork.freedesktop.org/series/69949/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7411_full -> Patchwork_15413_full
====================================================

Summary
-------

  **FAILURE**

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

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_param@vm:
    - shard-skl:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl3/igt@gem_ctx_param@vm.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl3/igt@gem_ctx_param@vm.html
    - shard-glk:          [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk5/igt@gem_ctx_param@vm.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk3/igt@gem_ctx_param@vm.html
    - shard-apl:          [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-apl2/igt@gem_ctx_param@vm.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-apl3/igt@gem_ctx_param@vm.html
    - shard-tglb:         [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@gem_ctx_param@vm.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb5/igt@gem_ctx_param@vm.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-render:
    - shard-tglb:         [PASS][9] -> [INCOMPLETE][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-render.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-skl:          [PASS][11] -> [INCOMPLETE][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl10/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl9/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          NOTRUN -> [INCOMPLETE][13]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@kms_plane@pixel-format-pipe-b-planes.html

  * igt@perf@oa-exponents:
    - shard-kbl:          [PASS][14] -> [TIMEOUT][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl1/igt@perf@oa-exponents.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl2/igt@perf@oa-exponents.html

  * igt@runner@aborted:
    - shard-kbl:          NOTRUN -> [FAIL][16]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl2/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@q-promotion-bsd2:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109276]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_ctx_shared@q-promotion-bsd2.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_ctx_shared@q-promotion-bsd2.html

  * igt@gem_exec_parallel@fds:
    - shard-tglb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#111867])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb9/igt@gem_exec_parallel@fds.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb6/igt@gem_exec_parallel@fds.html

  * igt@gem_exec_schedule@preempt-queue-chain-bsd2:
    - shard-tglb:         [PASS][21] -> [INCOMPLETE][22] ([fdo#111677])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb9/igt@gem_exec_schedule@preempt-queue-chain-bsd2.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb6/igt@gem_exec_schedule@preempt-queue-chain-bsd2.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#112146]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb6/igt@gem_exec_schedule@reorder-wide-bsd.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_exec_store@basic-vcs1:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#112080]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_exec_store@basic-vcs1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_exec_store@basic-vcs1.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-hsw:          [PASS][27] -> [TIMEOUT][28] ([fdo#112068 ])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-hsw5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-hsw5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [PASS][29] -> [DMESG-WARN][30] ([fdo#111870])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb6/igt@gem_userptr_blits@sync-unmap.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb6/igt@gem_userptr_blits@sync-unmap.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-glk:          [PASS][31] -> [INCOMPLETE][32] ([fdo#103359] / [k.org#198133])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk2/igt@gem_userptr_blits@sync-unmap-after-close.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk3/igt@gem_userptr_blits@sync-unmap-after-close.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-tglb:         [PASS][33] -> [INCOMPLETE][34] ([fdo#111832] / [fdo#111850]) +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@i915_pm_backlight@fade_with_suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb8/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@i915_pm_dc@dc5-dpms:
    - shard-iclb:         [PASS][35] -> [FAIL][36] ([fdo#111795 ])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@i915_pm_dc@dc5-dpms.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@i915_pm_dc@dc5-dpms.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-0:
    - shard-skl:          [PASS][37] -> [INCOMPLETE][38] ([fdo#104108] / [fdo#112347])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl1/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl9/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html

  * igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge:
    - shard-hsw:          [PASS][39] -> [INCOMPLETE][40] ([fdo#103540])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-hsw6/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-hsw2/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled:
    - shard-iclb:         [PASS][41] -> [INCOMPLETE][42] ([fdo#107713]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb6/igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][43] -> [FAIL][44] ([fdo#105363])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-snb:          [PASS][45] -> [INCOMPLETE][46] ([fdo#105411])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb1/igt@kms_flip@flip-vs-suspend.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          [PASS][47] -> [FAIL][48] ([fdo#103167])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk3/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][49] -> [DMESG-WARN][50] ([fdo#108566]) +4 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
    - shard-tglb:         [PASS][51] -> [INCOMPLETE][52] ([fdo#111832] / [fdo#111850] / [fdo#111884])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][53] -> [INCOMPLETE][54] ([fdo#106978] / [fdo#107713])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-skl:          [PASS][55] -> [INCOMPLETE][56] ([fdo#104108])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl9/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-kbl:          [PASS][57] -> [INCOMPLETE][58] ([fdo#103665])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl6/igt@kms_plane@pixel-format-pipe-a-planes.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl2/igt@kms_plane@pixel-format-pipe-a-planes.html

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-skl:          [PASS][59] -> [DMESG-WARN][60] ([fdo#106885])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl1/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl1/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html

  * igt@kms_psr@psr2_suspend:
    - shard-tglb:         [PASS][61] -> [DMESG-WARN][62] ([fdo#111600])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@kms_psr@psr2_suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb5/igt@kms_psr@psr2_suspend.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-skl:          [INCOMPLETE][63] ([fdo#104108]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl7/igt@gem_ctx_isolation@bcs0-s3.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_isolation@vcs0-s3:
    - shard-tglb:         [INCOMPLETE][65] ([fdo#111832]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb4/igt@gem_ctx_isolation@vcs0-s3.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb5/igt@gem_ctx_isolation@vcs0-s3.html

  * igt@gem_eio@unwedge-stress:
    - shard-glk:          [FAIL][67] ([fdo#109661]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk2/igt@gem_eio@unwedge-stress.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk8/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][69] ([fdo#112146]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [SKIP][71] ([fdo#109276]) -> [PASS][72] +5 similar issues
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb6/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd1.html
    - shard-tglb:         [INCOMPLETE][73] ([fdo#111677]) -> [PASS][74] +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb6/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb7/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@preempt-queue-chain-bsd1:
    - shard-tglb:         [INCOMPLETE][75] ([fdo#111606] / [fdo#111677]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb6/igt@gem_exec_schedule@preempt-queue-chain-bsd1.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb7/igt@gem_exec_schedule@preempt-queue-chain-bsd1.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-snb:          [TIMEOUT][77] ([fdo#112068 ]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-iclb:         [FAIL][79] ([fdo#112037]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][81] ([fdo#108566]) -> [PASS][82] +1 similar issue
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-apl1/igt@gem_softpin@noreloc-s3.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-apl3/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-hsw:          [DMESG-WARN][83] ([fdo#110789] / [fdo#111870]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-hsw5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@gem_wait@busy-vcs1:
    - shard-iclb:         [SKIP][85] ([fdo#112080]) -> [PASS][86] +3 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb5/igt@gem_wait@busy-vcs1.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@gem_wait@busy-vcs1.html

  * igt@i915_suspend@debugfs-reader:
    - shard-tglb:         [INCOMPLETE][87] ([fdo#111832] / [fdo#111850]) -> [PASS][88] +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb1/igt@i915_suspend@debugfs-reader.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb3/igt@i915_suspend@debugfs-reader.html

  * igt@i915_suspend@sysfs-reader:
    - shard-kbl:          [DMESG-WARN][89] ([fdo#108566]) -> [PASS][90] +2 similar issues
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl6/igt@i915_suspend@sysfs-reader.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl1/igt@i915_suspend@sysfs-reader.html

  * igt@kms_draw_crc@fill-fb:
    - shard-snb:          [SKIP][91] ([fdo#109271]) -> [PASS][92] +8 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb1/igt@kms_draw_crc@fill-fb.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb6/igt@kms_draw_crc@fill-fb.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-iclb:         [DMESG-WARN][93] ([fdo#111764]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-tglb:         [FAIL][95] ([fdo#103167]) -> [PASS][96] +4 similar issues
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][97] ([fdo#108145] / [fdo#110403]) -> [PASS][98] +1 similar issue
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [SKIP][99] ([fdo#109441]) -> [PASS][100]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb5/igt@kms_psr@psr2_primary_blt.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@kms_psr@psr2_primary_blt.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs2-nonpriv-switch:
    - shard-tglb:         [SKIP][101] ([fdo#112080]) -> [SKIP][102] ([fdo#111912] / [fdo#112080])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb9/igt@gem_ctx_isolation@vcs2-nonpriv-switch.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb6/igt@gem_ctx_isolation@vcs2-nonpriv-switch.html

  * igt@gem_eio@kms:
    - shard-snb:          [DMESG-WARN][103] ([fdo# 112000 ] / [fdo#111781]) -> [INCOMPLETE][104] ([fdo#105411])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb4/igt@gem_eio@kms.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb4/igt@gem_eio@kms.html

  * igt@gem_exec_schedule@deep-vebox:
    - shard-tglb:         [INCOMPLETE][105] ([fdo#111671]) -> [FAIL][106] ([fdo#111646])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@gem_exec_schedule@deep-vebox.html
   [106]: https://intel-gfx-ci.01.or

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-24 22:42   ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-11-24 22:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
URL   : https://patchwork.freedesktop.org/series/69949/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7411_full -> Patchwork_15413_full
====================================================

Summary
-------

  **FAILURE**

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

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_param@vm:
    - shard-skl:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl3/igt@gem_ctx_param@vm.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl3/igt@gem_ctx_param@vm.html
    - shard-glk:          [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk5/igt@gem_ctx_param@vm.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk3/igt@gem_ctx_param@vm.html
    - shard-apl:          [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-apl2/igt@gem_ctx_param@vm.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-apl3/igt@gem_ctx_param@vm.html
    - shard-tglb:         [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@gem_ctx_param@vm.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb5/igt@gem_ctx_param@vm.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-render:
    - shard-tglb:         [PASS][9] -> [INCOMPLETE][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-render.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-skl:          [PASS][11] -> [INCOMPLETE][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl10/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl9/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          NOTRUN -> [INCOMPLETE][13]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@kms_plane@pixel-format-pipe-b-planes.html

  * igt@perf@oa-exponents:
    - shard-kbl:          [PASS][14] -> [TIMEOUT][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl1/igt@perf@oa-exponents.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl2/igt@perf@oa-exponents.html

  * igt@runner@aborted:
    - shard-kbl:          NOTRUN -> [FAIL][16]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl2/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@q-promotion-bsd2:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109276]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_ctx_shared@q-promotion-bsd2.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_ctx_shared@q-promotion-bsd2.html

  * igt@gem_exec_parallel@fds:
    - shard-tglb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#111867])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb9/igt@gem_exec_parallel@fds.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb6/igt@gem_exec_parallel@fds.html

  * igt@gem_exec_schedule@preempt-queue-chain-bsd2:
    - shard-tglb:         [PASS][21] -> [INCOMPLETE][22] ([fdo#111677])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb9/igt@gem_exec_schedule@preempt-queue-chain-bsd2.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb6/igt@gem_exec_schedule@preempt-queue-chain-bsd2.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#112146]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb6/igt@gem_exec_schedule@reorder-wide-bsd.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_exec_store@basic-vcs1:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#112080]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_exec_store@basic-vcs1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_exec_store@basic-vcs1.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-hsw:          [PASS][27] -> [TIMEOUT][28] ([fdo#112068 ])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-hsw5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-hsw5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [PASS][29] -> [DMESG-WARN][30] ([fdo#111870])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb6/igt@gem_userptr_blits@sync-unmap.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb6/igt@gem_userptr_blits@sync-unmap.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-glk:          [PASS][31] -> [INCOMPLETE][32] ([fdo#103359] / [k.org#198133])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk2/igt@gem_userptr_blits@sync-unmap-after-close.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk3/igt@gem_userptr_blits@sync-unmap-after-close.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-tglb:         [PASS][33] -> [INCOMPLETE][34] ([fdo#111832] / [fdo#111850]) +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@i915_pm_backlight@fade_with_suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb8/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@i915_pm_dc@dc5-dpms:
    - shard-iclb:         [PASS][35] -> [FAIL][36] ([fdo#111795 ])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@i915_pm_dc@dc5-dpms.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@i915_pm_dc@dc5-dpms.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-0:
    - shard-skl:          [PASS][37] -> [INCOMPLETE][38] ([fdo#104108] / [fdo#112347])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl1/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl9/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html

  * igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge:
    - shard-hsw:          [PASS][39] -> [INCOMPLETE][40] ([fdo#103540])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-hsw6/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-hsw2/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled:
    - shard-iclb:         [PASS][41] -> [INCOMPLETE][42] ([fdo#107713]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb6/igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][43] -> [FAIL][44] ([fdo#105363])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-snb:          [PASS][45] -> [INCOMPLETE][46] ([fdo#105411])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb1/igt@kms_flip@flip-vs-suspend.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          [PASS][47] -> [FAIL][48] ([fdo#103167])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk3/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][49] -> [DMESG-WARN][50] ([fdo#108566]) +4 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
    - shard-tglb:         [PASS][51] -> [INCOMPLETE][52] ([fdo#111832] / [fdo#111850] / [fdo#111884])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][53] -> [INCOMPLETE][54] ([fdo#106978] / [fdo#107713])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-skl:          [PASS][55] -> [INCOMPLETE][56] ([fdo#104108])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl9/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-kbl:          [PASS][57] -> [INCOMPLETE][58] ([fdo#103665])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl6/igt@kms_plane@pixel-format-pipe-a-planes.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl2/igt@kms_plane@pixel-format-pipe-a-planes.html

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-skl:          [PASS][59] -> [DMESG-WARN][60] ([fdo#106885])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl1/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl1/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html

  * igt@kms_psr@psr2_suspend:
    - shard-tglb:         [PASS][61] -> [DMESG-WARN][62] ([fdo#111600])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@kms_psr@psr2_suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb5/igt@kms_psr@psr2_suspend.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-skl:          [INCOMPLETE][63] ([fdo#104108]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl7/igt@gem_ctx_isolation@bcs0-s3.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_isolation@vcs0-s3:
    - shard-tglb:         [INCOMPLETE][65] ([fdo#111832]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb4/igt@gem_ctx_isolation@vcs0-s3.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb5/igt@gem_ctx_isolation@vcs0-s3.html

  * igt@gem_eio@unwedge-stress:
    - shard-glk:          [FAIL][67] ([fdo#109661]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-glk2/igt@gem_eio@unwedge-stress.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-glk8/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][69] ([fdo#112146]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [SKIP][71] ([fdo#109276]) -> [PASS][72] +5 similar issues
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb6/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd1.html
    - shard-tglb:         [INCOMPLETE][73] ([fdo#111677]) -> [PASS][74] +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb6/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb7/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@preempt-queue-chain-bsd1:
    - shard-tglb:         [INCOMPLETE][75] ([fdo#111606] / [fdo#111677]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb6/igt@gem_exec_schedule@preempt-queue-chain-bsd1.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb7/igt@gem_exec_schedule@preempt-queue-chain-bsd1.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-snb:          [TIMEOUT][77] ([fdo#112068 ]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-iclb:         [FAIL][79] ([fdo#112037]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb1/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][81] ([fdo#108566]) -> [PASS][82] +1 similar issue
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-apl1/igt@gem_softpin@noreloc-s3.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-apl3/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-hsw:          [DMESG-WARN][83] ([fdo#110789] / [fdo#111870]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-hsw5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@gem_wait@busy-vcs1:
    - shard-iclb:         [SKIP][85] ([fdo#112080]) -> [PASS][86] +3 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb5/igt@gem_wait@busy-vcs1.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@gem_wait@busy-vcs1.html

  * igt@i915_suspend@debugfs-reader:
    - shard-tglb:         [INCOMPLETE][87] ([fdo#111832] / [fdo#111850]) -> [PASS][88] +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb1/igt@i915_suspend@debugfs-reader.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb3/igt@i915_suspend@debugfs-reader.html

  * igt@i915_suspend@sysfs-reader:
    - shard-kbl:          [DMESG-WARN][89] ([fdo#108566]) -> [PASS][90] +2 similar issues
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-kbl6/igt@i915_suspend@sysfs-reader.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-kbl1/igt@i915_suspend@sysfs-reader.html

  * igt@kms_draw_crc@fill-fb:
    - shard-snb:          [SKIP][91] ([fdo#109271]) -> [PASS][92] +8 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb1/igt@kms_draw_crc@fill-fb.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb6/igt@kms_draw_crc@fill-fb.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-iclb:         [DMESG-WARN][93] ([fdo#111764]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-tglb:         [FAIL][95] ([fdo#103167]) -> [PASS][96] +4 similar issues
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][97] ([fdo#108145] / [fdo#110403]) -> [PASS][98] +1 similar issue
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [SKIP][99] ([fdo#109441]) -> [PASS][100]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-iclb5/igt@kms_psr@psr2_primary_blt.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-iclb2/igt@kms_psr@psr2_primary_blt.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs2-nonpriv-switch:
    - shard-tglb:         [SKIP][101] ([fdo#112080]) -> [SKIP][102] ([fdo#111912] / [fdo#112080])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb9/igt@gem_ctx_isolation@vcs2-nonpriv-switch.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-tglb6/igt@gem_ctx_isolation@vcs2-nonpriv-switch.html

  * igt@gem_eio@kms:
    - shard-snb:          [DMESG-WARN][103] ([fdo# 112000 ] / [fdo#111781]) -> [INCOMPLETE][104] ([fdo#105411])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-snb4/igt@gem_eio@kms.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/shard-snb4/igt@gem_eio@kms.html

  * igt@gem_exec_schedule@deep-vebox:
    - shard-tglb:         [INCOMPLETE][105] ([fdo#111671]) -> [FAIL][106] ([fdo#111646])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7411/shard-tglb5/igt@gem_exec_schedule@deep-vebox.html
   [106]: https://intel-gfx-ci.01.or

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15413/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-25  9:16   ` Mika Kuoppala
  0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2019-11-25  9:16 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since we want to do a lockless read of the current active request, and
> that request is written to by process_csb also without serialisation, we
> need to instruct gcc to take care in reading the pointer itself.
>
> Otherwise, we have observed execlists_active() to report 0x40.
>
> [ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
> [ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
> [ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
> [ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940

Where is this exact tracepoint? My grep skills are failing me.

>
> which is impossible!
>
> The answer is that as we keep the existing execlists->active pointing
> into the array as we copy over that array, the unserialised read may see
> a partial pointer value.

...otherwise we will see ?

Also, the 0x40 is bothering me as I didn't find the tracepoint. If we
only displayed pointer values, where did the offset appear. 

>
> Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
>  drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
>  2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index bc3b72bfa9e3..01765a7ec18f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
>  static inline struct i915_request *
>  execlists_active(const struct intel_engine_execlists *execlists)
>  {
> -	GEM_BUG_ON(execlists->active - execlists->inflight >
> -		   execlists_num_ports(execlists));
> -	return READ_ONCE(*execlists->active);
> +	return *READ_ONCE(execlists->active);

Yes this seems proper as we need apriori read before deferencing.

>  }
>  
>  static inline void
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 0e2065a13f24..0d0dca3d6724 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
>  		else
>  			promote = gen8_csb_parse(execlists, buf + 2 * head);
>  		if (promote) {
> +			struct i915_request * const *old = execlists->active;
> +
> +			/* Point active to the new ELSP; prevent overwriting */
> +			WRITE_ONCE(execlists->active, execlists->pending);
> +			set_timeslice(engine);

If we set the active to pending here...

> +
>  			if (!inject_preempt_hang(execlists))
>  				ring_set_paused(engine, 0);
>  
>  			/* cancel old inflight, prepare for switch */
> -			trace_ports(execlists, "preempted", execlists->active);
> -			while (*execlists->active)
> -				execlists_schedule_out(*execlists->active++);
> +			trace_ports(execlists, "preempted", old);
> +			while (*old)
> +				execlists_schedule_out(*old++);
>  
>  			/* switch pending to inflight */
>  			GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
> -			execlists->active =
> -				memcpy(execlists->inflight,
> -				       execlists->pending,
> -				       execlists_num_ports(execlists) *
> -				       sizeof(*execlists->pending));
> -
> -			set_timeslice(engine);
> +			WRITE_ONCE(execlists->active,
> +				   memcpy(execlists->inflight,
> +					  execlists->pending,
> +					  execlists_num_ports(execlists) *
> +					  sizeof(*execlists->pending)));

Why we rewrite it in here, is the pending moving beneath us?

-Mika

>  
>  			WRITE_ONCE(execlists->pending[0], NULL);
>  		} else {
> -- 
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-25  9:16   ` Mika Kuoppala
  0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2019-11-25  9:16 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since we want to do a lockless read of the current active request, and
> that request is written to by process_csb also without serialisation, we
> need to instruct gcc to take care in reading the pointer itself.
>
> Otherwise, we have observed execlists_active() to report 0x40.
>
> [ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
> [ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
> [ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
> [ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940

Where is this exact tracepoint? My grep skills are failing me.

>
> which is impossible!
>
> The answer is that as we keep the existing execlists->active pointing
> into the array as we copy over that array, the unserialised read may see
> a partial pointer value.

...otherwise we will see ?

Also, the 0x40 is bothering me as I didn't find the tracepoint. If we
only displayed pointer values, where did the offset appear. 

>
> Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
>  drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
>  2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index bc3b72bfa9e3..01765a7ec18f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
>  static inline struct i915_request *
>  execlists_active(const struct intel_engine_execlists *execlists)
>  {
> -	GEM_BUG_ON(execlists->active - execlists->inflight >
> -		   execlists_num_ports(execlists));
> -	return READ_ONCE(*execlists->active);
> +	return *READ_ONCE(execlists->active);

Yes this seems proper as we need apriori read before deferencing.

>  }
>  
>  static inline void
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 0e2065a13f24..0d0dca3d6724 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
>  		else
>  			promote = gen8_csb_parse(execlists, buf + 2 * head);
>  		if (promote) {
> +			struct i915_request * const *old = execlists->active;
> +
> +			/* Point active to the new ELSP; prevent overwriting */
> +			WRITE_ONCE(execlists->active, execlists->pending);
> +			set_timeslice(engine);

If we set the active to pending here...

> +
>  			if (!inject_preempt_hang(execlists))
>  				ring_set_paused(engine, 0);
>  
>  			/* cancel old inflight, prepare for switch */
> -			trace_ports(execlists, "preempted", execlists->active);
> -			while (*execlists->active)
> -				execlists_schedule_out(*execlists->active++);
> +			trace_ports(execlists, "preempted", old);
> +			while (*old)
> +				execlists_schedule_out(*old++);
>  
>  			/* switch pending to inflight */
>  			GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
> -			execlists->active =
> -				memcpy(execlists->inflight,
> -				       execlists->pending,
> -				       execlists_num_ports(execlists) *
> -				       sizeof(*execlists->pending));
> -
> -			set_timeslice(engine);
> +			WRITE_ONCE(execlists->active,
> +				   memcpy(execlists->inflight,
> +					  execlists->pending,
> +					  execlists_num_ports(execlists) *
> +					  sizeof(*execlists->pending)));

Why we rewrite it in here, is the pending moving beneath us?

-Mika

>  
>  			WRITE_ONCE(execlists->pending[0], NULL);
>  		} else {
> -- 
> 2.24.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-25  9:23     ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-25  9:23 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-11-25 09:16:30)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since we want to do a lockless read of the current active request, and
> > that request is written to by process_csb also without serialisation, we
> > need to instruct gcc to take care in reading the pointer itself.
> >
> > Otherwise, we have observed execlists_active() to report 0x40.
> >
> > [ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
> > [ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
> > [ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
> > [ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940
> 
> Where is this exact tracepoint? My grep skills are failing me.

I added to see
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7388/fi-bsw-n3050/igt@i915_selftest@live_gem_contexts.html

> >
> > which is impossible!
> >
> > The answer is that as we keep the existing execlists->active pointing
> > into the array as we copy over that array, the unserialised read may see
> > a partial pointer value.
> 
> ...otherwise we will see ?
> 
> Also, the 0x40 is bothering me as I didn't find the tracepoint. If we
> only displayed pointer values, where did the offset appear. 

Because we did a byte-by-byte copy of pending to inflight as
execlists_active() reads *active [pointing into inflight]

So inflight is a random mix of NULL + rq, starting at the LSB.

> > Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
> >  2 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> > index bc3b72bfa9e3..01765a7ec18f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> > @@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
> >  static inline struct i915_request *
> >  execlists_active(const struct intel_engine_execlists *execlists)
> >  {
> > -     GEM_BUG_ON(execlists->active - execlists->inflight >
> > -                execlists_num_ports(execlists));
> > -     return READ_ONCE(*execlists->active);
> > +     return *READ_ONCE(execlists->active);
> 
> Yes this seems proper as we need apriori read before deferencing.
> 
> >  }
> >  
> >  static inline void
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 0e2065a13f24..0d0dca3d6724 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
> >               else
> >                       promote = gen8_csb_parse(execlists, buf + 2 * head);
> >               if (promote) {
> > +                     struct i915_request * const *old = execlists->active;
> > +
> > +                     /* Point active to the new ELSP; prevent overwriting */
> > +                     WRITE_ONCE(execlists->active, execlists->pending);
> > +                     set_timeslice(engine);
> 
> If we set the active to pending here...
> 
> > +
> >                       if (!inject_preempt_hang(execlists))
> >                               ring_set_paused(engine, 0);
> >  
> >                       /* cancel old inflight, prepare for switch */
> > -                     trace_ports(execlists, "preempted", execlists->active);
> > -                     while (*execlists->active)
> > -                             execlists_schedule_out(*execlists->active++);
> > +                     trace_ports(execlists, "preempted", old);
> > +                     while (*old)
> > +                             execlists_schedule_out(*old++);
> >  
> >                       /* switch pending to inflight */
> >                       GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
> > -                     execlists->active =
> > -                             memcpy(execlists->inflight,
> > -                                    execlists->pending,
> > -                                    execlists_num_ports(execlists) *
> > -                                    sizeof(*execlists->pending));
> > -
> > -                     set_timeslice(engine);
> > +                     WRITE_ONCE(execlists->active,
> > +                                memcpy(execlists->inflight,
> > +                                       execlists->pending,
> > +                                       execlists_num_ports(execlists) *
> > +                                       sizeof(*execlists->pending)));
> 
> Why we rewrite it in here, is the pending moving beneath us?

Yes. Pending is where we track the next submit, inflight + active the
current. pending[0] = NULL is the next line, and pending[] is then set
in dequeue.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-25  9:23     ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-25  9:23 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-11-25 09:16:30)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since we want to do a lockless read of the current active request, and
> > that request is written to by process_csb also without serialisation, we
> > need to instruct gcc to take care in reading the pointer itself.
> >
> > Otherwise, we have observed execlists_active() to report 0x40.
> >
> > [ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
> > [ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
> > [ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
> > [ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940
> 
> Where is this exact tracepoint? My grep skills are failing me.

I added to see
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7388/fi-bsw-n3050/igt@i915_selftest@live_gem_contexts.html

> >
> > which is impossible!
> >
> > The answer is that as we keep the existing execlists->active pointing
> > into the array as we copy over that array, the unserialised read may see
> > a partial pointer value.
> 
> ...otherwise we will see ?
> 
> Also, the 0x40 is bothering me as I didn't find the tracepoint. If we
> only displayed pointer values, where did the offset appear. 

Because we did a byte-by-byte copy of pending to inflight as
execlists_active() reads *active [pointing into inflight]

So inflight is a random mix of NULL + rq, starting at the LSB.

> > Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
> >  2 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> > index bc3b72bfa9e3..01765a7ec18f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> > @@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
> >  static inline struct i915_request *
> >  execlists_active(const struct intel_engine_execlists *execlists)
> >  {
> > -     GEM_BUG_ON(execlists->active - execlists->inflight >
> > -                execlists_num_ports(execlists));
> > -     return READ_ONCE(*execlists->active);
> > +     return *READ_ONCE(execlists->active);
> 
> Yes this seems proper as we need apriori read before deferencing.
> 
> >  }
> >  
> >  static inline void
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 0e2065a13f24..0d0dca3d6724 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
> >               else
> >                       promote = gen8_csb_parse(execlists, buf + 2 * head);
> >               if (promote) {
> > +                     struct i915_request * const *old = execlists->active;
> > +
> > +                     /* Point active to the new ELSP; prevent overwriting */
> > +                     WRITE_ONCE(execlists->active, execlists->pending);
> > +                     set_timeslice(engine);
> 
> If we set the active to pending here...
> 
> > +
> >                       if (!inject_preempt_hang(execlists))
> >                               ring_set_paused(engine, 0);
> >  
> >                       /* cancel old inflight, prepare for switch */
> > -                     trace_ports(execlists, "preempted", execlists->active);
> > -                     while (*execlists->active)
> > -                             execlists_schedule_out(*execlists->active++);
> > +                     trace_ports(execlists, "preempted", old);
> > +                     while (*old)
> > +                             execlists_schedule_out(*old++);
> >  
> >                       /* switch pending to inflight */
> >                       GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
> > -                     execlists->active =
> > -                             memcpy(execlists->inflight,
> > -                                    execlists->pending,
> > -                                    execlists_num_ports(execlists) *
> > -                                    sizeof(*execlists->pending));
> > -
> > -                     set_timeslice(engine);
> > +                     WRITE_ONCE(execlists->active,
> > +                                memcpy(execlists->inflight,
> > +                                       execlists->pending,
> > +                                       execlists_num_ports(execlists) *
> > +                                       sizeof(*execlists->pending)));
> 
> Why we rewrite it in here, is the pending moving beneath us?

Yes. Pending is where we track the next submit, inflight + active the
current. pending[0] = NULL is the next line, and pending[] is then set
in dequeue.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-25  9:38       ` Mika Kuoppala
  0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2019-11-25  9:38 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-11-25 09:16:30)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Since we want to do a lockless read of the current active request, and
>> > that request is written to by process_csb also without serialisation, we
>> > need to instruct gcc to take care in reading the pointer itself.
>> >
>> > Otherwise, we have observed execlists_active() to report 0x40.
>> >
>> > [ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
>> > [ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
>> > [ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
>> > [ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940
>> 
>> Where is this exact tracepoint? My grep skills are failing me.
>
> I added to see
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7388/fi-bsw-n3050/igt@i915_selftest@live_gem_contexts.html
>
>> >
>> > which is impossible!
>> >
>> > The answer is that as we keep the existing execlists->active pointing
>> > into the array as we copy over that array, the unserialised read may see
>> > a partial pointer value.
>> 
>> ...otherwise we will see ?
>> 
>> Also, the 0x40 is bothering me as I didn't find the tracepoint. If we
>> only displayed pointer values, where did the offset appear. 
>
> Because we did a byte-by-byte copy of pending to inflight as
> execlists_active() reads *active [pointing into inflight]
>
> So inflight is a random mix of NULL + rq, starting at the LSB.

Seems so, yeah we can't really assume memcpy would do anything
fancier.

Ok, put a WRITE_ONCE for changing the active on
cancel_port_requests() too, for symmetry.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>
>> > Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
>> >  drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
>> >  2 files changed, 15 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
>> > index bc3b72bfa9e3..01765a7ec18f 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
>> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
>> > @@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
>> >  static inline struct i915_request *
>> >  execlists_active(const struct intel_engine_execlists *execlists)
>> >  {
>> > -     GEM_BUG_ON(execlists->active - execlists->inflight >
>> > -                execlists_num_ports(execlists));
>> > -     return READ_ONCE(*execlists->active);
>> > +     return *READ_ONCE(execlists->active);
>> 
>> Yes this seems proper as we need apriori read before deferencing.
>> 
>> >  }
>> >  
>> >  static inline void
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > index 0e2065a13f24..0d0dca3d6724 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > @@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
>> >               else
>> >                       promote = gen8_csb_parse(execlists, buf + 2 * head);
>> >               if (promote) {
>> > +                     struct i915_request * const *old = execlists->active;
>> > +
>> > +                     /* Point active to the new ELSP; prevent overwriting */
>> > +                     WRITE_ONCE(execlists->active, execlists->pending);
>> > +                     set_timeslice(engine);
>> 
>> If we set the active to pending here...
>> 
>> > +
>> >                       if (!inject_preempt_hang(execlists))
>> >                               ring_set_paused(engine, 0);
>> >  
>> >                       /* cancel old inflight, prepare for switch */
>> > -                     trace_ports(execlists, "preempted", execlists->active);
>> > -                     while (*execlists->active)
>> > -                             execlists_schedule_out(*execlists->active++);
>> > +                     trace_ports(execlists, "preempted", old);
>> > +                     while (*old)
>> > +                             execlists_schedule_out(*old++);
>> >  
>> >                       /* switch pending to inflight */
>> >                       GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
>> > -                     execlists->active =
>> > -                             memcpy(execlists->inflight,
>> > -                                    execlists->pending,
>> > -                                    execlists_num_ports(execlists) *
>> > -                                    sizeof(*execlists->pending));
>> > -
>> > -                     set_timeslice(engine);
>> > +                     WRITE_ONCE(execlists->active,
>> > +                                memcpy(execlists->inflight,
>> > +                                       execlists->pending,
>> > +                                       execlists_num_ports(execlists) *
>> > +                                       sizeof(*execlists->pending)));
>> 
>> Why we rewrite it in here, is the pending moving beneath us?
>
> Yes. Pending is where we track the next submit, inflight + active the
> current. pending[0] = NULL is the next line, and pending[] is then set
> in dequeue.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access
@ 2019-11-25  9:38       ` Mika Kuoppala
  0 siblings, 0 replies; 24+ messages in thread
From: Mika Kuoppala @ 2019-11-25  9:38 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-11-25 09:16:30)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Since we want to do a lockless read of the current active request, and
>> > that request is written to by process_csb also without serialisation, we
>> > need to instruct gcc to take care in reading the pointer itself.
>> >
>> > Otherwise, we have observed execlists_active() to report 0x40.
>> >
>> > [ 2400.760381] igt/para-4098    1..s. 2376479300us : process_csb: rcs0 cs-irq head=3, tail=4
>> > [ 2400.760826] igt/para-4098    1..s. 2376479303us : process_csb: rcs0 csb[4]: status=0x00000001:0x00000000
>> > [ 2400.761271] igt/para-4098    1..s. 2376479306us : trace_ports: rcs0: promote { b9c59:2622, b9c55:2624 }
>> > [ 2400.761726] igt/para-4097    0d... 2376479311us : __i915_schedule: rcs0: -2147483648->3, inflight:0000000000000040, rq:ffff888208c1e940
>> 
>> Where is this exact tracepoint? My grep skills are failing me.
>
> I added to see
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7388/fi-bsw-n3050/igt@i915_selftest@live_gem_contexts.html
>
>> >
>> > which is impossible!
>> >
>> > The answer is that as we keep the existing execlists->active pointing
>> > into the array as we copy over that array, the unserialised read may see
>> > a partial pointer value.
>> 
>> ...otherwise we will see ?
>> 
>> Also, the 0x40 is bothering me as I didn't find the tracepoint. If we
>> only displayed pointer values, where did the offset appear. 
>
> Because we did a byte-by-byte copy of pending to inflight as
> execlists_active() reads *active [pointing into inflight]
>
> So inflight is a random mix of NULL + rq, starting at the LSB.

Seems so, yeah we can't really assume memcpy would do anything
fancier.

Ok, put a WRITE_ONCE for changing the active on
cancel_port_requests() too, for symmetry.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>
>> > Fixes: df403069029d ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/i915/gt/intel_engine.h |  4 +---
>> >  drivers/gpu/drm/i915/gt/intel_lrc.c    | 24 ++++++++++++++----------
>> >  2 files changed, 15 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
>> > index bc3b72bfa9e3..01765a7ec18f 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
>> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
>> > @@ -100,9 +100,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
>> >  static inline struct i915_request *
>> >  execlists_active(const struct intel_engine_execlists *execlists)
>> >  {
>> > -     GEM_BUG_ON(execlists->active - execlists->inflight >
>> > -                execlists_num_ports(execlists));
>> > -     return READ_ONCE(*execlists->active);
>> > +     return *READ_ONCE(execlists->active);
>> 
>> Yes this seems proper as we need apriori read before deferencing.
>> 
>> >  }
>> >  
>> >  static inline void
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > index 0e2065a13f24..0d0dca3d6724 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> > @@ -2169,23 +2169,27 @@ static void process_csb(struct intel_engine_cs *engine)
>> >               else
>> >                       promote = gen8_csb_parse(execlists, buf + 2 * head);
>> >               if (promote) {
>> > +                     struct i915_request * const *old = execlists->active;
>> > +
>> > +                     /* Point active to the new ELSP; prevent overwriting */
>> > +                     WRITE_ONCE(execlists->active, execlists->pending);
>> > +                     set_timeslice(engine);
>> 
>> If we set the active to pending here...
>> 
>> > +
>> >                       if (!inject_preempt_hang(execlists))
>> >                               ring_set_paused(engine, 0);
>> >  
>> >                       /* cancel old inflight, prepare for switch */
>> > -                     trace_ports(execlists, "preempted", execlists->active);
>> > -                     while (*execlists->active)
>> > -                             execlists_schedule_out(*execlists->active++);
>> > +                     trace_ports(execlists, "preempted", old);
>> > +                     while (*old)
>> > +                             execlists_schedule_out(*old++);
>> >  
>> >                       /* switch pending to inflight */
>> >                       GEM_BUG_ON(!assert_pending_valid(execlists, "promote"));
>> > -                     execlists->active =
>> > -                             memcpy(execlists->inflight,
>> > -                                    execlists->pending,
>> > -                                    execlists_num_ports(execlists) *
>> > -                                    sizeof(*execlists->pending));
>> > -
>> > -                     set_timeslice(engine);
>> > +                     WRITE_ONCE(execlists->active,
>> > +                                memcpy(execlists->inflight,
>> > +                                       execlists->pending,
>> > +                                       execlists_num_ports(execlists) *
>> > +                                       sizeof(*execlists->pending)));
>> 
>> Why we rewrite it in here, is the pending moving beneath us?
>
> Yes. Pending is where we track the next submit, inflight + active the
> current. pending[0] = NULL is the next line, and pending[] is then set
> in dequeue.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context
@ 2019-11-25 10:44     ` Tvrtko Ursulin
  0 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2019-11-25 10:44 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 24/11/2019 17:05, Chris Wilson wrote:
> As the engine->kernel_context is used within the engine-pm barrier, we
> have to be careful when emitting requests outside of the barrier, as the
> strict timeline locking rules do not apply. Instead, we must ensure the
> engine_park() cannot be entered as we build the request, which is
> simplest by taking an explicit engine-pm wakeref around the request
> construction.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  3 +-
>   .../i915/gem/selftests/i915_gem_client_blt.c  |  2 +
>   .../i915/gem/selftests/i915_gem_coherency.c   |  3 +-
>   .../drm/i915/gem/selftests/i915_gem_context.c |  7 +++-
>   .../drm/i915/gem/selftests/i915_gem_mman.c    |  3 +-
>   .../i915/gem/selftests/i915_gem_object_blt.c  | 18 ++++++---
>   .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 14 +++++--
>   drivers/gpu/drm/i915/gt/intel_engine_pm.h     | 21 ++++++++++
>   drivers/gpu/drm/i915/gt/intel_workarounds.c   |  3 ++
>   drivers/gpu/drm/i915/gt/selftest_context.c    |  2 +-
>   drivers/gpu/drm/i915/gt/selftest_engine_cs.c  | 12 ++++++
>   drivers/gpu/drm/i915/gt/selftest_lrc.c        |  6 +--
>   drivers/gpu/drm/i915/gt/selftest_mocs.c       |  2 +
>   drivers/gpu/drm/i915/gt/selftest_timeline.c   |  6 +--
>   drivers/gpu/drm/i915/i915_perf.c              |  4 +-
>   drivers/gpu/drm/i915/selftests/i915_active.c  |  2 +-
>   drivers/gpu/drm/i915/selftests/i915_perf.c    |  2 +-
>   drivers/gpu/drm/i915/selftests/i915_request.c | 40 ++++++++++++++-----
>   .../drm/i915/selftests/intel_memory_region.c  |  2 +
>   19 files changed, 119 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 6f1e6181f67a..c94ac838401a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -70,6 +70,7 @@
>   #include <drm/i915_drm.h>
>   
>   #include "gt/intel_engine_heartbeat.h"
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_engine_user.h"
>   #include "gt/intel_lrc_reg.h"
>   #include "gt/intel_ring.h"
> @@ -1265,7 +1266,7 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
>   	if (!intel_context_is_pinned(ce))
>   		return 0;
>   
> -	rq = i915_request_create(ce->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(ce->engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
> index da8edee4fe0a..b972be165e85 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
> @@ -24,6 +24,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
>   
>   	prandom_seed_state(&prng, i915_selftest.random_seed);
>   
> +	intel_engine_pm_get(engine);
>   	do {
>   		const u32 max_block_size = S16_MAX * PAGE_SIZE;
>   		u32 sz = min_t(u64, ce->vm->total >> 4, prandom_u32_state(&prng));
> @@ -99,6 +100,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
>   err_flush:
>   	if (err == -ENOMEM)
>   		err = 0;
> +	intel_engine_pm_put(engine);
>   
>   	return err;
>   }
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
> index 2b29f6b4e1dd..9d3cd1af61f6 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
> @@ -6,6 +6,7 @@
>   
>   #include <linux/prime_numbers.h>
>   
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   #include "gt/intel_gt_pm.h"
>   #include "gt/intel_ring.h"
> @@ -200,7 +201,7 @@ static int gpu_set(struct context *ctx, unsigned long offset, u32 v)
>   	if (IS_ERR(vma))
>   		return PTR_ERR(vma);
>   
> -	rq = i915_request_create(ctx->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(ctx->engine);
>   	if (IS_ERR(rq)) {
>   		i915_vma_unpin(vma);
>   		return PTR_ERR(rq);
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index e1d8ccd11409..2ea4790f3721 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -7,6 +7,7 @@
>   #include <linux/prime_numbers.h>
>   
>   #include "gem/i915_gem_pm.h"
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   #include "gt/intel_gt_requests.h"
>   #include "gt/intel_reset.h"
> @@ -1190,9 +1191,11 @@ __sseu_test(const char *name,
>   	struct igt_spinner *spin = NULL;
>   	int ret;
>   
> +	intel_engine_pm_get(ce->engine);
> +
>   	ret = __sseu_prepare(name, flags, ce, &spin);
>   	if (ret)
> -		return ret;
> +		goto out_pm;
>   
>   	ret = intel_context_reconfigure_sseu(ce, sseu);
>   	if (ret)
> @@ -1207,6 +1210,8 @@ __sseu_test(const char *name,
>   		igt_spinner_fini(spin);
>   		kfree(spin);
>   	}
> +out_pm:
> +	intel_engine_pm_put(ce->engine);
>   	return ret;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 9f1a69027a04..6ce9167f8c9f 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -6,6 +6,7 @@
>   
>   #include <linux/prime_numbers.h>
>   
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   #include "gt/intel_gt_pm.h"
>   #include "huge_gem_object.h"
> @@ -536,7 +537,7 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
>   		if (err)
>   			return err;
>   
> -		rq = i915_request_create(engine->kernel_context);
> +		rq = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(rq)) {
>   			i915_vma_unpin(vma);
>   			return PTR_ERR(rq);
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> index 675c1a20a2f1..62077fe46715 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> @@ -41,6 +41,7 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
>   		if (!engine)
>   			return 0;
>   
> +		intel_engine_pm_get(engine);
>   		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
>   			struct intel_context *ce = engine->kernel_context;
>   			ktime_t t0, t1;
> @@ -49,17 +50,20 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
>   
>   			err = i915_gem_object_fill_blt(obj, ce, 0);
>   			if (err)
> -				return err;
> +				break;
>   
>   			err = i915_gem_object_wait(obj,
>   						   I915_WAIT_ALL,
>   						   MAX_SCHEDULE_TIMEOUT);
>   			if (err)
> -				return err;
> +				break;
>   
>   			t1 = ktime_get();
>   			t[pass] = ktime_sub(t1, t0);
>   		}
> +		intel_engine_pm_put(engine);
> +		if (err)
> +			return err;
>   
>   		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
>   		pr_info("%s: blt %zd KiB fill: %lld MiB/s\n",
> @@ -109,6 +113,7 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
>   		struct intel_engine_cs *engine;
>   		ktime_t t[5];
>   		int pass;
> +		int err = 0;
>   
>   		engine = intel_engine_lookup_user(i915,
>   						  I915_ENGINE_CLASS_COPY,
> @@ -116,26 +121,29 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
>   		if (!engine)
>   			return 0;
>   
> +		intel_engine_pm_get(engine);
>   		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
>   			struct intel_context *ce = engine->kernel_context;
>   			ktime_t t0, t1;
> -			int err;
>   
>   			t0 = ktime_get();
>   
>   			err = i915_gem_object_copy_blt(src, dst, ce);
>   			if (err)
> -				return err;
> +				break;
>   
>   			err = i915_gem_object_wait(dst,
>   						   I915_WAIT_ALL,
>   						   MAX_SCHEDULE_TIMEOUT);
>   			if (err)
> -				return err;
> +				break;
>   
>   			t1 = ktime_get();
>   			t[pass] = ktime_sub(t1, t0);
>   		}
> +		intel_engine_pm_put(engine);
> +		if (err)
> +			return err;
>   
>   		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
>   		pr_info("%s: blt %zd KiB copy: %lld MiB/s\n",
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> index c91fd4e4af29..742628e40201 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> @@ -215,18 +215,26 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
>   int intel_engine_flush_barriers(struct intel_engine_cs *engine)
>   {
>   	struct i915_request *rq;
> +	int err = 0;
>   
>   	if (llist_empty(&engine->barrier_tasks))
>   		return 0;
>   
> +	if (!intel_engine_pm_get_if_awake(engine))
> +		return 0;
> +
>   	rq = i915_request_create(engine->kernel_context);
> -	if (IS_ERR(rq))
> -		return PTR_ERR(rq);
> +	if (IS_ERR(rq)) {
> +		err = PTR_ERR(rq);
> +		goto out_rpm;
> +	}
>   
>   	idle_pulse(engine, rq);
>   	i915_request_add(rq);
>   
> -	return 0;
> +out_rpm:
> +	intel_engine_pm_put(engine);
> +	return err;
>   }
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> index 24e20344dc22..e52c2b0cb245 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> @@ -7,6 +7,7 @@
>   #ifndef INTEL_ENGINE_PM_H
>   #define INTEL_ENGINE_PM_H
>   
> +#include "i915_request.h"
>   #include "intel_engine_types.h"
>   #include "intel_wakeref.h"
>   
> @@ -41,6 +42,26 @@ static inline void intel_engine_pm_flush(struct intel_engine_cs *engine)
>   	intel_wakeref_unlock_wait(&engine->wakeref);
>   }
>   
> +static inline struct i915_request *
> +intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> +{
> +	struct i915_request *rq;
> +
> +	/*
> +	 * The engine->kernel_context is special as it is used inside
> +	 * the engine-pm barrier (see __engine_park()), circumventing
> +	 * the usual mutexes and relying on the engine-pm barrier
> +	 * instead. So whenever we use the engine->kernel_context
> +	 * outside of the barrier, we must manually handle the
> +	 * engine wakeref to serialise with the use inside.
> +	 */
> +	intel_engine_pm_get(engine);
> +	rq = i915_request_create(engine->kernel_context);
> +	intel_engine_pm_put(engine);

i915_request_add does not have to be covered by the pm ref?

I am slightly confused by how patch converts some to this helper and at 
some places it open codes it.

Regards,

Tvrtko

> +
> +	return rq;
> +}
> +
>   void intel_engine_init__pm(struct intel_engine_cs *engine);
>   
>   #endif /* INTEL_ENGINE_PM_H */
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 226bd4cccb48..0c6d398980ba 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -6,6 +6,7 @@
>   
>   #include "i915_drv.h"
>   #include "intel_context.h"
> +#include "intel_engine_pm.h"
>   #include "intel_gt.h"
>   #include "intel_ring.h"
>   #include "intel_workarounds.h"
> @@ -1582,7 +1583,9 @@ static int engine_wa_list_verify(struct intel_context *ce,
>   	if (IS_ERR(vma))
>   		return PTR_ERR(vma);
>   
> +	intel_engine_pm_get(ce->engine);
>   	rq = intel_context_create_request(ce);
> +	intel_engine_pm_put(ce->engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_vma;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> index 5bc124574170..af354ccdbf40 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -121,7 +121,7 @@ static int __live_context_size(struct intel_engine_cs *engine,
>   		goto err_unpin;
>   
>   	/* Force the context switch */
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_unpin;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
> index 5981a7b71ec9..761d81f4bd68 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
> @@ -132,14 +132,18 @@ static int perf_mi_bb_start(void *arg)
>   		u32 cycles[COUNT];
>   		int i;
>   
> +		intel_engine_pm_get(engine);
> +
>   		batch = create_empty_batch(ce);
>   		if (IS_ERR(batch)) {
>   			err = PTR_ERR(batch);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
>   		err = i915_vma_sync(batch);
>   		if (err) {
> +			intel_engine_pm_put(engine);
>   			i915_vma_put(batch);
>   			break;
>   		}
> @@ -180,6 +184,7 @@ static int perf_mi_bb_start(void *arg)
>   			cycles[i] = rq->hwsp_seqno[3] - rq->hwsp_seqno[2];
>   		}
>   		i915_vma_put(batch);
> +		intel_engine_pm_put(engine);
>   		if (err)
>   			break;
>   
> @@ -251,15 +256,19 @@ static int perf_mi_noop(void *arg)
>   		u32 cycles[COUNT];
>   		int i;
>   
> +		intel_engine_pm_get(engine);
> +
>   		base = create_empty_batch(ce);
>   		if (IS_ERR(base)) {
>   			err = PTR_ERR(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
>   		err = i915_vma_sync(base);
>   		if (err) {
>   			i915_vma_put(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
> @@ -267,6 +276,7 @@ static int perf_mi_noop(void *arg)
>   		if (IS_ERR(nop)) {
>   			err = PTR_ERR(nop);
>   			i915_vma_put(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
> @@ -274,6 +284,7 @@ static int perf_mi_noop(void *arg)
>   		if (err) {
>   			i915_vma_put(nop);
>   			i915_vma_put(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
> @@ -327,6 +338,7 @@ static int perf_mi_noop(void *arg)
>   		}
>   		i915_vma_put(nop);
>   		i915_vma_put(base);
> +		intel_engine_pm_put(engine);
>   		if (err)
>   			break;
>   
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index fc142dd61dd1..ac8b9116d307 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -348,7 +348,7 @@ release_queue(struct intel_engine_cs *engine,
>   	struct i915_request *rq;
>   	u32 *cs;
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> @@ -497,7 +497,7 @@ static struct i915_request *nop_request(struct intel_engine_cs *engine)
>   {
>   	struct i915_request *rq;
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		return rq;
>   
> @@ -3698,7 +3698,7 @@ static int gpr_make_dirty(struct intel_engine_cs *engine)
>   	u32 *cs;
>   	int n;
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index a34d4fb52fa1..de010f527757 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -261,7 +261,9 @@ static int live_mocs_kernel(void *arg)
>   		return err;
>   
>   	for_each_engine(engine, gt, id) {
> +		intel_engine_pm_get(engine);
>   		err = check_mocs_engine(&mocs, engine->kernel_context);
> +		intel_engine_pm_put(engine);
>   		if (err)
>   			break;
>   	}
> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> index f04a59fe5d2c..e2d78cc22fb4 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> @@ -458,7 +458,7 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>   		goto out;
>   	}
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		goto out_unpin;
>   
> @@ -675,9 +675,7 @@ static int live_hwsp_wrap(void *arg)
>   		if (!intel_engine_can_store_dword(engine))
>   			continue;
>   
> -		intel_engine_pm_get(engine);
> -		rq = i915_request_create(engine->kernel_context);
> -		intel_engine_pm_put(engine);
> +		rq = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(rq)) {
>   			err = PTR_ERR(rq);
>   			goto out;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 608e6c3f3c1a..b46715b57576 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1968,7 +1968,9 @@ static int emit_oa_config(struct i915_perf_stream *stream,
>   	if (err)
>   		goto err_vma_put;
>   
> +	intel_engine_pm_get(ce->engine);
>   	rq = i915_request_create(ce);
> +	intel_engine_pm_put(ce->engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_vma_unpin;
> @@ -2165,7 +2167,7 @@ static int gen8_modify_context(struct intel_context *ce,
>   
>   	lockdep_assert_held(&ce->pin_mutex);
>   
> -	rq = i915_request_create(ce->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(ce->engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
> index 60290f78750d..6c1db3ded446 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_active.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_active.c
> @@ -99,7 +99,7 @@ __live_active_setup(struct drm_i915_private *i915)
>   	for_each_uabi_engine(engine, i915) {
>   		struct i915_request *rq;
>   
> -		rq = i915_request_create(engine->kernel_context);
> +		rq = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(rq)) {
>   			err = PTR_ERR(rq);
>   			break;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c b/drivers/gpu/drm/i915/selftests/i915_perf.c
> index aabd07f67e49..d1a1568c47ba 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_perf.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
> @@ -132,7 +132,7 @@ static int live_noa_delay(void *arg)
>   	for (i = 0; i < 4; i++)
>   		intel_write_status_page(stream->engine, 0x100 + i, 0);
>   
> -	rq = i915_request_create(stream->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(stream->engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto out;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
> index c16d1efd2ad4..99c94b4f69fb 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_request.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_request.c
> @@ -27,6 +27,7 @@
>   #include "gem/i915_gem_pm.h"
>   #include "gem/selftests/mock_context.h"
>   
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   
>   #include "i915_random.h"
> @@ -541,6 +542,7 @@ static int live_nop_request(void *arg)
>   		if (err)
>   			return err;
>   
> +		intel_engine_pm_get(engine);
>   		for_each_prime_number_from(prime, 1, 8192) {
>   			struct i915_request *request = NULL;
>   
> @@ -579,6 +581,7 @@ static int live_nop_request(void *arg)
>   			if (__igt_timeout(end_time, NULL))
>   				break;
>   		}
> +		intel_engine_pm_put(engine);
>   
>   		err = igt_live_test_end(&t);
>   		if (err)
> @@ -693,10 +696,13 @@ static int live_empty_request(void *arg)
>   		if (err)
>   			goto out_batch;
>   
> +		intel_engine_pm_get(engine);
> +
>   		/* Warmup / preload */
>   		request = empty_request(engine, batch);
>   		if (IS_ERR(request)) {
>   			err = PTR_ERR(request);
> +			intel_engine_pm_put(engine);
>   			goto out_batch;
>   		}
>   		i915_request_wait(request, 0, MAX_SCHEDULE_TIMEOUT);
> @@ -709,6 +715,7 @@ static int live_empty_request(void *arg)
>   				request = empty_request(engine, batch);
>   				if (IS_ERR(request)) {
>   					err = PTR_ERR(request);
> +					intel_engine_pm_put(engine);
>   					goto out_batch;
>   				}
>   			}
> @@ -722,6 +729,7 @@ static int live_empty_request(void *arg)
>   				break;
>   		}
>   		i915_request_put(request);
> +		intel_engine_pm_put(engine);
>   
>   		err = igt_live_test_end(&t);
>   		if (err)
> @@ -846,7 +854,7 @@ static int live_all_engines(void *arg)
>   
>   	idx = 0;
>   	for_each_uabi_engine(engine, i915) {
> -		request[idx] = i915_request_create(engine->kernel_context);
> +		request[idx] = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(request[idx])) {
>   			err = PTR_ERR(request[idx]);
>   			pr_err("%s: Request allocation failed with err=%d\n",
> @@ -963,7 +971,7 @@ static int live_sequential_engines(void *arg)
>   			goto out_free;
>   		}
>   
> -		request[idx] = i915_request_create(engine->kernel_context);
> +		request[idx] = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(request[idx])) {
>   			err = PTR_ERR(request[idx]);
>   			pr_err("%s: Request allocation failed for %s with err=%d\n",
> @@ -1068,15 +1076,19 @@ static int __live_parallel_engine1(void *arg)
>   	struct intel_engine_cs *engine = arg;
>   	IGT_TIMEOUT(end_time);
>   	unsigned long count;
> +	int err = 0;
>   
>   	count = 0;
> +	intel_engine_pm_get(engine);
>   	do {
>   		struct i915_request *rq;
> -		int err;
>   
>   		rq = i915_request_create(engine->kernel_context);
> -		if (IS_ERR(rq))
> -			return PTR_ERR(rq);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			if (err)
> +				break;
> +		}
>   
>   		i915_request_get(rq);
>   		i915_request_add(rq);
> @@ -1086,13 +1098,14 @@ static int __live_parallel_engine1(void *arg)
>   			err = -ETIME;
>   		i915_request_put(rq);
>   		if (err)
> -			return err;
> +			break;
>   
>   		count++;
>   	} while (!__igt_timeout(end_time, NULL));
> +	intel_engine_pm_put(engine);
>   
>   	pr_info("%s: %lu request + sync\n", engine->name, count);
> -	return 0;
> +	return err;
>   }
>   
>   static int __live_parallel_engineN(void *arg)
> @@ -1100,21 +1113,26 @@ static int __live_parallel_engineN(void *arg)
>   	struct intel_engine_cs *engine = arg;
>   	IGT_TIMEOUT(end_time);
>   	unsigned long count;
> +	int err = 0;
>   
>   	count = 0;
> +	intel_engine_pm_get(engine);
>   	do {
>   		struct i915_request *rq;
>   
>   		rq = i915_request_create(engine->kernel_context);
> -		if (IS_ERR(rq))
> -			return PTR_ERR(rq);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			break;
> +		}
>   
>   		i915_request_add(rq);
>   		count++;
>   	} while (!__igt_timeout(end_time, NULL));
> +	intel_engine_pm_put(engine);
>   
>   	pr_info("%s: %lu requests\n", engine->name, count);
> -	return 0;
> +	return err;
>   }
>   
>   static bool wake_all(struct drm_i915_private *i915)
> @@ -1158,9 +1176,11 @@ static int __live_parallel_spin(void *arg)
>   		return -ENOMEM;
>   	}
>   
> +	intel_engine_pm_get(engine);
>   	rq = igt_spinner_create_request(&spin,
>   					engine->kernel_context,
>   					MI_NOOP); /* no preemption */
> +	intel_engine_pm_put(engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		if (err == -ENODEV)
> diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> index b60916561462..04d0aa7b349e 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> @@ -506,7 +506,9 @@ static int igt_lmem_write_cpu(void *arg)
>   	}
>   
>   	/* Put the pages into a known state -- from the gpu for added fun */
> +	intel_engine_pm_get(engine);
>   	err = i915_gem_object_fill_blt(obj, engine->kernel_context, 0xdeadbeaf);
> +	intel_engine_pm_put(engine);
>   	if (err)
>   		goto out_unpin;
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context
@ 2019-11-25 10:44     ` Tvrtko Ursulin
  0 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2019-11-25 10:44 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 24/11/2019 17:05, Chris Wilson wrote:
> As the engine->kernel_context is used within the engine-pm barrier, we
> have to be careful when emitting requests outside of the barrier, as the
> strict timeline locking rules do not apply. Instead, we must ensure the
> engine_park() cannot be entered as we build the request, which is
> simplest by taking an explicit engine-pm wakeref around the request
> construction.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  3 +-
>   .../i915/gem/selftests/i915_gem_client_blt.c  |  2 +
>   .../i915/gem/selftests/i915_gem_coherency.c   |  3 +-
>   .../drm/i915/gem/selftests/i915_gem_context.c |  7 +++-
>   .../drm/i915/gem/selftests/i915_gem_mman.c    |  3 +-
>   .../i915/gem/selftests/i915_gem_object_blt.c  | 18 ++++++---
>   .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 14 +++++--
>   drivers/gpu/drm/i915/gt/intel_engine_pm.h     | 21 ++++++++++
>   drivers/gpu/drm/i915/gt/intel_workarounds.c   |  3 ++
>   drivers/gpu/drm/i915/gt/selftest_context.c    |  2 +-
>   drivers/gpu/drm/i915/gt/selftest_engine_cs.c  | 12 ++++++
>   drivers/gpu/drm/i915/gt/selftest_lrc.c        |  6 +--
>   drivers/gpu/drm/i915/gt/selftest_mocs.c       |  2 +
>   drivers/gpu/drm/i915/gt/selftest_timeline.c   |  6 +--
>   drivers/gpu/drm/i915/i915_perf.c              |  4 +-
>   drivers/gpu/drm/i915/selftests/i915_active.c  |  2 +-
>   drivers/gpu/drm/i915/selftests/i915_perf.c    |  2 +-
>   drivers/gpu/drm/i915/selftests/i915_request.c | 40 ++++++++++++++-----
>   .../drm/i915/selftests/intel_memory_region.c  |  2 +
>   19 files changed, 119 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 6f1e6181f67a..c94ac838401a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -70,6 +70,7 @@
>   #include <drm/i915_drm.h>
>   
>   #include "gt/intel_engine_heartbeat.h"
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_engine_user.h"
>   #include "gt/intel_lrc_reg.h"
>   #include "gt/intel_ring.h"
> @@ -1265,7 +1266,7 @@ gen8_modify_rpcs(struct intel_context *ce, struct intel_sseu sseu)
>   	if (!intel_context_is_pinned(ce))
>   		return 0;
>   
> -	rq = i915_request_create(ce->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(ce->engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
> index da8edee4fe0a..b972be165e85 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
> @@ -24,6 +24,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
>   
>   	prandom_seed_state(&prng, i915_selftest.random_seed);
>   
> +	intel_engine_pm_get(engine);
>   	do {
>   		const u32 max_block_size = S16_MAX * PAGE_SIZE;
>   		u32 sz = min_t(u64, ce->vm->total >> 4, prandom_u32_state(&prng));
> @@ -99,6 +100,7 @@ static int __igt_client_fill(struct intel_engine_cs *engine)
>   err_flush:
>   	if (err == -ENOMEM)
>   		err = 0;
> +	intel_engine_pm_put(engine);
>   
>   	return err;
>   }
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
> index 2b29f6b4e1dd..9d3cd1af61f6 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
> @@ -6,6 +6,7 @@
>   
>   #include <linux/prime_numbers.h>
>   
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   #include "gt/intel_gt_pm.h"
>   #include "gt/intel_ring.h"
> @@ -200,7 +201,7 @@ static int gpu_set(struct context *ctx, unsigned long offset, u32 v)
>   	if (IS_ERR(vma))
>   		return PTR_ERR(vma);
>   
> -	rq = i915_request_create(ctx->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(ctx->engine);
>   	if (IS_ERR(rq)) {
>   		i915_vma_unpin(vma);
>   		return PTR_ERR(rq);
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index e1d8ccd11409..2ea4790f3721 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -7,6 +7,7 @@
>   #include <linux/prime_numbers.h>
>   
>   #include "gem/i915_gem_pm.h"
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   #include "gt/intel_gt_requests.h"
>   #include "gt/intel_reset.h"
> @@ -1190,9 +1191,11 @@ __sseu_test(const char *name,
>   	struct igt_spinner *spin = NULL;
>   	int ret;
>   
> +	intel_engine_pm_get(ce->engine);
> +
>   	ret = __sseu_prepare(name, flags, ce, &spin);
>   	if (ret)
> -		return ret;
> +		goto out_pm;
>   
>   	ret = intel_context_reconfigure_sseu(ce, sseu);
>   	if (ret)
> @@ -1207,6 +1210,8 @@ __sseu_test(const char *name,
>   		igt_spinner_fini(spin);
>   		kfree(spin);
>   	}
> +out_pm:
> +	intel_engine_pm_put(ce->engine);
>   	return ret;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 9f1a69027a04..6ce9167f8c9f 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -6,6 +6,7 @@
>   
>   #include <linux/prime_numbers.h>
>   
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   #include "gt/intel_gt_pm.h"
>   #include "huge_gem_object.h"
> @@ -536,7 +537,7 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
>   		if (err)
>   			return err;
>   
> -		rq = i915_request_create(engine->kernel_context);
> +		rq = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(rq)) {
>   			i915_vma_unpin(vma);
>   			return PTR_ERR(rq);
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> index 675c1a20a2f1..62077fe46715 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> @@ -41,6 +41,7 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
>   		if (!engine)
>   			return 0;
>   
> +		intel_engine_pm_get(engine);
>   		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
>   			struct intel_context *ce = engine->kernel_context;
>   			ktime_t t0, t1;
> @@ -49,17 +50,20 @@ static int __perf_fill_blt(struct drm_i915_gem_object *obj)
>   
>   			err = i915_gem_object_fill_blt(obj, ce, 0);
>   			if (err)
> -				return err;
> +				break;
>   
>   			err = i915_gem_object_wait(obj,
>   						   I915_WAIT_ALL,
>   						   MAX_SCHEDULE_TIMEOUT);
>   			if (err)
> -				return err;
> +				break;
>   
>   			t1 = ktime_get();
>   			t[pass] = ktime_sub(t1, t0);
>   		}
> +		intel_engine_pm_put(engine);
> +		if (err)
> +			return err;
>   
>   		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
>   		pr_info("%s: blt %zd KiB fill: %lld MiB/s\n",
> @@ -109,6 +113,7 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
>   		struct intel_engine_cs *engine;
>   		ktime_t t[5];
>   		int pass;
> +		int err = 0;
>   
>   		engine = intel_engine_lookup_user(i915,
>   						  I915_ENGINE_CLASS_COPY,
> @@ -116,26 +121,29 @@ static int __perf_copy_blt(struct drm_i915_gem_object *src,
>   		if (!engine)
>   			return 0;
>   
> +		intel_engine_pm_get(engine);
>   		for (pass = 0; pass < ARRAY_SIZE(t); pass++) {
>   			struct intel_context *ce = engine->kernel_context;
>   			ktime_t t0, t1;
> -			int err;
>   
>   			t0 = ktime_get();
>   
>   			err = i915_gem_object_copy_blt(src, dst, ce);
>   			if (err)
> -				return err;
> +				break;
>   
>   			err = i915_gem_object_wait(dst,
>   						   I915_WAIT_ALL,
>   						   MAX_SCHEDULE_TIMEOUT);
>   			if (err)
> -				return err;
> +				break;
>   
>   			t1 = ktime_get();
>   			t[pass] = ktime_sub(t1, t0);
>   		}
> +		intel_engine_pm_put(engine);
> +		if (err)
> +			return err;
>   
>   		sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
>   		pr_info("%s: blt %zd KiB copy: %lld MiB/s\n",
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> index c91fd4e4af29..742628e40201 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> @@ -215,18 +215,26 @@ int intel_engine_pulse(struct intel_engine_cs *engine)
>   int intel_engine_flush_barriers(struct intel_engine_cs *engine)
>   {
>   	struct i915_request *rq;
> +	int err = 0;
>   
>   	if (llist_empty(&engine->barrier_tasks))
>   		return 0;
>   
> +	if (!intel_engine_pm_get_if_awake(engine))
> +		return 0;
> +
>   	rq = i915_request_create(engine->kernel_context);
> -	if (IS_ERR(rq))
> -		return PTR_ERR(rq);
> +	if (IS_ERR(rq)) {
> +		err = PTR_ERR(rq);
> +		goto out_rpm;
> +	}
>   
>   	idle_pulse(engine, rq);
>   	i915_request_add(rq);
>   
> -	return 0;
> +out_rpm:
> +	intel_engine_pm_put(engine);
> +	return err;
>   }
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> index 24e20344dc22..e52c2b0cb245 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
> @@ -7,6 +7,7 @@
>   #ifndef INTEL_ENGINE_PM_H
>   #define INTEL_ENGINE_PM_H
>   
> +#include "i915_request.h"
>   #include "intel_engine_types.h"
>   #include "intel_wakeref.h"
>   
> @@ -41,6 +42,26 @@ static inline void intel_engine_pm_flush(struct intel_engine_cs *engine)
>   	intel_wakeref_unlock_wait(&engine->wakeref);
>   }
>   
> +static inline struct i915_request *
> +intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> +{
> +	struct i915_request *rq;
> +
> +	/*
> +	 * The engine->kernel_context is special as it is used inside
> +	 * the engine-pm barrier (see __engine_park()), circumventing
> +	 * the usual mutexes and relying on the engine-pm barrier
> +	 * instead. So whenever we use the engine->kernel_context
> +	 * outside of the barrier, we must manually handle the
> +	 * engine wakeref to serialise with the use inside.
> +	 */
> +	intel_engine_pm_get(engine);
> +	rq = i915_request_create(engine->kernel_context);
> +	intel_engine_pm_put(engine);

i915_request_add does not have to be covered by the pm ref?

I am slightly confused by how patch converts some to this helper and at 
some places it open codes it.

Regards,

Tvrtko

> +
> +	return rq;
> +}
> +
>   void intel_engine_init__pm(struct intel_engine_cs *engine);
>   
>   #endif /* INTEL_ENGINE_PM_H */
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 226bd4cccb48..0c6d398980ba 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -6,6 +6,7 @@
>   
>   #include "i915_drv.h"
>   #include "intel_context.h"
> +#include "intel_engine_pm.h"
>   #include "intel_gt.h"
>   #include "intel_ring.h"
>   #include "intel_workarounds.h"
> @@ -1582,7 +1583,9 @@ static int engine_wa_list_verify(struct intel_context *ce,
>   	if (IS_ERR(vma))
>   		return PTR_ERR(vma);
>   
> +	intel_engine_pm_get(ce->engine);
>   	rq = intel_context_create_request(ce);
> +	intel_engine_pm_put(ce->engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_vma;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> index 5bc124574170..af354ccdbf40 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -121,7 +121,7 @@ static int __live_context_size(struct intel_engine_cs *engine,
>   		goto err_unpin;
>   
>   	/* Force the context switch */
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_unpin;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
> index 5981a7b71ec9..761d81f4bd68 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
> @@ -132,14 +132,18 @@ static int perf_mi_bb_start(void *arg)
>   		u32 cycles[COUNT];
>   		int i;
>   
> +		intel_engine_pm_get(engine);
> +
>   		batch = create_empty_batch(ce);
>   		if (IS_ERR(batch)) {
>   			err = PTR_ERR(batch);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
>   		err = i915_vma_sync(batch);
>   		if (err) {
> +			intel_engine_pm_put(engine);
>   			i915_vma_put(batch);
>   			break;
>   		}
> @@ -180,6 +184,7 @@ static int perf_mi_bb_start(void *arg)
>   			cycles[i] = rq->hwsp_seqno[3] - rq->hwsp_seqno[2];
>   		}
>   		i915_vma_put(batch);
> +		intel_engine_pm_put(engine);
>   		if (err)
>   			break;
>   
> @@ -251,15 +256,19 @@ static int perf_mi_noop(void *arg)
>   		u32 cycles[COUNT];
>   		int i;
>   
> +		intel_engine_pm_get(engine);
> +
>   		base = create_empty_batch(ce);
>   		if (IS_ERR(base)) {
>   			err = PTR_ERR(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
>   		err = i915_vma_sync(base);
>   		if (err) {
>   			i915_vma_put(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
> @@ -267,6 +276,7 @@ static int perf_mi_noop(void *arg)
>   		if (IS_ERR(nop)) {
>   			err = PTR_ERR(nop);
>   			i915_vma_put(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
> @@ -274,6 +284,7 @@ static int perf_mi_noop(void *arg)
>   		if (err) {
>   			i915_vma_put(nop);
>   			i915_vma_put(base);
> +			intel_engine_pm_put(engine);
>   			break;
>   		}
>   
> @@ -327,6 +338,7 @@ static int perf_mi_noop(void *arg)
>   		}
>   		i915_vma_put(nop);
>   		i915_vma_put(base);
> +		intel_engine_pm_put(engine);
>   		if (err)
>   			break;
>   
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index fc142dd61dd1..ac8b9116d307 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -348,7 +348,7 @@ release_queue(struct intel_engine_cs *engine,
>   	struct i915_request *rq;
>   	u32 *cs;
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> @@ -497,7 +497,7 @@ static struct i915_request *nop_request(struct intel_engine_cs *engine)
>   {
>   	struct i915_request *rq;
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		return rq;
>   
> @@ -3698,7 +3698,7 @@ static int gpr_make_dirty(struct intel_engine_cs *engine)
>   	u32 *cs;
>   	int n;
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index a34d4fb52fa1..de010f527757 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -261,7 +261,9 @@ static int live_mocs_kernel(void *arg)
>   		return err;
>   
>   	for_each_engine(engine, gt, id) {
> +		intel_engine_pm_get(engine);
>   		err = check_mocs_engine(&mocs, engine->kernel_context);
> +		intel_engine_pm_put(engine);
>   		if (err)
>   			break;
>   	}
> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> index f04a59fe5d2c..e2d78cc22fb4 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> @@ -458,7 +458,7 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>   		goto out;
>   	}
>   
> -	rq = i915_request_create(engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(engine);
>   	if (IS_ERR(rq))
>   		goto out_unpin;
>   
> @@ -675,9 +675,7 @@ static int live_hwsp_wrap(void *arg)
>   		if (!intel_engine_can_store_dword(engine))
>   			continue;
>   
> -		intel_engine_pm_get(engine);
> -		rq = i915_request_create(engine->kernel_context);
> -		intel_engine_pm_put(engine);
> +		rq = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(rq)) {
>   			err = PTR_ERR(rq);
>   			goto out;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 608e6c3f3c1a..b46715b57576 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1968,7 +1968,9 @@ static int emit_oa_config(struct i915_perf_stream *stream,
>   	if (err)
>   		goto err_vma_put;
>   
> +	intel_engine_pm_get(ce->engine);
>   	rq = i915_request_create(ce);
> +	intel_engine_pm_put(ce->engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto err_vma_unpin;
> @@ -2165,7 +2167,7 @@ static int gen8_modify_context(struct intel_context *ce,
>   
>   	lockdep_assert_held(&ce->pin_mutex);
>   
> -	rq = i915_request_create(ce->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(ce->engine);
>   	if (IS_ERR(rq))
>   		return PTR_ERR(rq);
>   
> diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
> index 60290f78750d..6c1db3ded446 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_active.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_active.c
> @@ -99,7 +99,7 @@ __live_active_setup(struct drm_i915_private *i915)
>   	for_each_uabi_engine(engine, i915) {
>   		struct i915_request *rq;
>   
> -		rq = i915_request_create(engine->kernel_context);
> +		rq = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(rq)) {
>   			err = PTR_ERR(rq);
>   			break;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c b/drivers/gpu/drm/i915/selftests/i915_perf.c
> index aabd07f67e49..d1a1568c47ba 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_perf.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
> @@ -132,7 +132,7 @@ static int live_noa_delay(void *arg)
>   	for (i = 0; i < 4; i++)
>   		intel_write_status_page(stream->engine, 0x100 + i, 0);
>   
> -	rq = i915_request_create(stream->engine->kernel_context);
> +	rq = intel_engine_create_kernel_request(stream->engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		goto out;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
> index c16d1efd2ad4..99c94b4f69fb 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_request.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_request.c
> @@ -27,6 +27,7 @@
>   #include "gem/i915_gem_pm.h"
>   #include "gem/selftests/mock_context.h"
>   
> +#include "gt/intel_engine_pm.h"
>   #include "gt/intel_gt.h"
>   
>   #include "i915_random.h"
> @@ -541,6 +542,7 @@ static int live_nop_request(void *arg)
>   		if (err)
>   			return err;
>   
> +		intel_engine_pm_get(engine);
>   		for_each_prime_number_from(prime, 1, 8192) {
>   			struct i915_request *request = NULL;
>   
> @@ -579,6 +581,7 @@ static int live_nop_request(void *arg)
>   			if (__igt_timeout(end_time, NULL))
>   				break;
>   		}
> +		intel_engine_pm_put(engine);
>   
>   		err = igt_live_test_end(&t);
>   		if (err)
> @@ -693,10 +696,13 @@ static int live_empty_request(void *arg)
>   		if (err)
>   			goto out_batch;
>   
> +		intel_engine_pm_get(engine);
> +
>   		/* Warmup / preload */
>   		request = empty_request(engine, batch);
>   		if (IS_ERR(request)) {
>   			err = PTR_ERR(request);
> +			intel_engine_pm_put(engine);
>   			goto out_batch;
>   		}
>   		i915_request_wait(request, 0, MAX_SCHEDULE_TIMEOUT);
> @@ -709,6 +715,7 @@ static int live_empty_request(void *arg)
>   				request = empty_request(engine, batch);
>   				if (IS_ERR(request)) {
>   					err = PTR_ERR(request);
> +					intel_engine_pm_put(engine);
>   					goto out_batch;
>   				}
>   			}
> @@ -722,6 +729,7 @@ static int live_empty_request(void *arg)
>   				break;
>   		}
>   		i915_request_put(request);
> +		intel_engine_pm_put(engine);
>   
>   		err = igt_live_test_end(&t);
>   		if (err)
> @@ -846,7 +854,7 @@ static int live_all_engines(void *arg)
>   
>   	idx = 0;
>   	for_each_uabi_engine(engine, i915) {
> -		request[idx] = i915_request_create(engine->kernel_context);
> +		request[idx] = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(request[idx])) {
>   			err = PTR_ERR(request[idx]);
>   			pr_err("%s: Request allocation failed with err=%d\n",
> @@ -963,7 +971,7 @@ static int live_sequential_engines(void *arg)
>   			goto out_free;
>   		}
>   
> -		request[idx] = i915_request_create(engine->kernel_context);
> +		request[idx] = intel_engine_create_kernel_request(engine);
>   		if (IS_ERR(request[idx])) {
>   			err = PTR_ERR(request[idx]);
>   			pr_err("%s: Request allocation failed for %s with err=%d\n",
> @@ -1068,15 +1076,19 @@ static int __live_parallel_engine1(void *arg)
>   	struct intel_engine_cs *engine = arg;
>   	IGT_TIMEOUT(end_time);
>   	unsigned long count;
> +	int err = 0;
>   
>   	count = 0;
> +	intel_engine_pm_get(engine);
>   	do {
>   		struct i915_request *rq;
> -		int err;
>   
>   		rq = i915_request_create(engine->kernel_context);
> -		if (IS_ERR(rq))
> -			return PTR_ERR(rq);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			if (err)
> +				break;
> +		}
>   
>   		i915_request_get(rq);
>   		i915_request_add(rq);
> @@ -1086,13 +1098,14 @@ static int __live_parallel_engine1(void *arg)
>   			err = -ETIME;
>   		i915_request_put(rq);
>   		if (err)
> -			return err;
> +			break;
>   
>   		count++;
>   	} while (!__igt_timeout(end_time, NULL));
> +	intel_engine_pm_put(engine);
>   
>   	pr_info("%s: %lu request + sync\n", engine->name, count);
> -	return 0;
> +	return err;
>   }
>   
>   static int __live_parallel_engineN(void *arg)
> @@ -1100,21 +1113,26 @@ static int __live_parallel_engineN(void *arg)
>   	struct intel_engine_cs *engine = arg;
>   	IGT_TIMEOUT(end_time);
>   	unsigned long count;
> +	int err = 0;
>   
>   	count = 0;
> +	intel_engine_pm_get(engine);
>   	do {
>   		struct i915_request *rq;
>   
>   		rq = i915_request_create(engine->kernel_context);
> -		if (IS_ERR(rq))
> -			return PTR_ERR(rq);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			break;
> +		}
>   
>   		i915_request_add(rq);
>   		count++;
>   	} while (!__igt_timeout(end_time, NULL));
> +	intel_engine_pm_put(engine);
>   
>   	pr_info("%s: %lu requests\n", engine->name, count);
> -	return 0;
> +	return err;
>   }
>   
>   static bool wake_all(struct drm_i915_private *i915)
> @@ -1158,9 +1176,11 @@ static int __live_parallel_spin(void *arg)
>   		return -ENOMEM;
>   	}
>   
> +	intel_engine_pm_get(engine);
>   	rq = igt_spinner_create_request(&spin,
>   					engine->kernel_context,
>   					MI_NOOP); /* no preemption */
> +	intel_engine_pm_put(engine);
>   	if (IS_ERR(rq)) {
>   		err = PTR_ERR(rq);
>   		if (err == -ENODEV)
> diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> index b60916561462..04d0aa7b349e 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> @@ -506,7 +506,9 @@ static int igt_lmem_write_cpu(void *arg)
>   	}
>   
>   	/* Put the pages into a known state -- from the gpu for added fun */
> +	intel_engine_pm_get(engine);
>   	err = i915_gem_object_fill_blt(obj, engine->kernel_context, 0xdeadbeaf);
> +	intel_engine_pm_put(engine);
>   	if (err)
>   		goto out_unpin;
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context
@ 2019-11-25 10:54       ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-25 10:54 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-11-25 10:44:15)
> 
> On 24/11/2019 17:05, Chris Wilson wrote:
> > +static inline struct i915_request *
> > +intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> > +{
> > +     struct i915_request *rq;
> > +
> > +     /*
> > +      * The engine->kernel_context is special as it is used inside
> > +      * the engine-pm barrier (see __engine_park()), circumventing
> > +      * the usual mutexes and relying on the engine-pm barrier
> > +      * instead. So whenever we use the engine->kernel_context
> > +      * outside of the barrier, we must manually handle the
> > +      * engine wakeref to serialise with the use inside.
> > +      */
> > +     intel_engine_pm_get(engine);
> > +     rq = i915_request_create(engine->kernel_context);
> > +     intel_engine_pm_put(engine);
> 
> i915_request_add does not have to be covered by the pm ref?

No, the normal course of action is:
i915_request_create:
	mutex_lock(timeline->mutex)
	intel_context_enter:
		if (!ce->active_count++) {
			intel_engine_pm_get();
			intel_timeline_enter();
		}

With the primary purpose of reducing atomics behind a simple usage
counter covered by the timeline->mutex.

Of course, engine-pm tries to be clever because it has to be called
under any timeline->mutex from retire, and so tries to fake
timeline->mutex itself.

> I am slightly confused by how patch converts some to this helper and at 
> some places it open codes it.

There were a few perf benchmarks, that I thought better to not add the
extra pair of atomics, and intel_engine_heartbeat.c is supposed to
cognisant of the rules and only operates on an active engine-pm.

The intent of intel_engine_create_kernel_request() was to apply a simple
fixup with a comment as to what is going on with the kernel_context.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context
@ 2019-11-25 10:54       ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2019-11-25 10:54 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-11-25 10:44:15)
> 
> On 24/11/2019 17:05, Chris Wilson wrote:
> > +static inline struct i915_request *
> > +intel_engine_create_kernel_request(struct intel_engine_cs *engine)
> > +{
> > +     struct i915_request *rq;
> > +
> > +     /*
> > +      * The engine->kernel_context is special as it is used inside
> > +      * the engine-pm barrier (see __engine_park()), circumventing
> > +      * the usual mutexes and relying on the engine-pm barrier
> > +      * instead. So whenever we use the engine->kernel_context
> > +      * outside of the barrier, we must manually handle the
> > +      * engine wakeref to serialise with the use inside.
> > +      */
> > +     intel_engine_pm_get(engine);
> > +     rq = i915_request_create(engine->kernel_context);
> > +     intel_engine_pm_put(engine);
> 
> i915_request_add does not have to be covered by the pm ref?

No, the normal course of action is:
i915_request_create:
	mutex_lock(timeline->mutex)
	intel_context_enter:
		if (!ce->active_count++) {
			intel_engine_pm_get();
			intel_timeline_enter();
		}

With the primary purpose of reducing atomics behind a simple usage
counter covered by the timeline->mutex.

Of course, engine-pm tries to be clever because it has to be called
under any timeline->mutex from retire, and so tries to fake
timeline->mutex itself.

> I am slightly confused by how patch converts some to this helper and at 
> some places it open codes it.

There were a few perf benchmarks, that I thought better to not add the
extra pair of atomics, and intel_engine_heartbeat.c is supposed to
cognisant of the rules and only operates on an active engine-pm.

The intent of intel_engine_create_kernel_request() was to apply a simple
fixup with a comment as to what is going on with the kernel_context.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2019-11-25 10:54 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-24 17:05 [CI 1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access Chris Wilson
2019-11-24 17:05 ` [Intel-gfx] " Chris Wilson
2019-11-24 17:05 ` [CI 2/4] drm/i915: Serialise with engine-pm around requests on the kernel_context Chris Wilson
2019-11-24 17:05   ` [Intel-gfx] " Chris Wilson
2019-11-25 10:44   ` Tvrtko Ursulin
2019-11-25 10:44     ` [Intel-gfx] " Tvrtko Ursulin
2019-11-25 10:54     ` Chris Wilson
2019-11-25 10:54       ` [Intel-gfx] " Chris Wilson
2019-11-24 17:05 ` [CI 3/4] drm/i915/gt: Adapt engine_park synchronisation rules for engine_retire Chris Wilson
2019-11-24 17:05   ` [Intel-gfx] " Chris Wilson
2019-11-24 17:05 ` [CI 4/4] drm/i915/gt: Schedule request retirement when timeline idles Chris Wilson
2019-11-24 17:05   ` [Intel-gfx] " Chris Wilson
2019-11-24 17:12 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/4] drm/i915/gt: Mark the execlists->active as the primary volatile access Patchwork
2019-11-24 17:12   ` [Intel-gfx] " Patchwork
2019-11-24 17:36 ` ✓ Fi.CI.BAT: success " Patchwork
2019-11-24 17:36   ` [Intel-gfx] " Patchwork
2019-11-24 22:42 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-11-24 22:42   ` [Intel-gfx] " Patchwork
2019-11-25  9:16 ` [CI 1/4] " Mika Kuoppala
2019-11-25  9:16   ` [Intel-gfx] " Mika Kuoppala
2019-11-25  9:23   ` Chris Wilson
2019-11-25  9:23     ` [Intel-gfx] " Chris Wilson
2019-11-25  9:38     ` Mika Kuoppala
2019-11-25  9:38       ` [Intel-gfx] " Mika Kuoppala

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.