On 19/10/2022 19:28, Niranjana Vishwanathapura wrote:
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@xxxxxxxxx>
Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
---
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@xxxxxxxxx>
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.
I was thinking more about how this potentially behaves with concurrent
bind/unbind, and what it might return in such cases. I'm assuming most
normal users will want to have an active pin and/or be holding the
object lock when calling this.
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))
Just realised that this leaks the fence. I guess we have yet to hit this
in testing.
+ 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
