All of lore.kernel.org
 help / color / mirror / Atom feed
* Short list of hopeful bug fixes
@ 2019-07-30 11:21 Chris Wilson
  2019-07-30 11:21 ` [PATCH 1/3] drm/i915/execlists: Always clear pending&inflight requests on reset Chris Wilson
                   ` (10 more replies)
  0 siblings, 11 replies; 21+ messages in thread
From: Chris Wilson @ 2019-07-30 11:21 UTC (permalink / raw)
  To: intel-gfx

Just trying to steal some of Tvrtko's time... You are all more than
welcome to look over the list and tut.
-Chris


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

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

* [PATCH 1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
@ 2019-07-30 11:21 ` Chris Wilson
  2019-07-30 11:21 ` [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests Chris Wilson
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2019-07-30 11:21 UTC (permalink / raw)
  To: intel-gfx

If we skip the reset as we found the engine inactive at the time of the
reset, we still need to clear the residual inflight & pending request
bookkeeping to reflect the current state of HW.

Otherwise, we may end up stuck in a loop like:

<7> [416.490346] hangcheck rcs0
<7> [416.490371] hangcheck 	Awake? 1
<7> [416.490376] hangcheck 	Hangcheck: 8003 ms ago
<7> [416.490380] hangcheck 	Reset count: 0 (global 0)
<7> [416.490383] hangcheck 	Requests:
<7> [416.491210] hangcheck 	RING_START: 0x0017b000
<7> [416.491983] hangcheck 	RING_HEAD:  0x00000048
<7> [416.491992] hangcheck 	RING_TAIL:  0x00000048
<7> [416.492006] hangcheck 	RING_CTL:   0x00000000
<7> [416.492037] hangcheck 	RING_MODE:  0x00000200 [idle]
<7> [416.492044] hangcheck 	RING_IMR: 00000000
<7> [416.492809] hangcheck 	ACTHD:  0x00000000_9ca00048
<7> [416.492824] hangcheck 	BBADDR: 0x00000000_00001004
<7> [416.492838] hangcheck 	DMA_FADDR: 0x00000000_00000000
<7> [416.492845] hangcheck 	IPEIR: 0x00000000
<7> [416.492852] hangcheck 	IPEHR: 0x00000000
<7> [416.492863] hangcheck 	Execlist status: 0x00018001 00000000, entries 12
<7> [416.492869] hangcheck 	Execlist CSB read 1, write 1, tasklet queued? no (enabled)
<7> [416.492938] hangcheck 		Pending[0] ring:{start:0017b000, hwsp:fedf9000, seqno:00016fd6}, rq:  20ffa:16fd6!+  prio=-4094 @ 8307ms: signaled
<7> [416.492972] hangcheck 		Queue priority hint: -4093
<7> [416.492979] hangcheck 		Q  20ffa:16fd8-  prio=-4093 @ 8307ms: [i915]
<7> [416.492985] hangcheck 		Q  20ffa:16fda  prio=-4094 @ 8307ms: [i915]
<7> [416.492990] hangcheck 		Q  20ffa:16fdc  prio=-4094 @ 8307ms: [i915]
<7> [416.492996] hangcheck 		Q  20ffa:16fde  prio=-4094 @ 8307ms: [i915]
<7> [416.493001] hangcheck 		Q  20ffa:16fe0  prio=-4094 @ 8307ms: [i915]
<7> [416.493007] hangcheck 		Q  20ffa:16fe2  prio=-4094 @ 8307ms: [i915]
<7> [416.493013] hangcheck 		Q  20ffa:16fe4  prio=-4094 @ 8307ms: [i915]
<7> [416.493021] hangcheck 		...skipping 21 queued requests...
<7> [416.493027] hangcheck 		Q  20ffa:17010  prio=-4094 @ 8307ms: [i915]
<7> [416.493081] hangcheck HWSP:
<7> [416.493089] hangcheck [0000] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<7> [416.493094] hangcheck *
<7> [416.493100] hangcheck [0040] 10008002 00000000 10000018 00000000 10000018 00000000 10000001 00000000
<7> [416.493106] hangcheck [0060] 10000018 00000000 10000001 00000000 10000018 00000000 10000001 00000000
<7> [416.493111] hangcheck *
<7> [416.493117] hangcheck [00a0] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000001
<7> [416.493123] hangcheck [00c0] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<7> [416.493127] hangcheck *
<7> [416.493132] hangcheck Idle? no
<6> [416.512124] i915 0000:00:02.0: GPU HANG: ecode 11:0:0x00000000, hang on rcs0
<6> [416.512205] [drm] GPU hangs can indicate a bug anywhere in the entire gfx stack, including userspace.
<6> [416.512207] [drm] Please file a _new_ bug report on bugs.freedesktop.org against DRI -> DRM/Intel
<6> [416.512208] [drm] drm/i915 developers can then reassign to the right component if it's not a kernel issue.
<6> [416.512210] [drm] The gpu crash dump is required to analyze gpu hangs, so please always attach it.
<6> [416.512212] [drm] GPU crash dump saved to /sys/class/drm/card0/error
<5> [416.513602] i915 0000:00:02.0: Resetting rcs0 for hang on rcs0
<7> [424.489258] hangcheck rcs0
<7> [424.489263] hangcheck 	Awake? 1
<7> [424.489267] hangcheck 	Hangcheck: 5954 ms ago
<7> [424.489271] hangcheck 	Reset count: 1 (global 0)
<7> [424.489274] hangcheck 	Requests:
<7> [424.490128] hangcheck 	RING_START: 0x00000000
<7> [424.490870] hangcheck 	RING_HEAD:  0x00000000
<7> [424.490877] hangcheck 	RING_TAIL:  0x00000000
<7> [424.490887] hangcheck 	RING_CTL:   0x00000000
<7> [424.490897] hangcheck 	RING_MODE:  0x00000200 [idle]
<7> [424.490904] hangcheck 	RING_IMR: 00000000
<7> [424.490917] hangcheck 	ACTHD:  0x00000000_00000000
<7> [424.490930] hangcheck 	BBADDR: 0x00000000_00000000
<7> [424.490943] hangcheck 	DMA_FADDR: 0x00000000_00000000
<7> [424.490950] hangcheck 	IPEIR: 0x00000000
<7> [424.490956] hangcheck 	IPEHR: 0x00000000
<7> [424.490968] hangcheck 	Execlist status: 0x00000001 00000000, entries 12
<7> [424.490972] hangcheck 	Execlist CSB read 11, write 11, tasklet queued? no (enabled)
<7> [424.490983] hangcheck 		Pending[0] ring:{start:0017b000, hwsp:fedf9000, seqno:00016fd6}, rq:  20ffa:16fd6!+  prio=-4094 @ 16305ms: signaled
<7> [424.490989] hangcheck 		Queue priority hint: -4093
<7> [424.490996] hangcheck 		Q  20ffa:16fd8-  prio=-4093 @ 16305ms: [i915]
<7> [424.491001] hangcheck 		Q  20ffa:16fda  prio=-4094 @ 16305ms: [i915]
<7> [424.491006] hangcheck 		Q  20ffa:16fdc  prio=-4094 @ 16305ms: [i915]
<7> [424.491011] hangcheck 		Q  20ffa:16fde  prio=-4094 @ 16305ms: [i915]
<7> [424.491016] hangcheck 		Q  20ffa:16fe0  prio=-4094 @ 16305ms: [i915]
<7> [424.491022] hangcheck 		Q  20ffa:16fe2  prio=-4094 @ 16305ms: [i915]
<7> [424.491048] hangcheck 		Q  20ffa:16fe4  prio=-4094 @ 16305ms: [i915]
<7> [424.491057] hangcheck 		...skipping 21 queued requests...
<7> [424.491063] hangcheck 		Q  20ffa:17010  prio=-4094 @ 16305ms: [i915]
<7> [424.491095] hangcheck HWSP:
<7> [424.491102] hangcheck [0000] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<7> [424.491106] hangcheck *
<7> [424.491113] hangcheck [0040] 10008002 00000000 10000018 00000000 10000018 00000000 10000001 00000000
<7> [424.491118] hangcheck [0060] 10000018 00000000 10000001 00000000 10000018 00000000 10000001 00000000
<7> [424.491122] hangcheck *
<7> [424.491127] hangcheck [00a0] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 0000000b
<7> [424.491133] hangcheck [00c0] 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
<7> [424.491136] hangcheck *
<7> [424.491141] hangcheck Idle? no
<5> [424.491834] i915 0000:00:02.0: Resetting rcs0 for hang on rcs0

Where not having cleared the pending array on reset, it persists
indefinitely.

Fixes: fff8102aaed5 ("drm/i915/execlists: Process interrupted context on reset")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 4d7c4d0dbf75..86dd1eddceac 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2283,18 +2283,6 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
 	GEM_BUG_ON(i915_active_is_idle(&ce->active));
 	GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
 	rq = active_request(rq);
-
-	/*
-	 * Catch up with any missed context-switch interrupts.
-	 *
-	 * Ideally we would just read the remaining CSB entries now that we
-	 * know the gpu is idle. However, the CSB registers are sometimes^W
-	 * often trashed across a GPU reset! Instead we have to rely on
-	 * guessing the missed context-switch events by looking at what
-	 * requests were completed.
-	 */
-	execlists_cancel_port_requests(execlists);
-
 	if (!rq) {
 		ce->ring->head = ce->ring->tail;
 		goto out_replay;
@@ -2356,6 +2344,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
 
 unwind:
 	/* Push back any incomplete requests for replay after the reset. */
+	execlists_cancel_port_requests(execlists);
 	__unwind_incomplete_requests(engine);
 }
 
-- 
2.22.0

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

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

* [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
  2019-07-30 11:21 ` [PATCH 1/3] drm/i915/execlists: Always clear pending&inflight requests on reset Chris Wilson
@ 2019-07-30 11:21 ` Chris Wilson
  2019-08-01 17:24   ` Tvrtko Ursulin
  2019-07-30 11:21   ` Chris Wilson
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Chris Wilson @ 2019-07-30 11:21 UTC (permalink / raw)
  To: intel-gfx

By placing our idle-barriers in the i915_active fence tree, we expose
those for reuse by other components that are issuing requests along the
kernel_context. Reusing the proto-barrier active_node is perfectly fine
as the new request implies a context-switch, and so an opportune point
to run the idle-barrier. However, the proto-barrier is not equivalent
to a normal active_node and care must be taken to avoid dereferencing the
ERR_PTR used as its request marker.

Reported-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context.c       |  40 ++-
 drivers/gpu/drm/i915/gt/intel_context.h       |  13 +-
 drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   2 +-
 drivers/gpu/drm/i915/gt/selftest_context.c    | 310 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_active.c            | 246 +++++++++++---
 drivers/gpu/drm/i915/i915_active.h            |   2 +-
 drivers/gpu/drm/i915/i915_active_types.h      |   2 +-
 .../drm/i915/selftests/i915_live_selftests.h  |   3 +-
 8 files changed, 555 insertions(+), 63 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_context.c

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index d64b45f7ec6d..211ac6568a5d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -162,23 +162,41 @@ static int __intel_context_active(struct i915_active *active)
 	if (err)
 		goto err_ring;
 
+	return 0;
+
+err_ring:
+	intel_ring_unpin(ce->ring);
+err_put:
+	intel_context_put(ce);
+	return err;
+}
+
+int intel_context_active_acquire(struct intel_context *ce)
+{
+	int err;
+
+	err = i915_active_acquire(&ce->active);
+	if (err)
+		return err;
+
 	/* Preallocate tracking nodes */
 	if (!i915_gem_context_is_kernel(ce->gem_context)) {
 		err = i915_active_acquire_preallocate_barrier(&ce->active,
 							      ce->engine);
-		if (err)
-			goto err_state;
+		if (err) {
+			i915_active_release(&ce->active);
+			return err;
+		}
 	}
 
 	return 0;
+}
 
-err_state:
-	__context_unpin_state(ce->state);
-err_ring:
-	intel_ring_unpin(ce->ring);
-err_put:
-	intel_context_put(ce);
-	return err;
+void intel_context_active_release(struct intel_context *ce)
+{
+	/* Nodes preallocated in intel_context_active() */
+	i915_active_acquire_barrier(&ce->active);
+	i915_active_release(&ce->active);
 }
 
 void
@@ -297,3 +315,7 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
 
 	return rq;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_context.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 23c7e4c0ce7c..07f9924de48f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -104,17 +104,8 @@ static inline void intel_context_exit(struct intel_context *ce)
 		ce->ops->exit(ce);
 }
 
-static inline int intel_context_active_acquire(struct intel_context *ce)
-{
-	return i915_active_acquire(&ce->active);
-}
-
-static inline void intel_context_active_release(struct intel_context *ce)
-{
-	/* Nodes preallocated in intel_context_active() */
-	i915_active_acquire_barrier(&ce->active);
-	i915_active_release(&ce->active);
-}
+int intel_context_active_acquire(struct intel_context *ce);
+void intel_context_active_release(struct intel_context *ce);
 
 static inline struct intel_context *intel_context_get(struct intel_context *ce)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index e74fbf04a68d..ce54092475da 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -90,7 +90,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 	/* Check again on the next retirement. */
 	engine->wakeref_serial = engine->serial + 1;
 
-	i915_request_add_barriers(rq);
+	i915_request_add_active_barriers(rq);
 	__i915_request_commit(rq);
 
 	return false;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
new file mode 100644
index 000000000000..d39b5594cb02
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -0,0 +1,310 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "intel_gt.h"
+
+#include "gem/selftests/mock_context.h"
+#include "selftests/igt_flush_test.h"
+#include "selftests/mock_drm.h"
+
+static int request_sync(struct i915_request *rq)
+{
+	long timeout;
+	int err = 0;
+
+	i915_request_get(rq);
+
+	i915_request_add(rq);
+	timeout = i915_request_wait(rq, 0, HZ / 10);
+	if (timeout < 0)
+		err = timeout;
+	else
+		i915_request_retire_upto(rq);
+
+	i915_request_put(rq);
+
+	return err;
+}
+
+static int context_sync(struct intel_context *ce)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err = 0;
+
+	do {
+		struct i915_request *rq;
+		long timeout;
+
+		rcu_read_lock();
+		rq = rcu_dereference(tl->last_request.request);
+		if (rq)
+			rq = i915_request_get_rcu(rq);
+		rcu_read_unlock();
+		if (!rq)
+			break;
+
+		timeout = i915_request_wait(rq, 0, HZ / 10);
+		if (timeout < 0)
+			err = timeout;
+		else
+			i915_request_retire_upto(rq);
+
+		i915_request_put(rq);
+	} while (!err);
+
+	return err;
+}
+
+static int __live_active_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *ce;
+	int pass;
+	int err;
+
+	/*
+	 * We keep active contexts alive until after a subsequent context
+	 * switch as the final write from the context-save will be after
+	 * we retire the final request. We track when we unpin the context,
+	 * under the presumption that the final pin is from the last request,
+	 * and instead of immediately unpinning the context, we add a task
+	 * to unpin the context from the next idle-barrier.
+	 *
+	 * This test makes sure that the context is kept alive until a
+	 * subsequent idle-barrier (emitted when the engine wakeref hits 0
+	 * with no more outstanding requests).
+	 */
+
+	if (intel_engine_pm_is_awake(engine)) {
+		pr_err("%s is awake before starting %s!\n",
+		       engine->name, __func__);
+		return -EINVAL;
+	}
+
+	ce = intel_context_create(fixme, engine);
+	if (!ce)
+		return -ENOMEM;
+
+	for (pass = 0; pass <= 2; pass++) {
+		struct i915_request *rq;
+
+		rq = intel_context_create_request(ce);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			goto err;
+		}
+
+		err = request_sync(rq);
+		if (err)
+			goto err;
+
+		/* Context will be kept active until after an idle-barrier. */
+		if (i915_active_is_idle(&ce->active)) {
+			pr_err("context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			goto err;
+		}
+
+		if (!intel_engine_pm_is_awake(engine)) {
+			pr_err("%s is asleep before idle-barrier\n",
+			       engine->name);
+			err = -EINVAL;
+			goto err;
+		}
+	}
+
+	/* Now make sure our idle-barriers are flushed */
+	err = context_sync(engine->kernel_context);
+	if (err)
+		goto err;
+
+	if (!i915_active_is_idle(&ce->active)) {
+		pr_err("context is still active!");
+		err = -EINVAL;
+	}
+
+	if (intel_engine_pm_is_awake(engine)) {
+		struct drm_printer p = drm_debug_printer(__func__);
+
+		intel_engine_dump(engine, &p,
+				  "%s is still awake after idle-barriers\n",
+				  engine->name);
+		GEM_TRACE_DUMP();
+
+		err = -EINVAL;
+		goto err;
+	}
+
+err:
+	intel_context_put(ce);
+	return err;
+}
+
+static int live_active_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_active_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+static int __remote_sync(struct intel_context *ce, struct intel_context *remote)
+{
+	struct i915_request *rq;
+	int err;
+
+	err = intel_context_pin(remote);
+	if (err)
+		return err;
+
+	rq = intel_context_create_request(ce);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto unpin;
+	}
+
+	err = intel_context_prepare_remote_request(remote, rq);
+	if (err) {
+		i915_request_add(rq);
+		goto unpin;
+	}
+
+	err = request_sync(rq);
+
+unpin:
+	intel_context_unpin(remote);
+	return err;
+}
+
+static int __live_remote_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *local, *remote;
+	int pass;
+	int err;
+
+	/*
+	 * Check that our idle barriers do not interfere with normal
+	 * activity tracking. In particular, check that operating
+	 * on the context image remotely (intel_context_prepare_remote_request),
+	 * which inserts foreign fences into intel_context.active, does not
+	 * clobber the idle-barrier.
+	 */
+
+	remote = intel_context_create(fixme, engine);
+	if (!remote)
+		return -ENOMEM;
+
+	local = intel_context_create(fixme, engine);
+	if (!local) {
+		err = -ENOMEM;
+		goto err_remote;
+	}
+
+	for (pass = 0; pass <= 2; pass++) {
+		err = __remote_sync(local, remote);
+		if (err)
+			break;
+
+		err = __remote_sync(engine->kernel_context, remote);
+		if (err)
+			break;
+
+		if (i915_active_is_idle(&remote->active)) {
+			pr_err("remote context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+	intel_context_put(local);
+err_remote:
+	intel_context_put(remote);
+	return err;
+}
+
+static int live_remote_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_remote_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+int intel_context_live_selftests(struct drm_i915_private *i915)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(live_active_context),
+		SUBTEST(live_remote_context),
+	};
+	struct intel_gt *gt = &i915->gt;
+
+	if (intel_gt_is_wedged(gt))
+		return 0;
+
+	return intel_gt_live_subtests(tests, gt);
+}
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index d32db8a4db5c..3d50a27ed16c 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -33,6 +33,38 @@ struct active_node {
 	u64 timeline;
 };
 
+static inline struct active_node *
+node_from_active(struct i915_active_request *active)
+{
+	return container_of(active, struct active_node, base);
+}
+
+#define get_preallocated_barriers(x) llist_del_all(&(x)->preallocated_barriers)
+
+static inline bool is_barrier(const struct i915_active_request *active)
+{
+	return IS_ERR(rcu_access_pointer(active->request));
+}
+
+static inline struct llist_node *barrier_to_ll(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct llist_node *)&node->base.link;
+}
+
+static inline struct intel_engine_cs *
+barrier_to_engine(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct intel_engine_cs *)node->base.link.prev;
+}
+
+static inline struct active_node *barrier_from_ll(struct llist_node *x)
+{
+	return container_of((struct list_head *)x,
+			    struct active_node, base.link);
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && IS_ENABLED(CONFIG_DEBUG_OBJECTS)
 
 static void *active_debug_hint(void *addr)
@@ -127,7 +159,7 @@ active_retire(struct i915_active *ref)
 static void
 node_retire(struct i915_active_request *base, struct i915_request *rq)
 {
-	active_retire(container_of(base, struct active_node, base)->ref);
+	active_retire(node_from_active(base)->ref);
 }
 
 static struct i915_active_request *
@@ -184,6 +216,7 @@ active_instance(struct i915_active *ref, u64 idx)
 	ref->cache = node;
 	mutex_unlock(&ref->mutex);
 
+	BUILD_BUG_ON(offsetof(typeof(*node), base));
 	return &node->base;
 }
 
@@ -201,11 +234,37 @@ void __i915_active_init(struct drm_i915_private *i915,
 	ref->retire = retire;
 	ref->tree = RB_ROOT;
 	ref->cache = NULL;
-	init_llist_head(&ref->barriers);
+	init_llist_head(&ref->preallocated_barriers);
 	atomic_set(&ref->count, 0);
 	__mutex_init(&ref->mutex, "i915_active", key);
 }
 
+static bool __active_del_barrier(struct i915_active *ref,
+				 struct active_node *node)
+{
+	struct intel_engine_cs *engine = barrier_to_engine(node);
+	struct llist_node *head = NULL, *tail = NULL;
+	struct llist_node *pos, *next;
+
+	GEM_BUG_ON(node->timeline != engine->kernel_context->ring->timeline->fence_context);
+
+	llist_for_each_safe(pos, next, llist_del_all(&engine->barrier_tasks)) {
+		if (node == barrier_from_ll(pos)) {
+			node = NULL;
+			continue;
+		}
+
+		pos->next = head;
+		head = pos;
+		if (!tail)
+			tail = pos;
+	}
+	if (head)
+		llist_add_batch(head, tail, &engine->barrier_tasks);
+
+	return !node;
+}
+
 int i915_active_ref(struct i915_active *ref,
 		    u64 timeline,
 		    struct i915_request *rq)
@@ -224,8 +283,15 @@ int i915_active_ref(struct i915_active *ref,
 		goto out;
 	}
 
-	if (!i915_active_request_isset(active))
-		atomic_inc(&ref->count);
+	if (is_barrier(active)) { /* proto-node used by our idle barrier */
+		__active_del_barrier(ref, node_from_active(active));
+		RCU_INIT_POINTER(active->request, NULL);
+		INIT_LIST_HEAD(&active->link);
+	} else {
+		if (!i915_active_request_isset(active))
+			atomic_inc(&ref->count);
+	}
+	GEM_BUG_ON(!atomic_read(&ref->count));
 	__i915_active_request_set(active, rq);
 
 out:
@@ -312,6 +378,11 @@ int i915_active_wait(struct i915_active *ref)
 	}
 
 	rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
+		if (is_barrier(&it->base)) { /* unconnected idle-barrier */
+			err = -EBUSY;
+			break;
+		}
+
 		err = i915_active_request_retire(&it->base, BKL(ref));
 		if (err)
 			break;
@@ -374,6 +445,79 @@ void i915_active_fini(struct i915_active *ref)
 }
 #endif
 
+static inline bool is_idle_barrier(struct active_node *node, u64 idx)
+{
+	return node->timeline == idx && !i915_active_request_isset(&node->base);
+}
+
+static struct active_node *idle_barrier(struct i915_active *ref, u64 idx)
+{
+	struct rb_node *prev, *p;
+
+	if (RB_EMPTY_ROOT(&ref->tree))
+		return NULL;
+
+	mutex_lock(&ref->mutex);
+	GEM_BUG_ON(i915_active_is_idle(ref));
+
+	/*
+	 * Try to reuse any existing barrier nodes already allocated for this
+	 * i915_active, due to overlapping active phases there is likely a
+	 * node kept alive (as we reuse before parking). We prefer to reuse
+	 * completely idle barriers (less hassle in manipulating the llists),
+	 * but otherwise any will do.
+	 */
+	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
+		p = &ref->cache->node;
+		goto match;
+	}
+
+	prev = NULL;
+	p = ref->tree.rb_node;
+	while (p) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (is_idle_barrier(node, idx))
+			goto match;
+
+		prev = p;
+		if (node->timeline < idx)
+			p = p->rb_right;
+		else
+			p = p->rb_left;
+	}
+
+	for (p = prev; p; p = rb_next(p)) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (node->timeline > idx)
+			break;
+
+		if (node->timeline < idx)
+			continue;
+
+		if (!i915_active_request_isset(&node->base))
+			goto match;
+
+		if (is_barrier(&node->base) && __active_del_barrier(ref, node))
+			goto match;
+	}
+
+	mutex_unlock(&ref->mutex);
+
+	return NULL;
+
+match:
+	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */
+	if (p == &ref->cache->node)
+		ref->cache = NULL;
+	mutex_unlock(&ref->mutex);
+
+	return rb_entry(p, struct active_node, node);
+}
+
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
@@ -382,39 +526,52 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	struct llist_node *pos, *next;
 	int err;
 
-	GEM_BUG_ON(!mask);
+	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
+
+	/*
+	 * Preallocate a node for each physical engine supporting the target
+	 * engine (remember virtual engines have more than one sibling).
+	 * We can then use the preallocated nodes in
+	 * i915_active_acquire_barrier()
+	 */
 	for_each_engine_masked(engine, i915, mask, tmp) {
-		struct intel_context *kctx = engine->kernel_context;
+		u64 idx = engine->kernel_context->ring->timeline->fence_context;
 		struct active_node *node;
 
-		node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
-		if (unlikely(!node)) {
-			err = -ENOMEM;
-			goto unwind;
+		node = idle_barrier(ref, idx);
+		if (!node) {
+			node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
+			if (!node) {
+				err = ENOMEM;
+				goto unwind;
+			}
+
+			RCU_INIT_POINTER(node->base.request, NULL);
+			node->base.retire = node_retire;
+			node->timeline = idx;
+			node->ref = ref;
 		}
 
-		i915_active_request_init(&node->base,
-					 (void *)engine, node_retire);
-		node->timeline = kctx->ring->timeline->fence_context;
-		node->ref = ref;
-		atomic_inc(&ref->count);
+		if (!i915_active_request_isset(&node->base)) {
+			RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
+			node->base.link.prev = (void *)engine;
+			atomic_inc(&ref->count);
+		}
 
+		GEM_BUG_ON(barrier_to_engine(node) != engine);
+		llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
 		intel_engine_pm_get(engine);
-		llist_add((struct llist_node *)&node->base.link,
-			  &ref->barriers);
 	}
 
 	return 0;
 
 unwind:
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct active_node *node;
+	llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
 
-		node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-		engine = (void *)rcu_access_pointer(node->base.request);
+		atomic_dec(&ref->count);
+		intel_engine_pm_put(barrier_to_engine(node));
 
-		intel_engine_pm_put(engine);
 		kmem_cache_free(global.slab_cache, node);
 	}
 	return err;
@@ -426,25 +583,27 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 
 	GEM_BUG_ON(i915_active_is_idle(ref));
 
+	/*
+	 * Transfer the list of preallocated barriers into the
+	 * i915_active rbtree, but only as proto-nodes. They will be
+	 * populated by i915_request_add_active_barrier() to point to the
+	 * request that will eventually release them.
+	 */
 	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct intel_engine_cs *engine;
-		struct active_node *node;
+	llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
+		struct intel_engine_cs *engine = barrier_to_engine(node);
 		struct rb_node **p, *parent;
 
-		node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-
-		engine = (void *)rcu_access_pointer(node->base.request);
-		RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
-
 		parent = NULL;
 		p = &ref->tree.rb_node;
 		while (*p) {
+			struct active_node *it;
+
 			parent = *p;
-			if (rb_entry(parent,
-				     struct active_node,
-				     node)->timeline < node->timeline)
+
+			it = rb_entry(parent, struct active_node, node);
+			if (it->timeline < node->timeline)
 				p = &parent->rb_right;
 			else
 				p = &parent->rb_left;
@@ -452,20 +611,29 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 		rb_link_node(&node->node, parent, p);
 		rb_insert_color(&node->node, &ref->tree);
 
-		llist_add((struct llist_node *)&node->base.link,
-			  &engine->barrier_tasks);
+		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
 		intel_engine_pm_put(engine);
 	}
 	mutex_unlock(&ref->mutex);
 }
 
-void i915_request_add_barriers(struct i915_request *rq)
+void i915_request_add_active_barriers(struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
 	struct llist_node *node, *next;
 
-	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks))
+	GEM_BUG_ON(intel_engine_is_virtual(engine));
+	GEM_BUG_ON(rq->timeline != engine->kernel_context->ring->timeline);
+
+	/*
+	 * Attach the list of proto-fences to the in-flight request such
+	 * that the parent i915_active will be released when this request
+	 * is retired.
+	 */
+	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
+		RCU_INIT_POINTER(barrier_from_ll(node)->base.request, rq);
 		list_add_tail((struct list_head *)node, &rq->active_list);
+	}
 }
 
 int i915_active_request_set(struct i915_active_request *active,
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index ba68b077ec6c..566336c99ed7 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -413,6 +413,6 @@ static inline void i915_active_fini(struct i915_active *ref) { }
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine);
 void i915_active_acquire_barrier(struct i915_active *ref);
-void i915_request_add_barriers(struct i915_request *rq);
+void i915_request_add_active_barriers(struct i915_request *rq);
 
 #endif /* _I915_ACTIVE_H_ */
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index 74743dd0d5f0..ae3ee441c114 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -42,7 +42,7 @@ struct i915_active {
 	int (*active)(struct i915_active *ref);
 	void (*retire)(struct i915_active *ref);
 
-	struct llist_head barriers;
+	struct llist_head preallocated_barriers;
 };
 
 #endif /* _I915_ACTIVE_TYPES_H_ */
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 2b31a4ee0b4c..a841d3f9bedc 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -15,6 +15,7 @@ selftest(workarounds, intel_workarounds_live_selftests)
 selftest(timelines, intel_timeline_live_selftests)
 selftest(requests, i915_request_live_selftests)
 selftest(active, i915_active_live_selftests)
+selftest(gt_contexts, intel_context_live_selftests)
 selftest(objects, i915_gem_object_live_selftests)
 selftest(mman, i915_gem_mman_live_selftests)
 selftest(dmabuf, i915_gem_dmabuf_live_selftests)
@@ -24,7 +25,7 @@ selftest(gtt, i915_gem_gtt_live_selftests)
 selftest(gem, i915_gem_live_selftests)
 selftest(evict, i915_gem_evict_live_selftests)
 selftest(hugepages, i915_gem_huge_page_live_selftests)
-selftest(contexts, i915_gem_context_live_selftests)
+selftest(gem_contexts, i915_gem_context_live_selftests)
 selftest(blt, i915_gem_object_blt_live_selftests)
 selftest(client, i915_gem_client_blt_live_selftests)
 selftest(reset, intel_reset_live_selftests)
-- 
2.22.0

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

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

* [PATCH 3/3] drm/i915: Flush extra hard after writing relocations through the GTT
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
@ 2019-07-30 11:21   ` Chris Wilson
  2019-07-30 11:21 ` [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests Chris Wilson
                     ` (9 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2019-07-30 11:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: tvrtko.ursulin, Chris Wilson, stable

Recently discovered in commit bdae33b8b82b ("drm/i915: Use maximum write
flush for pwrite_gtt") was that we needed to our full write barrier
before changing the GGTT PTE to ensure that our indirect writes through
the GTT landed before the PTE changed (and the writes end up in a
different page). That also applies to our GGTT relocation path.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 8a2047c4e7c3..01901dad33f7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1019,11 +1019,12 @@ static void reloc_cache_reset(struct reloc_cache *cache)
 		kunmap_atomic(vaddr);
 		i915_gem_object_finish_access((struct drm_i915_gem_object *)cache->node.mm);
 	} else {
-		wmb();
+		struct i915_ggtt *ggtt = cache_to_ggtt(cache);
+
+		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 		io_mapping_unmap_atomic((void __iomem *)vaddr);
-		if (cache->node.allocated) {
-			struct i915_ggtt *ggtt = cache_to_ggtt(cache);
 
+		if (cache->node.allocated) {
 			ggtt->vm.clear_range(&ggtt->vm,
 					     cache->node.start,
 					     cache->node.size);
@@ -1078,6 +1079,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 	void *vaddr;
 
 	if (cache->vaddr) {
+		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 		io_mapping_unmap_atomic((void __force __iomem *) unmask_page(cache->vaddr));
 	} else {
 		struct i915_vma *vma;
@@ -1119,7 +1121,6 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 
 	offset = cache->node.start;
 	if (cache->node.allocated) {
-		wmb();
 		ggtt->vm.insert_page(&ggtt->vm,
 				     i915_gem_object_get_dma_address(obj, page),
 				     offset, I915_CACHE_NONE, 0);
-- 
2.22.0


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

* [PATCH 3/3] drm/i915: Flush extra hard after writing relocations through the GTT
@ 2019-07-30 11:21   ` Chris Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2019-07-30 11:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

Recently discovered in commit bdae33b8b82b ("drm/i915: Use maximum write
flush for pwrite_gtt") was that we needed to our full write barrier
before changing the GGTT PTE to ensure that our indirect writes through
the GTT landed before the PTE changed (and the writes end up in a
different page). That also applies to our GGTT relocation path.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 8a2047c4e7c3..01901dad33f7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1019,11 +1019,12 @@ static void reloc_cache_reset(struct reloc_cache *cache)
 		kunmap_atomic(vaddr);
 		i915_gem_object_finish_access((struct drm_i915_gem_object *)cache->node.mm);
 	} else {
-		wmb();
+		struct i915_ggtt *ggtt = cache_to_ggtt(cache);
+
+		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 		io_mapping_unmap_atomic((void __iomem *)vaddr);
-		if (cache->node.allocated) {
-			struct i915_ggtt *ggtt = cache_to_ggtt(cache);
 
+		if (cache->node.allocated) {
 			ggtt->vm.clear_range(&ggtt->vm,
 					     cache->node.start,
 					     cache->node.size);
@@ -1078,6 +1079,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 	void *vaddr;
 
 	if (cache->vaddr) {
+		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 		io_mapping_unmap_atomic((void __force __iomem *) unmask_page(cache->vaddr));
 	} else {
 		struct i915_vma *vma;
@@ -1119,7 +1121,6 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 
 	offset = cache->node.start;
 	if (cache->node.allocated) {
-		wmb();
 		ggtt->vm.insert_page(&ggtt->vm,
 				     i915_gem_object_get_dma_address(obj, page),
 				     offset, I915_CACHE_NONE, 0);
-- 
2.22.0

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (2 preceding siblings ...)
  2019-07-30 11:21   ` Chris Wilson
@ 2019-07-30 12:39 ` Patchwork
  2019-07-30 13:01 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-07-30 12:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
URL   : https://patchwork.freedesktop.org/series/64426/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
879a5691d4f1 drm/i915/execlists: Always clear pending&inflight requests on reset
-:29: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#29: 
<7> [416.492863] hangcheck 	Execlist status: 0x00018001 00000000, entries 12

total: 0 errors, 1 warnings, 0 checks, 25 lines checked
629b8a978763 drm/i915: Allow sharing the idle-barrier from other kernel requests
-:123: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#123: 
new file mode 100644

-:128: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#128: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:1:
+/*

-:129: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#129: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:2:
+ * SPDX-License-Identifier: GPL-2.0

total: 0 errors, 3 warnings, 0 checks, 765 lines checked
32fcbf1b85e9 drm/i915: Flush extra hard after writing relocations through the GTT

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (3 preceding siblings ...)
  2019-07-30 12:39 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset Patchwork
@ 2019-07-30 13:01 ` Patchwork
  2019-07-30 22:46 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-07-30 13:01 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
URL   : https://patchwork.freedesktop.org/series/64426/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6581 -> Patchwork_13798
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/

New tests
---------

  New tests have been introduced between CI_DRM_6581 and Patchwork_13798:

### New IGT tests (2) ###

  * igt@i915_selftest@live_gem_contexts:
    - Statuses : 44 pass(s)
    - Exec time: [0.42, 28.55] s

  * igt@i915_selftest@live_gt_contexts:
    - Statuses : 44 pass(s)
    - Exec time: [0.42, 1.34] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@basic-read:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-icl-u3/igt@gem_workarounds@basic-read.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-icl-u3/igt@gem_workarounds@basic-read.html

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       [PASS][3] -> [SKIP][4] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       [PASS][5] -> [WARN][6] ([fdo#109380])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-kbl-7567u:       [PASS][7] -> [SKIP][8] ([fdo#109271]) +23 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-icl-guc}:       [INCOMPLETE][9] ([fdo#107713] / [fdo#109100]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-icl-guc/igt@gem_ctx_create@basic-files.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-icl-guc/igt@gem_ctx_create@basic-files.html
    - fi-cml-u:           [INCOMPLETE][11] ([fdo#110566]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-cml-u/igt@gem_ctx_create@basic-files.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-cml-u/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [INCOMPLETE][13] ([fdo#107718]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_pm_rpm@module-reload:
    - fi-icl-u3:          [DMESG-WARN][15] ([fdo#107724]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-icl-u3/igt@i915_pm_rpm@module-reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-icl-u3/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_reset:
    - fi-icl-u2:          [INCOMPLETE][17] ([fdo#107713]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-icl-u2/igt@i915_selftest@live_reset.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-icl-u2/igt@i915_selftest@live_reset.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-cml-u2:          [FAIL][19] ([fdo#106766]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html

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

  [fdo#106766]: https://bugs.freedesktop.org/show_bug.cgi?id=106766
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096


Participating hosts (55 -> 46)
------------------------------

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6581 -> Patchwork_13798

  CI-20190529: 20190529
  CI_DRM_6581: 53dee2a1faf99813437ddda7cf25a9adec5b01ce @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5115: 21be7a02ac8a8ff46b561c36a69e4dd5a0c2938b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13798: 32fcbf1b85e92d67d8c2f0e175eba47b78bf9584 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

32fcbf1b85e9 drm/i915: Flush extra hard after writing relocations through the GTT
629b8a978763 drm/i915: Allow sharing the idle-barrier from other kernel requests
879a5691d4f1 drm/i915/execlists: Always clear pending&inflight requests on reset

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (4 preceding siblings ...)
  2019-07-30 13:01 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-07-30 22:46 ` Patchwork
  2019-08-02  8:44 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2) Patchwork
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-07-30 22:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset
URL   : https://patchwork.freedesktop.org/series/64426/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6581_full -> Patchwork_13798_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

New tests
---------

  New tests have been introduced between CI_DRM_6581_full and Patchwork_13798_full:

### New IGT tests (2) ###

  * igt@i915_selftest@live_gem_contexts:
    - Statuses : 7 pass(s)
    - Exec time: [5.07, 35.74] s

  * igt@i915_selftest@live_gt_contexts:
    - Statuses : 7 pass(s)
    - Exec time: [0.46, 2.36] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@gem-mmap-cpu:
    - shard-iclb:         [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#108840])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb1/igt@i915_pm_rpm@gem-mmap-cpu.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb2/igt@i915_pm_rpm@gem-mmap-cpu.html

  * igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([fdo#103232])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-glk1/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-glk9/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-iclb:         [PASS][5] -> [INCOMPLETE][6] ([fdo#107713]) +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb3/igt@kms_fbcon_fbt@fbc-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb3/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          [PASS][7] -> [FAIL][8] ([fdo#105363])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-glk5/igt@kms_flip@flip-vs-expired-vblank.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-glk7/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [PASS][11] -> [INCOMPLETE][12] ([fdo#103540])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-hsw1/igt@kms_flip@flip-vs-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-hsw8/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][13] -> [INCOMPLETE][14] ([fdo#109507])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl9/igt@kms_flip@flip-vs-suspend-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +4 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][19] -> [DMESG-WARN][20] ([fdo#108566]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145] / [fdo#110403])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109441]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb4/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          [PASS][25] -> [INCOMPLETE][26] ([fdo#104108])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl9/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl6/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@tools_test@tools_test:
    - shard-apl:          [PASS][27] -> [SKIP][28] ([fdo#109271])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-apl8/igt@tools_test@tools_test.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-apl3/igt@tools_test@tools_test.html

  
#### Possible fixes ####

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][29] ([fdo#104108]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl7/igt@gem_softpin@noreloc-s3.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl9/igt@gem_softpin@noreloc-s3.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-apl:          [DMESG-WARN][31] ([fdo#108686]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-apl8/igt@gem_tiled_swapping@non-threaded.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-apl1/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [DMESG-WARN][33] ([fdo#108566]) -> [PASS][34] +5 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-apl1/igt@gem_workarounds@suspend-resume-context.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-apl2/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque:
    - shard-kbl:          [FAIL][35] ([fdo#103232]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-ytiled:
    - shard-skl:          [FAIL][37] ([fdo#103184] / [fdo#103232]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl5/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-ytiled.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl5/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-ytiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled:
    - shard-skl:          [FAIL][39] ([fdo#103184] / [fdo#103232] / [fdo#108472]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl8/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl2/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][41] ([fdo#105363]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][43] ([fdo#109507]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl7/igt@kms_flip@flip-vs-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl5/igt@kms_flip@flip-vs-suspend.html

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

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-snb:          [DMESG-WARN][47] ([fdo#102365]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-snb1/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-snb4/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [DMESG-WARN][49] ([fdo#108566]) -> [PASS][50] +3 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][51] ([fdo#108145] / [fdo#110403]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][53] ([fdo#103166]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][55] ([fdo#109642] / [fdo#111068]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb6/igt@kms_psr2_su@frontbuffer.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [SKIP][57] ([fdo#109441]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb6/igt@kms_psr@psr2_dpms.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb2/igt@kms_psr@psr2_dpms.html

  * igt@kms_psr@suspend:
    - shard-skl:          [INCOMPLETE][59] ([fdo#108972]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-skl2/igt@kms_psr@suspend.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-skl6/igt@kms_psr@suspend.html

  * igt@tools_test@tools_test:
    - shard-iclb:         [SKIP][61] ([fdo#109352]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb6/igt@tools_test@tools_test.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb7/igt@tools_test@tools_test.html

  
#### Warnings ####

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][63] ([fdo#109349]) -> [DMESG-WARN][64] ([fdo#107724])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6581/shard-iclb8/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13798/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  
  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108472]: https://bugs.freedesktop.org/show_bug.cgi?id=108472
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108972]: https://bugs.freedesktop.org/show_bug.cgi?id=108972
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109352]: https://bugs.freedesktop.org/show_bug.cgi?id=109352
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068


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

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6581 -> Patchwork_13798

  CI-20190529: 20190529
  CI_DRM_6581: 53dee2a1faf99813437ddda7cf25a9adec5b01ce @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5115: 21be7a02ac8a8ff46b561c36a69e4dd5a0c2938b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13798: 32fcbf1b85e92d67d8c2f0e175eba47b78bf9584 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests
  2019-07-30 11:21 ` [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests Chris Wilson
@ 2019-08-01 17:24   ` Tvrtko Ursulin
  2019-08-01 17:55     ` Chris Wilson
                       ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 17:24 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 30/07/2019 12:21, Chris Wilson wrote:
> By placing our idle-barriers in the i915_active fence tree, we expose
> those for reuse by other components that are issuing requests along the
> kernel_context. Reusing the proto-barrier active_node is perfectly fine
> as the new request implies a context-switch, and so an opportune point
> to run the idle-barrier. However, the proto-barrier is not equivalent
> to a normal active_node and care must be taken to avoid dereferencing the
> ERR_PTR used as its request marker.
> 
> Reported-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
> Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_context.c       |  40 ++-
>   drivers/gpu/drm/i915/gt/intel_context.h       |  13 +-
>   drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   2 +-
>   drivers/gpu/drm/i915/gt/selftest_context.c    | 310 ++++++++++++++++++
>   drivers/gpu/drm/i915/i915_active.c            | 246 +++++++++++---
>   drivers/gpu/drm/i915/i915_active.h            |   2 +-
>   drivers/gpu/drm/i915/i915_active_types.h      |   2 +-
>   .../drm/i915/selftests/i915_live_selftests.h  |   3 +-
>   8 files changed, 555 insertions(+), 63 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gt/selftest_context.c
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index d64b45f7ec6d..211ac6568a5d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -162,23 +162,41 @@ static int __intel_context_active(struct i915_active *active)
>   	if (err)
>   		goto err_ring;
>   
> +	return 0;
> +
> +err_ring:
> +	intel_ring_unpin(ce->ring);
> +err_put:
> +	intel_context_put(ce);
> +	return err;
> +}
> +
> +int intel_context_active_acquire(struct intel_context *ce)
> +{
> +	int err;
> +
> +	err = i915_active_acquire(&ce->active);
> +	if (err)
> +		return err;
> +
>   	/* Preallocate tracking nodes */
>   	if (!i915_gem_context_is_kernel(ce->gem_context)) {
>   		err = i915_active_acquire_preallocate_barrier(&ce->active,
>   							      ce->engine);
> -		if (err)
> -			goto err_state;
> +		if (err) {
> +			i915_active_release(&ce->active);
> +			return err;
> +		}
>   	}
>   
>   	return 0;
> +}
>   
> -err_state:
> -	__context_unpin_state(ce->state);
> -err_ring:
> -	intel_ring_unpin(ce->ring);
> -err_put:
> -	intel_context_put(ce);
> -	return err;
> +void intel_context_active_release(struct intel_context *ce)
> +{
> +	/* Nodes preallocated in intel_context_active() */
> +	i915_active_acquire_barrier(&ce->active);
> +	i915_active_release(&ce->active);
>   }
>   
>   void
> @@ -297,3 +315,7 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
>   
>   	return rq;
>   }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_context.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> index 23c7e4c0ce7c..07f9924de48f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -104,17 +104,8 @@ static inline void intel_context_exit(struct intel_context *ce)
>   		ce->ops->exit(ce);
>   }
>   
> -static inline int intel_context_active_acquire(struct intel_context *ce)
> -{
> -	return i915_active_acquire(&ce->active);
> -}
> -
> -static inline void intel_context_active_release(struct intel_context *ce)
> -{
> -	/* Nodes preallocated in intel_context_active() */
> -	i915_active_acquire_barrier(&ce->active);
> -	i915_active_release(&ce->active);
> -}
> +int intel_context_active_acquire(struct intel_context *ce);
> +void intel_context_active_release(struct intel_context *ce);
>   
>   static inline struct intel_context *intel_context_get(struct intel_context *ce)
>   {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index e74fbf04a68d..ce54092475da 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -90,7 +90,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
>   	/* Check again on the next retirement. */
>   	engine->wakeref_serial = engine->serial + 1;
>   
> -	i915_request_add_barriers(rq);
> +	i915_request_add_active_barriers(rq);
>   	__i915_request_commit(rq);
>   
>   	return false;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> new file mode 100644
> index 000000000000..d39b5594cb02
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -0,0 +1,310 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#include "i915_selftest.h"
> +#include "intel_gt.h"
> +
> +#include "gem/selftests/mock_context.h"
> +#include "selftests/igt_flush_test.h"
> +#include "selftests/mock_drm.h"
> +
> +static int request_sync(struct i915_request *rq)
> +{
> +	long timeout;
> +	int err = 0;
> +
> +	i915_request_get(rq);
> +
> +	i915_request_add(rq);
> +	timeout = i915_request_wait(rq, 0, HZ / 10);
> +	if (timeout < 0)
> +		err = timeout;
> +	else
> +		i915_request_retire_upto(rq);
> +
> +	i915_request_put(rq);
> +
> +	return err;
> +}
> +
> +static int context_sync(struct intel_context *ce)
> +{
> +	struct intel_timeline *tl = ce->ring->timeline;
> +	int err = 0;
> +
> +	do {
> +		struct i915_request *rq;
> +		long timeout;
> +
> +		rcu_read_lock();
> +		rq = rcu_dereference(tl->last_request.request);
> +		if (rq)
> +			rq = i915_request_get_rcu(rq);
> +		rcu_read_unlock();
> +		if (!rq)
> +			break;
> +
> +		timeout = i915_request_wait(rq, 0, HZ / 10);
> +		if (timeout < 0)
> +			err = timeout;
> +		else
> +			i915_request_retire_upto(rq);
> +
> +		i915_request_put(rq);
> +	} while (!err);
> +
> +	return err;
> +}
> +
> +static int __live_active_context(struct intel_engine_cs *engine,
> +				 struct i915_gem_context *fixme)
> +{
> +	struct intel_context *ce;
> +	int pass;
> +	int err;
> +
> +	/*
> +	 * We keep active contexts alive until after a subsequent context
> +	 * switch as the final write from the context-save will be after
> +	 * we retire the final request. We track when we unpin the context,
> +	 * under the presumption that the final pin is from the last request,
> +	 * and instead of immediately unpinning the context, we add a task
> +	 * to unpin the context from the next idle-barrier.
> +	 *
> +	 * This test makes sure that the context is kept alive until a
> +	 * subsequent idle-barrier (emitted when the engine wakeref hits 0
> +	 * with no more outstanding requests).
> +	 */
> +
> +	if (intel_engine_pm_is_awake(engine)) {
> +		pr_err("%s is awake before starting %s!\n",
> +		       engine->name, __func__);
> +		return -EINVAL;
> +	}
> +
> +	ce = intel_context_create(fixme, engine);
> +	if (!ce)
> +		return -ENOMEM;
> +
> +	for (pass = 0; pass <= 2; pass++) {
> +		struct i915_request *rq;
> +
> +		rq = intel_context_create_request(ce);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			goto err;
> +		}
> +
> +		err = request_sync(rq);
> +		if (err)
> +			goto err;
> +
> +		/* Context will be kept active until after an idle-barrier. */
> +		if (i915_active_is_idle(&ce->active)) {
> +			pr_err("context is not active; expected idle-barrier (%s pass %d)\n",
> +			       engine->name, pass);
> +			err = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (!intel_engine_pm_is_awake(engine)) {
> +			pr_err("%s is asleep before idle-barrier\n",
> +			       engine->name);
> +			err = -EINVAL;
> +			goto err;
> +		}
> +	}
> +
> +	/* Now make sure our idle-barriers are flushed */
> +	err = context_sync(engine->kernel_context);
> +	if (err)
> +		goto err;
> +
> +	if (!i915_active_is_idle(&ce->active)) {
> +		pr_err("context is still active!");
> +		err = -EINVAL;
> +	}
> +
> +	if (intel_engine_pm_is_awake(engine)) {
> +		struct drm_printer p = drm_debug_printer(__func__);
> +
> +		intel_engine_dump(engine, &p,
> +				  "%s is still awake after idle-barriers\n",
> +				  engine->name);
> +		GEM_TRACE_DUMP();
> +
> +		err = -EINVAL;
> +		goto err;
> +	}
> +
> +err:
> +	intel_context_put(ce);
> +	return err;
> +}
> +
> +static int live_active_context(void *arg)
> +{
> +	struct intel_gt *gt = arg;
> +	struct intel_engine_cs *engine;
> +	struct i915_gem_context *fixme;
> +	enum intel_engine_id id;
> +	struct drm_file *file;
> +	int err = 0;
> +
> +	file = mock_file(gt->i915);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	mutex_lock(&gt->i915->drm.struct_mutex);
> +
> +	fixme = live_context(gt->i915, file);
> +	if (!fixme) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	for_each_engine(engine, gt->i915, id) {
> +		err = __live_active_context(engine, fixme);
> +		if (err)
> +			break;
> +
> +		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
> +		if (err)
> +			break;
> +	}
> +
> +unlock:
> +	mutex_unlock(&gt->i915->drm.struct_mutex);
> +	mock_file_free(gt->i915, file);
> +	return err;
> +}
> +
> +static int __remote_sync(struct intel_context *ce, struct intel_context *remote)
> +{
> +	struct i915_request *rq;
> +	int err;
> +
> +	err = intel_context_pin(remote);
> +	if (err)
> +		return err;
> +
> +	rq = intel_context_create_request(ce);
> +	if (IS_ERR(rq)) {
> +		err = PTR_ERR(rq);
> +		goto unpin;
> +	}
> +
> +	err = intel_context_prepare_remote_request(remote, rq);
> +	if (err) {
> +		i915_request_add(rq);
> +		goto unpin;
> +	}
> +
> +	err = request_sync(rq);
> +
> +unpin:
> +	intel_context_unpin(remote);
> +	return err;
> +}
> +
> +static int __live_remote_context(struct intel_engine_cs *engine,
> +				 struct i915_gem_context *fixme)
> +{
> +	struct intel_context *local, *remote;
> +	int pass;
> +	int err;
> +
> +	/*
> +	 * Check that our idle barriers do not interfere with normal
> +	 * activity tracking. In particular, check that operating
> +	 * on the context image remotely (intel_context_prepare_remote_request),
> +	 * which inserts foreign fences into intel_context.active, does not
> +	 * clobber the idle-barrier.
> +	 */
> +
> +	remote = intel_context_create(fixme, engine);
> +	if (!remote)
> +		return -ENOMEM;
> +
> +	local = intel_context_create(fixme, engine);
> +	if (!local) {
> +		err = -ENOMEM;
> +		goto err_remote;
> +	}
> +
> +	for (pass = 0; pass <= 2; pass++) {
> +		err = __remote_sync(local, remote);
> +		if (err)
> +			break;
> +
> +		err = __remote_sync(engine->kernel_context, remote);
> +		if (err)
> +			break;
> +
> +		if (i915_active_is_idle(&remote->active)) {
> +			pr_err("remote context is not active; expected idle-barrier (%s pass %d)\n",
> +			       engine->name, pass);
> +			err = -EINVAL;
> +			break;
> +		}
> +	}
> +
> +	intel_context_put(local);
> +err_remote:
> +	intel_context_put(remote);
> +	return err;
> +}
> +
> +static int live_remote_context(void *arg)
> +{
> +	struct intel_gt *gt = arg;
> +	struct intel_engine_cs *engine;
> +	struct i915_gem_context *fixme;
> +	enum intel_engine_id id;
> +	struct drm_file *file;
> +	int err = 0;
> +
> +	file = mock_file(gt->i915);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	mutex_lock(&gt->i915->drm.struct_mutex);
> +
> +	fixme = live_context(gt->i915, file);
> +	if (!fixme) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	for_each_engine(engine, gt->i915, id) {
> +		err = __live_remote_context(engine, fixme);
> +		if (err)
> +			break;
> +
> +		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
> +		if (err)
> +			break;
> +	}
> +
> +unlock:
> +	mutex_unlock(&gt->i915->drm.struct_mutex);
> +	mock_file_free(gt->i915, file);
> +	return err;
> +}
> +
> +int intel_context_live_selftests(struct drm_i915_private *i915)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(live_active_context),
> +		SUBTEST(live_remote_context),
> +	};
> +	struct intel_gt *gt = &i915->gt;
> +
> +	if (intel_gt_is_wedged(gt))
> +		return 0;
> +
> +	return intel_gt_live_subtests(tests, gt);
> +}
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index d32db8a4db5c..3d50a27ed16c 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -33,6 +33,38 @@ struct active_node {
>   	u64 timeline;
>   };
>   
> +static inline struct active_node *
> +node_from_active(struct i915_active_request *active)
> +{
> +	return container_of(active, struct active_node, base);
> +}
> +
> +#define get_preallocated_barriers(x) llist_del_all(&(x)->preallocated_barriers)
> +
> +static inline bool is_barrier(const struct i915_active_request *active)
> +{
> +	return IS_ERR(rcu_access_pointer(active->request));
> +}
> +
> +static inline struct llist_node *barrier_to_ll(struct active_node *node)
> +{
> +	GEM_BUG_ON(!is_barrier(&node->base));
> +	return (struct llist_node *)&node->base.link;
> +}
> +
> +static inline struct intel_engine_cs *
> +barrier_to_engine(struct active_node *node)
> +{
> +	GEM_BUG_ON(!is_barrier(&node->base));
> +	return (struct intel_engine_cs *)node->base.link.prev;
> +}
> +
> +static inline struct active_node *barrier_from_ll(struct llist_node *x)
> +{
> +	return container_of((struct list_head *)x,
> +			    struct active_node, base.link);
> +}
> +
>   #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && IS_ENABLED(CONFIG_DEBUG_OBJECTS)
>   
>   static void *active_debug_hint(void *addr)
> @@ -127,7 +159,7 @@ active_retire(struct i915_active *ref)
>   static void
>   node_retire(struct i915_active_request *base, struct i915_request *rq)
>   {
> -	active_retire(container_of(base, struct active_node, base)->ref);
> +	active_retire(node_from_active(base)->ref);
>   }
>   
>   static struct i915_active_request *
> @@ -184,6 +216,7 @@ active_instance(struct i915_active *ref, u64 idx)
>   	ref->cache = node;
>   	mutex_unlock(&ref->mutex);
>   
> +	BUILD_BUG_ON(offsetof(typeof(*node), base));
>   	return &node->base;
>   }
>   
> @@ -201,11 +234,37 @@ void __i915_active_init(struct drm_i915_private *i915,
>   	ref->retire = retire;
>   	ref->tree = RB_ROOT;
>   	ref->cache = NULL;
> -	init_llist_head(&ref->barriers);
> +	init_llist_head(&ref->preallocated_barriers);
>   	atomic_set(&ref->count, 0);
>   	__mutex_init(&ref->mutex, "i915_active", key);
>   }
>   
> +static bool __active_del_barrier(struct i915_active *ref,
> +				 struct active_node *node)
> +{
> +	struct intel_engine_cs *engine = barrier_to_engine(node);
> +	struct llist_node *head = NULL, *tail = NULL;
> +	struct llist_node *pos, *next;
> +
> +	GEM_BUG_ON(node->timeline != engine->kernel_context->ring->timeline->fence_context);
> +
> +	llist_for_each_safe(pos, next, llist_del_all(&engine->barrier_tasks)) {
> +		if (node == barrier_from_ll(pos)) {
> +			node = NULL;
> +			continue;
> +		}
> +
> +		pos->next = head;
> +		head = pos;
> +		if (!tail)
> +			tail = pos;
> +	}
> +	if (head)
> +		llist_add_batch(head, tail, &engine->barrier_tasks);
> +
> +	return !node;
> +}
> +
>   int i915_active_ref(struct i915_active *ref,
>   		    u64 timeline,
>   		    struct i915_request *rq)
> @@ -224,8 +283,15 @@ int i915_active_ref(struct i915_active *ref,
>   		goto out;
>   	}
>   
> -	if (!i915_active_request_isset(active))
> -		atomic_inc(&ref->count);
> +	if (is_barrier(active)) { /* proto-node used by our idle barrier */
> +		__active_del_barrier(ref, node_from_active(active));
> +		RCU_INIT_POINTER(active->request, NULL);
> +		INIT_LIST_HEAD(&active->link);

So this, when prepare_remote_request calls it on ce->active, will remove 
the idle barrier from the engine->barriers_list and immediately transfer 
it to the rq->active_list? Then when this request is retired we know the 
context is idle? But what if it is the same context? Then it is not idle 
yet.

> +	} else {
> +		if (!i915_active_request_isset(active))
> +			atomic_inc(&ref->count);
> +	}
> +	GEM_BUG_ON(!atomic_read(&ref->count));
>   	__i915_active_request_set(active, rq);
>   
>   out:
> @@ -312,6 +378,11 @@ int i915_active_wait(struct i915_active *ref)
>   	}
>   
>   	rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
> +		if (is_barrier(&it->base)) { /* unconnected idle-barrier */
> +			err = -EBUSY;
> +			break;
> +		}
> +
>   		err = i915_active_request_retire(&it->base, BKL(ref));
>   		if (err)
>   			break;
> @@ -374,6 +445,79 @@ void i915_active_fini(struct i915_active *ref)
>   }
>   #endif
>   
> +static inline bool is_idle_barrier(struct active_node *node, u64 idx)
> +{
> +	return node->timeline == idx && !i915_active_request_isset(&node->base);
> +}
> +
> +static struct active_node *idle_barrier(struct i915_active *ref, u64 idx)
> +{
> +	struct rb_node *prev, *p;
> +
> +	if (RB_EMPTY_ROOT(&ref->tree))
> +		return NULL;
> +
> +	mutex_lock(&ref->mutex);
> +	GEM_BUG_ON(i915_active_is_idle(ref));
> +
> +	/*
> +	 * Try to reuse any existing barrier nodes already allocated for this
> +	 * i915_active, due to overlapping active phases there is likely a

For this i915_active or for this idx? It is this i915_active by 
definition, no? And idx also has to match in both loops below.

> +	 * node kept alive (as we reuse before parking). We prefer to reuse
> +	 * completely idle barriers (less hassle in manipulating the llists),
> +	 * but otherwise any will do.
> +	 */
> +	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
> +		p = &ref->cache->node;
> +		goto match;
> +	}
> +
> +	prev = NULL;
> +	p = ref->tree.rb_node;
> +	while (p) {
> +		struct active_node *node =
> +			rb_entry(p, struct active_node, node);
> +
> +		if (is_idle_barrier(node, idx))
> +			goto match;
> +
> +		prev = p;
> +		if (node->timeline < idx)
> +			p = p->rb_right;
> +		else
> +			p = p->rb_left;
> +	}
> +
> +	for (p = prev; p; p = rb_next(p)) {
> +		struct active_node *node =
> +			rb_entry(p, struct active_node, node);
> +
> +		if (node->timeline > idx)
> +			break;
> +
> +		if (node->timeline < idx)
> +			continue;
> +
> +		if (!i915_active_request_isset(&node->base))
> +			goto match;

Isn't this idle barrier, so same as above? How can above not find it, 
and find it here?

> +
> +		if (is_barrier(&node->base) && __active_del_barrier(ref, node))
> +			goto match;

And this is yet unused barrier with the right idx. Under what 
circumstances can __active_del_barrier fail then? I think a comment is 
needed.

> +	}
> +
> +	mutex_unlock(&ref->mutex);
> +
> +	return NULL;
> +
> +match:
> +	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */
> +	if (p == &ref->cache->node)
> +		ref->cache = NULL;
> +	mutex_unlock(&ref->mutex);
> +
> +	return rb_entry(p, struct active_node, node);
> +}
> +
>   int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   					    struct intel_engine_cs *engine)
>   {
> @@ -382,39 +526,52 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   	struct llist_node *pos, *next;
>   	int err;
>   
> -	GEM_BUG_ON(!mask);
> +	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
> +
> +	/*
> +	 * Preallocate a node for each physical engine supporting the target
> +	 * engine (remember virtual engines have more than one sibling).
> +	 * We can then use the preallocated nodes in
> +	 * i915_active_acquire_barrier()
> +	 */
>   	for_each_engine_masked(engine, i915, mask, tmp) {
> -		struct intel_context *kctx = engine->kernel_context;
> +		u64 idx = engine->kernel_context->ring->timeline->fence_context;
>   		struct active_node *node;
>   
> -		node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
> -		if (unlikely(!node)) {
> -			err = -ENOMEM;
> -			goto unwind;
> +		node = idle_barrier(ref, idx);
> +		if (!node) {
> +			node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
> +			if (!node) {
> +				err = ENOMEM;
> +				goto unwind;
> +			}
> +
> +			RCU_INIT_POINTER(node->base.request, NULL);
> +			node->base.retire = node_retire;
> +			node->timeline = idx;
> +			node->ref = ref;
>   		}
>   
> -		i915_active_request_init(&node->base,
> -					 (void *)engine, node_retire);
> -		node->timeline = kctx->ring->timeline->fence_context;
> -		node->ref = ref;
> -		atomic_inc(&ref->count);
> +		if (!i915_active_request_isset(&node->base)) {
> +			RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
> +			node->base.link.prev = (void *)engine;

Comment here please - what for and why it is safe.

> +			atomic_inc(&ref->count);
> +		}
>   
> +		GEM_BUG_ON(barrier_to_engine(node) != engine);
> +		llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
>   		intel_engine_pm_get(engine);
> -		llist_add((struct llist_node *)&node->base.link,
> -			  &ref->barriers);
>   	}
>   
>   	return 0;
>   
>   unwind:
> -	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
> -		struct active_node *node;
> +	llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {

s/get/take/ or remove, extract?

> +		struct active_node *node = barrier_from_ll(pos);
>   
> -		node = container_of((struct list_head *)pos,
> -				    typeof(*node), base.link);
> -		engine = (void *)rcu_access_pointer(node->base.request);
> +		atomic_dec(&ref->count);
> +		intel_engine_pm_put(barrier_to_engine(node));
>   
> -		intel_engine_pm_put(engine);
>   		kmem_cache_free(global.slab_cache, node);
>   	}
>   	return err;
> @@ -426,25 +583,27 @@ void i915_active_acquire_barrier(struct i915_active *ref)
>   
>   	GEM_BUG_ON(i915_active_is_idle(ref));
>   
> +	/*
> +	 * Transfer the list of preallocated barriers into the
> +	 * i915_active rbtree, but only as proto-nodes. They will be
> +	 * populated by i915_request_add_active_barrier() to point to the
> +	 * request that will eventually release them.
> +	 */
>   	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
> -	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
> -		struct intel_engine_cs *engine;
> -		struct active_node *node;
> +	llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {
> +		struct active_node *node = barrier_from_ll(pos);
> +		struct intel_engine_cs *engine = barrier_to_engine(node);
>   		struct rb_node **p, *parent;
>   
> -		node = container_of((struct list_head *)pos,
> -				    typeof(*node), base.link);
> -
> -		engine = (void *)rcu_access_pointer(node->base.request);
> -		RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
> -
>   		parent = NULL;
>   		p = &ref->tree.rb_node;
>   		while (*p) {
> +			struct active_node *it;
> +
>   			parent = *p;
> -			if (rb_entry(parent,
> -				     struct active_node,
> -				     node)->timeline < node->timeline)
> +
> +			it = rb_entry(parent, struct active_node, node);
> +			if (it->timeline < node->timeline)
>   				p = &parent->rb_right;
>   			else
>   				p = &parent->rb_left;
> @@ -452,20 +611,29 @@ void i915_active_acquire_barrier(struct i915_active *ref)
>   		rb_link_node(&node->node, parent, p);
>   		rb_insert_color(&node->node, &ref->tree);
>   
> -		llist_add((struct llist_node *)&node->base.link,
> -			  &engine->barrier_tasks);
> +		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
>   		intel_engine_pm_put(engine);
>   	}
>   	mutex_unlock(&ref->mutex);
>   }
>   
> -void i915_request_add_barriers(struct i915_request *rq)
> +void i915_request_add_active_barriers(struct i915_request *rq)
>   {
>   	struct intel_engine_cs *engine = rq->engine;
>   	struct llist_node *node, *next;
>   
> -	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks))
> +	GEM_BUG_ON(intel_engine_is_virtual(engine));
> +	GEM_BUG_ON(rq->timeline != engine->kernel_context->ring->timeline);
> +
> +	/*
> +	 * Attach the list of proto-fences to the in-flight request such
> +	 * that the parent i915_active will be released when this request
> +	 * is retired.
> +	 */
> +	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
> +		RCU_INIT_POINTER(barrier_from_ll(node)->base.request, rq);
>   		list_add_tail((struct list_head *)node, &rq->active_list);
> +	}
>   }
>   
>   int i915_active_request_set(struct i915_active_request *active,
> diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
> index ba68b077ec6c..566336c99ed7 100644
> --- a/drivers/gpu/drm/i915/i915_active.h
> +++ b/drivers/gpu/drm/i915/i915_active.h
> @@ -413,6 +413,6 @@ static inline void i915_active_fini(struct i915_active *ref) { }
>   int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   					    struct intel_engine_cs *engine);
>   void i915_active_acquire_barrier(struct i915_active *ref);
> -void i915_request_add_barriers(struct i915_request *rq);
> +void i915_request_add_active_barriers(struct i915_request *rq);
>   
>   #endif /* _I915_ACTIVE_H_ */
> diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
> index 74743dd0d5f0..ae3ee441c114 100644
> --- a/drivers/gpu/drm/i915/i915_active_types.h
> +++ b/drivers/gpu/drm/i915/i915_active_types.h
> @@ -42,7 +42,7 @@ struct i915_active {
>   	int (*active)(struct i915_active *ref);
>   	void (*retire)(struct i915_active *ref);
>   
> -	struct llist_head barriers;
> +	struct llist_head preallocated_barriers;
>   };
>   
>   #endif /* _I915_ACTIVE_TYPES_H_ */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> index 2b31a4ee0b4c..a841d3f9bedc 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> @@ -15,6 +15,7 @@ selftest(workarounds, intel_workarounds_live_selftests)
>   selftest(timelines, intel_timeline_live_selftests)
>   selftest(requests, i915_request_live_selftests)
>   selftest(active, i915_active_live_selftests)
> +selftest(gt_contexts, intel_context_live_selftests)
>   selftest(objects, i915_gem_object_live_selftests)
>   selftest(mman, i915_gem_mman_live_selftests)
>   selftest(dmabuf, i915_gem_dmabuf_live_selftests)
> @@ -24,7 +25,7 @@ selftest(gtt, i915_gem_gtt_live_selftests)
>   selftest(gem, i915_gem_live_selftests)
>   selftest(evict, i915_gem_evict_live_selftests)
>   selftest(hugepages, i915_gem_huge_page_live_selftests)
> -selftest(contexts, i915_gem_context_live_selftests)
> +selftest(gem_contexts, i915_gem_context_live_selftests)
>   selftest(blt, i915_gem_object_blt_live_selftests)
>   selftest(client, i915_gem_client_blt_live_selftests)
>   selftest(reset, intel_reset_live_selftests)
> 

It's extremely complicated!

Regards,

Tvrtko

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

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

* Re: [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests
  2019-08-01 17:24   ` Tvrtko Ursulin
@ 2019-08-01 17:55     ` Chris Wilson
  2019-08-02  8:34     ` [PATCH v2] " Chris Wilson
  2019-08-02 10:00     ` [PATCH v3] " Chris Wilson
  2 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2019-08-01 17:55 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-08-01 18:24:50)
> 
> On 30/07/2019 12:21, Chris Wilson wrote:
> > @@ -224,8 +283,15 @@ int i915_active_ref(struct i915_active *ref,
> >               goto out;
> >       }
> >   
> > -     if (!i915_active_request_isset(active))
> > -             atomic_inc(&ref->count);
> > +     if (is_barrier(active)) { /* proto-node used by our idle barrier */
> > +             __active_del_barrier(ref, node_from_active(active));
> > +             RCU_INIT_POINTER(active->request, NULL);
> > +             INIT_LIST_HEAD(&active->link);
> 
> So this, when prepare_remote_request calls it on ce->active, will remove 
> the idle barrier from the engine->barriers_list and immediately transfer 
> it to the rq->active_list?

Yes. So long as the context matches the idle-barrier, i.e. we are
operating on this context from the kernel_context.

> Then when this request is retired we know the 
> context is idle?

So long as the context itself hasn't claimed a new active reference for
its own requests.

> But what if it is the same context? Then it is not idle yet.

Correct, but that would only happens for the kernel_context, which is the
special case where we force idle on parking and exclude from the
deferred active-barrier (as it would continually defer instead of
parking).

> > +static inline bool is_idle_barrier(struct active_node *node, u64 idx)
> > +{
> > +     return node->timeline == idx && !i915_active_request_isset(&node->base);
> > +}
> > +
> > +static struct active_node *idle_barrier(struct i915_active *ref, u64 idx)
> > +{
> > +     struct rb_node *prev, *p;
> > +
> > +     if (RB_EMPTY_ROOT(&ref->tree))
> > +             return NULL;
> > +
> > +     mutex_lock(&ref->mutex);
> > +     GEM_BUG_ON(i915_active_is_idle(ref));
> > +
> > +     /*
> > +      * Try to reuse any existing barrier nodes already allocated for this
> > +      * i915_active, due to overlapping active phases there is likely a
> 
> For this i915_active or for this idx? It is this i915_active by 
> definition, no? And idx also has to match in both loops below.

idx is a timeline, which although would normally be unique in the tree,
due to the active-barrier we may have more than one instance, because we
may have overlapping i915_active phases of activity.

This function exists to try and reuse any of the older phases that we
could not reap as we never completely idled. (Before we would
accidentally keep growing the tree with new nodes until we hit a point
where we retired the idle timeline after parking.)

> > +      * node kept alive (as we reuse before parking). We prefer to reuse
> > +      * completely idle barriers (less hassle in manipulating the llists),
> > +      * but otherwise any will do.
> > +      */
> > +     if (ref->cache && is_idle_barrier(ref->cache, idx)) {
> > +             p = &ref->cache->node;
> > +             goto match;
> > +     }
> > +
> > +     prev = NULL;
> > +     p = ref->tree.rb_node;
> > +     while (p) {
> > +             struct active_node *node =
> > +                     rb_entry(p, struct active_node, node);
> > +
> > +             if (is_idle_barrier(node, idx))
> > +                     goto match;
> > +
> > +             prev = p;
> > +             if (node->timeline < idx)
> > +                     p = p->rb_right;
> > +             else
> > +                     p = p->rb_left;
> > +     }
> > +
> > +     for (p = prev; p; p = rb_next(p)) {
> > +             struct active_node *node =
> > +                     rb_entry(p, struct active_node, node);
> > +
> > +             if (node->timeline > idx)
> > +                     break;
> > +
> > +             if (node->timeline < idx)
> > +                     continue;
> > +
> > +             if (!i915_active_request_isset(&node->base))
> > +                     goto match;
> 
> Isn't this idle barrier, so same as above? How can above not find it, 
> and find it here?

We didn't check all nodes, only followed the binary tree down one
branch to find the starting point. I anticipate we may end up with more
than one idle-barrier in the tree.

> > +
> > +             if (is_barrier(&node->base) && __active_del_barrier(ref, node))
> > +                     goto match;
> 
> And this is yet unused barrier with the right idx. Under what 
> circumstances can __active_del_barrier fail then? I think a comment is 
> needed.

__active_del_barrier() here can theoretically race with
i915_request_add_active_barriers(). In the other callsite, we must be
holding the kernel_context timeline mutex and so cannot be concurrently
calling add_active_barriers(). [Here is the weakness of using
__active_del_barrier(), in order to reuse an activated idle-barrier, we
may cause add_active_barriers() to miss the others and so postpone the
retirement of other contexts. Under the current scheme of parking, that
is not an issue. In the future, it means retirement may miss a
heartbeat.]

> > +             if (!i915_active_request_isset(&node->base)) {
> > +                     RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
> > +                     node->base.link.prev = (void *)engine;
> 
> Comment here please - what for and why it is safe.

We're simply stashing pointers inside a block of memory that we own and
pass to ourselves later on. We've made sure it is not accessible from
the tree.

> > +                     atomic_inc(&ref->count);
> > +             }
> >   
> > +             GEM_BUG_ON(barrier_to_engine(node) != engine);
> > +             llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
> >               intel_engine_pm_get(engine);
> > -             llist_add((struct llist_node *)&node->base.link,
> > -                       &ref->barriers);
> >       }
> >   
> >       return 0;
> >   
> >   unwind:
> > -     llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
> > -             struct active_node *node;
> > +     llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {
> 
> s/get/take/ or remove, extract?

take, hopefully there's a column spare.

> It's extremely complicated!

No kidding! last_retired_context was so much easier to understand.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 3/3] drm/i915: Flush extra hard after writing relocations through the GTT
  2019-08-01 20:33   ` [Intel-gfx] " Kumar Valsan, Prathap
@ 2019-08-01 20:21     ` Chris Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2019-08-01 20:21 UTC (permalink / raw)
  To: Kumar Valsan, Prathap; +Cc: intel-gfx, stable

Quoting Kumar Valsan, Prathap (2019-08-01 21:33:44)
> On Tue, Jul 30, 2019 at 12:21:51PM +0100, Chris Wilson wrote:
> > Recently discovered in commit bdae33b8b82b ("drm/i915: Use maximum write
> > flush for pwrite_gtt") was that we needed to our full write barrier
> > before changing the GGTT PTE to ensure that our indirect writes through
> > the GTT landed before the PTE changed (and the writes end up in a
> > different page). That also applies to our GGTT relocation path.
> 
> Chris,
> 
> As i understand, changing the GGTT PTE also an indirect write. If so, isn't a wmb()
> should be good enough.

Ha! If only that was true.
-Chris

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

* Re: [Intel-gfx] [PATCH 3/3] drm/i915: Flush extra hard after writing relocations through the GTT
  2019-07-30 11:21   ` Chris Wilson
  (?)
@ 2019-08-01 20:33   ` Kumar Valsan, Prathap
  2019-08-01 20:21     ` Chris Wilson
  -1 siblings, 1 reply; 21+ messages in thread
From: Kumar Valsan, Prathap @ 2019-08-01 20:33 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, stable

On Tue, Jul 30, 2019 at 12:21:51PM +0100, Chris Wilson wrote:
> Recently discovered in commit bdae33b8b82b ("drm/i915: Use maximum write
> flush for pwrite_gtt") was that we needed to our full write barrier
> before changing the GGTT PTE to ensure that our indirect writes through
> the GTT landed before the PTE changed (and the writes end up in a
> different page). That also applies to our GGTT relocation path.

Chris,

As i understand, changing the GGTT PTE also an indirect write. If so, isn't a wmb()
should be good enough.

Thanks,
Prathap
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 8a2047c4e7c3..01901dad33f7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1019,11 +1019,12 @@ static void reloc_cache_reset(struct reloc_cache *cache)
>  		kunmap_atomic(vaddr);
>  		i915_gem_object_finish_access((struct drm_i915_gem_object *)cache->node.mm);
>  	} else {
> -		wmb();
> +		struct i915_ggtt *ggtt = cache_to_ggtt(cache);
> +
> +		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>  		io_mapping_unmap_atomic((void __iomem *)vaddr);
> -		if (cache->node.allocated) {
> -			struct i915_ggtt *ggtt = cache_to_ggtt(cache);
>  
> +		if (cache->node.allocated) {
>  			ggtt->vm.clear_range(&ggtt->vm,
>  					     cache->node.start,
>  					     cache->node.size);
> @@ -1078,6 +1079,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
>  	void *vaddr;
>  
>  	if (cache->vaddr) {
> +		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>  		io_mapping_unmap_atomic((void __force __iomem *) unmask_page(cache->vaddr));
>  	} else {
>  		struct i915_vma *vma;
> @@ -1119,7 +1121,6 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
>  
>  	offset = cache->node.start;
>  	if (cache->node.allocated) {
> -		wmb();
>  		ggtt->vm.insert_page(&ggtt->vm,
>  				     i915_gem_object_get_dma_address(obj, page),
>  				     offset, I915_CACHE_NONE, 0);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 3/3] drm/i915: Flush extra hard after writing relocations through the GTT
  2019-07-30 11:21   ` Chris Wilson
  (?)
  (?)
@ 2019-08-01 23:08   ` Kumar Valsan, Prathap
  -1 siblings, 0 replies; 21+ messages in thread
From: Kumar Valsan, Prathap @ 2019-08-01 23:08 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, stable

On Tue, Jul 30, 2019 at 12:21:51PM +0100, Chris Wilson wrote:
> Recently discovered in commit bdae33b8b82b ("drm/i915: Use maximum write
> flush for pwrite_gtt") was that we needed to our full write barrier
> before changing the GGTT PTE to ensure that our indirect writes through
> the GTT landed before the PTE changed (and the writes end up in a
> different page). That also applies to our GGTT relocation path.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org

Reviewed-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 8a2047c4e7c3..01901dad33f7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1019,11 +1019,12 @@ static void reloc_cache_reset(struct reloc_cache *cache)
>  		kunmap_atomic(vaddr);
>  		i915_gem_object_finish_access((struct drm_i915_gem_object *)cache->node.mm);
>  	} else {
> -		wmb();
> +		struct i915_ggtt *ggtt = cache_to_ggtt(cache);
> +
> +		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>  		io_mapping_unmap_atomic((void __iomem *)vaddr);
> -		if (cache->node.allocated) {
> -			struct i915_ggtt *ggtt = cache_to_ggtt(cache);
>  
> +		if (cache->node.allocated) {
>  			ggtt->vm.clear_range(&ggtt->vm,
>  					     cache->node.start,
>  					     cache->node.size);
> @@ -1078,6 +1079,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
>  	void *vaddr;
>  
>  	if (cache->vaddr) {
> +		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>  		io_mapping_unmap_atomic((void __force __iomem *) unmask_page(cache->vaddr));
>  	} else {
>  		struct i915_vma *vma;
> @@ -1119,7 +1121,6 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
>  
>  	offset = cache->node.start;
>  	if (cache->node.allocated) {
> -		wmb();
>  		ggtt->vm.insert_page(&ggtt->vm,
>  				     i915_gem_object_get_dma_address(obj, page),
>  				     offset, I915_CACHE_NONE, 0);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2] drm/i915: Allow sharing the idle-barrier from other kernel requests
  2019-08-01 17:24   ` Tvrtko Ursulin
  2019-08-01 17:55     ` Chris Wilson
@ 2019-08-02  8:34     ` Chris Wilson
  2019-08-02 10:00     ` [PATCH v3] " Chris Wilson
  2 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2019-08-02  8:34 UTC (permalink / raw)
  To: intel-gfx

By placing our idle-barriers in the i915_active fence tree, we expose
those for reuse by other components that are issuing requests along the
kernel_context. Reusing the proto-barrier active_node is perfectly fine
as the new request implies a context-switch, and so an opportune point
to run the idle-barrier. However, the proto-barrier is not equivalent
to a normal active_node and care must be taken to avoid dereferencing the
ERR_PTR used as its request marker.

v2: Comment the more egregious cheek

Reported-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context.c       |  40 ++-
 drivers/gpu/drm/i915/gt/intel_context.h       |  13 +-
 drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   2 +-
 drivers/gpu/drm/i915/gt/selftest_context.c    | 310 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_active.c            | 265 ++++++++++++---
 drivers/gpu/drm/i915/i915_active.h            |   2 +-
 drivers/gpu/drm/i915/i915_active_types.h      |   2 +-
 .../drm/i915/selftests/i915_live_selftests.h  |   3 +-
 8 files changed, 574 insertions(+), 63 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_context.c

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index f30441a140f8..34c8e37a73b8 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -162,23 +162,41 @@ static int __intel_context_active(struct i915_active *active)
 	if (err)
 		goto err_ring;
 
+	return 0;
+
+err_ring:
+	intel_ring_unpin(ce->ring);
+err_put:
+	intel_context_put(ce);
+	return err;
+}
+
+int intel_context_active_acquire(struct intel_context *ce)
+{
+	int err;
+
+	err = i915_active_acquire(&ce->active);
+	if (err)
+		return err;
+
 	/* Preallocate tracking nodes */
 	if (!i915_gem_context_is_kernel(ce->gem_context)) {
 		err = i915_active_acquire_preallocate_barrier(&ce->active,
 							      ce->engine);
-		if (err)
-			goto err_state;
+		if (err) {
+			i915_active_release(&ce->active);
+			return err;
+		}
 	}
 
 	return 0;
+}
 
-err_state:
-	__context_unpin_state(ce->state);
-err_ring:
-	intel_ring_unpin(ce->ring);
-err_put:
-	intel_context_put(ce);
-	return err;
+void intel_context_active_release(struct intel_context *ce)
+{
+	/* Nodes preallocated in intel_context_active() */
+	i915_active_acquire_barrier(&ce->active);
+	i915_active_release(&ce->active);
 }
 
 void
@@ -301,3 +319,7 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
 
 	return rq;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_context.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 23c7e4c0ce7c..07f9924de48f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -104,17 +104,8 @@ static inline void intel_context_exit(struct intel_context *ce)
 		ce->ops->exit(ce);
 }
 
-static inline int intel_context_active_acquire(struct intel_context *ce)
-{
-	return i915_active_acquire(&ce->active);
-}
-
-static inline void intel_context_active_release(struct intel_context *ce)
-{
-	/* Nodes preallocated in intel_context_active() */
-	i915_active_acquire_barrier(&ce->active);
-	i915_active_release(&ce->active);
-}
+int intel_context_active_acquire(struct intel_context *ce);
+void intel_context_active_release(struct intel_context *ce);
 
 static inline struct intel_context *intel_context_get(struct intel_context *ce)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index e74fbf04a68d..ce54092475da 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -90,7 +90,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 	/* Check again on the next retirement. */
 	engine->wakeref_serial = engine->serial + 1;
 
-	i915_request_add_barriers(rq);
+	i915_request_add_active_barriers(rq);
 	__i915_request_commit(rq);
 
 	return false;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
new file mode 100644
index 000000000000..d39b5594cb02
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -0,0 +1,310 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "intel_gt.h"
+
+#include "gem/selftests/mock_context.h"
+#include "selftests/igt_flush_test.h"
+#include "selftests/mock_drm.h"
+
+static int request_sync(struct i915_request *rq)
+{
+	long timeout;
+	int err = 0;
+
+	i915_request_get(rq);
+
+	i915_request_add(rq);
+	timeout = i915_request_wait(rq, 0, HZ / 10);
+	if (timeout < 0)
+		err = timeout;
+	else
+		i915_request_retire_upto(rq);
+
+	i915_request_put(rq);
+
+	return err;
+}
+
+static int context_sync(struct intel_context *ce)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err = 0;
+
+	do {
+		struct i915_request *rq;
+		long timeout;
+
+		rcu_read_lock();
+		rq = rcu_dereference(tl->last_request.request);
+		if (rq)
+			rq = i915_request_get_rcu(rq);
+		rcu_read_unlock();
+		if (!rq)
+			break;
+
+		timeout = i915_request_wait(rq, 0, HZ / 10);
+		if (timeout < 0)
+			err = timeout;
+		else
+			i915_request_retire_upto(rq);
+
+		i915_request_put(rq);
+	} while (!err);
+
+	return err;
+}
+
+static int __live_active_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *ce;
+	int pass;
+	int err;
+
+	/*
+	 * We keep active contexts alive until after a subsequent context
+	 * switch as the final write from the context-save will be after
+	 * we retire the final request. We track when we unpin the context,
+	 * under the presumption that the final pin is from the last request,
+	 * and instead of immediately unpinning the context, we add a task
+	 * to unpin the context from the next idle-barrier.
+	 *
+	 * This test makes sure that the context is kept alive until a
+	 * subsequent idle-barrier (emitted when the engine wakeref hits 0
+	 * with no more outstanding requests).
+	 */
+
+	if (intel_engine_pm_is_awake(engine)) {
+		pr_err("%s is awake before starting %s!\n",
+		       engine->name, __func__);
+		return -EINVAL;
+	}
+
+	ce = intel_context_create(fixme, engine);
+	if (!ce)
+		return -ENOMEM;
+
+	for (pass = 0; pass <= 2; pass++) {
+		struct i915_request *rq;
+
+		rq = intel_context_create_request(ce);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			goto err;
+		}
+
+		err = request_sync(rq);
+		if (err)
+			goto err;
+
+		/* Context will be kept active until after an idle-barrier. */
+		if (i915_active_is_idle(&ce->active)) {
+			pr_err("context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			goto err;
+		}
+
+		if (!intel_engine_pm_is_awake(engine)) {
+			pr_err("%s is asleep before idle-barrier\n",
+			       engine->name);
+			err = -EINVAL;
+			goto err;
+		}
+	}
+
+	/* Now make sure our idle-barriers are flushed */
+	err = context_sync(engine->kernel_context);
+	if (err)
+		goto err;
+
+	if (!i915_active_is_idle(&ce->active)) {
+		pr_err("context is still active!");
+		err = -EINVAL;
+	}
+
+	if (intel_engine_pm_is_awake(engine)) {
+		struct drm_printer p = drm_debug_printer(__func__);
+
+		intel_engine_dump(engine, &p,
+				  "%s is still awake after idle-barriers\n",
+				  engine->name);
+		GEM_TRACE_DUMP();
+
+		err = -EINVAL;
+		goto err;
+	}
+
+err:
+	intel_context_put(ce);
+	return err;
+}
+
+static int live_active_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_active_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+static int __remote_sync(struct intel_context *ce, struct intel_context *remote)
+{
+	struct i915_request *rq;
+	int err;
+
+	err = intel_context_pin(remote);
+	if (err)
+		return err;
+
+	rq = intel_context_create_request(ce);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto unpin;
+	}
+
+	err = intel_context_prepare_remote_request(remote, rq);
+	if (err) {
+		i915_request_add(rq);
+		goto unpin;
+	}
+
+	err = request_sync(rq);
+
+unpin:
+	intel_context_unpin(remote);
+	return err;
+}
+
+static int __live_remote_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *local, *remote;
+	int pass;
+	int err;
+
+	/*
+	 * Check that our idle barriers do not interfere with normal
+	 * activity tracking. In particular, check that operating
+	 * on the context image remotely (intel_context_prepare_remote_request),
+	 * which inserts foreign fences into intel_context.active, does not
+	 * clobber the idle-barrier.
+	 */
+
+	remote = intel_context_create(fixme, engine);
+	if (!remote)
+		return -ENOMEM;
+
+	local = intel_context_create(fixme, engine);
+	if (!local) {
+		err = -ENOMEM;
+		goto err_remote;
+	}
+
+	for (pass = 0; pass <= 2; pass++) {
+		err = __remote_sync(local, remote);
+		if (err)
+			break;
+
+		err = __remote_sync(engine->kernel_context, remote);
+		if (err)
+			break;
+
+		if (i915_active_is_idle(&remote->active)) {
+			pr_err("remote context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+	intel_context_put(local);
+err_remote:
+	intel_context_put(remote);
+	return err;
+}
+
+static int live_remote_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_remote_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+int intel_context_live_selftests(struct drm_i915_private *i915)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(live_active_context),
+		SUBTEST(live_remote_context),
+	};
+	struct intel_gt *gt = &i915->gt;
+
+	if (intel_gt_is_wedged(gt))
+		return 0;
+
+	return intel_gt_live_subtests(tests, gt);
+}
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index d32db8a4db5c..f1c50bdb0e85 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -33,6 +33,38 @@ struct active_node {
 	u64 timeline;
 };
 
+static inline struct active_node *
+node_from_active(struct i915_active_request *active)
+{
+	return container_of(active, struct active_node, base);
+}
+
+#define take_preallocated_barriers(x) llist_del_all(&(x)->preallocated_barriers)
+
+static inline bool is_barrier(const struct i915_active_request *active)
+{
+	return IS_ERR(rcu_access_pointer(active->request));
+}
+
+static inline struct llist_node *barrier_to_ll(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct llist_node *)&node->base.link;
+}
+
+static inline struct intel_engine_cs *
+barrier_to_engine(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct intel_engine_cs *)node->base.link.prev;
+}
+
+static inline struct active_node *barrier_from_ll(struct llist_node *x)
+{
+	return container_of((struct list_head *)x,
+			    struct active_node, base.link);
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && IS_ENABLED(CONFIG_DEBUG_OBJECTS)
 
 static void *active_debug_hint(void *addr)
@@ -127,7 +159,7 @@ active_retire(struct i915_active *ref)
 static void
 node_retire(struct i915_active_request *base, struct i915_request *rq)
 {
-	active_retire(container_of(base, struct active_node, base)->ref);
+	active_retire(node_from_active(base)->ref);
 }
 
 static struct i915_active_request *
@@ -184,6 +216,7 @@ active_instance(struct i915_active *ref, u64 idx)
 	ref->cache = node;
 	mutex_unlock(&ref->mutex);
 
+	BUILD_BUG_ON(offsetof(typeof(*node), base));
 	return &node->base;
 }
 
@@ -201,11 +234,37 @@ void __i915_active_init(struct drm_i915_private *i915,
 	ref->retire = retire;
 	ref->tree = RB_ROOT;
 	ref->cache = NULL;
-	init_llist_head(&ref->barriers);
+	init_llist_head(&ref->preallocated_barriers);
 	atomic_set(&ref->count, 0);
 	__mutex_init(&ref->mutex, "i915_active", key);
 }
 
+static bool __active_del_barrier(struct i915_active *ref,
+				 struct active_node *node)
+{
+	struct intel_engine_cs *engine = barrier_to_engine(node);
+	struct llist_node *head = NULL, *tail = NULL;
+	struct llist_node *pos, *next;
+
+	GEM_BUG_ON(node->timeline != engine->kernel_context->ring->timeline->fence_context);
+
+	llist_for_each_safe(pos, next, llist_del_all(&engine->barrier_tasks)) {
+		if (node == barrier_from_ll(pos)) {
+			node = NULL;
+			continue;
+		}
+
+		pos->next = head;
+		head = pos;
+		if (!tail)
+			tail = pos;
+	}
+	if (head)
+		llist_add_batch(head, tail, &engine->barrier_tasks);
+
+	return !node;
+}
+
 int i915_active_ref(struct i915_active *ref,
 		    u64 timeline,
 		    struct i915_request *rq)
@@ -224,8 +283,20 @@ int i915_active_ref(struct i915_active *ref,
 		goto out;
 	}
 
-	if (!i915_active_request_isset(active))
-		atomic_inc(&ref->count);
+	if (is_barrier(active)) { /* proto-node used by our idle barrier */
+		/*
+		 * This request is on the kernel_context timeline, and so
+		 * we can use it to substitute for the pending idle-barrer
+		 * request that we want to emit on the kernel_context.
+		 */
+		__active_del_barrier(ref, node_from_active(active));
+		RCU_INIT_POINTER(active->request, NULL);
+		INIT_LIST_HEAD(&active->link);
+	} else {
+		if (!i915_active_request_isset(active))
+			atomic_inc(&ref->count);
+	}
+	GEM_BUG_ON(!atomic_read(&ref->count));
 	__i915_active_request_set(active, rq);
 
 out:
@@ -312,6 +383,11 @@ int i915_active_wait(struct i915_active *ref)
 	}
 
 	rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
+		if (is_barrier(&it->base)) { /* unconnected idle-barrier */
+			err = -EBUSY;
+			break;
+		}
+
 		err = i915_active_request_retire(&it->base, BKL(ref));
 		if (err)
 			break;
@@ -374,6 +450,92 @@ void i915_active_fini(struct i915_active *ref)
 }
 #endif
 
+static inline bool is_idle_barrier(struct active_node *node, u64 idx)
+{
+	return node->timeline == idx && !i915_active_request_isset(&node->base);
+}
+
+static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
+{
+	struct rb_node *prev, *p;
+
+	if (RB_EMPTY_ROOT(&ref->tree))
+		return NULL;
+
+	mutex_lock(&ref->mutex);
+	GEM_BUG_ON(i915_active_is_idle(ref));
+
+	/*
+	 * Try to reuse any existing barrier nodes already allocated for this
+	 * i915_active, due to overlapping active phases there is likely a
+	 * node kept alive (as we reuse before parking). We prefer to reuse
+	 * completely idle barriers (less hassle in manipulating the llists),
+	 * but otherwise any will do.
+	 */
+	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
+		p = &ref->cache->node;
+		goto match;
+	}
+
+	prev = NULL;
+	p = ref->tree.rb_node;
+	while (p) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (is_idle_barrier(node, idx))
+			goto match;
+
+		prev = p;
+		if (node->timeline < idx)
+			p = p->rb_right;
+		else
+			p = p->rb_left;
+	}
+
+	/*
+	 * No quick match, but we did find the leftmost rb_node for the
+	 * kernel_context. Walk the rb_tree in-order to see if there were
+	 * any idle-barriers on this timeline that we missed, or just use
+	 * the first pending barrier.
+	 */
+	for (p = prev; p; p = rb_next(p)) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (node->timeline > idx)
+			break;
+
+		if (node->timeline < idx)
+			continue;
+
+		if (!i915_active_request_isset(&node->base))
+			goto match;
+
+		/*
+		 * The list of pending barriers is protected by the
+		 * kernel_context timeline, which notably we do not hold
+		 * here. i915_request_add_active_barriers() may consume
+		 * the barrier before we claim it, so we have to check
+		 * for success.
+		 */
+		if (is_barrier(&node->base) && __active_del_barrier(ref, node))
+			goto match;
+	}
+
+	mutex_unlock(&ref->mutex);
+
+	return NULL;
+
+match:
+	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */
+	if (p == &ref->cache->node)
+		ref->cache = NULL;
+	mutex_unlock(&ref->mutex);
+
+	return rb_entry(p, struct active_node, node);
+}
+
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
@@ -382,39 +544,53 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	struct llist_node *pos, *next;
 	int err;
 
-	GEM_BUG_ON(!mask);
+	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
+
+	/*
+	 * Preallocate a node for each physical engine supporting the target
+	 * engine (remember virtual engines have more than one sibling).
+	 * We can then use the preallocated nodes in
+	 * i915_active_acquire_barrier()
+	 */
 	for_each_engine_masked(engine, i915, mask, tmp) {
-		struct intel_context *kctx = engine->kernel_context;
+		u64 idx = engine->kernel_context->ring->timeline->fence_context;
 		struct active_node *node;
 
-		node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
-		if (unlikely(!node)) {
-			err = -ENOMEM;
-			goto unwind;
+		node = reuse_idle_barrier(ref, idx);
+		if (!node) {
+			node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
+			if (!node) {
+				err = ENOMEM;
+				goto unwind;
+			}
+
+			RCU_INIT_POINTER(node->base.request, NULL);
+			node->base.retire = node_retire;
+			node->timeline = idx;
+			node->ref = ref;
 		}
 
-		i915_active_request_init(&node->base,
-					 (void *)engine, node_retire);
-		node->timeline = kctx->ring->timeline->fence_context;
-		node->ref = ref;
-		atomic_inc(&ref->count);
+		if (!i915_active_request_isset(&node->base)) {
+			/* Mark this as being *our* unconnected proto-node */
+			RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
+			node->base.link.prev = (void *)engine;
+			atomic_inc(&ref->count);
+		}
 
+		GEM_BUG_ON(barrier_to_engine(node) != engine);
+		llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
 		intel_engine_pm_get(engine);
-		llist_add((struct llist_node *)&node->base.link,
-			  &ref->barriers);
 	}
 
 	return 0;
 
 unwind:
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct active_node *node;
+	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
 
-		node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-		engine = (void *)rcu_access_pointer(node->base.request);
+		atomic_dec(&ref->count);
+		intel_engine_pm_put(barrier_to_engine(node));
 
-		intel_engine_pm_put(engine);
 		kmem_cache_free(global.slab_cache, node);
 	}
 	return err;
@@ -426,25 +602,27 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 
 	GEM_BUG_ON(i915_active_is_idle(ref));
 
+	/*
+	 * Transfer the list of preallocated barriers into the
+	 * i915_active rbtree, but only as proto-nodes. They will be
+	 * populated by i915_request_add_active_barrier() to point to the
+	 * request that will eventually release them.
+	 */
 	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct intel_engine_cs *engine;
-		struct active_node *node;
+	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
+		struct intel_engine_cs *engine = barrier_to_engine(node);
 		struct rb_node **p, *parent;
 
-		node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-
-		engine = (void *)rcu_access_pointer(node->base.request);
-		RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
-
 		parent = NULL;
 		p = &ref->tree.rb_node;
 		while (*p) {
+			struct active_node *it;
+
 			parent = *p;
-			if (rb_entry(parent,
-				     struct active_node,
-				     node)->timeline < node->timeline)
+
+			it = rb_entry(parent, struct active_node, node);
+			if (it->timeline < node->timeline)
 				p = &parent->rb_right;
 			else
 				p = &parent->rb_left;
@@ -452,20 +630,29 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 		rb_link_node(&node->node, parent, p);
 		rb_insert_color(&node->node, &ref->tree);
 
-		llist_add((struct llist_node *)&node->base.link,
-			  &engine->barrier_tasks);
+		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
 		intel_engine_pm_put(engine);
 	}
 	mutex_unlock(&ref->mutex);
 }
 
-void i915_request_add_barriers(struct i915_request *rq)
+void i915_request_add_active_barriers(struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
 	struct llist_node *node, *next;
 
-	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks))
+	GEM_BUG_ON(intel_engine_is_virtual(engine));
+	GEM_BUG_ON(rq->timeline != engine->kernel_context->ring->timeline);
+
+	/*
+	 * Attach the list of proto-fences to the in-flight request such
+	 * that the parent i915_active will be released when this request
+	 * is retired.
+	 */
+	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
+		RCU_INIT_POINTER(barrier_from_ll(node)->base.request, rq);
 		list_add_tail((struct list_head *)node, &rq->active_list);
+	}
 }
 
 int i915_active_request_set(struct i915_active_request *active,
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index ba68b077ec6c..566336c99ed7 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -413,6 +413,6 @@ static inline void i915_active_fini(struct i915_active *ref) { }
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine);
 void i915_active_acquire_barrier(struct i915_active *ref);
-void i915_request_add_barriers(struct i915_request *rq);
+void i915_request_add_active_barriers(struct i915_request *rq);
 
 #endif /* _I915_ACTIVE_H_ */
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index 74743dd0d5f0..ae3ee441c114 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -42,7 +42,7 @@ struct i915_active {
 	int (*active)(struct i915_active *ref);
 	void (*retire)(struct i915_active *ref);
 
-	struct llist_head barriers;
+	struct llist_head preallocated_barriers;
 };
 
 #endif /* _I915_ACTIVE_TYPES_H_ */
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 2b31a4ee0b4c..a841d3f9bedc 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -15,6 +15,7 @@ selftest(workarounds, intel_workarounds_live_selftests)
 selftest(timelines, intel_timeline_live_selftests)
 selftest(requests, i915_request_live_selftests)
 selftest(active, i915_active_live_selftests)
+selftest(gt_contexts, intel_context_live_selftests)
 selftest(objects, i915_gem_object_live_selftests)
 selftest(mman, i915_gem_mman_live_selftests)
 selftest(dmabuf, i915_gem_dmabuf_live_selftests)
@@ -24,7 +25,7 @@ selftest(gtt, i915_gem_gtt_live_selftests)
 selftest(gem, i915_gem_live_selftests)
 selftest(evict, i915_gem_evict_live_selftests)
 selftest(hugepages, i915_gem_huge_page_live_selftests)
-selftest(contexts, i915_gem_context_live_selftests)
+selftest(gem_contexts, i915_gem_context_live_selftests)
 selftest(blt, i915_gem_object_blt_live_selftests)
 selftest(client, i915_gem_client_blt_live_selftests)
 selftest(reset, intel_reset_live_selftests)
-- 
2.23.0.rc0

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2)
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (5 preceding siblings ...)
  2019-07-30 22:46 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-08-02  8:44 ` Patchwork
  2019-08-02  9:27 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-08-02  8:44 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2)
URL   : https://patchwork.freedesktop.org/series/64426/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
fade66ed3996 drm/i915: Allow sharing the idle-barrier from other kernel requests
-:125: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#125: 
new file mode 100644

-:130: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#130: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:1:
+/*

-:131: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#131: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:2:
+ * SPDX-License-Identifier: GPL-2.0

total: 0 errors, 3 warnings, 0 checks, 784 lines checked

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2)
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (6 preceding siblings ...)
  2019-08-02  8:44 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2) Patchwork
@ 2019-08-02  9:27 ` Patchwork
  2019-08-02 10:29 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3) Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-08-02  9:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2)
URL   : https://patchwork.freedesktop.org/series/64426/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6612 -> Patchwork_13846
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload-no-display:
    - fi-cfl-8109u:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-cfl-8109u/igt@i915_module_load@reload-no-display.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-cfl-8109u/igt@i915_module_load@reload-no-display.html

  
New tests
---------

  New tests have been introduced between CI_DRM_6612 and Patchwork_13846:

### New IGT tests (2) ###

  * igt@i915_selftest@live_gem_contexts:
    - Statuses : 41 pass(s)
    - Exec time: [1.25, 28.46] s

  * igt@i915_selftest@live_gt_contexts:
    - Statuses : 41 pass(s)
    - Exec time: [0.37, 1.36] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@create-close:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-icl-u3/igt@gem_basic@create-close.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-icl-u3/igt@gem_basic@create-close.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][5] -> [INCOMPLETE][6] ([fdo#107718])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

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

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [PASS][9] -> [DMESG-FAIL][10] ([fdo#111108])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_reset:
    - fi-icl-u3:          [PASS][11] -> [INCOMPLETE][12] ([fdo#107713])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-icl-u3/igt@i915_selftest@live_reset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-icl-u3/igt@i915_selftest@live_reset.html

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       [PASS][13] -> [SKIP][14] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html

  * igt@prime_vgem@basic-fence-read:
    - fi-bwr-2160:        [PASS][15] -> [INCOMPLETE][16] ([fdo#111278])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-bwr-2160/igt@prime_vgem@basic-fence-read.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-bwr-2160/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@gem_exec_reloc@basic-softpin:
    - fi-icl-u3:          [DMESG-WARN][17] ([fdo#107724]) -> [PASS][18] +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-icl-u3/igt@gem_exec_reloc@basic-softpin.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-icl-u3/igt@gem_exec_reloc@basic-softpin.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       [WARN][19] ([fdo#109380]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7567u:       [FAIL][21] ([fdo#109485]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u4}:        [FAIL][23] ([fdo#103167]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-u2:          [FAIL][25] ([fdo#103167]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-kbl-7567u:       [SKIP][27] ([fdo#109271]) -> [PASS][28] +23 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  * igt@prime_vgem@basic-fence-read:
    - fi-pnv-d510:        [INCOMPLETE][29] ([fdo#110740]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-pnv-d510/igt@prime_vgem@basic-fence-read.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-pnv-d510/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-sync-default:
    - fi-bxt-j4205:       [FAIL][31] ([fdo#111277]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-bxt-j4205/igt@prime_vgem@basic-sync-default.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-bxt-j4205/igt@prime_vgem@basic-sync-default.html
    - fi-bxt-dsi:         [FAIL][33] ([fdo#111277]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-bxt-dsi/igt@prime_vgem@basic-sync-default.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13846/fi-bxt-dsi/igt@prime_vgem@basic-sync-default.html

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108
  [fdo#111277]: https://bugs.freedesktop.org/show_bug.cgi?id=111277
  [fdo#111278]: https://bugs.freedesktop.org/show_bug.cgi?id=111278


Participating hosts (50 -> 44)
------------------------------

  Additional (1): fi-icl-guc 
  Missing    (7): fi-kbl-soraka fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-icl-y fi-icl-dsi fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6612 -> Patchwork_13846

  CI-20190529: 20190529
  CI_DRM_6612: c85e7a40c6feee8cb16ceee3386189fc00f48cc1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5120: b3138fbea79d5d7935e53530b90efe3e816236f4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13846: fade66ed3996316c5711e63c8f83b96070c3b3fc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fade66ed3996 drm/i915: Allow sharing the idle-barrier from other kernel requests

== Logs ==

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

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

* [PATCH v3] drm/i915: Allow sharing the idle-barrier from other kernel requests
  2019-08-01 17:24   ` Tvrtko Ursulin
  2019-08-01 17:55     ` Chris Wilson
  2019-08-02  8:34     ` [PATCH v2] " Chris Wilson
@ 2019-08-02 10:00     ` Chris Wilson
  2019-08-02 10:11       ` Tvrtko Ursulin
  2 siblings, 1 reply; 21+ messages in thread
From: Chris Wilson @ 2019-08-02 10:00 UTC (permalink / raw)
  To: intel-gfx

By placing our idle-barriers in the i915_active fence tree, we expose
those for reuse by other components that are issuing requests along the
kernel_context. Reusing the proto-barrier active_node is perfectly fine
as the new request implies a context-switch, and so an opportune point
to run the idle-barrier. However, the proto-barrier is not equivalent
to a normal active_node and care must be taken to avoid dereferencing the
ERR_PTR used as its request marker.

v2: Comment the more egregious cheek
v3: A glossary!

Reported-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_context.c       |  40 ++-
 drivers/gpu/drm/i915/gt/intel_context.h       |  13 +-
 drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   2 +-
 drivers/gpu/drm/i915/gt/selftest_context.c    | 310 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_active.c            | 288 +++++++++++++---
 drivers/gpu/drm/i915/i915_active.h            |   2 +-
 drivers/gpu/drm/i915/i915_active_types.h      |   2 +-
 .../drm/i915/selftests/i915_live_selftests.h  |   3 +-
 8 files changed, 597 insertions(+), 63 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_context.c

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index f30441a140f8..34c8e37a73b8 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -162,23 +162,41 @@ static int __intel_context_active(struct i915_active *active)
 	if (err)
 		goto err_ring;
 
+	return 0;
+
+err_ring:
+	intel_ring_unpin(ce->ring);
+err_put:
+	intel_context_put(ce);
+	return err;
+}
+
+int intel_context_active_acquire(struct intel_context *ce)
+{
+	int err;
+
+	err = i915_active_acquire(&ce->active);
+	if (err)
+		return err;
+
 	/* Preallocate tracking nodes */
 	if (!i915_gem_context_is_kernel(ce->gem_context)) {
 		err = i915_active_acquire_preallocate_barrier(&ce->active,
 							      ce->engine);
-		if (err)
-			goto err_state;
+		if (err) {
+			i915_active_release(&ce->active);
+			return err;
+		}
 	}
 
 	return 0;
+}
 
-err_state:
-	__context_unpin_state(ce->state);
-err_ring:
-	intel_ring_unpin(ce->ring);
-err_put:
-	intel_context_put(ce);
-	return err;
+void intel_context_active_release(struct intel_context *ce)
+{
+	/* Nodes preallocated in intel_context_active() */
+	i915_active_acquire_barrier(&ce->active);
+	i915_active_release(&ce->active);
 }
 
 void
@@ -301,3 +319,7 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
 
 	return rq;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_context.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 23c7e4c0ce7c..07f9924de48f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -104,17 +104,8 @@ static inline void intel_context_exit(struct intel_context *ce)
 		ce->ops->exit(ce);
 }
 
-static inline int intel_context_active_acquire(struct intel_context *ce)
-{
-	return i915_active_acquire(&ce->active);
-}
-
-static inline void intel_context_active_release(struct intel_context *ce)
-{
-	/* Nodes preallocated in intel_context_active() */
-	i915_active_acquire_barrier(&ce->active);
-	i915_active_release(&ce->active);
-}
+int intel_context_active_acquire(struct intel_context *ce);
+void intel_context_active_release(struct intel_context *ce);
 
 static inline struct intel_context *intel_context_get(struct intel_context *ce)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index e74fbf04a68d..ce54092475da 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -90,7 +90,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 	/* Check again on the next retirement. */
 	engine->wakeref_serial = engine->serial + 1;
 
-	i915_request_add_barriers(rq);
+	i915_request_add_active_barriers(rq);
 	__i915_request_commit(rq);
 
 	return false;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
new file mode 100644
index 000000000000..d39b5594cb02
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -0,0 +1,310 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "intel_gt.h"
+
+#include "gem/selftests/mock_context.h"
+#include "selftests/igt_flush_test.h"
+#include "selftests/mock_drm.h"
+
+static int request_sync(struct i915_request *rq)
+{
+	long timeout;
+	int err = 0;
+
+	i915_request_get(rq);
+
+	i915_request_add(rq);
+	timeout = i915_request_wait(rq, 0, HZ / 10);
+	if (timeout < 0)
+		err = timeout;
+	else
+		i915_request_retire_upto(rq);
+
+	i915_request_put(rq);
+
+	return err;
+}
+
+static int context_sync(struct intel_context *ce)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err = 0;
+
+	do {
+		struct i915_request *rq;
+		long timeout;
+
+		rcu_read_lock();
+		rq = rcu_dereference(tl->last_request.request);
+		if (rq)
+			rq = i915_request_get_rcu(rq);
+		rcu_read_unlock();
+		if (!rq)
+			break;
+
+		timeout = i915_request_wait(rq, 0, HZ / 10);
+		if (timeout < 0)
+			err = timeout;
+		else
+			i915_request_retire_upto(rq);
+
+		i915_request_put(rq);
+	} while (!err);
+
+	return err;
+}
+
+static int __live_active_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *ce;
+	int pass;
+	int err;
+
+	/*
+	 * We keep active contexts alive until after a subsequent context
+	 * switch as the final write from the context-save will be after
+	 * we retire the final request. We track when we unpin the context,
+	 * under the presumption that the final pin is from the last request,
+	 * and instead of immediately unpinning the context, we add a task
+	 * to unpin the context from the next idle-barrier.
+	 *
+	 * This test makes sure that the context is kept alive until a
+	 * subsequent idle-barrier (emitted when the engine wakeref hits 0
+	 * with no more outstanding requests).
+	 */
+
+	if (intel_engine_pm_is_awake(engine)) {
+		pr_err("%s is awake before starting %s!\n",
+		       engine->name, __func__);
+		return -EINVAL;
+	}
+
+	ce = intel_context_create(fixme, engine);
+	if (!ce)
+		return -ENOMEM;
+
+	for (pass = 0; pass <= 2; pass++) {
+		struct i915_request *rq;
+
+		rq = intel_context_create_request(ce);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			goto err;
+		}
+
+		err = request_sync(rq);
+		if (err)
+			goto err;
+
+		/* Context will be kept active until after an idle-barrier. */
+		if (i915_active_is_idle(&ce->active)) {
+			pr_err("context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			goto err;
+		}
+
+		if (!intel_engine_pm_is_awake(engine)) {
+			pr_err("%s is asleep before idle-barrier\n",
+			       engine->name);
+			err = -EINVAL;
+			goto err;
+		}
+	}
+
+	/* Now make sure our idle-barriers are flushed */
+	err = context_sync(engine->kernel_context);
+	if (err)
+		goto err;
+
+	if (!i915_active_is_idle(&ce->active)) {
+		pr_err("context is still active!");
+		err = -EINVAL;
+	}
+
+	if (intel_engine_pm_is_awake(engine)) {
+		struct drm_printer p = drm_debug_printer(__func__);
+
+		intel_engine_dump(engine, &p,
+				  "%s is still awake after idle-barriers\n",
+				  engine->name);
+		GEM_TRACE_DUMP();
+
+		err = -EINVAL;
+		goto err;
+	}
+
+err:
+	intel_context_put(ce);
+	return err;
+}
+
+static int live_active_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_active_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+static int __remote_sync(struct intel_context *ce, struct intel_context *remote)
+{
+	struct i915_request *rq;
+	int err;
+
+	err = intel_context_pin(remote);
+	if (err)
+		return err;
+
+	rq = intel_context_create_request(ce);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto unpin;
+	}
+
+	err = intel_context_prepare_remote_request(remote, rq);
+	if (err) {
+		i915_request_add(rq);
+		goto unpin;
+	}
+
+	err = request_sync(rq);
+
+unpin:
+	intel_context_unpin(remote);
+	return err;
+}
+
+static int __live_remote_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *local, *remote;
+	int pass;
+	int err;
+
+	/*
+	 * Check that our idle barriers do not interfere with normal
+	 * activity tracking. In particular, check that operating
+	 * on the context image remotely (intel_context_prepare_remote_request),
+	 * which inserts foreign fences into intel_context.active, does not
+	 * clobber the idle-barrier.
+	 */
+
+	remote = intel_context_create(fixme, engine);
+	if (!remote)
+		return -ENOMEM;
+
+	local = intel_context_create(fixme, engine);
+	if (!local) {
+		err = -ENOMEM;
+		goto err_remote;
+	}
+
+	for (pass = 0; pass <= 2; pass++) {
+		err = __remote_sync(local, remote);
+		if (err)
+			break;
+
+		err = __remote_sync(engine->kernel_context, remote);
+		if (err)
+			break;
+
+		if (i915_active_is_idle(&remote->active)) {
+			pr_err("remote context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+	intel_context_put(local);
+err_remote:
+	intel_context_put(remote);
+	return err;
+}
+
+static int live_remote_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_remote_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+int intel_context_live_selftests(struct drm_i915_private *i915)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(live_active_context),
+		SUBTEST(live_remote_context),
+	};
+	struct intel_gt *gt = &i915->gt;
+
+	if (intel_gt_is_wedged(gt))
+		return 0;
+
+	return intel_gt_live_subtests(tests, gt);
+}
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index d32db8a4db5c..3a8127face79 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -33,6 +33,38 @@ struct active_node {
 	u64 timeline;
 };
 
+static inline struct active_node *
+node_from_active(struct i915_active_request *active)
+{
+	return container_of(active, struct active_node, base);
+}
+
+#define take_preallocated_barriers(x) llist_del_all(&(x)->preallocated_barriers)
+
+static inline bool is_barrier(const struct i915_active_request *active)
+{
+	return IS_ERR(rcu_access_pointer(active->request));
+}
+
+static inline struct llist_node *barrier_to_ll(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct llist_node *)&node->base.link;
+}
+
+static inline struct intel_engine_cs *
+barrier_to_engine(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct intel_engine_cs *)node->base.link.prev;
+}
+
+static inline struct active_node *barrier_from_ll(struct llist_node *x)
+{
+	return container_of((struct list_head *)x,
+			    struct active_node, base.link);
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && IS_ENABLED(CONFIG_DEBUG_OBJECTS)
 
 static void *active_debug_hint(void *addr)
@@ -127,7 +159,7 @@ active_retire(struct i915_active *ref)
 static void
 node_retire(struct i915_active_request *base, struct i915_request *rq)
 {
-	active_retire(container_of(base, struct active_node, base)->ref);
+	active_retire(node_from_active(base)->ref);
 }
 
 static struct i915_active_request *
@@ -184,6 +216,7 @@ active_instance(struct i915_active *ref, u64 idx)
 	ref->cache = node;
 	mutex_unlock(&ref->mutex);
 
+	BUILD_BUG_ON(offsetof(typeof(*node), base));
 	return &node->base;
 }
 
@@ -201,11 +234,52 @@ void __i915_active_init(struct drm_i915_private *i915,
 	ref->retire = retire;
 	ref->tree = RB_ROOT;
 	ref->cache = NULL;
-	init_llist_head(&ref->barriers);
+	init_llist_head(&ref->preallocated_barriers);
 	atomic_set(&ref->count, 0);
 	__mutex_init(&ref->mutex, "i915_active", key);
 }
 
+static bool __active_del_barrier(struct i915_active *ref,
+				 struct active_node *node)
+{
+	struct intel_engine_cs *engine = barrier_to_engine(node);
+	struct llist_node *head = NULL, *tail = NULL;
+	struct llist_node *pos, *next;
+
+	GEM_BUG_ON(node->timeline != engine->kernel_context->ring->timeline->fence_context);
+
+	/*
+	 * Rebuild the llist excluding our node. We may perform this
+	 * outside of the kernel_context timeline mutex and so someone
+	 * else may be manipulating the engine->barrier_tasks, in
+	 * which case either we or they will be upset :)
+	 *
+	 * A second __active_del_barrier() will report failure to claim
+	 * the active_node and the caller will just shrug and know not to
+	 * claim ownership of its node.
+	 *
+	 * A concurrent i915_request_add_active_barriers() will miss adding
+	 * any of the tasks, but we will try again on the next -- and since
+	 * we are actively using the barrier, we know that there will be
+	 * at least another opportunity when we idle.
+	 */
+	llist_for_each_safe(pos, next, llist_del_all(&engine->barrier_tasks)) {
+		if (node == barrier_from_ll(pos)) {
+			node = NULL;
+			continue;
+		}
+
+		pos->next = head;
+		head = pos;
+		if (!tail)
+			tail = pos;
+	}
+	if (head)
+		llist_add_batch(head, tail, &engine->barrier_tasks);
+
+	return !node;
+}
+
 int i915_active_ref(struct i915_active *ref,
 		    u64 timeline,
 		    struct i915_request *rq)
@@ -224,8 +298,20 @@ int i915_active_ref(struct i915_active *ref,
 		goto out;
 	}
 
-	if (!i915_active_request_isset(active))
-		atomic_inc(&ref->count);
+	if (is_barrier(active)) { /* proto-node used by our idle barrier */
+		/*
+		 * This request is on the kernel_context timeline, and so
+		 * we can use it to substitute for the pending idle-barrer
+		 * request that we want to emit on the kernel_context.
+		 */
+		__active_del_barrier(ref, node_from_active(active));
+		RCU_INIT_POINTER(active->request, NULL);
+		INIT_LIST_HEAD(&active->link);
+	} else {
+		if (!i915_active_request_isset(active))
+			atomic_inc(&ref->count);
+	}
+	GEM_BUG_ON(!atomic_read(&ref->count));
 	__i915_active_request_set(active, rq);
 
 out:
@@ -312,6 +398,11 @@ int i915_active_wait(struct i915_active *ref)
 	}
 
 	rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
+		if (is_barrier(&it->base)) { /* unconnected idle-barrier */
+			err = -EBUSY;
+			break;
+		}
+
 		err = i915_active_request_retire(&it->base, BKL(ref));
 		if (err)
 			break;
@@ -374,6 +465,92 @@ void i915_active_fini(struct i915_active *ref)
 }
 #endif
 
+static inline bool is_idle_barrier(struct active_node *node, u64 idx)
+{
+	return node->timeline == idx && !i915_active_request_isset(&node->base);
+}
+
+static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
+{
+	struct rb_node *prev, *p;
+
+	if (RB_EMPTY_ROOT(&ref->tree))
+		return NULL;
+
+	mutex_lock(&ref->mutex);
+	GEM_BUG_ON(i915_active_is_idle(ref));
+
+	/*
+	 * Try to reuse any existing barrier nodes already allocated for this
+	 * i915_active, due to overlapping active phases there is likely a
+	 * node kept alive (as we reuse before parking). We prefer to reuse
+	 * completely idle barriers (less hassle in manipulating the llists),
+	 * but otherwise any will do.
+	 */
+	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
+		p = &ref->cache->node;
+		goto match;
+	}
+
+	prev = NULL;
+	p = ref->tree.rb_node;
+	while (p) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (is_idle_barrier(node, idx))
+			goto match;
+
+		prev = p;
+		if (node->timeline < idx)
+			p = p->rb_right;
+		else
+			p = p->rb_left;
+	}
+
+	/*
+	 * No quick match, but we did find the leftmost rb_node for the
+	 * kernel_context. Walk the rb_tree in-order to see if there were
+	 * any idle-barriers on this timeline that we missed, or just use
+	 * the first pending barrier.
+	 */
+	for (p = prev; p; p = rb_next(p)) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (node->timeline > idx)
+			break;
+
+		if (node->timeline < idx)
+			continue;
+
+		if (is_idle_barrier(node, idx))
+			goto match;
+
+		/*
+		 * The list of pending barriers is protected by the
+		 * kernel_context timeline, which notably we do not hold
+		 * here. i915_request_add_active_barriers() may consume
+		 * the barrier before we claim it, so we have to check
+		 * for success.
+		 */
+		if (is_barrier(&node->base) && __active_del_barrier(ref, node))
+			goto match;
+	}
+
+	mutex_unlock(&ref->mutex);
+
+	return NULL;
+
+match:
+	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */
+	if (p == &ref->cache->node)
+		ref->cache = NULL;
+	mutex_unlock(&ref->mutex);
+
+	return rb_entry(p, struct active_node, node);
+}
+
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine)
 {
@@ -382,39 +559,61 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	struct llist_node *pos, *next;
 	int err;
 
-	GEM_BUG_ON(!mask);
+	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
+
+	/*
+	 * Preallocate a node for each physical engine supporting the target
+	 * engine (remember virtual engines have more than one sibling).
+	 * We can then use the preallocated nodes in
+	 * i915_active_acquire_barrier()
+	 */
 	for_each_engine_masked(engine, i915, mask, tmp) {
-		struct intel_context *kctx = engine->kernel_context;
+		u64 idx = engine->kernel_context->ring->timeline->fence_context;
 		struct active_node *node;
 
-		node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
-		if (unlikely(!node)) {
-			err = -ENOMEM;
-			goto unwind;
+		node = reuse_idle_barrier(ref, idx);
+		if (!node) {
+			node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
+			if (!node) {
+				err = ENOMEM;
+				goto unwind;
+			}
+
+			RCU_INIT_POINTER(node->base.request, NULL);
+			node->base.retire = node_retire;
+			node->timeline = idx;
+			node->ref = ref;
 		}
 
-		i915_active_request_init(&node->base,
-					 (void *)engine, node_retire);
-		node->timeline = kctx->ring->timeline->fence_context;
-		node->ref = ref;
-		atomic_inc(&ref->count);
+		if (!i915_active_request_isset(&node->base)) {
+			/*
+			 * Mark this as being *our* unconnected proto-node.
+			 *
+			 * Since this node is not in any list, and we have
+			 * decoupled it from the rbtree, we can reuse the
+			 * request to indicate this is an idle-barrier node
+			 * and then we can use the rb_node and list pointers
+			 * for our tracking of the pending barrier.
+			 */
+			RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
+			node->base.link.prev = (void *)engine;
+			atomic_inc(&ref->count);
+		}
 
+		GEM_BUG_ON(barrier_to_engine(node) != engine);
+		llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
 		intel_engine_pm_get(engine);
-		llist_add((struct llist_node *)&node->base.link,
-			  &ref->barriers);
 	}
 
 	return 0;
 
 unwind:
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct active_node *node;
+	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
 
-		node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-		engine = (void *)rcu_access_pointer(node->base.request);
+		atomic_dec(&ref->count);
+		intel_engine_pm_put(barrier_to_engine(node));
 
-		intel_engine_pm_put(engine);
 		kmem_cache_free(global.slab_cache, node);
 	}
 	return err;
@@ -426,25 +625,27 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 
 	GEM_BUG_ON(i915_active_is_idle(ref));
 
+	/*
+	 * Transfer the list of preallocated barriers into the
+	 * i915_active rbtree, but only as proto-nodes. They will be
+	 * populated by i915_request_add_active_barrier() to point to the
+	 * request that will eventually release them.
+	 */
 	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct intel_engine_cs *engine;
-		struct active_node *node;
+	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
+		struct intel_engine_cs *engine = barrier_to_engine(node);
 		struct rb_node **p, *parent;
 
-		node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-
-		engine = (void *)rcu_access_pointer(node->base.request);
-		RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
-
 		parent = NULL;
 		p = &ref->tree.rb_node;
 		while (*p) {
+			struct active_node *it;
+
 			parent = *p;
-			if (rb_entry(parent,
-				     struct active_node,
-				     node)->timeline < node->timeline)
+
+			it = rb_entry(parent, struct active_node, node);
+			if (it->timeline < node->timeline)
 				p = &parent->rb_right;
 			else
 				p = &parent->rb_left;
@@ -452,20 +653,29 @@ void i915_active_acquire_barrier(struct i915_active *ref)
 		rb_link_node(&node->node, parent, p);
 		rb_insert_color(&node->node, &ref->tree);
 
-		llist_add((struct llist_node *)&node->base.link,
-			  &engine->barrier_tasks);
+		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
 		intel_engine_pm_put(engine);
 	}
 	mutex_unlock(&ref->mutex);
 }
 
-void i915_request_add_barriers(struct i915_request *rq)
+void i915_request_add_active_barriers(struct i915_request *rq)
 {
 	struct intel_engine_cs *engine = rq->engine;
 	struct llist_node *node, *next;
 
-	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks))
+	GEM_BUG_ON(intel_engine_is_virtual(engine));
+	GEM_BUG_ON(rq->timeline != engine->kernel_context->ring->timeline);
+
+	/*
+	 * Attach the list of proto-fences to the in-flight request such
+	 * that the parent i915_active will be released when this request
+	 * is retired.
+	 */
+	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
+		RCU_INIT_POINTER(barrier_from_ll(node)->base.request, rq);
 		list_add_tail((struct list_head *)node, &rq->active_list);
+	}
 }
 
 int i915_active_request_set(struct i915_active_request *active,
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index ba68b077ec6c..566336c99ed7 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -413,6 +413,6 @@ static inline void i915_active_fini(struct i915_active *ref) { }
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 					    struct intel_engine_cs *engine);
 void i915_active_acquire_barrier(struct i915_active *ref);
-void i915_request_add_barriers(struct i915_request *rq);
+void i915_request_add_active_barriers(struct i915_request *rq);
 
 #endif /* _I915_ACTIVE_H_ */
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index 74743dd0d5f0..ae3ee441c114 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -42,7 +42,7 @@ struct i915_active {
 	int (*active)(struct i915_active *ref);
 	void (*retire)(struct i915_active *ref);
 
-	struct llist_head barriers;
+	struct llist_head preallocated_barriers;
 };
 
 #endif /* _I915_ACTIVE_TYPES_H_ */
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 2b31a4ee0b4c..a841d3f9bedc 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -15,6 +15,7 @@ selftest(workarounds, intel_workarounds_live_selftests)
 selftest(timelines, intel_timeline_live_selftests)
 selftest(requests, i915_request_live_selftests)
 selftest(active, i915_active_live_selftests)
+selftest(gt_contexts, intel_context_live_selftests)
 selftest(objects, i915_gem_object_live_selftests)
 selftest(mman, i915_gem_mman_live_selftests)
 selftest(dmabuf, i915_gem_dmabuf_live_selftests)
@@ -24,7 +25,7 @@ selftest(gtt, i915_gem_gtt_live_selftests)
 selftest(gem, i915_gem_live_selftests)
 selftest(evict, i915_gem_evict_live_selftests)
 selftest(hugepages, i915_gem_huge_page_live_selftests)
-selftest(contexts, i915_gem_context_live_selftests)
+selftest(gem_contexts, i915_gem_context_live_selftests)
 selftest(blt, i915_gem_object_blt_live_selftests)
 selftest(client, i915_gem_client_blt_live_selftests)
 selftest(reset, intel_reset_live_selftests)
-- 
2.23.0.rc0

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

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

* Re: [PATCH v3] drm/i915: Allow sharing the idle-barrier from other kernel requests
  2019-08-02 10:00     ` [PATCH v3] " Chris Wilson
@ 2019-08-02 10:11       ` Tvrtko Ursulin
  0 siblings, 0 replies; 21+ messages in thread
From: Tvrtko Ursulin @ 2019-08-02 10:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 02/08/2019 11:00, Chris Wilson wrote:
> By placing our idle-barriers in the i915_active fence tree, we expose
> those for reuse by other components that are issuing requests along the
> kernel_context. Reusing the proto-barrier active_node is perfectly fine
> as the new request implies a context-switch, and so an opportune point
> to run the idle-barrier. However, the proto-barrier is not equivalent
> to a normal active_node and care must be taken to avoid dereferencing the
> ERR_PTR used as its request marker.
> 
> v2: Comment the more egregious cheek
> v3: A glossary!
> 
> Reported-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
> Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_context.c       |  40 ++-
>   drivers/gpu/drm/i915/gt/intel_context.h       |  13 +-
>   drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   2 +-
>   drivers/gpu/drm/i915/gt/selftest_context.c    | 310 ++++++++++++++++++
>   drivers/gpu/drm/i915/i915_active.c            | 288 +++++++++++++---
>   drivers/gpu/drm/i915/i915_active.h            |   2 +-
>   drivers/gpu/drm/i915/i915_active_types.h      |   2 +-
>   .../drm/i915/selftests/i915_live_selftests.h  |   3 +-
>   8 files changed, 597 insertions(+), 63 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gt/selftest_context.c
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index f30441a140f8..34c8e37a73b8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -162,23 +162,41 @@ static int __intel_context_active(struct i915_active *active)
>   	if (err)
>   		goto err_ring;
>   
> +	return 0;
> +
> +err_ring:
> +	intel_ring_unpin(ce->ring);
> +err_put:
> +	intel_context_put(ce);
> +	return err;
> +}
> +
> +int intel_context_active_acquire(struct intel_context *ce)
> +{
> +	int err;
> +
> +	err = i915_active_acquire(&ce->active);
> +	if (err)
> +		return err;
> +
>   	/* Preallocate tracking nodes */
>   	if (!i915_gem_context_is_kernel(ce->gem_context)) {
>   		err = i915_active_acquire_preallocate_barrier(&ce->active,
>   							      ce->engine);
> -		if (err)
> -			goto err_state;
> +		if (err) {
> +			i915_active_release(&ce->active);
> +			return err;
> +		}
>   	}
>   
>   	return 0;
> +}
>   
> -err_state:
> -	__context_unpin_state(ce->state);
> -err_ring:
> -	intel_ring_unpin(ce->ring);
> -err_put:
> -	intel_context_put(ce);
> -	return err;
> +void intel_context_active_release(struct intel_context *ce)
> +{
> +	/* Nodes preallocated in intel_context_active() */
> +	i915_active_acquire_barrier(&ce->active);
> +	i915_active_release(&ce->active);
>   }
>   
>   void
> @@ -301,3 +319,7 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
>   
>   	return rq;
>   }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_context.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> index 23c7e4c0ce7c..07f9924de48f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -104,17 +104,8 @@ static inline void intel_context_exit(struct intel_context *ce)
>   		ce->ops->exit(ce);
>   }
>   
> -static inline int intel_context_active_acquire(struct intel_context *ce)
> -{
> -	return i915_active_acquire(&ce->active);
> -}
> -
> -static inline void intel_context_active_release(struct intel_context *ce)
> -{
> -	/* Nodes preallocated in intel_context_active() */
> -	i915_active_acquire_barrier(&ce->active);
> -	i915_active_release(&ce->active);
> -}
> +int intel_context_active_acquire(struct intel_context *ce);
> +void intel_context_active_release(struct intel_context *ce);
>   
>   static inline struct intel_context *intel_context_get(struct intel_context *ce)
>   {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index e74fbf04a68d..ce54092475da 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -90,7 +90,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
>   	/* Check again on the next retirement. */
>   	engine->wakeref_serial = engine->serial + 1;
>   
> -	i915_request_add_barriers(rq);
> +	i915_request_add_active_barriers(rq);
>   	__i915_request_commit(rq);
>   
>   	return false;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> new file mode 100644
> index 000000000000..d39b5594cb02
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -0,0 +1,310 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#include "i915_selftest.h"
> +#include "intel_gt.h"
> +
> +#include "gem/selftests/mock_context.h"
> +#include "selftests/igt_flush_test.h"
> +#include "selftests/mock_drm.h"
> +
> +static int request_sync(struct i915_request *rq)
> +{
> +	long timeout;
> +	int err = 0;
> +
> +	i915_request_get(rq);
> +
> +	i915_request_add(rq);
> +	timeout = i915_request_wait(rq, 0, HZ / 10);
> +	if (timeout < 0)
> +		err = timeout;
> +	else
> +		i915_request_retire_upto(rq);
> +
> +	i915_request_put(rq);
> +
> +	return err;
> +}
> +
> +static int context_sync(struct intel_context *ce)
> +{
> +	struct intel_timeline *tl = ce->ring->timeline;
> +	int err = 0;
> +
> +	do {
> +		struct i915_request *rq;
> +		long timeout;
> +
> +		rcu_read_lock();
> +		rq = rcu_dereference(tl->last_request.request);
> +		if (rq)
> +			rq = i915_request_get_rcu(rq);
> +		rcu_read_unlock();
> +		if (!rq)
> +			break;
> +
> +		timeout = i915_request_wait(rq, 0, HZ / 10);
> +		if (timeout < 0)
> +			err = timeout;
> +		else
> +			i915_request_retire_upto(rq);
> +
> +		i915_request_put(rq);
> +	} while (!err);
> +
> +	return err;
> +}
> +
> +static int __live_active_context(struct intel_engine_cs *engine,
> +				 struct i915_gem_context *fixme)
> +{
> +	struct intel_context *ce;
> +	int pass;
> +	int err;
> +
> +	/*
> +	 * We keep active contexts alive until after a subsequent context
> +	 * switch as the final write from the context-save will be after
> +	 * we retire the final request. We track when we unpin the context,
> +	 * under the presumption that the final pin is from the last request,
> +	 * and instead of immediately unpinning the context, we add a task
> +	 * to unpin the context from the next idle-barrier.
> +	 *
> +	 * This test makes sure that the context is kept alive until a
> +	 * subsequent idle-barrier (emitted when the engine wakeref hits 0
> +	 * with no more outstanding requests).
> +	 */
> +
> +	if (intel_engine_pm_is_awake(engine)) {
> +		pr_err("%s is awake before starting %s!\n",
> +		       engine->name, __func__);
> +		return -EINVAL;
> +	}
> +
> +	ce = intel_context_create(fixme, engine);
> +	if (!ce)
> +		return -ENOMEM;
> +
> +	for (pass = 0; pass <= 2; pass++) {
> +		struct i915_request *rq;
> +
> +		rq = intel_context_create_request(ce);
> +		if (IS_ERR(rq)) {
> +			err = PTR_ERR(rq);
> +			goto err;
> +		}
> +
> +		err = request_sync(rq);
> +		if (err)
> +			goto err;
> +
> +		/* Context will be kept active until after an idle-barrier. */
> +		if (i915_active_is_idle(&ce->active)) {
> +			pr_err("context is not active; expected idle-barrier (%s pass %d)\n",
> +			       engine->name, pass);
> +			err = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (!intel_engine_pm_is_awake(engine)) {
> +			pr_err("%s is asleep before idle-barrier\n",
> +			       engine->name);
> +			err = -EINVAL;
> +			goto err;
> +		}
> +	}
> +
> +	/* Now make sure our idle-barriers are flushed */
> +	err = context_sync(engine->kernel_context);
> +	if (err)
> +		goto err;
> +
> +	if (!i915_active_is_idle(&ce->active)) {
> +		pr_err("context is still active!");
> +		err = -EINVAL;
> +	}
> +
> +	if (intel_engine_pm_is_awake(engine)) {
> +		struct drm_printer p = drm_debug_printer(__func__);
> +
> +		intel_engine_dump(engine, &p,
> +				  "%s is still awake after idle-barriers\n",
> +				  engine->name);
> +		GEM_TRACE_DUMP();
> +
> +		err = -EINVAL;
> +		goto err;
> +	}
> +
> +err:
> +	intel_context_put(ce);
> +	return err;
> +}
> +
> +static int live_active_context(void *arg)
> +{
> +	struct intel_gt *gt = arg;
> +	struct intel_engine_cs *engine;
> +	struct i915_gem_context *fixme;
> +	enum intel_engine_id id;
> +	struct drm_file *file;
> +	int err = 0;
> +
> +	file = mock_file(gt->i915);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	mutex_lock(&gt->i915->drm.struct_mutex);
> +
> +	fixme = live_context(gt->i915, file);
> +	if (!fixme) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	for_each_engine(engine, gt->i915, id) {
> +		err = __live_active_context(engine, fixme);
> +		if (err)
> +			break;
> +
> +		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
> +		if (err)
> +			break;
> +	}
> +
> +unlock:
> +	mutex_unlock(&gt->i915->drm.struct_mutex);
> +	mock_file_free(gt->i915, file);
> +	return err;
> +}
> +
> +static int __remote_sync(struct intel_context *ce, struct intel_context *remote)
> +{
> +	struct i915_request *rq;
> +	int err;
> +
> +	err = intel_context_pin(remote);
> +	if (err)
> +		return err;
> +
> +	rq = intel_context_create_request(ce);
> +	if (IS_ERR(rq)) {
> +		err = PTR_ERR(rq);
> +		goto unpin;
> +	}
> +
> +	err = intel_context_prepare_remote_request(remote, rq);
> +	if (err) {
> +		i915_request_add(rq);
> +		goto unpin;
> +	}
> +
> +	err = request_sync(rq);
> +
> +unpin:
> +	intel_context_unpin(remote);
> +	return err;
> +}
> +
> +static int __live_remote_context(struct intel_engine_cs *engine,
> +				 struct i915_gem_context *fixme)
> +{
> +	struct intel_context *local, *remote;
> +	int pass;
> +	int err;
> +
> +	/*
> +	 * Check that our idle barriers do not interfere with normal
> +	 * activity tracking. In particular, check that operating
> +	 * on the context image remotely (intel_context_prepare_remote_request),
> +	 * which inserts foreign fences into intel_context.active, does not
> +	 * clobber the idle-barrier.
> +	 */
> +
> +	remote = intel_context_create(fixme, engine);
> +	if (!remote)
> +		return -ENOMEM;
> +
> +	local = intel_context_create(fixme, engine);
> +	if (!local) {
> +		err = -ENOMEM;
> +		goto err_remote;
> +	}
> +
> +	for (pass = 0; pass <= 2; pass++) {
> +		err = __remote_sync(local, remote);
> +		if (err)
> +			break;
> +
> +		err = __remote_sync(engine->kernel_context, remote);
> +		if (err)
> +			break;
> +
> +		if (i915_active_is_idle(&remote->active)) {
> +			pr_err("remote context is not active; expected idle-barrier (%s pass %d)\n",
> +			       engine->name, pass);
> +			err = -EINVAL;
> +			break;
> +		}
> +	}
> +
> +	intel_context_put(local);
> +err_remote:
> +	intel_context_put(remote);
> +	return err;
> +}
> +
> +static int live_remote_context(void *arg)
> +{
> +	struct intel_gt *gt = arg;
> +	struct intel_engine_cs *engine;
> +	struct i915_gem_context *fixme;
> +	enum intel_engine_id id;
> +	struct drm_file *file;
> +	int err = 0;
> +
> +	file = mock_file(gt->i915);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	mutex_lock(&gt->i915->drm.struct_mutex);
> +
> +	fixme = live_context(gt->i915, file);
> +	if (!fixme) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	for_each_engine(engine, gt->i915, id) {
> +		err = __live_remote_context(engine, fixme);
> +		if (err)
> +			break;
> +
> +		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
> +		if (err)
> +			break;
> +	}
> +
> +unlock:
> +	mutex_unlock(&gt->i915->drm.struct_mutex);
> +	mock_file_free(gt->i915, file);
> +	return err;
> +}
> +
> +int intel_context_live_selftests(struct drm_i915_private *i915)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(live_active_context),
> +		SUBTEST(live_remote_context),
> +	};
> +	struct intel_gt *gt = &i915->gt;
> +
> +	if (intel_gt_is_wedged(gt))
> +		return 0;
> +
> +	return intel_gt_live_subtests(tests, gt);
> +}
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index d32db8a4db5c..3a8127face79 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -33,6 +33,38 @@ struct active_node {
>   	u64 timeline;
>   };
>   
> +static inline struct active_node *
> +node_from_active(struct i915_active_request *active)
> +{
> +	return container_of(active, struct active_node, base);
> +}
> +
> +#define take_preallocated_barriers(x) llist_del_all(&(x)->preallocated_barriers)
> +
> +static inline bool is_barrier(const struct i915_active_request *active)
> +{
> +	return IS_ERR(rcu_access_pointer(active->request));
> +}
> +
> +static inline struct llist_node *barrier_to_ll(struct active_node *node)
> +{
> +	GEM_BUG_ON(!is_barrier(&node->base));
> +	return (struct llist_node *)&node->base.link;
> +}
> +
> +static inline struct intel_engine_cs *
> +barrier_to_engine(struct active_node *node)
> +{
> +	GEM_BUG_ON(!is_barrier(&node->base));
> +	return (struct intel_engine_cs *)node->base.link.prev;
> +}
> +
> +static inline struct active_node *barrier_from_ll(struct llist_node *x)
> +{
> +	return container_of((struct list_head *)x,
> +			    struct active_node, base.link);
> +}
> +
>   #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && IS_ENABLED(CONFIG_DEBUG_OBJECTS)
>   
>   static void *active_debug_hint(void *addr)
> @@ -127,7 +159,7 @@ active_retire(struct i915_active *ref)
>   static void
>   node_retire(struct i915_active_request *base, struct i915_request *rq)
>   {
> -	active_retire(container_of(base, struct active_node, base)->ref);
> +	active_retire(node_from_active(base)->ref);
>   }
>   
>   static struct i915_active_request *
> @@ -184,6 +216,7 @@ active_instance(struct i915_active *ref, u64 idx)
>   	ref->cache = node;
>   	mutex_unlock(&ref->mutex);
>   
> +	BUILD_BUG_ON(offsetof(typeof(*node), base));
>   	return &node->base;
>   }
>   
> @@ -201,11 +234,52 @@ void __i915_active_init(struct drm_i915_private *i915,
>   	ref->retire = retire;
>   	ref->tree = RB_ROOT;
>   	ref->cache = NULL;
> -	init_llist_head(&ref->barriers);
> +	init_llist_head(&ref->preallocated_barriers);
>   	atomic_set(&ref->count, 0);
>   	__mutex_init(&ref->mutex, "i915_active", key);
>   }
>   
> +static bool __active_del_barrier(struct i915_active *ref,
> +				 struct active_node *node)
> +{
> +	struct intel_engine_cs *engine = barrier_to_engine(node);
> +	struct llist_node *head = NULL, *tail = NULL;
> +	struct llist_node *pos, *next;
> +
> +	GEM_BUG_ON(node->timeline != engine->kernel_context->ring->timeline->fence_context);
> +
> +	/*
> +	 * Rebuild the llist excluding our node. We may perform this
> +	 * outside of the kernel_context timeline mutex and so someone
> +	 * else may be manipulating the engine->barrier_tasks, in
> +	 * which case either we or they will be upset :)
> +	 *
> +	 * A second __active_del_barrier() will report failure to claim
> +	 * the active_node and the caller will just shrug and know not to
> +	 * claim ownership of its node.
> +	 *
> +	 * A concurrent i915_request_add_active_barriers() will miss adding
> +	 * any of the tasks, but we will try again on the next -- and since
> +	 * we are actively using the barrier, we know that there will be
> +	 * at least another opportunity when we idle.
> +	 */
> +	llist_for_each_safe(pos, next, llist_del_all(&engine->barrier_tasks)) {
> +		if (node == barrier_from_ll(pos)) {
> +			node = NULL;
> +			continue;
> +		}
> +
> +		pos->next = head;
> +		head = pos;
> +		if (!tail)
> +			tail = pos;
> +	}
> +	if (head)
> +		llist_add_batch(head, tail, &engine->barrier_tasks);
> +
> +	return !node;
> +}
> +
>   int i915_active_ref(struct i915_active *ref,
>   		    u64 timeline,
>   		    struct i915_request *rq)
> @@ -224,8 +298,20 @@ int i915_active_ref(struct i915_active *ref,
>   		goto out;
>   	}
>   
> -	if (!i915_active_request_isset(active))
> -		atomic_inc(&ref->count);
> +	if (is_barrier(active)) { /* proto-node used by our idle barrier */
> +		/*
> +		 * This request is on the kernel_context timeline, and so
> +		 * we can use it to substitute for the pending idle-barrer
> +		 * request that we want to emit on the kernel_context.
> +		 */
> +		__active_del_barrier(ref, node_from_active(active));
> +		RCU_INIT_POINTER(active->request, NULL);
> +		INIT_LIST_HEAD(&active->link);
> +	} else {
> +		if (!i915_active_request_isset(active))
> +			atomic_inc(&ref->count);
> +	}
> +	GEM_BUG_ON(!atomic_read(&ref->count));
>   	__i915_active_request_set(active, rq);
>   
>   out:
> @@ -312,6 +398,11 @@ int i915_active_wait(struct i915_active *ref)
>   	}
>   
>   	rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
> +		if (is_barrier(&it->base)) { /* unconnected idle-barrier */
> +			err = -EBUSY;
> +			break;
> +		}
> +
>   		err = i915_active_request_retire(&it->base, BKL(ref));
>   		if (err)
>   			break;
> @@ -374,6 +465,92 @@ void i915_active_fini(struct i915_active *ref)
>   }
>   #endif
>   
> +static inline bool is_idle_barrier(struct active_node *node, u64 idx)
> +{
> +	return node->timeline == idx && !i915_active_request_isset(&node->base);
> +}
> +
> +static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
> +{
> +	struct rb_node *prev, *p;
> +
> +	if (RB_EMPTY_ROOT(&ref->tree))
> +		return NULL;
> +
> +	mutex_lock(&ref->mutex);
> +	GEM_BUG_ON(i915_active_is_idle(ref));
> +
> +	/*
> +	 * Try to reuse any existing barrier nodes already allocated for this
> +	 * i915_active, due to overlapping active phases there is likely a
> +	 * node kept alive (as we reuse before parking). We prefer to reuse
> +	 * completely idle barriers (less hassle in manipulating the llists),
> +	 * but otherwise any will do.
> +	 */
> +	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
> +		p = &ref->cache->node;
> +		goto match;
> +	}
> +
> +	prev = NULL;
> +	p = ref->tree.rb_node;
> +	while (p) {
> +		struct active_node *node =
> +			rb_entry(p, struct active_node, node);
> +
> +		if (is_idle_barrier(node, idx))
> +			goto match;
> +
> +		prev = p;
> +		if (node->timeline < idx)
> +			p = p->rb_right;
> +		else
> +			p = p->rb_left;
> +	}
> +
> +	/*
> +	 * No quick match, but we did find the leftmost rb_node for the
> +	 * kernel_context. Walk the rb_tree in-order to see if there were
> +	 * any idle-barriers on this timeline that we missed, or just use
> +	 * the first pending barrier.
> +	 */
> +	for (p = prev; p; p = rb_next(p)) {
> +		struct active_node *node =
> +			rb_entry(p, struct active_node, node);
> +
> +		if (node->timeline > idx)
> +			break;
> +
> +		if (node->timeline < idx)
> +			continue;
> +
> +		if (is_idle_barrier(node, idx))
> +			goto match;
> +
> +		/*
> +		 * The list of pending barriers is protected by the
> +		 * kernel_context timeline, which notably we do not hold
> +		 * here. i915_request_add_active_barriers() may consume
> +		 * the barrier before we claim it, so we have to check
> +		 * for success.
> +		 */
> +		if (is_barrier(&node->base) && __active_del_barrier(ref, node))
> +			goto match;
> +	}
> +
> +	mutex_unlock(&ref->mutex);
> +
> +	return NULL;
> +
> +match:
> +	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */
> +	if (p == &ref->cache->node)
> +		ref->cache = NULL;
> +	mutex_unlock(&ref->mutex);
> +
> +	return rb_entry(p, struct active_node, node);
> +}
> +
>   int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   					    struct intel_engine_cs *engine)
>   {
> @@ -382,39 +559,61 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   	struct llist_node *pos, *next;
>   	int err;
>   
> -	GEM_BUG_ON(!mask);
> +	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
> +
> +	/*
> +	 * Preallocate a node for each physical engine supporting the target
> +	 * engine (remember virtual engines have more than one sibling).
> +	 * We can then use the preallocated nodes in
> +	 * i915_active_acquire_barrier()
> +	 */
>   	for_each_engine_masked(engine, i915, mask, tmp) {
> -		struct intel_context *kctx = engine->kernel_context;
> +		u64 idx = engine->kernel_context->ring->timeline->fence_context;
>   		struct active_node *node;
>   
> -		node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
> -		if (unlikely(!node)) {
> -			err = -ENOMEM;
> -			goto unwind;
> +		node = reuse_idle_barrier(ref, idx);
> +		if (!node) {
> +			node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
> +			if (!node) {
> +				err = ENOMEM;
> +				goto unwind;
> +			}
> +
> +			RCU_INIT_POINTER(node->base.request, NULL);
> +			node->base.retire = node_retire;
> +			node->timeline = idx;
> +			node->ref = ref;
>   		}
>   
> -		i915_active_request_init(&node->base,
> -					 (void *)engine, node_retire);
> -		node->timeline = kctx->ring->timeline->fence_context;
> -		node->ref = ref;
> -		atomic_inc(&ref->count);
> +		if (!i915_active_request_isset(&node->base)) {
> +			/*
> +			 * Mark this as being *our* unconnected proto-node.
> +			 *
> +			 * Since this node is not in any list, and we have
> +			 * decoupled it from the rbtree, we can reuse the
> +			 * request to indicate this is an idle-barrier node
> +			 * and then we can use the rb_node and list pointers
> +			 * for our tracking of the pending barrier.
> +			 */
> +			RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
> +			node->base.link.prev = (void *)engine;
> +			atomic_inc(&ref->count);
> +		}
>   
> +		GEM_BUG_ON(barrier_to_engine(node) != engine);
> +		llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
>   		intel_engine_pm_get(engine);
> -		llist_add((struct llist_node *)&node->base.link,
> -			  &ref->barriers);
>   	}
>   
>   	return 0;
>   
>   unwind:
> -	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
> -		struct active_node *node;
> +	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {
> +		struct active_node *node = barrier_from_ll(pos);
>   
> -		node = container_of((struct list_head *)pos,
> -				    typeof(*node), base.link);
> -		engine = (void *)rcu_access_pointer(node->base.request);
> +		atomic_dec(&ref->count);
> +		intel_engine_pm_put(barrier_to_engine(node));
>   
> -		intel_engine_pm_put(engine);
>   		kmem_cache_free(global.slab_cache, node);
>   	}
>   	return err;
> @@ -426,25 +625,27 @@ void i915_active_acquire_barrier(struct i915_active *ref)
>   
>   	GEM_BUG_ON(i915_active_is_idle(ref));
>   
> +	/*
> +	 * Transfer the list of preallocated barriers into the
> +	 * i915_active rbtree, but only as proto-nodes. They will be
> +	 * populated by i915_request_add_active_barrier() to point to the
> +	 * request that will eventually release them.
> +	 */
>   	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
> -	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
> -		struct intel_engine_cs *engine;
> -		struct active_node *node;
> +	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {
> +		struct active_node *node = barrier_from_ll(pos);
> +		struct intel_engine_cs *engine = barrier_to_engine(node);
>   		struct rb_node **p, *parent;
>   
> -		node = container_of((struct list_head *)pos,
> -				    typeof(*node), base.link);
> -
> -		engine = (void *)rcu_access_pointer(node->base.request);
> -		RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
> -
>   		parent = NULL;
>   		p = &ref->tree.rb_node;
>   		while (*p) {
> +			struct active_node *it;
> +
>   			parent = *p;
> -			if (rb_entry(parent,
> -				     struct active_node,
> -				     node)->timeline < node->timeline)
> +
> +			it = rb_entry(parent, struct active_node, node);
> +			if (it->timeline < node->timeline)
>   				p = &parent->rb_right;
>   			else
>   				p = &parent->rb_left;
> @@ -452,20 +653,29 @@ void i915_active_acquire_barrier(struct i915_active *ref)
>   		rb_link_node(&node->node, parent, p);
>   		rb_insert_color(&node->node, &ref->tree);
>   
> -		llist_add((struct llist_node *)&node->base.link,
> -			  &engine->barrier_tasks);
> +		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
>   		intel_engine_pm_put(engine);
>   	}
>   	mutex_unlock(&ref->mutex);
>   }
>   
> -void i915_request_add_barriers(struct i915_request *rq)
> +void i915_request_add_active_barriers(struct i915_request *rq)
>   {
>   	struct intel_engine_cs *engine = rq->engine;
>   	struct llist_node *node, *next;
>   
> -	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks))
> +	GEM_BUG_ON(intel_engine_is_virtual(engine));
> +	GEM_BUG_ON(rq->timeline != engine->kernel_context->ring->timeline);
> +
> +	/*
> +	 * Attach the list of proto-fences to the in-flight request such
> +	 * that the parent i915_active will be released when this request
> +	 * is retired.
> +	 */
> +	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
> +		RCU_INIT_POINTER(barrier_from_ll(node)->base.request, rq);
>   		list_add_tail((struct list_head *)node, &rq->active_list);
> +	}
>   }
>   
>   int i915_active_request_set(struct i915_active_request *active,
> diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
> index ba68b077ec6c..566336c99ed7 100644
> --- a/drivers/gpu/drm/i915/i915_active.h
> +++ b/drivers/gpu/drm/i915/i915_active.h
> @@ -413,6 +413,6 @@ static inline void i915_active_fini(struct i915_active *ref) { }
>   int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>   					    struct intel_engine_cs *engine);
>   void i915_active_acquire_barrier(struct i915_active *ref);
> -void i915_request_add_barriers(struct i915_request *rq);
> +void i915_request_add_active_barriers(struct i915_request *rq);
>   
>   #endif /* _I915_ACTIVE_H_ */
> diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
> index 74743dd0d5f0..ae3ee441c114 100644
> --- a/drivers/gpu/drm/i915/i915_active_types.h
> +++ b/drivers/gpu/drm/i915/i915_active_types.h
> @@ -42,7 +42,7 @@ struct i915_active {
>   	int (*active)(struct i915_active *ref);
>   	void (*retire)(struct i915_active *ref);
>   
> -	struct llist_head barriers;
> +	struct llist_head preallocated_barriers;
>   };
>   
>   #endif /* _I915_ACTIVE_TYPES_H_ */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> index 2b31a4ee0b4c..a841d3f9bedc 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> @@ -15,6 +15,7 @@ selftest(workarounds, intel_workarounds_live_selftests)
>   selftest(timelines, intel_timeline_live_selftests)
>   selftest(requests, i915_request_live_selftests)
>   selftest(active, i915_active_live_selftests)
> +selftest(gt_contexts, intel_context_live_selftests)
>   selftest(objects, i915_gem_object_live_selftests)
>   selftest(mman, i915_gem_mman_live_selftests)
>   selftest(dmabuf, i915_gem_dmabuf_live_selftests)
> @@ -24,7 +25,7 @@ selftest(gtt, i915_gem_gtt_live_selftests)
>   selftest(gem, i915_gem_live_selftests)
>   selftest(evict, i915_gem_evict_live_selftests)
>   selftest(hugepages, i915_gem_huge_page_live_selftests)
> -selftest(contexts, i915_gem_context_live_selftests)
> +selftest(gem_contexts, i915_gem_context_live_selftests)
>   selftest(blt, i915_gem_object_blt_live_selftests)
>   selftest(client, i915_gem_client_blt_live_selftests)
>   selftest(reset, intel_reset_live_selftests)
> 

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

Regards,

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3)
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (7 preceding siblings ...)
  2019-08-02  9:27 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-08-02 10:29 ` Patchwork
  2019-08-02 10:49 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-08-03 15:26 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-08-02 10:29 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3)
URL   : https://patchwork.freedesktop.org/series/64426/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e6d3307c25e3 drm/i915: Allow sharing the idle-barrier from other kernel requests
-:127: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#127: 
new file mode 100644

-:132: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#132: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:1:
+/*

-:133: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#133: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:2:
+ * SPDX-License-Identifier: GPL-2.0

total: 0 errors, 3 warnings, 0 checks, 807 lines checked

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3)
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (8 preceding siblings ...)
  2019-08-02 10:29 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3) Patchwork
@ 2019-08-02 10:49 ` Patchwork
  2019-08-03 15:26 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-08-02 10:49 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3)
URL   : https://patchwork.freedesktop.org/series/64426/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6612 -> Patchwork_13847
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/

New tests
---------

  New tests have been introduced between CI_DRM_6612 and Patchwork_13847:

### New IGT tests (2) ###

  * igt@i915_selftest@live_gem_contexts:
    - Statuses : 38 pass(s)
    - Exec time: [1.25, 28.49] s

  * igt@i915_selftest@live_gt_contexts:
    - Statuses : 38 pass(s)
    - Exec time: [0.41, 1.38] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@prime_vgem@basic-fence-mmap:
    - fi-pnv-d510:        [PASS][3] -> [INCOMPLETE][4] ([fdo#110740] / [fdo#111276])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-pnv-d510/igt@prime_vgem@basic-fence-mmap.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-pnv-d510/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-fence-read:
    - fi-bsw-kefka:       [PASS][5] -> [INCOMPLETE][6] ([fdo#111278])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-bsw-kefka/igt@prime_vgem@basic-fence-read.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-bsw-kefka/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-wait-default:
    - fi-bxt-j4205:       [PASS][7] -> [FAIL][8] ([fdo#111277])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-bxt-j4205/igt@prime_vgem@basic-wait-default.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-bxt-j4205/igt@prime_vgem@basic-wait-default.html

  
#### Possible fixes ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       [WARN][9] ([fdo#109380]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7567u:       [FAIL][11] ([fdo#109485]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u4}:        [FAIL][13] ([fdo#103167]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-kbl-7567u:       [SKIP][15] ([fdo#109271]) -> [PASS][16] +23 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  * igt@prime_vgem@basic-fence-read:
    - fi-pnv-d510:        [INCOMPLETE][17] ([fdo#110740]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-pnv-d510/igt@prime_vgem@basic-fence-read.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-pnv-d510/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-wait-default:
    - fi-bxt-dsi:         [FAIL][19] ([fdo#111277]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/fi-bxt-dsi/igt@prime_vgem@basic-wait-default.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/fi-bxt-dsi/igt@prime_vgem@basic-wait-default.html

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
  [fdo#111276]: https://bugs.freedesktop.org/show_bug.cgi?id=111276
  [fdo#111277]: https://bugs.freedesktop.org/show_bug.cgi?id=111277
  [fdo#111278]: https://bugs.freedesktop.org/show_bug.cgi?id=111278


Participating hosts (50 -> 41)
------------------------------

  Missing    (9): fi-kbl-soraka fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-skl-6600u fi-icl-y fi-bdw-samus fi-byt-clapper fi-skl-6700k2 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6612 -> Patchwork_13847

  CI-20190529: 20190529
  CI_DRM_6612: c85e7a40c6feee8cb16ceee3386189fc00f48cc1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5120: b3138fbea79d5d7935e53530b90efe3e816236f4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13847: e6d3307c25e3760d667f88901953b439534c2575 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e6d3307c25e3 drm/i915: Allow sharing the idle-barrier from other kernel requests

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3)
  2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
                   ` (9 preceding siblings ...)
  2019-08-02 10:49 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-08-03 15:26 ` Patchwork
  10 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-08-03 15:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3)
URL   : https://patchwork.freedesktop.org/series/64426/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6612_full -> Patchwork_13847_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen:
    - shard-snb:          [PASS][1] -> [SKIP][2] ([fdo#109271])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-snb5/igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-snb7/igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-apl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-apl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][5] -> [FAIL][6] ([fdo#105363])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-skl7/igt@kms_flip@flip-vs-expired-vblank.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-skl1/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][7] -> [FAIL][8] ([fdo#103167]) +4 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#108145])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-skl4/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#109441]) +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb6/igt@kms_psr@psr2_cursor_blt.html

  * igt@perf@polling:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#110728])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-skl10/igt@perf@polling.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-skl1/igt@perf@polling.html

  * igt@perf_pmu@rc6:
    - shard-kbl:          [PASS][15] -> [SKIP][16] ([fdo#109271])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-kbl4/igt@perf_pmu@rc6.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-kbl1/igt@perf_pmu@rc6.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@q-smoketest-default:
    - shard-iclb:         [INCOMPLETE][17] ([fdo#107713]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb7/igt@gem_ctx_shared@q-smoketest-default.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb1/igt@gem_ctx_shared@q-smoketest-default.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][19] ([fdo#110854]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb6/igt@gem_exec_balancer@smoke.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb4/igt@gem_exec_balancer@smoke.html

  * igt@i915_pm_rpm@pm-tiling:
    - shard-iclb:         [INCOMPLETE][21] ([fdo#107713] / [fdo#108840]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb2/igt@i915_pm_rpm@pm-tiling.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb2/igt@i915_pm_rpm@pm-tiling.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding:
    - shard-skl:          [FAIL][23] ([fdo#103232]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-skl5/igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][25] ([fdo#105363]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][27] ([fdo#103167]) -> [PASS][28] +7 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-apl:          [DMESG-WARN][29] ([fdo#108566]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][31] ([fdo#108145] / [fdo#110403]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][33] ([fdo#103166]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [SKIP][35] ([fdo#109441]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-iclb4/igt@kms_psr@psr2_dpms.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-iclb2/igt@kms_psr@psr2_dpms.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +7 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@prime_vgem@busy-bsd:
    - shard-apl:          [FAIL][39] ([fdo#111276]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-apl7/igt@prime_vgem@busy-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-apl1/igt@prime_vgem@busy-bsd.html

  * igt@syncobj_wait@wait-all-for-submit-complex:
    - shard-apl:          [INCOMPLETE][41] ([fdo#103927]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6612/shard-apl8/igt@syncobj_wait@wait-all-for-submit-complex.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13847/shard-apl6/igt@syncobj_wait@wait-all-for-submit-complex.html

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111276]: https://bugs.freedesktop.org/show_bug.cgi?id=111276


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

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6612 -> Patchwork_13847

  CI-20190529: 20190529
  CI_DRM_6612: c85e7a40c6feee8cb16ceee3386189fc00f48cc1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5120: b3138fbea79d5d7935e53530b90efe3e816236f4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13847: e6d3307c25e3760d667f88901953b439534c2575 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

end of thread, other threads:[~2019-08-03 15:26 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30 11:21 Short list of hopeful bug fixes Chris Wilson
2019-07-30 11:21 ` [PATCH 1/3] drm/i915/execlists: Always clear pending&inflight requests on reset Chris Wilson
2019-07-30 11:21 ` [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests Chris Wilson
2019-08-01 17:24   ` Tvrtko Ursulin
2019-08-01 17:55     ` Chris Wilson
2019-08-02  8:34     ` [PATCH v2] " Chris Wilson
2019-08-02 10:00     ` [PATCH v3] " Chris Wilson
2019-08-02 10:11       ` Tvrtko Ursulin
2019-07-30 11:21 ` [PATCH 3/3] drm/i915: Flush extra hard after writing relocations through the GTT Chris Wilson
2019-07-30 11:21   ` Chris Wilson
2019-08-01 20:33   ` [Intel-gfx] " Kumar Valsan, Prathap
2019-08-01 20:21     ` Chris Wilson
2019-08-01 23:08   ` Kumar Valsan, Prathap
2019-07-30 12:39 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset Patchwork
2019-07-30 13:01 ` ✓ Fi.CI.BAT: success " Patchwork
2019-07-30 22:46 ` ✓ Fi.CI.IGT: " Patchwork
2019-08-02  8:44 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev2) Patchwork
2019-08-02  9:27 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-08-02 10:29 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/execlists: Always clear pending&inflight requests on reset (rev3) Patchwork
2019-08-02 10:49 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-03 15:26 ` ✓ Fi.CI.IGT: " Patchwork

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