[Intel-gfx] [PATCH] drm/i915: Unlock the shared hwsp_gtt object after pinning

[Chris Wilson]

Quoting Mika Kuoppala (2020-09-03 14:31:44)
> From: Thomas Hellström <thomas.hellstrom at intel.com>
> 
> The hwsp_gtt object is used for sub-allocation and could therefore
> be shared by many contexts causing unnecessary contention during
> concurrent context pinning.
> However since we're currently locking it only for pinning, it remains
> resident until we unpin it, and therefore it's safe to drop the
> lock early, allowing for concurrent thread access.
> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_context.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 61b05cd4c47a..65e956ba19e1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -271,6 +271,13 @@ int __intel_context_do_pin_ww(struct intel_context *ce,
>         i915_active_release(&ce->active);
>  err_ctx_unpin:
>         intel_context_post_unpin(ce);
> +
> +       /*
> +        * Unlock the hwsp_ggtt object since it's shared. This is fine for now
> +        * since the lock has been used for pinning only, not fencing.
> +        */
> +       i915_gem_ww_unlock_single(ce->timeline->hwsp_ggtt->obj);

The timeline is not special here, the same rules for locking/unlock can
equally be applied to all the global state. You may even go as far as
putting a local acquire context here, which would then have avoided the
cross driver pollution.

Anyway, if it works for the timeline, there is no reason to keep the
other globals locked. They are pinned and on completely different usage
cycles to the user objects. [They can't be evicted without interacting
with the HW to ensure the context has been flushed, so far there has
been no way to do so without stalling for a freshly submitted request,
hence we let them retire gracefully and kick the kernel timelines to
flush completed contexts.]
-Chris