[PATCH 2/2] drm/i915: Try to move over intel_context completely to i915_active
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Wed Jun 30 11:10:52 UTC 2021
Remove the double refcount and pin_mutex locking, we could do it from a single spot.
This slightly changes ops, unpin() may be called after post_unpin(), it's only used to
prepare pinning now to be called without ww. Might need rename, shrug..
Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 8 +++
drivers/gpu/drm/i915/gt/gen6_ppgtt.h | 1 +
drivers/gpu/drm/i915/gt/intel_context.c | 61 ++++++-------------
drivers/gpu/drm/i915/gt/intel_context.h | 26 +++++---
drivers/gpu/drm/i915/gt/intel_context_sseu.c | 2 +-
drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++
.../gpu/drm/i915/gt/intel_ring_submission.c | 7 +++
9 files changed, 57 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7720b8c22c81..55bde628cf18 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1266,7 +1266,7 @@ static bool skip_ppgtt_update(struct intel_context *ce, void *data)
if (HAS_LOGICAL_RING_CONTEXTS(ce->engine->i915))
return !ce->state;
else
- return !atomic_read(&ce->pin_count);
+ return !intel_context_is_pinned(ce);
}
static int set_ppgtt(struct drm_i915_file_private *file_priv,
diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index 1aee5e6b1b23..185935c3ef35 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -362,6 +362,14 @@ static struct i915_vma *pd_vma_create(struct gen6_ppgtt *ppgtt, int size)
return vma;
}
+void __gen6_ppgtt_pin(struct i915_ppgtt *base)
+{
+ struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(base);
+
+ GEM_BUG_ON(!atomic_read(&ppgtt->pin_count));
+ atomic_inc(&ppgtt->pin_count);
+}
+
int gen6_ppgtt_pin(struct i915_ppgtt *base, struct i915_gem_ww_ctx *ww)
{
struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(base);
diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.h b/drivers/gpu/drm/i915/gt/gen6_ppgtt.h
index 6a61a5c3a85a..1f25c7a7329f 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.h
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.h
@@ -70,6 +70,7 @@ static inline struct gen6_ppgtt *to_gen6_ppgtt(struct i915_ppgtt *base)
++iter)
int gen6_ppgtt_pin(struct i915_ppgtt *base, struct i915_gem_ww_ctx *ww);
+void __gen6_ppgtt_pin(struct i915_ppgtt *base);
void gen6_ppgtt_unpin(struct i915_ppgtt *base);
void gen6_ppgtt_unpin_all(struct i915_ppgtt *base);
void gen6_ppgtt_enable(struct intel_gt *gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 4033184f13b9..80cfec029c71 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -53,7 +53,7 @@ int intel_context_alloc_state(struct intel_context *ce)
{
int err = 0;
- if (mutex_lock_interruptible(&ce->pin_mutex))
+ if (mutex_lock_interruptible(&ce->active.mutex))
return -EINTR;
if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
@@ -70,7 +70,7 @@ int intel_context_alloc_state(struct intel_context *ce)
}
unlock:
- mutex_unlock(&ce->pin_mutex);
+ mutex_unlock(&ce->active.mutex);
return err;
}
@@ -203,8 +203,6 @@ static void intel_context_post_unpin(struct intel_context *ce)
int __intel_context_do_pin_ww(struct intel_context *ce,
struct i915_gem_ww_ctx *ww)
{
- bool handoff = false;
- void *vaddr;
int err = 0;
if (unlikely(!test_bit(CONTEXT_ALLOC_BIT, &ce->flags))) {
@@ -229,52 +227,29 @@ int __intel_context_do_pin_ww(struct intel_context *ce,
if (err)
return err;
- err = i915_active_acquire(&ce->active);
+ err = ce->ops->pre_pin(ce, ww, &ce->vaddr);
if (err)
goto err_ctx_unpin;
- err = ce->ops->pre_pin(ce, ww, &vaddr);
- if (err)
- goto err_release;
-
- err = mutex_lock_interruptible(&ce->pin_mutex);
+ err = i915_active_acquire(&ce->active);
if (err)
- goto err_post_unpin;
+ goto err_ctx_unpin;
if (unlikely(intel_context_is_closed(ce))) {
err = -ENOENT;
- goto err_unlock;
+ goto err_release;
}
- if (likely(!atomic_add_unless(&ce->pin_count, 1, 0))) {
- err = intel_context_active_acquire(ce);
- if (unlikely(err))
- goto err_unlock;
-
- err = ce->ops->pin(ce, vaddr);
- if (err) {
- intel_context_active_release(ce);
- goto err_unlock;
- }
-
- CE_TRACE(ce, "pin ring:{start:%08x, head:%04x, tail:%04x}\n",
- i915_ggtt_offset(ce->ring->vma),
- ce->ring->head, ce->ring->tail);
-
- handoff = true;
- smp_mb__before_atomic(); /* flush pin before it is visible */
- atomic_inc(&ce->pin_count);
- }
+ err = intel_context_active_acquire(ce);
+ if (unlikely(err))
+ goto err_release;
GEM_BUG_ON(!intel_context_is_pinned(ce)); /* no overflow! */
-err_unlock:
- mutex_unlock(&ce->pin_mutex);
-err_post_unpin:
- if (!handoff)
- ce->ops->post_unpin(ce);
err_release:
i915_active_release(&ce->active);
+err_post_unpin:
+ ce->ops->post_unpin(ce);
err_ctx_unpin:
intel_context_post_unpin(ce);
@@ -308,12 +283,7 @@ int __intel_context_do_pin(struct intel_context *ce)
void intel_context_unpin(struct intel_context *ce)
{
- if (!atomic_dec_and_test(&ce->pin_count))
- return;
-
CE_TRACE(ce, "unpin\n");
- ce->ops->unpin(ce);
- ce->ops->post_unpin(ce);
/*
* Once released, we may asynchronously drop the active reference.
@@ -335,6 +305,7 @@ static void __intel_context_retire(struct i915_active *active)
intel_context_get_avg_runtime_ns(ce));
set_bit(CONTEXT_VALID_BIT, &ce->flags);
+ ce->ops->unpin(ce);
intel_context_post_unpin(ce);
intel_context_put(ce);
}
@@ -342,6 +313,11 @@ static void __intel_context_retire(struct i915_active *active)
static int __intel_context_active(struct i915_active *active)
{
struct intel_context *ce = container_of(active, typeof(*ce), active);
+ int err;
+
+ err = ce->ops->pin(ce, ce->vaddr);
+ if (err)
+ return err;
intel_context_get(ce);
@@ -381,8 +357,6 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine)
spin_lock_init(&ce->signal_lock);
INIT_LIST_HEAD(&ce->signals);
- mutex_init(&ce->pin_mutex);
-
i915_active_init(&ce->active,
__intel_context_active, __intel_context_retire, 0);
}
@@ -393,7 +367,6 @@ void intel_context_fini(struct intel_context *ce)
intel_timeline_put(ce->timeline);
i915_vm_put(ce->vm);
- mutex_destroy(&ce->pin_mutex);
i915_active_fini(&ce->active);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index f83a73a2b39f..9bed8cd387d5 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -49,9 +49,9 @@ int intel_context_reconfigure_sseu(struct intel_context *ce,
* intel_context_is_pinned() remains stable.
*/
static inline int intel_context_lock_pinned(struct intel_context *ce)
- __acquires(ce->pin_mutex)
+ __acquires(ce->active.mutex)
{
- return mutex_lock_interruptible(&ce->pin_mutex);
+ return mutex_lock_interruptible(&ce->active.mutex);
}
/**
@@ -66,7 +66,7 @@ static inline int intel_context_lock_pinned(struct intel_context *ce)
static inline bool
intel_context_is_pinned(struct intel_context *ce)
{
- return atomic_read(&ce->pin_count);
+ return !i915_active_is_idle(&ce->active);
}
/**
@@ -76,9 +76,9 @@ intel_context_is_pinned(struct intel_context *ce)
* Releases the lock earlier acquired by intel_context_unlock_pinned().
*/
static inline void intel_context_unlock_pinned(struct intel_context *ce)
- __releases(ce->pin_mutex)
+ __releases(ce->active.mutex)
{
- mutex_unlock(&ce->pin_mutex);
+ mutex_unlock(&ce->active.mutex);
}
int __intel_context_do_pin(struct intel_context *ce);
@@ -87,13 +87,20 @@ int __intel_context_do_pin_ww(struct intel_context *ce,
static inline bool intel_context_pin_if_active(struct intel_context *ce)
{
- return atomic_inc_not_zero(&ce->pin_count);
+ return i915_active_acquire_if_busy(&ce->active);
}
static inline int intel_context_pin(struct intel_context *ce)
{
- if (likely(intel_context_pin_if_active(ce)))
- return 0;
+ if (likely(intel_context_pin_if_active(ce))) {
+ int err = i915_active_acquire_preallocate_barrier(&ce->active,
+ ce->engine);
+
+ if (err)
+ i915_active_release(&ce->active);
+
+ return err;
+ }
return __intel_context_do_pin(ce);
}
@@ -110,11 +117,10 @@ static inline int intel_context_pin_ww(struct intel_context *ce,
static inline void __intel_context_pin(struct intel_context *ce)
{
GEM_BUG_ON(!intel_context_is_pinned(ce));
- atomic_inc(&ce->pin_count);
+ __i915_active_acquire(&ce->active);
}
void intel_context_unpin(struct intel_context *ce);
-
void intel_context_enter_engine(struct intel_context *ce);
void intel_context_exit_engine(struct intel_context *ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_sseu.c b/drivers/gpu/drm/i915/gt/intel_context_sseu.c
index e86d8255feec..2b72a8c15b7c 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_context_sseu.c
@@ -42,7 +42,7 @@ gen8_modify_rpcs(struct intel_context *ce, const struct intel_sseu sseu)
struct i915_request *rq;
int ret;
- lockdep_assert_held(&ce->pin_mutex);
+ lockdep_assert_held(&ce->active.mutex);
/*
* If the context is not idle, we have to submit an ordered request to
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index ed8c447a7346..fd72e8517523 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -121,15 +121,14 @@ struct intel_context {
unsigned int active_count; /* protected by timeline->mutex */
- atomic_t pin_count;
- struct mutex pin_mutex; /* guards pinning and associated on-gpuing */
-
/**
* active: Active tracker for the rq activity (inc. external) on this
* intel_context object.
*/
struct i915_active active;
+ void *vaddr;
+
const struct intel_context_ops *ops;
/** sseu: Control eu/slice partitioning */
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a27bac0a4bfb..4a0a6780c086 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -922,6 +922,8 @@ lrc_pin(struct intel_context *ce,
lrc_init_state(ce, engine, vaddr);
ce->lrc.lrca = lrc_update_regs(ce, engine, ce->ring->tail);
+
+ __i915_gem_object_pin_pages(ce->state->obj);
return 0;
}
@@ -929,6 +931,8 @@ void lrc_unpin(struct intel_context *ce)
{
check_redzone((void *)ce->lrc_reg_state - LRC_STATE_OFFSET,
ce->engine);
+
+ i915_gem_object_unpin_map(ce->state->obj);
}
void lrc_post_unpin(struct intel_context *ce)
diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
index 5d42a12ef3d6..8b1942942986 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
@@ -499,6 +499,7 @@ static void __context_unpin_ppgtt(struct intel_context *ce)
static void ring_context_unpin(struct intel_context *ce)
{
+ __context_unpin_ppgtt(ce);
}
static void ring_context_post_unpin(struct intel_context *ce)
@@ -574,6 +575,12 @@ static int ring_context_alloc(struct intel_context *ce)
static int ring_context_pin(struct intel_context *ce, void *unused)
{
+ struct i915_address_space *vm;
+
+ vm = vm_alias(ce->vm);
+ if (vm)
+ __gen6_ppgtt_pin(i915_vm_to_ppgtt((vm)));
+
return 0;
}
--
2.31.0
More information about the Intel-gfx-trybot
mailing list