[Intel-gfx] [PATCH v5 09/19] drm/i915/vm_bind: Add out fence support
Niranjana Vishwanathapura
niranjana.vishwanathapura at intel.com
Wed Oct 26 18:59:49 UTC 2022
On Wed, Oct 26, 2022 at 06:15:09PM +0100, Matthew Auld wrote:
>On 25/10/2022 07:58, Niranjana Vishwanathapura wrote:
>>Add support for handling out fence for vm_bind call.
>>
>>v2: Reset vma->vm_bind_fence.syncobj to NULL at the end
>> of vm_bind call.
>>v3: Remove vm_unbind out fence uapi which is not supported yet.
>>v4: Return error if I915_TIMELINE_FENCE_WAIT fence flag is set.
>> Wait for bind to complete iff I915_TIMELINE_FENCE_SIGNAL is
>> not specified.
>>
>>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
>>Signed-off-by: Andi Shyti <andi.shyti at linux.intel.com>
>>---
>> drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h | 4 +
>> .../drm/i915/gem/i915_gem_vm_bind_object.c | 93 +++++++++++++++++++
>> drivers/gpu/drm/i915/i915_vma.c | 7 +-
>> drivers/gpu/drm/i915/i915_vma_types.h | 7 ++
>> include/uapi/drm/i915_drm.h | 49 +++++++++-
>> 5 files changed, 157 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>>index 36262a6357b5..b70e900e35ab 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>>@@ -8,6 +8,7 @@
>> #include <linux/types.h>
>>+struct dma_fence;
>> struct drm_device;
>> struct drm_file;
>> struct i915_address_space;
>>@@ -23,4 +24,7 @@ int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data,
>> void i915_gem_vm_unbind_all(struct i915_address_space *vm);
>>+void i915_vm_bind_signal_fence(struct i915_vma *vma,
>>+ struct dma_fence * const fence);
>>+
>> #endif /* __I915_GEM_VM_BIND_H */
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>index 863bd17c9253..fca6d6eb9ef8 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>@@ -7,6 +7,8 @@
>> #include <linux/interval_tree_generic.h>
>>+#include <drm/drm_syncobj.h>
>>+
>> #include "gem/i915_gem_context.h"
>> #include "gem/i915_gem_vm_bind.h"
>>@@ -100,6 +102,76 @@ static void i915_gem_vm_bind_remove(struct i915_vma *vma, bool release_obj)
>> i915_gem_object_put(vma->obj);
>> }
>>+static int i915_vm_bind_add_fence(struct drm_file *file, struct i915_vma *vma,
>>+ u32 handle, u64 point)
>>+{
>>+ struct drm_syncobj *syncobj;
>>+
>>+ syncobj = drm_syncobj_find(file, handle);
>>+ if (!syncobj) {
>>+ DRM_DEBUG("Invalid syncobj handle provided\n");
>
>AFAIK drm_dbg() is preferred, if possible.
ok, will fix.
>
>>+ return -ENOENT;
>>+ }
>>+
>>+ /*
>>+ * For timeline syncobjs we need to preallocate chains for
>>+ * later signaling.
>>+ */
>>+ if (point) {
>>+ vma->vm_bind_fence.chain_fence = dma_fence_chain_alloc();
>>+ if (!vma->vm_bind_fence.chain_fence) {
>>+ drm_syncobj_put(syncobj);
>>+ return -ENOMEM;
>>+ }
>>+ } else {
>>+ vma->vm_bind_fence.chain_fence = NULL;
>>+ }
>>+ vma->vm_bind_fence.syncobj = syncobj;
>>+ vma->vm_bind_fence.value = point;
>>+
>>+ return 0;
>>+}
>>+
>>+static void i915_vm_bind_put_fence(struct i915_vma *vma)
>>+{
>>+ if (!vma->vm_bind_fence.syncobj)
>>+ return;
>>+
>>+ drm_syncobj_put(vma->vm_bind_fence.syncobj);
>>+ dma_fence_chain_free(vma->vm_bind_fence.chain_fence);
>>+ vma->vm_bind_fence.syncobj = NULL;
>>+}
>>+
>>+/**
>>+ * i915_vm_bind_signal_fence() - Add fence to vm_bind syncobj
>>+ * @vma: vma mapping requiring signaling
>>+ * @fence: fence to be added
>>+ *
>>+ * Associate specified @fence with the @vma's syncobj to be
>>+ * signaled after the @fence work completes.
>>+ */
>>+void i915_vm_bind_signal_fence(struct i915_vma *vma,
>>+ struct dma_fence * const fence)
>>+{
>>+ struct drm_syncobj *syncobj = vma->vm_bind_fence.syncobj;
>>+
>>+ if (!syncobj)
>>+ return;
>>+
>>+ if (vma->vm_bind_fence.chain_fence) {
>>+ drm_syncobj_add_point(syncobj,
>>+ vma->vm_bind_fence.chain_fence,
>>+ fence, vma->vm_bind_fence.value);
>>+ /*
>>+ * The chain's ownership is transferred to the
>>+ * timeline.
>>+ */
>>+ vma->vm_bind_fence.chain_fence = NULL;
>>+ } else {
>>+ drm_syncobj_replace_fence(syncobj, fence);
>>+ }
>>+}
>>+
>> static int i915_gem_vm_unbind_vma(struct i915_address_space *vm,
>> struct drm_i915_gem_vm_unbind *va)
>> {
>>@@ -205,6 +277,10 @@ static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>> if (!va->length || !IS_ALIGNED(va->start, I915_GTT_PAGE_SIZE))
>> ret = -EINVAL;
>>+ /* In fences are not supported */
>>+ if (va->fence.flags & I915_TIMELINE_FENCE_WAIT)
>>+ ret = -EINVAL;
>
>I guess also:
>
>if (flags & __I915_TIMELINE_FENCE_UNKNOWN_FLAGS)
>
ok, will add
>Reviewed-by: Matthew Auld <matthew.auld at intel.com>
>
Thanks,
Niranjana
>>+
>> obj = i915_gem_object_lookup(file, va->handle);
>> if (!obj)
>> return -ENOENT;
>>@@ -237,6 +313,13 @@ static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>> goto unlock_vm;
>> }
>>+ if (va->fence.flags & I915_TIMELINE_FENCE_SIGNAL) {
>>+ ret = i915_vm_bind_add_fence(file, vma, va->fence.handle,
>>+ va->fence.value);
>>+ if (ret)
>>+ goto put_vma;
>>+ }
>>+
>> pin_flags = va->start | PIN_OFFSET_FIXED | PIN_USER |
>> PIN_VALIDATE | PIN_NOEVICT;
>>@@ -249,6 +332,13 @@ static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>> if (ret)
>> continue;
>>+ /* If out fence is not requested, wait for bind to complete */
>>+ if (!(va->fence.flags & I915_TIMELINE_FENCE_SIGNAL)) {
>>+ ret = i915_vma_wait_for_bind(vma);
>>+ if (ret)
>>+ continue;
>>+ }
>>+
>> list_add_tail(&vma->vm_bind_link, &vm->vm_bound_list);
>> i915_vm_bind_it_insert(vma, &vm->va);
>> if (!obj->priv_root)
>>@@ -259,6 +349,9 @@ static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>> i915_gem_object_get(vma->obj);
>> }
>>+ if (va->fence.flags & I915_TIMELINE_FENCE_SIGNAL)
>>+ i915_vm_bind_put_fence(vma);
>>+put_vma:
>> if (ret)
>> i915_vma_destroy(vma);
>> unlock_vm:
>>diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>>index 04abdb92c2b2..eaa13e9ba966 100644
>>--- a/drivers/gpu/drm/i915/i915_vma.c
>>+++ b/drivers/gpu/drm/i915/i915_vma.c
>>@@ -29,6 +29,7 @@
>> #include "display/intel_frontbuffer.h"
>> #include "gem/i915_gem_lmem.h"
>> #include "gem/i915_gem_tiling.h"
>>+#include "gem/i915_gem_vm_bind.h"
>> #include "gt/intel_engine.h"
>> #include "gt/intel_engine_heartbeat.h"
>> #include "gt/intel_gt.h"
>>@@ -1567,8 +1568,12 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>> err_vma_res:
>> i915_vma_resource_free(vma_res);
>> err_fence:
>>- if (work)
>>+ if (work) {
>>+ if (i915_vma_is_persistent(vma))
>>+ i915_vm_bind_signal_fence(vma, &work->base.dma);
>>+
>> dma_fence_work_commit_imm(&work->base);
>>+ }
>> err_rpm:
>> if (wakeref)
>> intel_runtime_pm_put(&vma->vm->i915->runtime_pm, wakeref);
>>diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
>>index 61d0ec1a4e18..7c8c293ddfcb 100644
>>--- a/drivers/gpu/drm/i915/i915_vma_types.h
>>+++ b/drivers/gpu/drm/i915/i915_vma_types.h
>>@@ -308,6 +308,13 @@ struct i915_vma {
>> /** @vm_rebind_link: link to vm_rebind_list and protected by vm_rebind_lock */
>> struct list_head vm_rebind_link; /* Link in vm_rebind_list */
>>+ /** Timeline fence for vm_bind completion notification */
>>+ struct {
>>+ struct dma_fence_chain *chain_fence;
>>+ struct drm_syncobj *syncobj;
>>+ u64 value;
>>+ } vm_bind_fence;
>>+
>> /** Interval tree structures for persistent vma */
>> /** @rb: node for the interval tree of vm for persistent vmas */
>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>>index 4383ed85f10a..87f5c2a470f5 100644
>>--- a/include/uapi/drm/i915_drm.h
>>+++ b/include/uapi/drm/i915_drm.h
>>@@ -1527,6 +1527,41 @@ struct drm_i915_gem_execbuffer2 {
>> #define i915_execbuffer2_get_context_id(eb2) \
>> ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK)
>>+/**
>>+ * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
>>+ *
>>+ * The operation will wait for input fence to signal.
>>+ *
>>+ * The returned output fence will be signaled after the completion of the
>>+ * operation.
>>+ */
>>+struct drm_i915_gem_timeline_fence {
>>+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
>>+ __u32 handle;
>>+
>>+ /**
>>+ * @flags: Supported flags are:
>>+ *
>>+ * I915_TIMELINE_FENCE_WAIT:
>>+ * Wait for the input fence before the operation.
>>+ *
>>+ * I915_TIMELINE_FENCE_SIGNAL:
>>+ * Return operation completion fence as output.
>>+ */
>>+ __u32 flags;
>>+#define I915_TIMELINE_FENCE_WAIT (1 << 0)
>>+#define I915_TIMELINE_FENCE_SIGNAL (1 << 1)
>>+#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL << 1))
>>+
>>+ /**
>>+ * @value: A point in the timeline.
>>+ * Value must be 0 for a binary drm_syncobj. A Value of 0 for a
>>+ * timeline drm_syncobj is invalid as it turns a drm_syncobj into a
>>+ * binary one.
>>+ */
>>+ __u64 value;
>>+};
>>+
>> struct drm_i915_gem_pin {
>> /** Handle of the buffer to be pinned. */
>> __u32 handle;
>>@@ -3826,8 +3861,18 @@ struct drm_i915_gem_vm_bind {
>> */
>> __u64 flags;
>>- /** @rsvd: Reserved, MBZ */
>>- __u64 rsvd[2];
>>+ /**
>>+ * @fence: Timeline fence for bind completion signaling.
>>+ *
>>+ * Timeline fence is of format struct drm_i915_gem_timeline_fence.
>>+ *
>>+ * It is an out fence, hence using I915_TIMELINE_FENCE_WAIT flag
>>+ * is invalid, and an error will be returned.
>>+ *
>>+ * If I915_TIMELINE_FENCE_SIGNAL flag is not set, then out fence
>>+ * is not requested and binding is completed synchronously.
>>+ */
>>+ struct drm_i915_gem_timeline_fence fence;
>> /**
>> * @extensions: Zero-terminated chain of extensions.
More information about the Intel-gfx
mailing list