* [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs
@ 2025-04-01 15:57 Rob Clark
2025-04-01 20:40 ` Rob Clark
0 siblings, 1 reply; 5+ messages in thread
From: Rob Clark @ 2025-04-01 15:57 UTC (permalink / raw)
To: dri-devel
Cc: Christian König, Dmitry Osipenko, Rob Clark,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sumit Semwal, open list,
open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b
From: Rob Clark <robdclark@chromium.org>
Add support for exporting a dma_fence fd for a specific point on a
timeline. This is needed for vtest/vpipe[1][2] to implement timeline
syncobj support, as it needs a way to turn a point on a timeline back
into a dma_fence fd. It also closes an odd omission from the syncobj
UAPI.
[1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433
[2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805
v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE
v3: Add unstaged uabi header hunk
v4: Also handle IMPORT_SYNC_FILE case
v5: Address comments from Dmitry
v6: checkpatch.pl nits
Signed-off-by: Rob Clark <robdclark@chromium.org>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
drivers/gpu/drm/drm_syncobj.c | 47 +++++++++++++++++++++++++++--------
include/uapi/drm/drm.h | 4 +++
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4f2ab8a7b50f..636cd83ca29e 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -741,7 +741,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
- int fd, int handle)
+ int fd, int handle, u64 point)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
@@ -755,14 +755,24 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, fence);
+ if (point) {
+ struct dma_fence_chain *chain = dma_fence_chain_alloc();
+
+ if (!chain)
+ return -ENOMEM;
+
+ drm_syncobj_add_point(syncobj, chain, fence, point);
+ } else {
+ drm_syncobj_replace_fence(syncobj, fence);
+ }
+
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return 0;
}
static int drm_syncobj_export_sync_file(struct drm_file *file_private,
- int handle, int *p_fd)
+ int handle, u64 point, int *p_fd)
{
int ret;
struct dma_fence *fence;
@@ -772,7 +782,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private,
if (fd < 0)
return fd;
- ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
+ ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
if (ret)
goto err_put_fd;
@@ -869,6 +879,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private)
{
struct drm_syncobj_handle *args = data;
+ unsigned int valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE |
+ DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE;
+ u64 point = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
return -EOPNOTSUPP;
@@ -876,13 +889,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
if (args->pad)
return -EINVAL;
- if (args->flags != 0 &&
- args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+ if (args->flags & ~valid_flags)
return -EINVAL;
+ if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE)
+ point = args->point;
+
if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
return drm_syncobj_export_sync_file(file_private, args->handle,
- &args->fd);
+ point, &args->fd);
+
+ if (args->point)
+ return -EINVAL;
return drm_syncobj_handle_to_fd(file_private, args->handle,
&args->fd);
@@ -893,6 +911,9 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private)
{
struct drm_syncobj_handle *args = data;
+ unsigned int valid_flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE |
+ DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE;
+ u64 point = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
return -EOPNOTSUPP;
@@ -900,14 +921,20 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
if (args->pad)
return -EINVAL;
- if (args->flags != 0 &&
- args->flags != DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
+ if (args->flags & ~valid_flags)
return -EINVAL;
+ if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE)
+ point = args->point;
+
if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
return drm_syncobj_import_sync_file_fence(file_private,
args->fd,
- args->handle);
+ args->handle,
+ point);
+
+ if (args->point)
+ return -EINVAL;
return drm_syncobj_fd_to_handle(file_private, args->fd,
&args->handle);
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 7fba37b94401..e63a71d3c607 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -905,13 +905,17 @@ struct drm_syncobj_destroy {
};
#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE (1 << 0)
+#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE (1 << 1)
#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE (1 << 0)
+#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE (1 << 1)
struct drm_syncobj_handle {
__u32 handle;
__u32 flags;
__s32 fd;
__u32 pad;
+
+ __u64 point;
};
struct drm_syncobj_transfer {
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs
2025-04-01 15:57 [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs Rob Clark
@ 2025-04-01 20:40 ` Rob Clark
2025-04-01 20:46 ` Dmitry Osipenko
0 siblings, 1 reply; 5+ messages in thread
From: Rob Clark @ 2025-04-01 20:40 UTC (permalink / raw)
To: dri-devel
Cc: Christian König, Dmitry Osipenko, Rob Clark,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sumit Semwal, open list,
open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b
On Tue, Apr 1, 2025 at 8:58 AM Rob Clark <robdclark@gmail.com> wrote:
>
> From: Rob Clark <robdclark@chromium.org>
>
> Add support for exporting a dma_fence fd for a specific point on a
> timeline. This is needed for vtest/vpipe[1][2] to implement timeline
> syncobj support, as it needs a way to turn a point on a timeline back
> into a dma_fence fd. It also closes an odd omission from the syncobj
> UAPI.
>
> [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433
> [2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805
>
> v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE
> v3: Add unstaged uabi header hunk
> v4: Also handle IMPORT_SYNC_FILE case
> v5: Address comments from Dmitry
> v6: checkpatch.pl nits
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> drivers/gpu/drm/drm_syncobj.c | 47 +++++++++++++++++++++++++++--------
> include/uapi/drm/drm.h | 4 +++
> 2 files changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 4f2ab8a7b50f..636cd83ca29e 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -741,7 +741,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
> }
>
> static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
> - int fd, int handle)
> + int fd, int handle, u64 point)
> {
> struct dma_fence *fence = sync_file_get_fence(fd);
> struct drm_syncobj *syncobj;
> @@ -755,14 +755,24 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
> return -ENOENT;
> }
>
> - drm_syncobj_replace_fence(syncobj, fence);
> + if (point) {
> + struct dma_fence_chain *chain = dma_fence_chain_alloc();
> +
> + if (!chain)
> + return -ENOMEM;
> +
> + drm_syncobj_add_point(syncobj, chain, fence, point);
> + } else {
> + drm_syncobj_replace_fence(syncobj, fence);
> + }
> +
> dma_fence_put(fence);
> drm_syncobj_put(syncobj);
> return 0;
> }
>
> static int drm_syncobj_export_sync_file(struct drm_file *file_private,
> - int handle, int *p_fd)
> + int handle, u64 point, int *p_fd)
> {
> int ret;
> struct dma_fence *fence;
> @@ -772,7 +782,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private,
> if (fd < 0)
> return fd;
>
> - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
> + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
> if (ret)
> goto err_put_fd;
>
> @@ -869,6 +879,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_private)
> {
> struct drm_syncobj_handle *args = data;
> + unsigned int valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE |
> + DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE;
> + u64 point = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> return -EOPNOTSUPP;
> @@ -876,13 +889,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> if (args->pad)
> return -EINVAL;
>
> - if (args->flags != 0 &&
> - args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
> + if (args->flags & ~valid_flags)
> return -EINVAL;
>
> + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE)
> + point = args->point;
> +
> if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
> return drm_syncobj_export_sync_file(file_private, args->handle,
> - &args->fd);
> + point, &args->fd);
> +
> + if (args->point)
> + return -EINVAL;
>
> return drm_syncobj_handle_to_fd(file_private, args->handle,
> &args->fd);
> @@ -893,6 +911,9 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_private)
> {
> struct drm_syncobj_handle *args = data;
> + unsigned int valid_flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE |
> + DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE;
> + u64 point = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> return -EOPNOTSUPP;
oh, I suppose I should add a check for DRIVER_SYNCOBJ_TIMELINE? I'll
send a v7 a bit later
BR,
-R
> @@ -900,14 +921,20 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> if (args->pad)
> return -EINVAL;
>
> - if (args->flags != 0 &&
> - args->flags != DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
> + if (args->flags & ~valid_flags)
> return -EINVAL;
>
> + if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE)
> + point = args->point;
> +
> if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
> return drm_syncobj_import_sync_file_fence(file_private,
> args->fd,
> - args->handle);
> + args->handle,
> + point);
> +
> + if (args->point)
> + return -EINVAL;
>
> return drm_syncobj_fd_to_handle(file_private, args->fd,
> &args->handle);
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 7fba37b94401..e63a71d3c607 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -905,13 +905,17 @@ struct drm_syncobj_destroy {
> };
>
> #define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE (1 << 0)
> +#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE (1 << 1)
> #define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE (1 << 0)
> +#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE (1 << 1)
> struct drm_syncobj_handle {
> __u32 handle;
> __u32 flags;
>
> __s32 fd;
> __u32 pad;
> +
> + __u64 point;
> };
>
> struct drm_syncobj_transfer {
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs
2025-04-01 20:40 ` Rob Clark
@ 2025-04-01 20:46 ` Dmitry Osipenko
2025-04-02 6:55 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2025-04-01 20:46 UTC (permalink / raw)
To: Rob Clark, dri-devel
Cc: Christian König, Rob Clark, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
open list,
open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b
On 4/1/25 23:40, Rob Clark wrote:
> On Tue, Apr 1, 2025 at 8:58 AM Rob Clark <robdclark@gmail.com> wrote:
>>
>> From: Rob Clark <robdclark@chromium.org>
>>
>> Add support for exporting a dma_fence fd for a specific point on a
>> timeline. This is needed for vtest/vpipe[1][2] to implement timeline
>> syncobj support, as it needs a way to turn a point on a timeline back
>> into a dma_fence fd. It also closes an odd omission from the syncobj
>> UAPI.
>>
>> [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433
>> [2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805
>>
>> v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE
>> v3: Add unstaged uabi header hunk
>> v4: Also handle IMPORT_SYNC_FILE case
>> v5: Address comments from Dmitry
>> v6: checkpatch.pl nits
>>
>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>> drivers/gpu/drm/drm_syncobj.c | 47 +++++++++++++++++++++++++++--------
>> include/uapi/drm/drm.h | 4 +++
>> 2 files changed, 41 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index 4f2ab8a7b50f..636cd83ca29e 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -741,7 +741,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
>> }
>>
>> static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
>> - int fd, int handle)
>> + int fd, int handle, u64 point)
>> {
>> struct dma_fence *fence = sync_file_get_fence(fd);
>> struct drm_syncobj *syncobj;
>> @@ -755,14 +755,24 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
>> return -ENOENT;
>> }
>>
>> - drm_syncobj_replace_fence(syncobj, fence);
>> + if (point) {
>> + struct dma_fence_chain *chain = dma_fence_chain_alloc();
>> +
>> + if (!chain)
>> + return -ENOMEM;
>> +
>> + drm_syncobj_add_point(syncobj, chain, fence, point);
>> + } else {
>> + drm_syncobj_replace_fence(syncobj, fence);
>> + }
>> +
>> dma_fence_put(fence);
>> drm_syncobj_put(syncobj);
>> return 0;
>> }
>>
>> static int drm_syncobj_export_sync_file(struct drm_file *file_private,
>> - int handle, int *p_fd)
>> + int handle, u64 point, int *p_fd)
>> {
>> int ret;
>> struct dma_fence *fence;
>> @@ -772,7 +782,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private,
>> if (fd < 0)
>> return fd;
>>
>> - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
>> + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
>> if (ret)
>> goto err_put_fd;
>>
>> @@ -869,6 +879,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *file_private)
>> {
>> struct drm_syncobj_handle *args = data;
>> + unsigned int valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE |
>> + DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE;
>> + u64 point = 0;
>>
>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>> return -EOPNOTSUPP;
>> @@ -876,13 +889,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>> if (args->pad)
>> return -EINVAL;
>>
>> - if (args->flags != 0 &&
>> - args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
>> + if (args->flags & ~valid_flags)
>> return -EINVAL;
>>
>> + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE)
>> + point = args->point;
>> +
>> if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
>> return drm_syncobj_export_sync_file(file_private, args->handle,
>> - &args->fd);
>> + point, &args->fd);
>> +
>> + if (args->point)
>> + return -EINVAL;
>>
>> return drm_syncobj_handle_to_fd(file_private, args->handle,
>> &args->fd);
>> @@ -893,6 +911,9 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *file_private)
>> {
>> struct drm_syncobj_handle *args = data;
>> + unsigned int valid_flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE |
>> + DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE;
>> + u64 point = 0;
>>
>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>> return -EOPNOTSUPP;
>
> oh, I suppose I should add a check for DRIVER_SYNCOBJ_TIMELINE? I'll
> send a v7 a bit later
Christian already applied to misc-test, please rebase and make it as a
new patch
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs
2025-04-01 20:46 ` Dmitry Osipenko
@ 2025-04-02 6:55 ` Christian König
2025-04-02 16:11 ` Rob Clark
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2025-04-02 6:55 UTC (permalink / raw)
To: Dmitry Osipenko, Rob Clark, dri-devel
Cc: Rob Clark, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Sumit Semwal, open list,
open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b
Am 01.04.25 um 22:46 schrieb Dmitry Osipenko:
> On 4/1/25 23:40, Rob Clark wrote:
>> On Tue, Apr 1, 2025 at 8:58 AM Rob Clark <robdclark@gmail.com> wrote:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> Add support for exporting a dma_fence fd for a specific point on a
>>> timeline. This is needed for vtest/vpipe[1][2] to implement timeline
>>> syncobj support, as it needs a way to turn a point on a timeline back
>>> into a dma_fence fd. It also closes an odd omission from the syncobj
>>> UAPI.
>>>
>>> [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433
>>> [2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805
>>>
>>> v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE
>>> v3: Add unstaged uabi header hunk
>>> v4: Also handle IMPORT_SYNC_FILE case
>>> v5: Address comments from Dmitry
>>> v6: checkpatch.pl nits
>>>
>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> ---
>>> drivers/gpu/drm/drm_syncobj.c | 47 +++++++++++++++++++++++++++--------
>>> include/uapi/drm/drm.h | 4 +++
>>> 2 files changed, 41 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index 4f2ab8a7b50f..636cd83ca29e 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -741,7 +741,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
>>> }
>>>
>>> static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
>>> - int fd, int handle)
>>> + int fd, int handle, u64 point)
>>> {
>>> struct dma_fence *fence = sync_file_get_fence(fd);
>>> struct drm_syncobj *syncobj;
>>> @@ -755,14 +755,24 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
>>> return -ENOENT;
>>> }
>>>
>>> - drm_syncobj_replace_fence(syncobj, fence);
>>> + if (point) {
>>> + struct dma_fence_chain *chain = dma_fence_chain_alloc();
>>> +
>>> + if (!chain)
>>> + return -ENOMEM;
>>> +
>>> + drm_syncobj_add_point(syncobj, chain, fence, point);
>>> + } else {
>>> + drm_syncobj_replace_fence(syncobj, fence);
>>> + }
>>> +
>>> dma_fence_put(fence);
>>> drm_syncobj_put(syncobj);
>>> return 0;
>>> }
>>>
>>> static int drm_syncobj_export_sync_file(struct drm_file *file_private,
>>> - int handle, int *p_fd)
>>> + int handle, u64 point, int *p_fd)
>>> {
>>> int ret;
>>> struct dma_fence *fence;
>>> @@ -772,7 +782,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private,
>>> if (fd < 0)
>>> return fd;
>>>
>>> - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
>>> + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
>>> if (ret)
>>> goto err_put_fd;
>>>
>>> @@ -869,6 +879,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>>> struct drm_file *file_private)
>>> {
>>> struct drm_syncobj_handle *args = data;
>>> + unsigned int valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE |
>>> + DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE;
>>> + u64 point = 0;
>>>
>>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>>> return -EOPNOTSUPP;
>>> @@ -876,13 +889,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
>>> if (args->pad)
>>> return -EINVAL;
>>>
>>> - if (args->flags != 0 &&
>>> - args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
>>> + if (args->flags & ~valid_flags)
>>> return -EINVAL;
>>>
>>> + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE)
>>> + point = args->point;
>>> +
>>> if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
>>> return drm_syncobj_export_sync_file(file_private, args->handle,
>>> - &args->fd);
>>> + point, &args->fd);
>>> +
>>> + if (args->point)
>>> + return -EINVAL;
>>>
>>> return drm_syncobj_handle_to_fd(file_private, args->handle,
>>> &args->fd);
>>> @@ -893,6 +911,9 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
>>> struct drm_file *file_private)
>>> {
>>> struct drm_syncobj_handle *args = data;
>>> + unsigned int valid_flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE |
>>> + DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE;
>>> + u64 point = 0;
>>>
>>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>>> return -EOPNOTSUPP;
>> oh, I suppose I should add a check for DRIVER_SYNCOBJ_TIMELINE? I'll
>> send a v7 a bit later
> Christian already applied to misc-test, please rebase and make it as a
> new patch
Yeah, sorry I was a bit to quick obviously.
On the other hand I don't see an immediate need for a check for DRIVER_SYNCOBJ_TIMELINE here.
The functions should work even when the driver doesn't handle timeline syncobj on it's own.
Regards,
Christian.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs
2025-04-02 6:55 ` Christian König
@ 2025-04-02 16:11 ` Rob Clark
0 siblings, 0 replies; 5+ messages in thread
From: Rob Clark @ 2025-04-02 16:11 UTC (permalink / raw)
To: Christian König
Cc: Dmitry Osipenko, dri-devel, Rob Clark, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sumit Semwal, open list,
open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b
On Tue, Apr 1, 2025 at 11:55 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 01.04.25 um 22:46 schrieb Dmitry Osipenko:
> > On 4/1/25 23:40, Rob Clark wrote:
> >> On Tue, Apr 1, 2025 at 8:58 AM Rob Clark <robdclark@gmail.com> wrote:
> >>> From: Rob Clark <robdclark@chromium.org>
> >>>
> >>> Add support for exporting a dma_fence fd for a specific point on a
> >>> timeline. This is needed for vtest/vpipe[1][2] to implement timeline
> >>> syncobj support, as it needs a way to turn a point on a timeline back
> >>> into a dma_fence fd. It also closes an odd omission from the syncobj
> >>> UAPI.
> >>>
> >>> [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433
> >>> [2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805
> >>>
> >>> v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE
> >>> v3: Add unstaged uabi header hunk
> >>> v4: Also handle IMPORT_SYNC_FILE case
> >>> v5: Address comments from Dmitry
> >>> v6: checkpatch.pl nits
> >>>
> >>> Signed-off-by: Rob Clark <robdclark@chromium.org>
> >>> Reviewed-by: Christian König <christian.koenig@amd.com>
> >>> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> >>> ---
> >>> drivers/gpu/drm/drm_syncobj.c | 47 +++++++++++++++++++++++++++--------
> >>> include/uapi/drm/drm.h | 4 +++
> >>> 2 files changed, 41 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> >>> index 4f2ab8a7b50f..636cd83ca29e 100644
> >>> --- a/drivers/gpu/drm/drm_syncobj.c
> >>> +++ b/drivers/gpu/drm/drm_syncobj.c
> >>> @@ -741,7 +741,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
> >>> }
> >>>
> >>> static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
> >>> - int fd, int handle)
> >>> + int fd, int handle, u64 point)
> >>> {
> >>> struct dma_fence *fence = sync_file_get_fence(fd);
> >>> struct drm_syncobj *syncobj;
> >>> @@ -755,14 +755,24 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
> >>> return -ENOENT;
> >>> }
> >>>
> >>> - drm_syncobj_replace_fence(syncobj, fence);
> >>> + if (point) {
> >>> + struct dma_fence_chain *chain = dma_fence_chain_alloc();
> >>> +
> >>> + if (!chain)
> >>> + return -ENOMEM;
> >>> +
> >>> + drm_syncobj_add_point(syncobj, chain, fence, point);
> >>> + } else {
> >>> + drm_syncobj_replace_fence(syncobj, fence);
> >>> + }
> >>> +
> >>> dma_fence_put(fence);
> >>> drm_syncobj_put(syncobj);
> >>> return 0;
> >>> }
> >>>
> >>> static int drm_syncobj_export_sync_file(struct drm_file *file_private,
> >>> - int handle, int *p_fd)
> >>> + int handle, u64 point, int *p_fd)
> >>> {
> >>> int ret;
> >>> struct dma_fence *fence;
> >>> @@ -772,7 +782,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private,
> >>> if (fd < 0)
> >>> return fd;
> >>>
> >>> - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
> >>> + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
> >>> if (ret)
> >>> goto err_put_fd;
> >>>
> >>> @@ -869,6 +879,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> >>> struct drm_file *file_private)
> >>> {
> >>> struct drm_syncobj_handle *args = data;
> >>> + unsigned int valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE |
> >>> + DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE;
> >>> + u64 point = 0;
> >>>
> >>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> >>> return -EOPNOTSUPP;
> >>> @@ -876,13 +889,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> >>> if (args->pad)
> >>> return -EINVAL;
> >>>
> >>> - if (args->flags != 0 &&
> >>> - args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
> >>> + if (args->flags & ~valid_flags)
> >>> return -EINVAL;
> >>>
> >>> + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE)
> >>> + point = args->point;
> >>> +
> >>> if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
> >>> return drm_syncobj_export_sync_file(file_private, args->handle,
> >>> - &args->fd);
> >>> + point, &args->fd);
> >>> +
> >>> + if (args->point)
> >>> + return -EINVAL;
> >>>
> >>> return drm_syncobj_handle_to_fd(file_private, args->handle,
> >>> &args->fd);
> >>> @@ -893,6 +911,9 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> >>> struct drm_file *file_private)
> >>> {
> >>> struct drm_syncobj_handle *args = data;
> >>> + unsigned int valid_flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_TIMELINE |
> >>> + DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE;
> >>> + u64 point = 0;
> >>>
> >>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> >>> return -EOPNOTSUPP;
> >> oh, I suppose I should add a check for DRIVER_SYNCOBJ_TIMELINE? I'll
> >> send a v7 a bit later
> > Christian already applied to misc-test, please rebase and make it as a
> > new patch
>
> Yeah, sorry I was a bit to quick obviously.
>
> On the other hand I don't see an immediate need for a check for DRIVER_SYNCOBJ_TIMELINE here.
>
> The functions should work even when the driver doesn't handle timeline syncobj on it's own.
Ok, no problem, I'll just put an explicit cap check in virglrenderer,
rather than relying on this to tell me also if the driver supports
timeline:
struct drm_syncobj_handle args = {
.handle = 0, /* invalid handle */
.flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE |
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE,
.fd = -1,
.point = 1,
};
errno = 0;
ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD, &args);
/* ENOENT means the kernel supports
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE
* but that we didn't provide a valid handle. EINVAL means
the kernel does
* not support DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE.
*/
if (errno == ENOENT) {
resp[0] = true;
resp[1] = 1;
} else {
assert(errno == EINVAL);
}
BR,
-R
> Regards,
> Christian.
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-02 16:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-01 15:57 [PATCH v6] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs Rob Clark
2025-04-01 20:40 ` Rob Clark
2025-04-01 20:46 ` Dmitry Osipenko
2025-04-02 6:55 ` Christian König
2025-04-02 16:11 ` Rob Clark
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox