Re: [Intel-gfx] [PATCH v4 13/17] drm/i915/vm_bind: Update i915_vma_verify_bind_complete()

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Wed, Oct 19, 2022 at 05:07:31PM +0100, Matthew Auld wrote:
>On 18/10/2022 08:16, Niranjana Vishwanathapura wrote:
>>Ensure i915_vma_verify_bind_complete() handles case where bind
>>is not initiated. Also make it non static, add documentation
>>and move it out of CONFIG_DRM_I915_DEBUG_GEM.
>>
>>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>>Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>>---
>>  drivers/gpu/drm/i915/i915_vma.c | 16 +++++++++++-----
>>  drivers/gpu/drm/i915/i915_vma.h |  1 +
>>  2 files changed, 12 insertions(+), 5 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>>index eaa13e9ba966..4975fc662c86 100644
>>--- a/drivers/gpu/drm/i915/i915_vma.c
>>+++ b/drivers/gpu/drm/i915/i915_vma.c
>>@@ -439,12 +439,21 @@ int i915_vma_sync(struct i915_vma *vma)
>>  	return i915_vm_sync(vma->vm);
>>  }
>>-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>>-static int i915_vma_verify_bind_complete(struct i915_vma *vma)
>>+/**
>>+ * i915_vma_verify_bind_complete() - Check for the bind completion of the vma
>>+ * @vma: vma to check for bind completion
>
>Maybe mention the locking since this is now more than just DEBUG_GEM 
>stuff. I assume we need the object lock or otherwise some guarantee 
>that the vma is pinned?
>
>Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>

I think they are not needed. The fence reference is obtained under rcu
lock anyhow (will add this to documentation). Only thing required is
that vma is not released, but that caller must ensure for all i915_vma
apis anyhow.

Thanks,
Niranjana

>>+ *
>>+ * Returns: 0 if the vma bind is completed. Error code otherwise.
>>+ */
>>+int i915_vma_verify_bind_complete(struct i915_vma *vma)
>>  {
>>  	struct dma_fence *fence = i915_active_fence_get(&vma->active.excl);
>>  	int err;
>>+	/* Ensure vma bind is initiated */
>>+	if (!i915_vma_is_bound(vma, I915_VMA_BIND_MASK))
>>+		return -EINVAL;
>>+
>>  	if (!fence)
>>  		return 0;
>>@@ -457,9 +466,6 @@ static int i915_vma_verify_bind_complete(struct i915_vma *vma)
>>  	return err;
>>  }
>>-#else
>>-#define i915_vma_verify_bind_complete(_vma) 0
>>-#endif
>>  I915_SELFTEST_EXPORT void
>>  i915_vma_resource_init_from_vma(struct i915_vma_resource *vma_res,
>>diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
>>index 1cadbf8fdedf..04770f8ba815 100644
>>--- a/drivers/gpu/drm/i915/i915_vma.h
>>+++ b/drivers/gpu/drm/i915/i915_vma.h
>>@@ -440,6 +440,7 @@ void i915_vma_make_purgeable(struct i915_vma *vma);
>>  int i915_vma_wait_for_bind(struct i915_vma *vma);
>>  int i915_vma_sync(struct i915_vma *vma);
>>+int i915_vma_verify_bind_complete(struct i915_vma *vma);
>>  /**
>>   * i915_vma_get_current_resource - Get the current resource of the vma