If we introduce a callback for i915_active that is only called the first
time we use the i915_active and is symmetrically paired with the
i915_active.retire callback, we can replace the open-coded and
non-atomic implementations -- which will be very fragile (i.e. broken)
upon removing the struct_mutex serialisation.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 8 +-
drivers/gpu/drm/i915/gt/intel_context.c | 65 +++---
drivers/gpu/drm/i915/gt/intel_context.h | 14 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 6 +-
drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 2 +-
drivers/gpu/drm/i915/gt/intel_timeline.c | 16 +-
drivers/gpu/drm/i915/gt/mock_engine.c | 2 +-
drivers/gpu/drm/i915/i915_active.c | 225 ++++++++++---------
drivers/gpu/drm/i915/i915_active.h | 25 ++-
drivers/gpu/drm/i915/i915_active_types.h | 10 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 22 +-
drivers/gpu/drm/i915/selftests/i915_active.c | 53 ++++-
13 files changed, 258 insertions(+), 192 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 628673d1d7f8..8a9787cf0cd0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -923,8 +923,12 @@ static int context_barrier_task(struct i915_gem_context *ctx,
if (!cb)
return -ENOMEM;
- i915_active_init(i915, &cb->base, cb_retire);
- i915_active_acquire(&cb->base);
+ i915_active_init(i915, &cb->base, NULL, cb_retire);
+ err = i915_active_acquire(&cb->base);
+ if (err) {
+ kfree(cb);
+ return err;
+ }
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 23120901c55f..938dd032b820 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -95,11 +95,15 @@ void intel_context_unpin(struct intel_context *ce)
intel_context_put(ce);
}
-static int __context_pin_state(struct i915_vma *vma, unsigned long flags)
+static int __context_pin_state(struct i915_vma *vma)
{
+ u64 flags;
int err;
- err = i915_vma_pin(vma, 0, 0, flags | PIN_GLOBAL);
+ flags = i915_ggtt_pin_bias(vma) | PIN_OFFSET_BIAS;
+ flags |= PIN_HIGH | PIN_GLOBAL;
+
+ err = i915_vma_pin(vma, 0, 0, flags);
if (err)
return err;
@@ -119,7 +123,7 @@ static void __context_unpin_state(struct i915_vma *vma)
__i915_vma_unpin(vma);
}
-static void intel_context_retire(struct i915_active *active)
+static void __intel_context_retire(struct i915_active *active)
{
struct intel_context *ce = container_of(active, typeof(*ce), active);
@@ -130,35 +134,11 @@ static void intel_context_retire(struct i915_active *active)
intel_context_put(ce);
}
-void
-intel_context_init(struct intel_context *ce,
- struct i915_gem_context *ctx,
- struct intel_engine_cs *engine)
-{
- GEM_BUG_ON(!engine->cops);
-
- kref_init(&ce->ref);
-
- ce->gem_context = ctx;
- ce->engine = engine;
- ce->ops = engine->cops;
- ce->sseu = engine->sseu;
-
- INIT_LIST_HEAD(&ce->signal_link);
- INIT_LIST_HEAD(&ce->signals);
-
- mutex_init(&ce->pin_mutex);
-
- i915_active_init(ctx->i915, &ce->active, intel_context_retire);
-}
-
-int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
+static int __intel_context_active(struct i915_active *active)
{
+ struct intel_context *ce = container_of(active, typeof(*ce), active);
int err;
- if (!i915_active_acquire(&ce->active))
- return 0;
-
intel_context_get(ce);
err = intel_ring_pin(ce->ring);
@@ -168,7 +148,7 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
if (!ce->state)
return 0;
- err = __context_pin_state(ce->state, flags);
+ err = __context_pin_state(ce->state);
if (err)
goto err_ring;
@@ -188,15 +168,30 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags)
intel_ring_unpin(ce->ring);
err_put:
intel_context_put(ce);
- i915_active_cancel(&ce->active);
return err;
}
-void intel_context_active_release(struct intel_context *ce)
+void
+intel_context_init(struct intel_context *ce,
+ struct i915_gem_context *ctx,
+ struct intel_engine_cs *engine)
{
- /* Nodes preallocated in intel_context_active() */
- i915_active_acquire_barrier(&ce->active);
- i915_active_release(&ce->active);
+ GEM_BUG_ON(!engine->cops);
+
+ kref_init(&ce->ref);
+
+ ce->gem_context = ctx;
+ ce->engine = engine;
+ ce->ops = engine->cops;
+ ce->sseu = engine->sseu;
+
+ INIT_LIST_HEAD(&ce->signal_link);
+ INIT_LIST_HEAD(&ce->signals);
+
+ mutex_init(&ce->pin_mutex);
+
+ i915_active_init(ctx->i915, &ce->active,
+ __intel_context_active, __intel_context_retire);
}
static void i915_global_context_shrink(void)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index a47275bc4f01..40cd8320fcc3 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -9,6 +9,7 @@
#include <linux/lockdep.h>
+#include "i915_active.h"
#include "intel_context_types.h"
#include "intel_engine_types.h"
@@ -102,8 +103,17 @@ static inline void intel_context_exit(struct intel_context *ce)
ce->ops->exit(ce);
}
-int intel_context_active_acquire(struct intel_context *ce, unsigned long flags);
-void intel_context_active_release(struct intel_context *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);
+}
static inline struct intel_context *intel_context_get(struct intel_context *ce)
{
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index ad7638da785d..c8a0c9b32764 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1542,12 +1542,10 @@ __execlists_context_pin(struct intel_context *ce,
goto err;
GEM_BUG_ON(!ce->state);
- ret = intel_context_active_acquire(ce,
- engine->i915->ggtt.pin_bias |
- PIN_OFFSET_BIAS |
- PIN_HIGH);
+ ret = intel_context_active_acquire(ce);
if (ret)
goto err;
+ GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
vaddr = i915_gem_object_pin_map(ce->state->obj,
i915_coherent_map_type(engine->i915) |
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index c9337e4b5ee0..f094406dcc56 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1455,7 +1455,7 @@ static int ring_context_pin(struct intel_context *ce)
ce->state = vma;
}
- err = intel_context_active_acquire(ce, PIN_HIGH);
+ err = intel_context_active_acquire(ce);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 44273b7c96f8..478258274986 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -146,6 +146,15 @@ static void __cacheline_retire(struct i915_active *active)
__idle_cacheline_free(cl);
}
+static int __cacheline_active(struct i915_active *active)
+{
+ struct intel_timeline_cacheline *cl =
+ container_of(active, typeof(*cl), active);
+
+ __i915_vma_pin(cl->hwsp->vma);
+ return 0;
+}
+
static struct intel_timeline_cacheline *
cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
{
@@ -168,15 +177,16 @@ cacheline_alloc(struct intel_timeline_hwsp *hwsp, unsigned int cacheline)
cl->hwsp = hwsp;
cl->vaddr = page_pack_bits(vaddr, cacheline);
- i915_active_init(hwsp->gt->i915, &cl->active, __cacheline_retire);
+ i915_active_init(hwsp->gt->i915, &cl->active,
+ __cacheline_active, __cacheline_retire);
return cl;
}
static void cacheline_acquire(struct intel_timeline_cacheline *cl)
{
- if (cl && i915_active_acquire(&cl->active))
- __i915_vma_pin(cl->hwsp->vma);
+ if (cl)
+ i915_active_acquire(&cl->active);
}
static void cacheline_release(struct intel_timeline_cacheline *cl)
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index bf0974b12f3d..490ebd121f4c 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -155,7 +155,7 @@ static int mock_context_pin(struct intel_context *ce)
return -ENOMEM;
}
- ret = intel_context_active_acquire(ce, PIN_HIGH);
+ ret = intel_context_active_acquire(ce);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index eb91a625c71f..cb6a1eadf7df 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -39,7 +39,7 @@ static void *active_debug_hint(void *addr)
{
struct i915_active *ref = addr;
- return (void *)ref->retire ?: (void *)ref;
+ return (void *)ref->active ?: (void *)ref->retire ?: (void *)ref;
}
static struct debug_obj_descr active_debug_desc = {
@@ -83,50 +83,58 @@ static inline void debug_active_assert(struct i915_active *ref) { }
#endif
static void
-__active_park(struct i915_active *ref)
+__active_retire(struct i915_active *ref)
{
struct active_node *it, *n;
+ struct rb_root root;
+ bool retire = false;
+
+ lockdep_assert_held(&ref->mutex);
+
+ /* return the unused nodes to our slabcache -- flushing the allocator */
+ if (atomic_dec_and_test(&ref->count)) {
+ debug_active_deactivate(ref);
+ root = ref->tree;
+ ref->tree = RB_ROOT;
+ ref->cache = NULL;
+ retire = true;
+ }
- rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
+ mutex_unlock(&ref->mutex);
+ if (!retire)
+ return;
+
+ ref->retire(ref);
+
+ rbtree_postorder_for_each_entry_safe(it, n, &root, node) {
GEM_BUG_ON(i915_active_request_isset(&it->base));
kmem_cache_free(global.slab_cache, it);
}
- ref->tree = RB_ROOT;
}
static void
-__active_retire(struct i915_active *ref)
+active_retire(struct i915_active *ref)
{
- GEM_BUG_ON(!ref->count);
- if (--ref->count)
+ GEM_BUG_ON(!atomic_read(&ref->count));
+ if (atomic_add_unless(&ref->count, -1, 1))
return;
- debug_active_deactivate(ref);
-
- /* return the unused nodes to our slabcache */
- __active_park(ref);
-
- ref->retire(ref);
+ /* One active may be flushed from inside the acquire of another */
+ mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
+ __active_retire(ref);
}
static void
node_retire(struct i915_active_request *base, struct i915_request *rq)
{
- __active_retire(container_of(base, struct active_node, base)->ref);
-}
-
-static void
-last_retire(struct i915_active_request *base, struct i915_request *rq)
-{
- __active_retire(container_of(base, struct i915_active, last));
+ active_retire(container_of(base, struct active_node, base)->ref);
}
static struct i915_active_request *
active_instance(struct i915_active *ref, u64 idx)
{
- struct active_node *node;
+ struct active_node *node, *prealloc;
struct rb_node **p, *parent;
- struct i915_request *old;
/*
* We track the most recently used timeline to skip a rbtree search
@@ -134,20 +142,18 @@ active_instance(struct i915_active *ref, u64 idx)
* at all. We can reuse the last slot if it is empty, that is
* after the previous activity has been retired, or if it matches the
* current timeline.
- *
- * Note that we allow the timeline to be active simultaneously in
- * the rbtree and the last cache. We do this to avoid having
- * to search and replace the rbtree element for a new timeline, with
- * the cost being that we must be aware that the ref may be retired
- * twice for the same timeline (as the older rbtree element will be
- * retired before the new request added to last).
*/
- old = i915_active_request_raw(&ref->last, BKL(ref));
- if (!old || old->fence.context == idx)
- goto out;
+ node = READ_ONCE(ref->cache);
+ if (node && node->timeline == idx)
+ return &node->base;
- /* Move the currently active fence into the rbtree */
- idx = old->fence.context;
+ /* Preallocate a replacement, just in case */
+ prealloc = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
+ if (!prealloc)
+ return NULL;
+
+ mutex_lock(&ref->mutex);
+ GEM_BUG_ON(i915_active_is_idle(ref));
parent = NULL;
p = &ref->tree.rb_node;
@@ -155,8 +161,10 @@ active_instance(struct i915_active *ref, u64 idx)
parent = *p;
node = rb_entry(parent, struct active_node, node);
- if (node->timeline == idx)
- goto replace;
+ if (node->timeline == idx) {
+ kmem_cache_free(global.slab_cache, prealloc);
+ goto out;
+ }
if (node->timeline < idx)
p = &parent->rb_right;
@@ -164,17 +172,7 @@ active_instance(struct i915_active *ref, u64 idx)
p = &parent->rb_left;
}
- node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
-
- /* kmalloc may retire the ref->last (thanks shrinker)! */
- if (unlikely(!i915_active_request_raw(&ref->last, BKL(ref)))) {
- kmem_cache_free(global.slab_cache, node);
- goto out;
- }
-
- if (unlikely(!node))
- return ERR_PTR(-ENOMEM);
-
+ node = prealloc;
i915_active_request_init(&node->base, NULL, node_retire);
node->ref = ref;
node->timeline = idx;
@@ -182,40 +180,29 @@ active_instance(struct i915_active *ref, u64 idx)
rb_link_node(&node->node, parent, p);
rb_insert_color(&node->node, &ref->tree);
-replace:
- /*
- * Overwrite the previous active slot in the rbtree with last,
- * leaving last zeroed. If the previous slot is still active,
- * we must be careful as we now only expect to receive one retire
- * callback not two, and so much undo the active counting for the
- * overwritten slot.
- */
- if (i915_active_request_isset(&node->base)) {
- /* Retire ourselves from the old rq->active_list */
- __list_del_entry(&node->base.link);
- ref->count--;
- GEM_BUG_ON(!ref->count);
- }
- GEM_BUG_ON(list_empty(&ref->last.link));
- list_replace_init(&ref->last.link, &node->base.link);
- node->base.request = fetch_and_zero(&ref->last.request);
-
out:
- return &ref->last;
+ ref->cache = node;
+ mutex_unlock(&ref->mutex);
+
+ return &node->base;
}
-void i915_active_init(struct drm_i915_private *i915,
- struct i915_active *ref,
- void (*retire)(struct i915_active *ref))
+void __i915_active_init(struct drm_i915_private *i915,
+ struct i915_active *ref,
+ int (*active)(struct i915_active *ref),
+ void (*retire)(struct i915_active *ref),
+ struct lock_class_key *key)
{
debug_active_init(ref);
ref->i915 = i915;
+ ref->active = active;
ref->retire = retire;
ref->tree = RB_ROOT;
- i915_active_request_init(&ref->last, NULL, last_retire);
+ ref->cache = NULL;
init_llist_head(&ref->barriers);
- ref->count = 0;
+ atomic_set(&ref->count, 0);
+ __mutex_init(&ref->mutex, "i915_active", key);
}
int i915_active_ref(struct i915_active *ref,
@@ -223,68 +210,84 @@ int i915_active_ref(struct i915_active *ref,
struct i915_request *rq)
{
struct i915_active_request *active;
- int err = 0;
+ int err;
/* Prevent reaping in case we malloc/wait while building the tree */
- i915_active_acquire(ref);
+ err = i915_active_acquire(ref);
+ if (err)
+ return err;
active = active_instance(ref, timeline);
- if (IS_ERR(active)) {
- err = PTR_ERR(active);
+ if (!active) {
+ err = -ENOMEM;
goto out;
}
if (!i915_active_request_isset(active))
- ref->count++;
+ atomic_inc(&ref->count);
__i915_active_request_set(active, rq);
- GEM_BUG_ON(!ref->count);
out:
i915_active_release(ref);
return err;
}
-bool i915_active_acquire(struct i915_active *ref)
+int i915_active_acquire(struct i915_active *ref)
{
+ int err;
+
debug_active_assert(ref);
- lockdep_assert_held(BKL(ref));
+ if (atomic_add_unless(&ref->count, 1, 0))
+ return 0;
+
+ err = mutex_lock_interruptible(&ref->mutex);
+ if (err)
+ return err;
- if (ref->count++)
- return false;
+ if (!atomic_read(&ref->count) && ref->active)
+ err = ref->active(ref);
+ if (!err) {
+ debug_active_activate(ref);
+ atomic_inc(&ref->count);
+ }
+
+ mutex_unlock(&ref->mutex);
- debug_active_activate(ref);
- return true;
+ return err;
}
void i915_active_release(struct i915_active *ref)
{
debug_active_assert(ref);
- lockdep_assert_held(BKL(ref));
-
- __active_retire(ref);
+ active_retire(ref);
}
int i915_active_wait(struct i915_active *ref)
{
struct active_node *it, *n;
- int ret = 0;
+ int err;
- if (i915_active_acquire(ref))
- goto out_release;
+ might_sleep();
+ if (RB_EMPTY_ROOT(&ref->tree))
+ return 0;
- ret = i915_active_request_retire(&ref->last, BKL(ref));
- if (ret)
- goto out_release;
+ err = mutex_lock_interruptible(&ref->mutex);
+ if (err)
+ return err;
+
+ if (!atomic_add_unless(&ref->count, 1, 0)) {
+ mutex_unlock(&ref->mutex);
+ return 0;
+ }
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
- ret = i915_active_request_retire(&it->base, BKL(ref));
- if (ret)
+ err = i915_active_request_retire(&it->base, BKL(ref));
+ if (err)
break;
}
-out_release:
- i915_active_release(ref);
- return ret;
+ __active_retire(ref);
+ return err;
}
int i915_request_await_active_request(struct i915_request *rq,
@@ -299,23 +302,24 @@ int i915_request_await_active_request(struct i915_request *rq,
int i915_request_await_active(struct i915_request *rq, struct i915_active *ref)
{
struct active_node *it, *n;
- int err = 0;
+ int err;
- /* await allocates and so we need to avoid hitting the shrinker */
- if (i915_active_acquire(ref))
- goto out; /* was idle */
+ if (RB_EMPTY_ROOT(&ref->tree))
+ return 0;
- err = i915_request_await_active_request(rq, &ref->last);
+ /* await allocates and so we need to avoid hitting the shrinker */
+ err = i915_active_acquire(ref);
if (err)
- goto out;
+ return err;
+ mutex_lock(&ref->mutex);
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
err = i915_request_await_active_request(rq, &it->base);
if (err)
- goto out;
+ break;
}
+ mutex_unlock(&ref->mutex);
-out:
i915_active_release(ref);
return err;
}
@@ -324,9 +328,9 @@ int i915_request_await_active(struct i915_request *rq, struct i915_active *ref)
void i915_active_fini(struct i915_active *ref)
{
debug_active_fini(ref);
- GEM_BUG_ON(i915_active_request_isset(&ref->last));
GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree));
- GEM_BUG_ON(ref->count);
+ GEM_BUG_ON(atomic_read(&ref->count));
+ mutex_destroy(&ref->mutex);
}
#endif
@@ -353,7 +357,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
(void *)engine, node_retire);
node->timeline = kctx->ring->timeline->fence_context;
node->ref = ref;
- ref->count++;
+ atomic_inc(&ref->count);
intel_engine_pm_get(engine);
llist_add((struct llist_node *)&node->base.link,
@@ -380,8 +384,9 @@ void i915_active_acquire_barrier(struct i915_active *ref)
{
struct llist_node *pos, *next;
- i915_active_acquire(ref);
+ GEM_BUG_ON(i915_active_is_idle(ref));
+ 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;
@@ -411,7 +416,7 @@ void i915_active_acquire_barrier(struct i915_active *ref)
&engine->barrier_tasks);
intel_engine_pm_put(engine);
}
- i915_active_release(ref);
+ mutex_unlock(&ref->mutex);
}
void i915_request_add_barriers(struct i915_request *rq)
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index c14eebf6d074..134166d31251 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -369,9 +369,16 @@ i915_active_request_retire(struct i915_active_request *active,
* synchronisation.
*/
-void i915_active_init(struct drm_i915_private *i915,
- struct i915_active *ref,
- void (*retire)(struct i915_active *ref));
+void __i915_active_init(struct drm_i915_private *i915,
+ struct i915_active *ref,
+ int (*active)(struct i915_active *ref),
+ void (*retire)(struct i915_active *ref),
+ struct lock_class_key *key);
+#define i915_active_init(i915, ref, active, retire) do { \
+ static struct lock_class_key __key; \
+ \
+ __i915_active_init(i915, ref, active, retire, &__key); \
+} while (0)
int i915_active_ref(struct i915_active *ref,
u64 timeline,
@@ -384,20 +391,14 @@ int i915_request_await_active(struct i915_request *rq,
int i915_request_await_active_request(struct i915_request *rq,
struct i915_active_request *active);
-bool i915_active_acquire(struct i915_active *ref);
-
-static inline void i915_active_cancel(struct i915_active *ref)
-{
- GEM_BUG_ON(ref->count != 1);
- ref->count = 0;
-}
-
+int i915_active_acquire(struct i915_active *ref);
void i915_active_release(struct i915_active *ref);
+void __i915_active_release_nested(struct i915_active *ref, int subclass);
static inline bool
i915_active_is_idle(const struct i915_active *ref)
{
- return !ref->count;
+ return !atomic_read(&ref->count);
}
#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index c025991b9233..5b0a3024ce24 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -7,7 +7,9 @@
#ifndef _I915_ACTIVE_TYPES_H_
#define _I915_ACTIVE_TYPES_H_
+#include <linux/atomic.h>
#include <linux/llist.h>
+#include <linux/mutex.h>
#include <linux/rbtree.h>
#include <linux/rcupdate.h>
@@ -24,13 +26,17 @@ struct i915_active_request {
i915_active_retire_fn retire;
};
+struct active_node;
+
struct i915_active {
struct drm_i915_private *i915;
+ struct active_node *cache;
struct rb_root tree;
- struct i915_active_request last;
- unsigned int count;
+ struct mutex mutex;
+ atomic_t count;
+ int (*active)(struct i915_active *ref);
void (*retire)(struct i915_active *ref);
struct llist_head barriers;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 90f367397656..ff1d5008a256 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2055,7 +2055,7 @@ static struct i915_vma *pd_vma_create(struct gen6_ppgtt *ppgtt, int size)
if (!vma)
return ERR_PTR(-ENOMEM);
- i915_active_init(i915, &vma->active, NULL);
+ i915_active_init(i915, &vma->active, NULL, NULL);
INIT_ACTIVE_REQUEST(&vma->last_fence);
vma->vm = &ggtt->vm;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index c13b86e6ef1f..c20a3022cd80 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -78,11 +78,20 @@ static void vma_print_allocator(struct i915_vma *vma, const char *reason)
#endif
-static void __i915_vma_retire(struct i915_active *ref)
+static inline struct i915_vma *active_to_vma(struct i915_active *ref)
{
- struct i915_vma *vma = container_of(ref, typeof(*vma), active);
+ return container_of(ref, typeof(struct i915_vma), active);
+}
- i915_vma_put(vma);
+static int __i915_vma_active(struct i915_active *ref)
+{
+ i915_vma_get(active_to_vma(ref));
+ return 0;
+}
+
+static void __i915_vma_retire(struct i915_active *ref)
+{
+ i915_vma_put(active_to_vma(ref));
}
static struct i915_vma *
@@ -107,7 +116,8 @@ vma_create(struct drm_i915_gem_object *obj,
vma->size = obj->base.size;
vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
- i915_active_init(vm->i915, &vma->active, __i915_vma_retire);
+ i915_active_init(vm->i915, &vma->active,
+ __i915_vma_active, __i915_vma_retire);
INIT_ACTIVE_REQUEST(&vma->last_fence);
INIT_LIST_HEAD(&vma->closed_link);
@@ -904,11 +914,7 @@ int i915_vma_move_to_active(struct i915_vma *vma,
* add the active reference first and queue for it to be dropped
* *last*.
*/
- if (i915_active_acquire(&vma->active))
- i915_vma_get(vma);
-
err = i915_active_ref(&vma->active, rq->fence.context, rq);
- i915_active_release(&vma->active);
if (unlikely(err))
return err;
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index 98493bcc91f2..84fce379c0de 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -4,6 +4,8 @@
* Copyright © 2018 Intel Corporation
*/
+#include <linux/kref.h>
+
#include "gem/i915_gem_pm.h"
#include "i915_selftest.h"
@@ -13,20 +15,47 @@
struct live_active {
struct i915_active base;
+ struct kref ref;
bool retired;
};
+static void __live_get(struct live_active *active)
+{
+ kref_get(&active->ref);
+}
+
static void __live_free(struct live_active *active)
{
i915_active_fini(&active->base);
kfree(active);
}
+static void __live_release(struct kref *ref)
+{
+ struct live_active *active = container_of(ref, typeof(*active), ref);
+
+ __live_free(active);
+}
+
+static void __live_put(struct live_active *active)
+{
+ kref_put(&active->ref, __live_release);
+}
+
+static int __live_active(struct i915_active *base)
+{
+ struct live_active *active = container_of(base, typeof(*active), base);
+
+ __live_get(active);
+ return 0;
+}
+
static void __live_retire(struct i915_active *base)
{
struct live_active *active = container_of(base, typeof(*active), base);
active->retired = true;
+ __live_put(active);
}
static struct live_active *__live_alloc(struct drm_i915_private *i915)
@@ -37,7 +66,8 @@ static struct live_active *__live_alloc(struct drm_i915_private *i915)
if (!active)
return NULL;
- i915_active_init(i915, &active->base, __live_retire);
+ kref_init(&active->ref);
+ i915_active_init(i915, &active->base, __live_active, __live_retire);
return active;
}
@@ -62,11 +92,9 @@ __live_active_setup(struct drm_i915_private *i915)
return ERR_PTR(-ENOMEM);
}
- if (!i915_active_acquire(&active->base)) {
- pr_err("First i915_active_acquire should report being idle\n");
- err = -EINVAL;
+ err = i915_active_acquire(&active->base);
+ if (err)
goto out;
- }
for_each_engine(engine, i915, id) {
struct i915_request *rq;
@@ -97,18 +125,21 @@ __live_active_setup(struct drm_i915_private *i915)
pr_err("i915_active retired before submission!\n");
err = -EINVAL;
}
- if (active->base.count != count) {
+ if (atomic_read(&active->base.count) != count) {
pr_err("i915_active not tracking all requests, found %d, expected %d\n",
- active->base.count, count);
+ atomic_read(&active->base.count), count);
err = -EINVAL;
}
out:
i915_sw_fence_commit(submit);
heap_fence_put(submit);
+ if (err) {
+ __live_put(active);
+ active = ERR_PTR(err);
+ }
- /* XXX leaks live_active on error */
- return err ? ERR_PTR(err) : active;
+ return active;
}
static int live_active_wait(void *arg)
@@ -135,7 +166,7 @@ static int live_active_wait(void *arg)
err = -EINVAL;
}
- __live_free(active);
+ __live_put(active);
if (igt_flush_test(i915, I915_WAIT_LOCKED))
err = -EIO;
@@ -174,7 +205,7 @@ static int live_active_retire(void *arg)
err = -EINVAL;
}
- __live_free(active);
+ __live_put(active);
err:
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
