On 08/04/2019 10:17, Chris Wilson wrote:
We wish to start segregating the power management into different control
domains, both with respect to the hardware and the user interface. The
first step is that at the lowest level flow of requests, we want to
process a context event (and not a global GEM operation). In this patch,
we introduce the context callbacks that in future patches will be
redirected to per-engine interfaces leading to global operations as
required.
The intent is that this will be guarded by the timeline->mutex, except
that retiring has not quite finished transitioning over from being
guarded by struct_mutex. So at the moment it is protected by
struct_mutex with a reminded to switch.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_context.c | 17 ++++++++++++++
drivers/gpu/drm/i915/gt/intel_context.h | 20 +++++++++++++++++
drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++++
drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +++
drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 3 +++
drivers/gpu/drm/i915/gt/mock_engine.c | 3 +++
drivers/gpu/drm/i915/i915_request.c | 22 ++++---------------
7 files changed, 55 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index ebd1e5919a4a..bf16f00a10a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -266,3 +266,20 @@ int __init i915_global_context_init(void)
i915_global_register(&global.base);
return 0;
}
+
+void __intel_context_enter(struct intel_context *ce)
+{
+ struct drm_i915_private *i915 = ce->gem_context->i915;
+
+ if (!i915->gt.active_requests++)
+ i915_gem_unpark(i915);
+}
+
+void __intel_context_exit(struct intel_context *ce)
+{
+ struct drm_i915_private *i915 = ce->gem_context->i915;
+
+ GEM_BUG_ON(!i915->gt.active_requests);
+ if (!--i915->gt.active_requests)
+ i915_gem_park(i915);
+}
In our normal nomenclature __intel_context_enter would be expected to be
directly called by intel_context_enter on the 0 -> 1 refcnt transition.
Here they are in fact common helpers, so one level of indirection, via a
different layer. I found this a bit confusing at first. My usual remedy
here is to prescribe a better name. __intel_gt_context_enter/exit? To
designate the glue between backend and GT.
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index ebc861b1a49e..95b1fdc5826a 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -73,6 +73,26 @@ static inline void __intel_context_pin(struct intel_context *ce)
void intel_context_unpin(struct intel_context *ce);
+void __intel_context_enter(struct intel_context *ce);
+void __intel_context_exit(struct intel_context *ce);
+
+static inline void intel_context_enter(struct intel_context *ce)
+{
+ if (!ce->active_count++)
+ ce->ops->enter(ce);
+}
+
+static inline void intel_context_mark_active(struct intel_context *ce)
+{
GEM_BUG_ON(ce->active_count == 0) ?
And some lockdep_assert_held in all three?
+ ++ce->active_count;
+}
+
+static inline void intel_context_exit(struct intel_context *ce)
+{
+ if (!--ce->active_count)
+ ce->ops->exit(ce);
+}
+
static inline struct intel_context *intel_context_get(struct intel_context *ce)
{
kref_get(&ce->ref);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 9ec4f787c908..f02d27734e3b 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -25,6 +25,9 @@ struct intel_context_ops {
int (*pin)(struct intel_context *ce);
void (*unpin)(struct intel_context *ce);
+ void (*enter)(struct intel_context *ce);
+ void (*exit)(struct intel_context *ce);
+
void (*reset)(struct intel_context *ce);
void (*destroy)(struct kref *kref);
};
@@ -46,6 +49,8 @@ struct intel_context {
u32 *lrc_reg_state;
u64 lrc_desc;
+ unsigned int active_count; /* notionally protected by timeline->mutex */
+
atomic_t pin_count;
struct mutex pin_mutex; /* guards pinning and associated on-gpuing */
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 1565b11b15f6..7da5e49dd385 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1404,6 +1404,9 @@ static const struct intel_context_ops execlists_context_ops = {
.pin = execlists_context_pin,
.unpin = execlists_context_unpin,
+ .enter = __intel_context_enter,
+ .exit = __intel_context_exit,
+
.reset = execlists_context_reset,
.destroy = execlists_context_destroy,
};
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index bcc842b8bbe7..5788e5ab818f 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1516,6 +1516,9 @@ static const struct intel_context_ops ring_context_ops = {
.pin = ring_context_pin,
.unpin = ring_context_unpin,
+ .enter = __intel_context_enter,
+ .exit = __intel_context_exit,
+
.reset = ring_context_reset,
.destroy = ring_context_destroy,
};
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index 414afd2f27fe..2e84ffb46489 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -157,6 +157,9 @@ static const struct intel_context_ops mock_context_ops = {
.pin = mock_context_pin,
.unpin = mock_context_unpin,
+ .enter = __intel_context_enter,
+ .exit = __intel_context_exit,
+
.destroy = mock_context_destroy,
};
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index f2b9e0e1910b..e72e4087bbfe 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -130,19 +130,6 @@ i915_request_remove_from_client(struct i915_request *request)
spin_unlock(&file_priv->mm.lock);
}
-static void reserve_gt(struct drm_i915_private *i915)
-{
- if (!i915->gt.active_requests++)
- i915_gem_unpark(i915);
-}
-
-static void unreserve_gt(struct drm_i915_private *i915)
-{
- GEM_BUG_ON(!i915->gt.active_requests);
- if (!--i915->gt.active_requests)
- i915_gem_park(i915);
-}
-
static void advance_ring(struct i915_request *request)
{
struct intel_ring *ring = request->ring;
@@ -300,11 +287,10 @@ static void i915_request_retire(struct i915_request *request)
i915_request_remove_from_client(request);
- intel_context_unpin(request->hw_context);
-
__retire_engine_upto(request->engine, request);
- unreserve_gt(request->i915);
+ intel_context_unpin(request->hw_context);
+ intel_context_exit(request->hw_context);
i915_sched_node_fini(&request->sched);
i915_request_put(request);
@@ -628,8 +614,8 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
if (IS_ERR(ce))
return ERR_CAST(ce);
- reserve_gt(i915);
mutex_lock(&ce->ring->timeline->mutex);
+ intel_context_enter(ce);
/* Move our oldest request to the slab-cache (if not in use!) */
rq = list_first_entry(&ce->ring->request_list, typeof(*rq), ring_link);
@@ -759,8 +745,8 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
err_free:
kmem_cache_free(global.slab_requests, rq);
err_unreserve:
+ intel_context_exit(ce);
mutex_unlock(&ce->ring->timeline->mutex);
- unreserve_gt(i915);
intel_context_unpin(ce);
return ERR_PTR(ret);
}
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
