* [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex
@ 2019-07-25 13:14 Chris Wilson
2019-07-25 13:14 ` [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests Chris Wilson
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Chris Wilson @ 2019-07-25 13:14 UTC (permalink / raw)
To: intel-gfx
Modifying a remote context requires careful serialisation with requests
on that context, and that serialisation requires us to take their
timeline->mutex. Make it so.
Note that while struct_mutex rules, we can't create more than one
request in parallel, but that age is soon coming to an end.
v2: Though it doesn't affect the current users, contexts may share
timelines so check if we already hold the right mutex.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gt/intel_context.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 9292b6ca5e9c..d64b45f7ec6d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -254,10 +254,18 @@ int intel_context_prepare_remote_request(struct intel_context *ce,
/* Only suitable for use in remotely modifying this context */
GEM_BUG_ON(rq->hw_context == ce);
- /* Queue this switch after all other activity by this context. */
- err = i915_active_request_set(&tl->last_request, rq);
- if (err)
- return err;
+ if (rq->timeline != tl) { /* beware timeline sharing */
+ err = mutex_lock_interruptible_nested(&tl->mutex,
+ SINGLE_DEPTH_NESTING);
+ if (err)
+ return err;
+
+ /* Queue this switch after current activity by this context. */
+ err = i915_active_request_set(&tl->last_request, rq);
+ if (err)
+ goto unlock;
+ }
+ lockdep_assert_held(&tl->mutex);
/*
* Guarantee context image and the timeline remains pinned until the
@@ -267,7 +275,12 @@ int intel_context_prepare_remote_request(struct intel_context *ce,
* words transfer the pinned ce object to tracked active request.
*/
GEM_BUG_ON(i915_active_is_idle(&ce->active));
- return i915_active_ref(&ce->active, rq->fence.context, rq);
+ err = i915_active_ref(&ce->active, rq->fence.context, rq);
+
+unlock:
+ if (rq->timeline != tl)
+ mutex_unlock(&tl->mutex);
+ return err;
}
struct i915_request *intel_context_create_request(struct intel_context *ce)
--
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] 12+ messages in thread
* [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
@ 2019-07-25 13:14 ` Chris Wilson
2019-07-26 18:24 ` [PATCH v2] drm/i915: Allow sharing " Chris Wilson
2019-07-29 7:46 ` [PATCH 2/2] drm/i915: Unshare " Tvrtko Ursulin
2019-07-25 14:45 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex Patchwork
` (5 subsequent siblings)
6 siblings, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2019-07-25 13:14 UTC (permalink / raw)
To: intel-gfx
Under some circumstances (see intel_context_prepare_remote_request), we
may use a request along a kernel context to modify the logical state of
another. To keep the target context in place while the request executes,
we take an active reference on it using the kernel timeline. This is the
same timeline as we use for the idle-barrier, and so we end up reusing
the same active node. Except that the idle barrier is special and cannot
be reused in this manner! Give the idle-barrier a reserved timeline
index (0) so that it will always be unique (give or take we may issue
multiple idle barriers across multiple engines).
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>
Tested-by: Lionel Landwerlin <lionel.g.landwerlin@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/selftest_context.c | 322 ++++++++++++++++++
drivers/gpu/drm/i915/i915_active.c | 67 +++-
.../drm/i915/selftests/i915_live_selftests.h | 3 +-
5 files changed, 408 insertions(+), 37 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/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
new file mode 100644
index 000000000000..b40efdaabdd5
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -0,0 +1,322 @@
+/*
+ * 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(>->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(>->i915->drm.struct_mutex);
+ mock_file_free(gt->i915, file);
+ return err;
+}
+
+static int __live_remote_context(struct intel_engine_cs *engine,
+ struct i915_gem_context *fixme)
+{
+ struct intel_context *local, *remote;
+ struct i915_request *rq;
+ 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 foriegn fences into intel_context.active are not
+ * clobbered.
+ */
+
+ 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 = intel_context_pin(remote);
+ if (err)
+ goto err_local;
+
+ rq = intel_context_create_request(local);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto err_unpin;
+ }
+
+ err = intel_context_prepare_remote_request(remote, rq);
+ if (err) {
+ i915_request_add(rq);
+ goto err_unpin;
+ }
+
+ err = request_sync(rq);
+ if (err)
+ goto err_unpin;
+
+ intel_context_unpin(remote);
+ err = intel_context_pin(remote);
+ if (err)
+ goto err_local;
+
+ rq = i915_request_create(engine->kernel_context);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto err_unpin;
+ }
+
+ err = intel_context_prepare_remote_request(remote, rq);
+ if (err) {
+ i915_request_add(rq);
+ goto err_unpin;
+ }
+
+ err = request_sync(rq);
+ if (err)
+ goto err_unpin;
+
+ intel_context_unpin(remote);
+
+ if (i915_active_is_idle(&remote->active)) {
+ pr_err("remote context is not active; expected idle-barrier (pass %d)\n", pass);
+ err = -EINVAL;
+ goto err_local;
+ }
+ }
+
+ goto err_local;
+
+err_unpin:
+ intel_context_unpin(remote);
+err_local:
+ 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(>->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(>->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 13f304a29fc8..e04afb519264 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -184,6 +184,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;
}
@@ -212,6 +213,8 @@ int i915_active_ref(struct i915_active *ref,
struct i915_active_request *active;
int err;
+ GEM_BUG_ON(!timeline); /* reserved for idle-barrier */
+
/* Prevent reaping in case we malloc/wait while building the tree */
err = i915_active_acquire(ref);
if (err)
@@ -222,6 +225,7 @@ int i915_active_ref(struct i915_active *ref,
err = -ENOMEM;
goto out;
}
+ GEM_BUG_ON(IS_ERR(active->request));
if (!i915_active_request_isset(active))
atomic_inc(&ref->count);
@@ -342,6 +346,34 @@ void i915_active_fini(struct i915_active *ref)
}
#endif
+static struct active_node *idle_barrier(struct i915_active *ref)
+{
+ struct active_node *idle = NULL;
+ struct rb_node *rb;
+
+ if (RB_EMPTY_ROOT(&ref->tree))
+ return NULL;
+
+ mutex_lock(&ref->mutex);
+ for (rb = rb_first(&ref->tree); rb; rb = rb_next(rb)) {
+ struct active_node *node;
+
+ node = rb_entry(rb, typeof(*node), node);
+ if (node->timeline)
+ break;
+
+ if (!i915_active_request_isset(&node->base)) {
+ GEM_BUG_ON(!list_empty(&node->base.link));
+ rb_erase(rb, &ref->tree);
+ idle = node;
+ break;
+ }
+ }
+ mutex_unlock(&ref->mutex);
+
+ return idle;
+}
+
int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
struct intel_engine_cs *engine)
{
@@ -352,22 +384,29 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
GEM_BUG_ON(!engine->mask);
for_each_engine_masked(engine, i915, engine->mask, tmp) {
- struct intel_context *kctx = engine->kernel_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);
+ if (!node) {
+ node = kmem_cache_alloc(global.slab_cache,
+ GFP_KERNEL |
+ __GFP_RETRY_MAYFAIL |
+ __GFP_NOWARN);
+ if (unlikely(!node)) {
+ err = -ENOMEM;
+ goto unwind;
+ }
+
+ node->ref = ref;
+ node->timeline = 0;
+ node->base.retire = node_retire;
}
- i915_active_request_init(&node->base,
- (void *)engine, node_retire);
- node->timeline = kctx->ring->timeline->fence_context;
- node->ref = ref;
+ intel_engine_pm_get(engine);
+
+ RCU_INIT_POINTER(node->base.request, (void *)engine);
atomic_inc(&ref->count);
- intel_engine_pm_get(engine);
llist_add((struct llist_node *)&node->base.link,
&ref->barriers);
}
@@ -402,6 +441,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
node = container_of((struct list_head *)pos,
typeof(*node), base.link);
+ GEM_BUG_ON(node->timeline);
engine = (void *)rcu_access_pointer(node->base.request);
RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
@@ -410,12 +450,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
p = &ref->tree.rb_node;
while (*p) {
parent = *p;
- if (rb_entry(parent,
- struct active_node,
- node)->timeline < node->timeline)
- p = &parent->rb_right;
- else
- p = &parent->rb_left;
+ p = &parent->rb_left;
}
rb_link_node(&node->node, parent, p);
rb_insert_color(&node->node, &ref->tree);
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] 12+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
2019-07-25 13:14 ` [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests Chris Wilson
@ 2019-07-25 14:45 ` Patchwork
2019-07-25 15:06 ` ✓ Fi.CI.BAT: success " Patchwork
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-07-25 14:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex
URL : https://patchwork.freedesktop.org/series/64227/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
400f113b3b2b drm/i915/gt: Add to timeline requires the timeline mutex
7b3634ed02fa drm/i915: Unshare the idle-barrier from other kernel requests
-:112: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#112:
new file mode 100644
-:117: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#117: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:1:
+/*
-:118: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#118: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:2:
+ * SPDX-License-Identifier: GPL-2.0
-:372: WARNING:LONG_LINE: line over 100 characters
#372: FILE: drivers/gpu/drm/i915/gt/selftest_context.c:256:
+ pr_err("remote context is not active; expected idle-barrier (pass %d)\n", pass);
total: 0 errors, 4 warnings, 0 checks, 528 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
2019-07-25 13:14 ` [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests Chris Wilson
2019-07-25 14:45 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex Patchwork
@ 2019-07-25 15:06 ` Patchwork
2019-07-26 3:33 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-07-25 15:06 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex
URL : https://patchwork.freedesktop.org/series/64227/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6551 -> Patchwork_13750
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/
New tests
---------
New tests have been introduced between CI_DRM_6551 and Patchwork_13750:
### New IGT tests (2) ###
* igt@i915_selftest@live_gem_contexts:
- Statuses : 46 pass(s)
- Exec time: [0.40, 28.30] s
* igt@i915_selftest@live_gt_contexts:
- Statuses : 46 pass(s)
- Exec time: [0.38, 1.34] s
Known issues
------------
Here are the changes found in Patchwork_13750 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_busy@basic-flip-a:
- fi-kbl-7567u: [PASS][1] -> [SKIP][2] ([fdo#109271] / [fdo#109278]) +2 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7567u: [PASS][3] -> [WARN][4] ([fdo#109380])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
* igt@kms_pipe_crc_basic@read-crc-pipe-c:
- fi-kbl-7567u: [PASS][5] -> [SKIP][6] ([fdo#109271]) +23 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
#### Possible fixes ####
* igt@gem_ctx_create@basic-files:
- fi-skl-gvtdvm: [TIMEOUT][7] -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-skl-gvtdvm/igt@gem_ctx_create@basic-files.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-skl-gvtdvm/igt@gem_ctx_create@basic-files.html
* {igt@gem_ctx_switch@rcs0}:
- {fi-icl-guc}: [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-guc/igt@gem_ctx_switch@rcs0.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-guc/igt@gem_ctx_switch@rcs0.html
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [INCOMPLETE][11] ([fdo#107718]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
* igt@i915_module_load@reload-with-fault-injection:
- fi-snb-2520m: [INCOMPLETE][13] ([fdo#105411]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-snb-2520m/igt@i915_module_load@reload-with-fault-injection.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-snb-2520m/igt@i915_module_load@reload-with-fault-injection.html
* igt@i915_pm_rpm@module-reload:
- fi-icl-dsi: [INCOMPLETE][15] ([fdo#107713] / [fdo#108840]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-dsi/igt@i915_pm_rpm@module-reload.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-dsi/igt@i915_pm_rpm@module-reload.html
* igt@kms_chamelium@hdmi-hpd-fast:
- {fi-icl-u4}: [FAIL][17] ([fdo#109485]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-u4/igt@kms_chamelium@hdmi-hpd-fast.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-u4/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u3: [FAIL][19] ([fdo#103167]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
- fi-icl-u2: [FAIL][21] ([fdo#103167]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
* igt@vgem_basic@setversion:
- fi-icl-dsi: [DMESG-WARN][23] ([fdo#106107]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-dsi/igt@vgem_basic@setversion.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-dsi/igt@vgem_basic@setversion.html
#### Warnings ####
* igt@i915_module_load@reload:
- fi-icl-u3: [TIMEOUT][25] ([fdo#111214]) -> [TIMEOUT][26] ([fdo#109673] / [fdo#111214])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/fi-icl-u3/igt@i915_module_load@reload.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/fi-icl-u3/igt@i915_module_load@reload.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#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
[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#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
[fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
Participating hosts (56 -> 47)
------------------------------
Missing (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6551 -> Patchwork_13750
CI-20190529: 20190529
CI_DRM_6551: 83b453519660e386eebc0fa9b6d974aed978b610 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5111: e8e8ee79bdcd0a0e138d6a9cebacc10b1322cc07 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13750: 7b3634ed02face564e144d9d8b83ab14186b53dd @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
7b3634ed02fa drm/i915: Unshare the idle-barrier from other kernel requests
400f113b3b2b drm/i915/gt: Add to timeline requires the timeline mutex
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
` (2 preceding siblings ...)
2019-07-25 15:06 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-07-26 3:33 ` Patchwork
2019-07-26 13:39 ` [PATCH 1/2] " Tvrtko Ursulin
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-07-26 3:33 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex
URL : https://patchwork.freedesktop.org/series/64227/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6551_full -> Patchwork_13750_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
New tests
---------
New tests have been introduced between CI_DRM_6551_full and Patchwork_13750_full:
### New IGT tests (2) ###
* igt@i915_selftest@live_gem_contexts:
- Statuses : 6 pass(s)
- Exec time: [5.25, 34.05] s
* igt@i915_selftest@live_gt_contexts:
- Statuses : 6 pass(s)
- Exec time: [0.44, 2.26] s
Known issues
------------
Here are the changes found in Patchwork_13750_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@rcs0-s3:
- shard-iclb: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#109100])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb2/igt@gem_ctx_isolation@rcs0-s3.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb7/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_exec_balancer@smoke:
- shard-iclb: [PASS][3] -> [SKIP][4] ([fdo#110854])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb2/igt@gem_exec_balancer@smoke.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb8/igt@gem_exec_balancer@smoke.html
* igt@gem_tiled_swapping@non-threaded:
- shard-apl: [PASS][5] -> [DMESG-WARN][6] ([fdo#108686])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-apl6/igt@gem_tiled_swapping@non-threaded.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-apl4/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_workarounds@suspend-resume-context:
- shard-apl: [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +6 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-apl3/igt@gem_workarounds@suspend-resume-context.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-apl5/igt@gem_workarounds@suspend-resume-context.html
* igt@i915_suspend@fence-restore-untiled:
- shard-kbl: [PASS][9] -> [DMESG-WARN][10] ([fdo#108566])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-kbl2/igt@i915_suspend@fence-restore-untiled.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-kbl2/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled:
- shard-apl: [PASS][11] -> [INCOMPLETE][12] ([fdo#103927])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-apl3/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-apl5/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: [PASS][13] -> [FAIL][14] ([fdo#105363])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-glk6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
- shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103167]) +7 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html
* igt@kms_plane@plane-position-covered-pipe-a-planes:
- shard-snb: [PASS][17] -> [SKIP][18] ([fdo#109271]) +2 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-snb2/igt@kms_plane@plane-position-covered-pipe-a-planes.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-snb2/igt@kms_plane@plane-position-covered-pipe-a-planes.html
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [PASS][19] -> [FAIL][20] ([fdo#103166])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-y.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html
* igt@kms_psr@psr2_cursor_render:
- shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) +4 similar issues
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb7/igt@kms_psr@psr2_cursor_render.html
#### Possible fixes ####
* igt@gem_eio@reset-stress:
- shard-snb: [FAIL][23] ([fdo#109661]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-snb7/igt@gem_eio@reset-stress.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-snb6/igt@gem_eio@reset-stress.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [INCOMPLETE][25] ([fdo#104108]) -> [PASS][26] +1 similar issue
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl2/igt@gem_softpin@noreloc-s3.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl5/igt@gem_softpin@noreloc-s3.html
* igt@kms_color@pipe-b-ctm-negative:
- shard-skl: [FAIL][27] ([fdo#107361]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl2/igt@kms_color@pipe-b-ctm-negative.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl5/igt@kms_color@pipe-b-ctm-negative.html
* igt@kms_cursor_crc@pipe-b-cursor-256x85-offscreen:
- shard-skl: [FAIL][29] ([fdo#103232]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl2/igt@kms_cursor_crc@pipe-b-cursor-256x85-offscreen.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl5/igt@kms_cursor_crc@pipe-b-cursor-256x85-offscreen.html
* igt@kms_cursor_crc@pipe-c-cursor-suspend:
- shard-kbl: [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +6 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
* igt@kms_flip@2x-flip-vs-expired-vblank:
- shard-glk: [FAIL][33] ([fdo#105363]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: [FAIL][35] ([fdo#105363]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
- shard-iclb: [FAIL][37] ([fdo#103167]) -> [PASS][38] +7 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-apl: [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40] +2 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-apl5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: [FAIL][41] ([fdo#108145]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: [FAIL][43] ([fdo#108145] / [fdo#110403]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
* igt@kms_psr@psr2_sprite_mmap_gtt:
- shard-iclb: [SKIP][45] ([fdo#109441]) -> [PASS][46] +2 similar issues
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-iclb7/igt@kms_psr@psr2_sprite_mmap_gtt.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
* igt@perf@blocking:
- shard-skl: [FAIL][47] ([fdo#110728]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-skl6/igt@perf@blocking.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-skl9/igt@perf@blocking.html
#### Warnings ####
* igt@gem_userptr_blits@coherency-sync:
- shard-apl: [INCOMPLETE][49] ([fdo#103927]) -> [SKIP][50] ([fdo#109271])
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6551/shard-apl3/igt@gem_userptr_blits@coherency-sync.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13750/shard-apl3/igt@gem_userptr_blits@coherency-sync.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#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#107361]: https://bugs.freedesktop.org/show_bug.cgi?id=107361
[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#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
[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
Participating hosts (9 -> 9)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6551 -> Patchwork_13750
CI-20190529: 20190529
CI_DRM_6551: 83b453519660e386eebc0fa9b6d974aed978b610 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5111: e8e8ee79bdcd0a0e138d6a9cebacc10b1322cc07 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13750: 7b3634ed02face564e144d9d8b83ab14186b53dd @ 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_13750/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
` (3 preceding siblings ...)
2019-07-26 3:33 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-07-26 13:39 ` Tvrtko Ursulin
2019-07-26 19:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2) Patchwork
2019-07-26 19:56 ` ✗ Fi.CI.BAT: failure " Patchwork
6 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-07-26 13:39 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 25/07/2019 14:14, Chris Wilson wrote:
> Modifying a remote context requires careful serialisation with requests
> on that context, and that serialisation requires us to take their
> timeline->mutex. Make it so.
>
> Note that while struct_mutex rules, we can't create more than one
> request in parallel, but that age is soon coming to an end.
>
> v2: Though it doesn't affect the current users, contexts may share
> timelines so check if we already hold the right mutex.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_context.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 9292b6ca5e9c..d64b45f7ec6d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -254,10 +254,18 @@ int intel_context_prepare_remote_request(struct intel_context *ce,
> /* Only suitable for use in remotely modifying this context */
> GEM_BUG_ON(rq->hw_context == ce);
>
> - /* Queue this switch after all other activity by this context. */
> - err = i915_active_request_set(&tl->last_request, rq);
> - if (err)
> - return err;
> + if (rq->timeline != tl) { /* beware timeline sharing */
> + err = mutex_lock_interruptible_nested(&tl->mutex,
> + SINGLE_DEPTH_NESTING);
> + if (err)
> + return err;
> +
> + /* Queue this switch after current activity by this context. */
> + err = i915_active_request_set(&tl->last_request, rq);
> + if (err)
> + goto unlock;
> + }
> + lockdep_assert_held(&tl->mutex);
>
> /*
> * Guarantee context image and the timeline remains pinned until the
> @@ -267,7 +275,12 @@ int intel_context_prepare_remote_request(struct intel_context *ce,
> * words transfer the pinned ce object to tracked active request.
> */
> GEM_BUG_ON(i915_active_is_idle(&ce->active));
> - return i915_active_ref(&ce->active, rq->fence.context, rq);
> + err = i915_active_ref(&ce->active, rq->fence.context, rq);
> +
> +unlock:
> + if (rq->timeline != tl)
> + mutex_unlock(&tl->mutex);
> + return err;
> }
>
> struct i915_request *intel_context_create_request(struct intel_context *ce)
>
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] 12+ messages in thread
* [PATCH v2] drm/i915: Allow sharing the idle-barrier from other kernel requests
2019-07-25 13:14 ` [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests Chris Wilson
@ 2019-07-26 18:24 ` Chris Wilson
2019-07-29 7:46 ` [PATCH 2/2] drm/i915: Unshare " Tvrtko Ursulin
1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-07-26 18:24 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 | 215 +++++++++---
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, 525 insertions(+), 62 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..e3b45fe747ae
--- /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(>->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(>->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 foriegn fences into intel_context.active are not
+ * clobbered.
+ */
+
+ 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(>->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(>->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..a23195270b69 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,33 @@ 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 void __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 (barrier_to_ll(node) == pos)
+ continue;
+
+ pos->next = head;
+ head = pos;
+ if (!tail)
+ tail = pos;
+ }
+ if (head)
+ llist_add_batch(head, tail, &engine->barrier_tasks);
+}
+
int i915_active_ref(struct i915_active *ref,
u64 timeline,
struct i915_request *rq)
@@ -224,8 +279,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 +374,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 +441,58 @@ void i915_active_fini(struct i915_active *ref)
}
#endif
+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));
+
+ prev = NULL;
+ p = ref->tree.rb_node;
+ while (p) {
+ struct active_node *node =
+ rb_entry(p, struct active_node, node);
+
+ if (node->timeline == idx &&
+ !i915_active_request_isset(&node->base))
+ 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;
+
+ }
+ mutex_unlock(&ref->mutex);
+
+ return NULL;
+
+match:
+ rb_erase(p, &ref->tree);
+ 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 +501,48 @@ 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;
+ }
+
+ 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;
+ RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
+ node->base.link.prev = (void *)engine;
atomic_inc(&ref->count);
+ 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 +554,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 +582,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] 12+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2)
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
` (4 preceding siblings ...)
2019-07-26 13:39 ` [PATCH 1/2] " Tvrtko Ursulin
@ 2019-07-26 19:35 ` Patchwork
2019-07-26 19:56 ` ✗ Fi.CI.BAT: failure " Patchwork
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-07-26 19:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2)
URL : https://patchwork.freedesktop.org/series/64227/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ff62d595c38e 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
-:607: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#607: FILE: drivers/gpu/drm/i915/i915_active.c:484:
+
+ }
total: 0 errors, 3 warnings, 1 checks, 735 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2)
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
` (5 preceding siblings ...)
2019-07-26 19:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2) Patchwork
@ 2019-07-26 19:56 ` Patchwork
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-07-26 19:56 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2)
URL : https://patchwork.freedesktop.org/series/64227/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6564 -> Patchwork_13774
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_13774 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13774, 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_13774/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_13774:
### IGT changes ###
#### Possible regressions ####
* {igt@i915_selftest@live_gt_contexts} (NEW):
- fi-hsw-4770r: NOTRUN -> [INCOMPLETE][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-hsw-4770r/igt@i915_selftest@live_gt_contexts.html
- fi-blb-e6850: NOTRUN -> [INCOMPLETE][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-blb-e6850/igt@i915_selftest@live_gt_contexts.html
- fi-bwr-2160: NOTRUN -> [INCOMPLETE][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-bwr-2160/igt@i915_selftest@live_gt_contexts.html
- fi-hsw-peppy: NOTRUN -> [INCOMPLETE][4]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-hsw-peppy/igt@i915_selftest@live_gt_contexts.html
- fi-snb-2520m: NOTRUN -> [INCOMPLETE][5]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-snb-2520m/igt@i915_selftest@live_gt_contexts.html
- fi-ilk-650: NOTRUN -> [INCOMPLETE][6]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-ilk-650/igt@i915_selftest@live_gt_contexts.html
- fi-ivb-3770: NOTRUN -> [INCOMPLETE][7]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-ivb-3770/igt@i915_selftest@live_gt_contexts.html
- fi-hsw-4770: NOTRUN -> [INCOMPLETE][8]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-hsw-4770/igt@i915_selftest@live_gt_contexts.html
* igt@runner@aborted:
- fi-ilk-650: NOTRUN -> [FAIL][9]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-ilk-650/igt@runner@aborted.html
- fi-pnv-d510: NOTRUN -> [FAIL][10]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-pnv-d510/igt@runner@aborted.html
- fi-hsw-peppy: NOTRUN -> [FAIL][11]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-hsw-peppy/igt@runner@aborted.html
- fi-gdg-551: NOTRUN -> [FAIL][12]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-gdg-551/igt@runner@aborted.html
- fi-snb-2520m: NOTRUN -> [FAIL][13]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-snb-2520m/igt@runner@aborted.html
- fi-hsw-4770: NOTRUN -> [FAIL][14]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-hsw-4770/igt@runner@aborted.html
- fi-ivb-3770: NOTRUN -> [FAIL][15]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-ivb-3770/igt@runner@aborted.html
- fi-byt-j1900: NOTRUN -> [FAIL][16]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-byt-j1900/igt@runner@aborted.html
- fi-blb-e6850: NOTRUN -> [FAIL][17]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-blb-e6850/igt@runner@aborted.html
- fi-hsw-4770r: NOTRUN -> [FAIL][18]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-hsw-4770r/igt@runner@aborted.html
- fi-byt-n2820: NOTRUN -> [FAIL][19]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-byt-n2820/igt@runner@aborted.html
- fi-snb-2600: NOTRUN -> [FAIL][20]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-snb-2600/igt@runner@aborted.html
- fi-elk-e7500: NOTRUN -> [FAIL][21]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-elk-e7500/igt@runner@aborted.html
New tests
---------
New tests have been introduced between CI_DRM_6564 and Patchwork_13774:
### New IGT tests (2) ###
* igt@i915_selftest@live_gem_contexts:
- Statuses : 29 pass(s)
- Exec time: [21.71, 28.22] s
* igt@i915_selftest@live_gt_contexts:
- Statuses : 14 incomplete(s) 29 pass(s)
- Exec time: [0.0, 1.11] s
Known issues
------------
Here are the changes found in Patchwork_13774 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_create@basic-files:
- fi-cml-u: [PASS][22] -> [INCOMPLETE][23] ([fdo#110566])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6564/fi-cml-u/igt@gem_ctx_create@basic-files.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-cml-u/igt@gem_ctx_create@basic-files.html
* igt@i915_module_load@reload-no-display:
- fi-icl-u3: [PASS][24] -> [DMESG-WARN][25] ([fdo#107724]) +2 similar issues
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6564/fi-icl-u3/igt@i915_module_load@reload-no-display.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-icl-u3/igt@i915_module_load@reload-no-display.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-apl-guc: [PASS][26] -> [DMESG-WARN][27] ([fdo#108566])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6564/fi-apl-guc/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-apl-guc/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Possible fixes ####
* igt@gem_exec_reloc@basic-gtt-cpu:
- fi-icl-u3: [DMESG-WARN][28] ([fdo#107724]) -> [PASS][29]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6564/fi-icl-u3/igt@gem_exec_reloc@basic-gtt-cpu.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-icl-u3/igt@gem_exec_reloc@basic-gtt-cpu.html
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u2: [FAIL][30] ([fdo#103167]) -> [PASS][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6564/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13774/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.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#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
Participating hosts (53 -> 45)
------------------------------
Additional (1): fi-cfl-8109u
Missing (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u 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_6564 -> Patchwork_13774
CI-20190529: 20190529
CI_DRM_6564: 26169fbe1a3c592f1be6792d642e3e084b7acc4f @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5113: f8f1bfbd25559e01c59a55635477cb74b326ea0b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13774: ff62d595c38e524db041053bc8a4b20f13d47f6e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
ff62d595c38e 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_13774/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests
2019-07-25 13:14 ` [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests Chris Wilson
2019-07-26 18:24 ` [PATCH v2] drm/i915: Allow sharing " Chris Wilson
@ 2019-07-29 7:46 ` Tvrtko Ursulin
2019-07-29 8:54 ` Chris Wilson
1 sibling, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-07-29 7:46 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 25/07/2019 14:14, Chris Wilson wrote:
> Under some circumstances (see intel_context_prepare_remote_request), we
> may use a request along a kernel context to modify the logical state of
> another. To keep the target context in place while the request executes,
> we take an active reference on it using the kernel timeline. This is the
> same timeline as we use for the idle-barrier, and so we end up reusing
> the same active node. Except that the idle barrier is special and cannot
> be reused in this manner! Give the idle-barrier a reserved timeline
> index (0) so that it will always be unique (give or take we may issue
> multiple idle barriers across multiple engines).
>
> 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>
> Tested-by: Lionel Landwerlin <lionel.g.landwerlin@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/selftest_context.c | 322 ++++++++++++++++++
> drivers/gpu/drm/i915/i915_active.c | 67 +++-
> .../drm/i915/selftests/i915_live_selftests.h | 3 +-
> 5 files changed, 408 insertions(+), 37 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/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> new file mode 100644
> index 000000000000..b40efdaabdd5
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -0,0 +1,322 @@
> +/*
> + * 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(>->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(>->i915->drm.struct_mutex);
> + mock_file_free(gt->i915, file);
> + return err;
> +}
> +
> +static int __live_remote_context(struct intel_engine_cs *engine,
> + struct i915_gem_context *fixme)
> +{
> + struct intel_context *local, *remote;
> + struct i915_request *rq;
> + 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 foriegn fences into intel_context.active are not
typo in foreign
"operating ... are not .." ? Foreign fences are not clobbered?
> + * clobbered.
> + */
> +
> + 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 = intel_context_pin(remote);
> + if (err)
> + goto err_local;
> +
> + rq = intel_context_create_request(local);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + goto err_unpin;
> + }
> +
> + err = intel_context_prepare_remote_request(remote, rq);
> + if (err) {
> + i915_request_add(rq);
> + goto err_unpin;
> + }
> +
> + err = request_sync(rq);
> + if (err)
> + goto err_unpin;
> +
> + intel_context_unpin(remote);
> + err = intel_context_pin(remote);
unpin-pin is to trigger transfer of idle barriers and maybe trigger some
asserts?
> + if (err)
> + goto err_local;
> +
> + rq = i915_request_create(engine->kernel_context);
Why a request on kernel context here, a third context?
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + goto err_unpin;
> + }
> +
> + err = intel_context_prepare_remote_request(remote, rq);
> + if (err) {
> + i915_request_add(rq);
> + goto err_unpin;
> + }
> +
> + err = request_sync(rq);
> + if (err)
> + goto err_unpin;
> +
> + intel_context_unpin(remote);
> +
> + if (i915_active_is_idle(&remote->active)) {
> + pr_err("remote context is not active; expected idle-barrier (pass %d)\n", pass);
> + err = -EINVAL;
> + goto err_local;
> + }
> + }
> +
> + goto err_local;
> +
> +err_unpin:
> + intel_context_unpin(remote);
> +err_local:
> + 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(>->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(>->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 13f304a29fc8..e04afb519264 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -184,6 +184,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;
> }
>
> @@ -212,6 +213,8 @@ int i915_active_ref(struct i915_active *ref,
> struct i915_active_request *active;
> int err;
>
> + GEM_BUG_ON(!timeline); /* reserved for idle-barrier */
> +
> /* Prevent reaping in case we malloc/wait while building the tree */
> err = i915_active_acquire(ref);
> if (err)
> @@ -222,6 +225,7 @@ int i915_active_ref(struct i915_active *ref,
> err = -ENOMEM;
> goto out;
> }
> + GEM_BUG_ON(IS_ERR(active->request));
>
> if (!i915_active_request_isset(active))
> atomic_inc(&ref->count);
> @@ -342,6 +346,34 @@ void i915_active_fini(struct i915_active *ref)
> }
> #endif
>
> +static struct active_node *idle_barrier(struct i915_active *ref)
> +{
> + struct active_node *idle = NULL;
> + struct rb_node *rb;
> +
> + if (RB_EMPTY_ROOT(&ref->tree))
> + return NULL;
> +
> + mutex_lock(&ref->mutex);
> + for (rb = rb_first(&ref->tree); rb; rb = rb_next(rb)) {
> + struct active_node *node;
> +
> + node = rb_entry(rb, typeof(*node), node);
> + if (node->timeline)
> + break;
> +
> + if (!i915_active_request_isset(&node->base)) {
> + GEM_BUG_ON(!list_empty(&node->base.link));
> + rb_erase(rb, &ref->tree);
> + idle = node;
> + break;
> + }
Under what circumstances does the walk continue? There can be two idle
barriers (timeline == 0) in the tree?
> + }
> + mutex_unlock(&ref->mutex);
> +
> + return idle;
> +}
> +
> int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
> struct intel_engine_cs *engine)
> {
> @@ -352,22 +384,29 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>
> GEM_BUG_ON(!engine->mask);
> for_each_engine_masked(engine, i915, engine->mask, tmp) {
> - struct intel_context *kctx = engine->kernel_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);
> + if (!node) {
> + node = kmem_cache_alloc(global.slab_cache,
> + GFP_KERNEL |
> + __GFP_RETRY_MAYFAIL |
> + __GFP_NOWARN);
> + if (unlikely(!node)) {
> + err = -ENOMEM;
> + goto unwind;
> + }
> +
> + node->ref = ref;
> + node->timeline = 0;
> + node->base.retire = node_retire;
> }
>
> - i915_active_request_init(&node->base,
> - (void *)engine, node_retire);
> - node->timeline = kctx->ring->timeline->fence_context;
> - node->ref = ref;
> + intel_engine_pm_get(engine);
> +
> + RCU_INIT_POINTER(node->base.request, (void *)engine);
> atomic_inc(&ref->count);
>
> - intel_engine_pm_get(engine);
> llist_add((struct llist_node *)&node->base.link,
> &ref->barriers);
> }
> @@ -402,6 +441,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
>
> node = container_of((struct list_head *)pos,
> typeof(*node), base.link);
> + GEM_BUG_ON(node->timeline);
>
> engine = (void *)rcu_access_pointer(node->base.request);
> RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
> @@ -410,12 +450,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
> p = &ref->tree.rb_node;
> while (*p) {
> parent = *p;
> - if (rb_entry(parent,
> - struct active_node,
> - node)->timeline < node->timeline)
> - p = &parent->rb_right;
> - else
> - p = &parent->rb_left;
> + p = &parent->rb_left;
> }
> rb_link_node(&node->node, parent, p);
> rb_insert_color(&node->node, &ref->tree);
> 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)
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests
2019-07-29 7:46 ` [PATCH 2/2] drm/i915: Unshare " Tvrtko Ursulin
@ 2019-07-29 8:54 ` Chris Wilson
2019-07-29 9:43 ` Tvrtko Ursulin
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-07-29 8:54 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-07-29 08:46:59)
>
> On 25/07/2019 14:14, Chris Wilson wrote:
> > +static int __live_remote_context(struct intel_engine_cs *engine,
> > + struct i915_gem_context *fixme)
> > +{
> > + struct intel_context *local, *remote;
> > + struct i915_request *rq;
> > + 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 foriegn fences into intel_context.active are not
>
> typo in foreign
>
> "operating ... are not .." ? Foreign fences are not clobbered?
, does not clobber the active tracking.
>
> > + * clobbered.
> > + */
> > +
> > + 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 = intel_context_pin(remote);
> > + if (err)
> > + goto err_local;
> > +
> > + rq = intel_context_create_request(local);
> > + if (IS_ERR(rq)) {
> > + err = PTR_ERR(rq);
> > + goto err_unpin;
> > + }
> > +
> > + err = intel_context_prepare_remote_request(remote, rq);
> > + if (err) {
> > + i915_request_add(rq);
> > + goto err_unpin;
> > + }
> > +
> > + err = request_sync(rq);
> > + if (err)
> > + goto err_unpin;
> > +
> > + intel_context_unpin(remote);
> > + err = intel_context_pin(remote);
>
> unpin-pin is to trigger transfer of idle barriers and maybe trigger some
> asserts?
unpin is to trigger the idle-barrier. The pin is just the start of the
next phase with another context. At first I tried doing two remote
requests within on pin-phase, but that doesn't hit the bug. It needed
the idle barrier in the middle of the test, not between passes.
v2 wrapped it with another subroutine so the unpin-pin is not so
glaringly obvious.
> > + if (err)
> > + goto err_local;
> > +
> > + rq = i915_request_create(engine->kernel_context);
>
> Why a request on kernel context here, a third context?
The kernel_context is most important since that's the one used by the
idle barrier. I included the normal context as well for completeness as
the intel_context_prepare_remote_request() interface should not assume
it is working from the kernel context.
> > + if (IS_ERR(rq)) {
> > + err = PTR_ERR(rq);
> > + goto err_unpin;
> > + }
> > +
> > + err = intel_context_prepare_remote_request(remote, rq);
> > + if (err) {
> > + i915_request_add(rq);
> > + goto err_unpin;
> > + }
> > +
> > + err = request_sync(rq);
> > + if (err)
> > + goto err_unpin;
> > +
> > + intel_context_unpin(remote);
> > +
> > + if (i915_active_is_idle(&remote->active)) {
> > + pr_err("remote context is not active; expected idle-barrier (pass %d)\n", pass);
> > + err = -EINVAL;
> > + goto err_local;
> > + }
> > + }
> > +
> > + goto err_local;
> > +
> > +err_unpin:
> > + intel_context_unpin(remote);
> > +err_local:
> > + 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(>->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(>->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 13f304a29fc8..e04afb519264 100644
> > --- a/drivers/gpu/drm/i915/i915_active.c
> > +++ b/drivers/gpu/drm/i915/i915_active.c
> > @@ -184,6 +184,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;
> > }
> >
> > @@ -212,6 +213,8 @@ int i915_active_ref(struct i915_active *ref,
> > struct i915_active_request *active;
> > int err;
> >
> > + GEM_BUG_ON(!timeline); /* reserved for idle-barrier */
> > +
> > /* Prevent reaping in case we malloc/wait while building the tree */
> > err = i915_active_acquire(ref);
> > if (err)
> > @@ -222,6 +225,7 @@ int i915_active_ref(struct i915_active *ref,
> > err = -ENOMEM;
> > goto out;
> > }
> > + GEM_BUG_ON(IS_ERR(active->request));
> >
> > if (!i915_active_request_isset(active))
> > atomic_inc(&ref->count);
> > @@ -342,6 +346,34 @@ void i915_active_fini(struct i915_active *ref)
> > }
> > #endif
> >
> > +static struct active_node *idle_barrier(struct i915_active *ref)
> > +{
> > + struct active_node *idle = NULL;
> > + struct rb_node *rb;
> > +
> > + if (RB_EMPTY_ROOT(&ref->tree))
> > + return NULL;
> > +
> > + mutex_lock(&ref->mutex);
> > + for (rb = rb_first(&ref->tree); rb; rb = rb_next(rb)) {
> > + struct active_node *node;
> > +
> > + node = rb_entry(rb, typeof(*node), node);
> > + if (node->timeline)
> > + break;
> > +
> > + if (!i915_active_request_isset(&node->base)) {
> > + GEM_BUG_ON(!list_empty(&node->base.link));
> > + rb_erase(rb, &ref->tree);
> > + idle = node;
> > + break;
> > + }
>
> Under what circumstances does the walk continue? There can be two idle
> barriers (timeline == 0) in the tree?
Yes, there can be more than one (virtual engines). It should be the case
that when i915_active becomes idle (all idle barriers are idle) that the
tree is reaped. But... if we overlap active phases, we will get multiple
idle barriers, some idle, some active, which we want to reuse to avoid
having a potentially unbounded allocation.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests
2019-07-29 8:54 ` Chris Wilson
@ 2019-07-29 9:43 ` Tvrtko Ursulin
0 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2019-07-29 9:43 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 29/07/2019 09:54, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-29 08:46:59)
>>
>> On 25/07/2019 14:14, Chris Wilson wrote:
>>> +static int __live_remote_context(struct intel_engine_cs *engine,
>>> + struct i915_gem_context *fixme)
>>> +{
>>> + struct intel_context *local, *remote;
>>> + struct i915_request *rq;
>>> + 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 foriegn fences into intel_context.active are not
>>
>> typo in foreign
>>
>> "operating ... are not .." ? Foreign fences are not clobbered?
>
> , does not clobber the active tracking.
>
>>
>>> + * clobbered.
>>> + */
>>> +
>>> + 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 = intel_context_pin(remote);
>>> + if (err)
>>> + goto err_local;
>>> +
>>> + rq = intel_context_create_request(local);
>>> + if (IS_ERR(rq)) {
>>> + err = PTR_ERR(rq);
>>> + goto err_unpin;
>>> + }
>>> +
>>> + err = intel_context_prepare_remote_request(remote, rq);
>>> + if (err) {
>>> + i915_request_add(rq);
>>> + goto err_unpin;
>>> + }
>>> +
>>> + err = request_sync(rq);
>>> + if (err)
>>> + goto err_unpin;
>>> +
>>> + intel_context_unpin(remote);
>>> + err = intel_context_pin(remote);
>>
>> unpin-pin is to trigger transfer of idle barriers and maybe trigger some
>> asserts?
>
> unpin is to trigger the idle-barrier. The pin is just the start of the
> next phase with another context. At first I tried doing two remote
> requests within on pin-phase, but that doesn't hit the bug. It needed
> the idle barrier in the middle of the test, not between passes.
>
> v2 wrapped it with another subroutine so the unpin-pin is not so
> glaringly obvious.
v*2*? :D Hard to know without revisioning and multiple sends... I've
found it now, I've been looking at a very different version here.
>
>>> + if (err)
>>> + goto err_local;
>>> +
>>> + rq = i915_request_create(engine->kernel_context);
>>
>> Why a request on kernel context here, a third context?
>
> The kernel_context is most important since that's the one used by the
> idle barrier. I included the normal context as well for completeness as
> the intel_context_prepare_remote_request() interface should not assume
> it is working from the kernel context.
Most important = worthy of a comment? ;)
>
>>> + if (IS_ERR(rq)) {
>>> + err = PTR_ERR(rq);
>>> + goto err_unpin;
>>> + }
>>> +
>>> + err = intel_context_prepare_remote_request(remote, rq);
>>> + if (err) {
>>> + i915_request_add(rq);
>>> + goto err_unpin;
>>> + }
>>> +
>>> + err = request_sync(rq);
>>> + if (err)
>>> + goto err_unpin;
>>> +
>>> + intel_context_unpin(remote);
>>> +
>>> + if (i915_active_is_idle(&remote->active)) {
>>> + pr_err("remote context is not active; expected idle-barrier (pass %d)\n", pass);
>>> + err = -EINVAL;
>>> + goto err_local;
>>> + }
>>> + }
>>> +
>>> + goto err_local;
>>> +
>>> +err_unpin:
>>> + intel_context_unpin(remote);
>>> +err_local:
>>> + 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(>->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(>->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 13f304a29fc8..e04afb519264 100644
>>> --- a/drivers/gpu/drm/i915/i915_active.c
>>> +++ b/drivers/gpu/drm/i915/i915_active.c
>>> @@ -184,6 +184,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;
>>> }
>>>
>>> @@ -212,6 +213,8 @@ int i915_active_ref(struct i915_active *ref,
>>> struct i915_active_request *active;
>>> int err;
>>>
>>> + GEM_BUG_ON(!timeline); /* reserved for idle-barrier */
>>> +
>>> /* Prevent reaping in case we malloc/wait while building the tree */
>>> err = i915_active_acquire(ref);
>>> if (err)
>>> @@ -222,6 +225,7 @@ int i915_active_ref(struct i915_active *ref,
>>> err = -ENOMEM;
>>> goto out;
>>> }
>>> + GEM_BUG_ON(IS_ERR(active->request));
>>>
>>> if (!i915_active_request_isset(active))
>>> atomic_inc(&ref->count);
>>> @@ -342,6 +346,34 @@ void i915_active_fini(struct i915_active *ref)
>>> }
>>> #endif
>>>
>>> +static struct active_node *idle_barrier(struct i915_active *ref)
>>> +{
>>> + struct active_node *idle = NULL;
>>> + struct rb_node *rb;
>>> +
>>> + if (RB_EMPTY_ROOT(&ref->tree))
>>> + return NULL;
>>> +
>>> + mutex_lock(&ref->mutex);
>>> + for (rb = rb_first(&ref->tree); rb; rb = rb_next(rb)) {
>>> + struct active_node *node;
>>> +
>>> + node = rb_entry(rb, typeof(*node), node);
>>> + if (node->timeline)
>>> + break;
>>> +
>>> + if (!i915_active_request_isset(&node->base)) {
>>> + GEM_BUG_ON(!list_empty(&node->base.link));
>>> + rb_erase(rb, &ref->tree);
>>> + idle = node;
>>> + break;
>>> + }
>>
>> Under what circumstances does the walk continue? There can be two idle
>> barriers (timeline == 0) in the tree?
>
> Yes, there can be more than one (virtual engines). It should be the case
> that when i915_active becomes idle (all idle barriers are idle) that the
> tree is reaped. But... if we overlap active phases, we will get multiple
> idle barriers, some idle, some active, which we want to reuse to avoid
> having a potentially unbounded allocation.
This feels is comment worthy as well.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-07-29 9:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 13:14 [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex Chris Wilson
2019-07-25 13:14 ` [PATCH 2/2] drm/i915: Unshare the idle-barrier from other kernel requests Chris Wilson
2019-07-26 18:24 ` [PATCH v2] drm/i915: Allow sharing " Chris Wilson
2019-07-29 7:46 ` [PATCH 2/2] drm/i915: Unshare " Tvrtko Ursulin
2019-07-29 8:54 ` Chris Wilson
2019-07-29 9:43 ` Tvrtko Ursulin
2019-07-25 14:45 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex Patchwork
2019-07-25 15:06 ` ✓ Fi.CI.BAT: success " Patchwork
2019-07-26 3:33 ` ✓ Fi.CI.IGT: " Patchwork
2019-07-26 13:39 ` [PATCH 1/2] " Tvrtko Ursulin
2019-07-26 19:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gt: Add to timeline requires the timeline mutex (rev2) Patchwork
2019-07-26 19:56 ` ✗ Fi.CI.BAT: failure " 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.