On Mon, 2022-11-07 at 00:52 -0800, Niranjana Vishwanathapura wrote:
> Asynchronously unbind the vma upon vm_unbind call.
> Fall back to synchronous unbind if backend doesn't support
> async unbind or if async unbind fails.
> 
> No need for vm_unbind out fence support as i915 will internally
> handle all sequencing and user need not try to sequence any
> operation with the unbind completion.

Can you please provide some more details on how this works from the
user space point of view? I want to be able to know with 100% certainty
if an unbind has already happened, so I can reuse that vma or whatever
else I may decide to do. I see the interface does not provide any sort
of drm_syncobjs for me to wait on the async unbind. So, when does the
unbind really happen? When can I be sure it's past so I can do stuff
with it? Why would you provide an async ioctl and provide no means for
user space to wait on it?

Thanks,
Paulo

> 
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathap...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vma.c | 51 ++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_vma.h |  1 +
>  2 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 08218e3a2f12..03c966fad87b 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -42,6 +42,8 @@
>  #include "i915_vma.h"
>  #include "i915_vma_resource.h"
>  
> 
> 
> 
> +static struct dma_fence *__i915_vma_unbind_async(struct i915_vma *vma);
> +
>  static inline void assert_vma_held_evict(const struct i915_vma *vma)
>  {
>       /*
> @@ -1711,7 +1713,7 @@ void i915_vma_reopen(struct i915_vma *vma)
>       spin_unlock_irq(&gt->closed_lock);
>  }
>  
> 
> 
> 
> -static void force_unbind(struct i915_vma *vma)
> +static void force_unbind(struct i915_vma *vma, bool async)
>  {
>       if (!drm_mm_node_allocated(&vma->node))
>               return;
> @@ -1725,7 +1727,21 @@ static void force_unbind(struct i915_vma *vma)
>               i915_vma_set_purged(vma);
>  
> 
> 
> 
>       atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
> -     WARN_ON(__i915_vma_unbind(vma));
> +     if (async) {
> +             struct dma_fence *fence;
> +
> +             fence = __i915_vma_unbind_async(vma);
> +             if (IS_ERR_OR_NULL(fence)) {
> +                     async = false;
> +             } else {
> +                     dma_resv_add_fence(vma->obj->base.resv, fence,
> +                                        DMA_RESV_USAGE_READ);
> +                     dma_fence_put(fence);
> +             }
> +     }
> +
> +     if (!async)
> +             WARN_ON(__i915_vma_unbind(vma));
>       GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
>  }
>  
> 
> 
> 
> @@ -1785,7 +1801,7 @@ void i915_vma_destroy_locked(struct i915_vma *vma)
>  {
>       lockdep_assert_held(&vma->vm->mutex);
>  
> 
> 
> 
> -     force_unbind(vma);
> +     force_unbind(vma, false);
>       list_del_init(&vma->vm_link);
>       release_references(vma, vma->vm->gt, false);
>  }
> @@ -1796,7 +1812,34 @@ void i915_vma_destroy(struct i915_vma *vma)
>       bool vm_ddestroy;
>  
> 
> 
> 
>       mutex_lock(&vma->vm->mutex);
> -     force_unbind(vma);
> +     force_unbind(vma, false);
> +     list_del_init(&vma->vm_link);
> +     vm_ddestroy = vma->vm_ddestroy;
> +     vma->vm_ddestroy = false;
> +
> +     /* vma->vm may be freed when releasing vma->vm->mutex. */
> +     gt = vma->vm->gt;
> +     mutex_unlock(&vma->vm->mutex);
> +     release_references(vma, gt, vm_ddestroy);
> +}
> +
> +void i915_vma_destroy_async(struct i915_vma *vma)
> +{
> +     bool vm_ddestroy, async = vma->obj->mm.rsgt;
> +     struct intel_gt *gt;
> +
> +     if (dma_resv_reserve_fences(vma->obj->base.resv, 1))
> +             async = false;
> +
> +     mutex_lock(&vma->vm->mutex);
> +     /*
> +      * Ensure any asynchronous binding is complete while using
> +      * async unbind as we will be releasing the vma here.
> +      */
> +     if (async && i915_active_wait(&vma->active))
> +             async = false;
> +
> +     force_unbind(vma, async);
>       list_del_init(&vma->vm_link);
>       vm_ddestroy = vma->vm_ddestroy;
>       vma->vm_ddestroy = false;
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 737ef310d046..25f15965dab8 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -272,6 +272,7 @@ void i915_vma_reopen(struct i915_vma *vma);
>  
> 
> 
> 
>  void i915_vma_destroy_locked(struct i915_vma *vma);
>  void i915_vma_destroy(struct i915_vma *vma);
> +void i915_vma_destroy_async(struct i915_vma *vma);
>  
> 
> 
> 
>  #define assert_vma_held(vma) dma_resv_assert_held((vma)->obj->base.resv)
>  
> 
> 
> 

Reply via email to