Re: [PATCH v3 1/1] drm/syncobj: add sideband payload

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/08/2019 17:01, Lionel Landwerlin wrote:
On 08/08/2019 16:42, Chunming Zhou wrote:
No, I just see your comment "The next vkQueueSubmit()
waiting on a the syncobj will read the sideband payload and wait for a
fence chain element with a seqno superior or equal to the sideband
payload value to be added into the fence chain and use that fence to
trigger the submission on the kernel driver. ".


That was meant to say wait on the chain fence to reach the sideband payload value.

It a bit confusing but I can't see any other way to word it.


In that, you mentioned wait for sideband.
So I want to know we how to use that, maybe I miss something in previous
discussing thread.


In QueueSubmit(), we start by reading the sideband payloads : https://gitlab.freedesktop.org/llandwerlin/mesa/blob/review/anv-timeline_semaphore_prep/src/intel/vulkan/anv_queue.c#L655


We only need to read/write sideband payloads for binary semaphores. The behavior for timeline semaphores remains unchanged. :)


-Lionel



Then build everything for the submission and hand it over to the submission thread.

Instead of the just waiting on the timeline semaphore values, we also wait on the binary semaphore sideband payload values.

Finally before exiting the QueueSubmit() call, we bump the sideband payloads of all the binary semaphores have have been signaled : https://gitlab.freedesktop.org/llandwerlin/mesa/blob/review/anv-timeline_semaphore_prep/src/intel/vulkan/anv_queue.c#L806


Whoever calls QueueSubmit() after that will pickup the new sideband payload values to wait on.


-Lionel




-DAvid


在 2019/8/8 21:38, Lionel Landwerlin 写道:
Interesting question :)

I didn't see any usecase for that.
This sideband payload value is used for a wait so waiting on the wait
value for another wait is bit meta :)

Do you have a use case for that?

-Lionel

On 08/08/2019 16:23, Chunming Zhou wrote:
The propursal is fine with me.

one question:

how to wait sideband payload? Following patch will show that?

-David

在 2019/8/8 21:14, Lionel Landwerlin 写道:
The Vulkan timeline semaphores allow signaling to happen on the point
of the timeline without all of the its dependencies to be created.

The current 2 implementations (AMD/Intel) of the Vulkan spec on top of
the Linux kernel are using a thread to wait on the dependencies of a
given point to materialize and delay actual submission to the kernel
driver until the wait completes.

If a binary semaphore is submitted for signaling along the side of a
timeline semaphore waiting for completion that means that the drm
syncobj associated with that binary semaphore will not have a DMA
fence associated with it by the time vkQueueSubmit() returns. This and
the fact that a binary semaphore can be signaled and unsignaled as
before its DMA fences materialize mean that we cannot just rely on the fence within the syncobj but we also need a sideband payload verifying
that the fence in the syncobj matches the last submission from the
Vulkan API point of view.

This change adds a sideband payload that is incremented with signaled
syncobj when vkQueueSubmit() is called. The next vkQueueSubmit()
waiting on a the syncobj will read the sideband payload and wait for a
fence chain element with a seqno superior or equal to the sideband
payload value to be added into the fence chain and use that fence to
trigger the submission on the kernel driver.

v2: Use a separate ioctl to get/set the sideband value (Christian)

v3: Use 2 ioctls for get/set (Christian)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
Cc: Christian Koenig <Christian.Koenig@xxxxxxx>
Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
Cc: David(ChunMing) Zhou <David1.Zhou@xxxxxxx>
---
    drivers/gpu/drm/drm_internal.h |  4 ++
    drivers/gpu/drm/drm_ioctl.c    |  5 +++
    drivers/gpu/drm/drm_syncobj.c  | 79
+++++++++++++++++++++++++++++++++-
    include/drm/drm_syncobj.h      |  9 ++++
    include/uapi/drm/drm.h         |  2 +
    5 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_internal.h
b/drivers/gpu/drm/drm_internal.h
index 51a2055c8f18..0c9736199df0 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -208,6 +208,10 @@ int drm_syncobj_timeline_signal_ioctl(struct
drm_device *dev, void *data,
                          struct drm_file *file_private);
    int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
                    struct drm_file *file_private);
+int drm_syncobj_get_sideband_ioctl(struct drm_device *dev, void *data,
+                   struct drm_file *file_private);
+int drm_syncobj_set_sideband_ioctl(struct drm_device *dev, void *data,
+                   struct drm_file *file_private);
       /* drm_framebuffer.c */
    void drm_framebuffer_print_info(struct drm_printer *p, unsigned
int indent,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index f675a3bb2c88..48500bf62801 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -703,6 +703,11 @@ static const struct drm_ioctl_desc drm_ioctls[]
= {
                  DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
                  DRM_RENDER_ALLOW),
+    DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_GET_SIDEBAND,
drm_syncobj_get_sideband_ioctl,
+              DRM_RENDER_ALLOW),
+    DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SET_SIDEBAND,
drm_syncobj_set_sideband_ioctl,
+              DRM_RENDER_ALLOW),
+
        DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE,
drm_crtc_get_sequence_ioctl, 0),
        DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE,
drm_crtc_queue_sequence_ioctl, 0),
        DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE,
drm_mode_create_lease_ioctl, DRM_MASTER),
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index b927e482e554..c90ef20b9242 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1150,8 +1150,10 @@ drm_syncobj_reset_ioctl(struct drm_device
*dev, void *data,
        if (ret < 0)
            return ret;
    -    for (i = 0; i < args->count_handles; i++)
+    for (i = 0; i < args->count_handles; i++) {
            drm_syncobj_replace_fence(syncobjs[i], NULL);
+        syncobjs[i]->sideband_payload = 0;
+    }
           drm_syncobj_array_free(syncobjs, args->count_handles);
    @@ -1321,6 +1323,81 @@ int drm_syncobj_query_ioctl(struct
drm_device *dev, void *data,
            if (ret)
                break;
        }
+
+    drm_syncobj_array_free(syncobjs, args->count_handles);
+
+    return ret;
+}
+
+int drm_syncobj_get_sideband_ioctl(struct drm_device *dev, void *data,
+                   struct drm_file *file_private)
+{
+    struct drm_syncobj_timeline_array *args = data;
+    struct drm_syncobj **syncobjs;
+    uint64_t __user *points = u64_to_user_ptr(args->points);
+    uint32_t i;
+    int ret;
+
+    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
+        return -EOPNOTSUPP;
+
+    if (args->pad != 0)
+        return -EINVAL;
+
+    if (args->count_handles == 0)
+        return -EINVAL;
+
+    ret = drm_syncobj_array_find(file_private,
+                     u64_to_user_ptr(args->handles),
+                     args->count_handles,
+                     &syncobjs);
+    if (ret < 0)
+        return ret;
+
+    for (i = 0; i < args->count_handles; i++) {
+        copy_to_user(&points[i], &syncobjs[i]->sideband_payload,
sizeof(uint64_t));
+        ret = ret ? -EFAULT : 0;
+        if (ret)
+            break;
+    }
+
+    drm_syncobj_array_free(syncobjs, args->count_handles);
+
+    return ret;
+}
+
+int drm_syncobj_set_sideband_ioctl(struct drm_device *dev, void *data,
+                   struct drm_file *file_private)
+{
+    struct drm_syncobj_timeline_array *args = data;
+    struct drm_syncobj **syncobjs;
+    uint64_t __user *points = u64_to_user_ptr(args->points);
+    uint32_t i;
+    int ret;
+
+    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
+        return -EOPNOTSUPP;
+
+    if (args->pad != 0)
+        return -EINVAL;
+
+    if (args->count_handles == 0)
+        return -EINVAL;
+
+    ret = drm_syncobj_array_find(file_private,
+                     u64_to_user_ptr(args->handles),
+                     args->count_handles,
+                     &syncobjs);
+    if (ret < 0)
+        return ret;
+
+    for (i = 0; i < args->count_handles; i++) {
+ copy_from_user(&syncobjs[i]->sideband_payload, &points[i],
sizeof(uint64_t));
+        ret = ret ? -EFAULT : 0;
+        if (ret)
+            break;
+    }
+
        drm_syncobj_array_free(syncobjs, args->count_handles);
           return ret;
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 6cf7243a1dc5..b587b8e07547 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -61,6 +61,15 @@ struct drm_syncobj {
         * @file: A file backing for this syncobj.
         */
        struct file *file;
+    /**
+     * @sideband_payload: A 64bit side band payload.
+     *
+     * We use the sideband payload value to wait on binary syncobj
fences
+     * to materialize. It is a reservation mechanism for the
signaler to
+     * express that at some point in the future a dma fence with
the same
+     * seqno will be put into the syncobj.
+     */
+    u64 sideband_payload;
    };
       void drm_syncobj_free(struct kref *kref);
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 8a5b2f8f8eb9..cffdc6c9772c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -946,6 +946,8 @@ extern "C" {
    #define DRM_IOCTL_SYNCOBJ_QUERY        DRM_IOWR(0xCB, struct
drm_syncobj_timeline_array)
    #define DRM_IOCTL_SYNCOBJ_TRANSFER    DRM_IOWR(0xCC, struct
drm_syncobj_transfer)
    #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD,
struct drm_syncobj_timeline_array)
+#define DRM_IOCTL_SYNCOBJ_GET_SIDEBAND    DRM_IOR(0xCE, struct
drm_syncobj_timeline_array)
+#define DRM_IOCTL_SYNCOBJ_SET_SIDEBAND    DRM_IOWR(0xCF, struct
drm_syncobj_timeline_array)
       /**
     * Device specific ioctls should only be in their respective headers


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux