[Intel-gfx] [PATCH] drm/i915/blt: Remove recursive vma->lock

[Matthew Auld]

On Tue, 18 Jun 2019 at 10:53, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
> As we have already plugged the w->dma into the reservation_object, and
> have set ourselves up to automatically signal the request and w->dma on
> completion, we do not need to export the rq->fence directly and just use
> the w->dma fence.
>
> This avoids having to take the reservation_lock inside the worker which
> cross-release lockdep would complain about. :)
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld at intel.com>
> ---
> Patch based on the i915_active overhaul, but on the face of it should be
> safe without. But let's pretend it does need the overhaul as this is one
> of the motivations :)
> -Chris
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_client_blt.c        | 10 ++++++----
>  .../gpu/drm/i915/gem/selftests/i915_gem_client_blt.c  | 11 -----------
>  2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
> index f253ec5765ad..83b5c5d13b93 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
> @@ -193,10 +193,12 @@ static void clear_pages_worker(struct work_struct *work)
>                         goto out_request;
>         }
>
> -       /* XXX: more feverish nightmares await */
> -       i915_vma_lock(vma);
> -       err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
> -       i915_vma_unlock(vma);
> +       /*
> +        * w->dma is already exported via (vma|obj)->resv we need only
> +        * keep track of the GPU activity within this vma/request, and
> +        * propagate the signal from the request to w->dma.
> +        */
> +       err = i915_active_ref(&vma->active, rq->fence.context, &rq->fence);
>         if (err)
>                 goto out_request;

Oh, wow ~o~

Yeah, seems to work locally with the extra ref + active count
tracking, so I guess should fit nicely with your i915_active.acquire,
Reviewed-by: Matthew Auld <matthew.auld at intel.com>