Op 08-01-2020 om 13:23 schreef Chris Wilson:
> While this is encroaching on midlayer territory, having already made the
> state allocation a previous step in pinning, we can now pull the common
> intel_context_active_acquire() into intel_context_pin() itself. This is
> a prelude to make the activation a separate step inside pinning, outside
> of the ce->pin_mutex
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Yeah, hopefully with both we can finally fix the issue.
With that, for both patches:
Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> drivers/gpu/drm/i915/gt/intel_context.c | 64 ++++++++++---------
> drivers/gpu/drm/i915/gt/intel_context.h | 3 -
> drivers/gpu/drm/i915/gt/intel_lrc.c | 16 +----
> .../gpu/drm/i915/gt/intel_ring_submission.c | 16 +----
> drivers/gpu/drm/i915/gt/mock_engine.c | 2 +-
> 5 files changed, 39 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
> b/drivers/gpu/drm/i915/gt/intel_context.c
> index eefe0996ade1..85f161fd46f8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -63,6 +63,34 @@ int intel_context_alloc_state(struct intel_context *ce)
> return 0;
> }
>
> +static 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 (!intel_context_is_barrier(ce)) {
> + err = i915_active_acquire_preallocate_barrier(&ce->active,
> + ce->engine);
> + if (err) {
> + i915_active_release(&ce->active);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static 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_do_pin(struct intel_context *ce)
> {
> int err;
> @@ -79,11 +107,15 @@ int __intel_context_do_pin(struct intel_context *ce)
> if (likely(!atomic_read(&ce->pin_count))) {
> intel_wakeref_t wakeref;
>
> + err = intel_context_active_acquire(ce);
> + if (unlikely(err))
> + goto err;
> +
> err = 0;
> with_intel_runtime_pm(ce->engine->uncore->rpm, wakeref)
> err = ce->ops->pin(ce);
> if (err)
> - goto err;
> + goto err_active;
>
> CE_TRACE(ce, "pin ring:{head:%04x, tail:%04x}\n",
> ce->ring->head, ce->ring->tail);
> @@ -97,6 +129,8 @@ int __intel_context_do_pin(struct intel_context *ce)
> mutex_unlock(&ce->pin_mutex);
> return 0;
>
> +err_active:
> + intel_context_active_release(ce);
> err:
> mutex_unlock(&ce->pin_mutex);
> return err;
> @@ -198,34 +232,6 @@ static int __intel_context_active(struct i915_active
> *active)
> 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 (!intel_context_is_barrier(ce)) {
> - err = i915_active_acquire_preallocate_barrier(&ce->active,
> - ce->engine);
> - if (err) {
> - i915_active_release(&ce->active);
> - return err;
> - }
> - }
> -
> - return 0;
> -}
> -
> -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
> intel_context_init(struct intel_context *ce,
> struct intel_engine_cs *engine)
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h
> b/drivers/gpu/drm/i915/gt/intel_context.h
> index 673f5fb2967a..f0f49b50b968 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -118,9 +118,6 @@ static inline void intel_context_exit(struct
> intel_context *ce)
> ce->ops->exit(ce);
> }
>
> -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)
> {
> kref_get(&ce->ref);
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index bd74b76c6403..28b1fbcb4370 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2566,33 +2566,21 @@ __execlists_context_pin(struct intel_context *ce,
> struct intel_engine_cs *engine)
> {
> void *vaddr;
> - int ret;
>
> GEM_BUG_ON(!ce->state);
> -
> - 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) |
> I915_MAP_OVERRIDE);
> - if (IS_ERR(vaddr)) {
> - ret = PTR_ERR(vaddr);
> - goto unpin_active;
> - }
> + if (IS_ERR(vaddr))
> + return PTR_ERR(vaddr);
>
> ce->lrc_desc = lrc_descriptor(ce, engine) | CTX_DESC_FORCE_RESTORE;
> ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
> __execlists_update_reg_state(ce, engine);
>
> return 0;
> -
> -unpin_active:
> - intel_context_active_release(ce);
> -err:
> - return ret;
> }
>
> static int execlists_context_pin(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 eca5ab048cd9..bc44fe8e5ffa 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -1329,21 +1329,7 @@ static int ring_context_alloc(struct intel_context *ce)
>
> static int ring_context_pin(struct intel_context *ce)
> {
> - int err;
> -
> - err = intel_context_active_acquire(ce);
> - if (err)
> - return err;
> -
> - err = __context_pin_ppgtt(ce);
> - if (err)
> - goto err_active;
> -
> - return 0;
> -
> -err_active:
> - intel_context_active_release(ce);
> - return err;
> + return __context_pin_ppgtt(ce);
> }
>
> static void ring_context_reset(struct intel_context *ce)
> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c
> b/drivers/gpu/drm/i915/gt/mock_engine.c
> index d0e68ce9aa51..a560b7eee2cd 100644
> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> @@ -149,7 +149,7 @@ static int mock_context_alloc(struct intel_context *ce)
>
> static int mock_context_pin(struct intel_context *ce)
> {
> - return intel_context_active_acquire(ce);
> + return 0;
> }
>
> static void mock_context_reset(struct intel_context *ce)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
