[Intel-gfx] [PATCH 10/29] drm/i915: Introduce context->enter() and context->exit()
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Wed Apr 10 10:05:13 UTC 2019
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 at chris-wilson.co.uk>
> ---
> 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
More information about the Intel-gfx
mailing list