| Message ID | 20230227193535.2822389-6-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dma-fence: Deadline awareness | expand |
On Mon, 27 Feb 2023 11:35:11 -0800 Rob Clark <robdclark@gmail.com> wrote: > From: Rob Clark <robdclark@chromium.org> > > The initial purpose is for igt tests, but this would also be useful for > compositors that wait until close to vblank deadline to make decisions > about which frame to show. > > The igt tests can be found at: > > https://gitlab.freedesktop.org/robclark/igt-gpu-tools/-/commits/fence-deadline > > v2: Clarify the timebase, add link to igt tests > v3: Use u64 value in ns to express deadline. > v4: More doc > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/dma-buf/dma-fence.c | 3 ++- > drivers/dma-buf/sync_file.c | 19 +++++++++++++++++++ > include/uapi/linux/sync_file.h | 22 ++++++++++++++++++++++ > 3 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index e103e821d993..7761ceeae620 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -933,7 +933,8 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); > * the GPU's devfreq to reduce frequency, when in fact the opposite is what is > * needed. > * > - * To this end, deadline hint(s) can be set on a &dma_fence via &dma_fence_set_deadline. > + * To this end, deadline hint(s) can be set on a &dma_fence via &dma_fence_set_deadline > + * (or indirectly via userspace facing ioctls like &SYNC_IOC_SET_DEADLINE). > * The deadline hint provides a way for the waiting driver, or userspace, to > * convey an appropriate sense of urgency to the signaling driver. Hi, when the kernel HTML doc is generated, I assume the above becomes a link to "DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence", right? > * > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > index af57799c86ce..418021cfb87c 100644 > --- a/drivers/dma-buf/sync_file.c > +++ b/drivers/dma-buf/sync_file.c > @@ -350,6 +350,22 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, > return ret; > } > > +static int sync_file_ioctl_set_deadline(struct sync_file *sync_file, > + unsigned long arg) > +{ > + struct sync_set_deadline ts; > + > + if (copy_from_user(&ts, (void __user *)arg, sizeof(ts))) > + return -EFAULT; > + > + if (ts.pad) > + return -EINVAL; > + > + dma_fence_set_deadline(sync_file->fence, ns_to_ktime(ts.deadline_ns)); > + > + return 0; > +} > + > static long sync_file_ioctl(struct file *file, unsigned int cmd, > unsigned long arg) > { > @@ -362,6 +378,9 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd, > case SYNC_IOC_FILE_INFO: > return sync_file_ioctl_fence_info(sync_file, arg); > > + case SYNC_IOC_SET_DEADLINE: > + return sync_file_ioctl_set_deadline(sync_file, arg); > + > default: > return -ENOTTY; > } > diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h > index ee2dcfb3d660..49325cf6749b 100644 > --- a/include/uapi/linux/sync_file.h > +++ b/include/uapi/linux/sync_file.h > @@ -67,6 +67,21 @@ struct sync_file_info { > __u64 sync_fence_info; > }; > > +/** > + * struct sync_set_deadline - set a deadline hint on a fence > + * @deadline_ns: absolute time of the deadline Is it legal to pass zero as deadline_ns? > + * @pad: must be zero > + * > + * The timebase for the deadline is CLOCK_MONOTONIC (same as vblank) Does something here provide doc links to "DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence" and to the "DOC: deadline hints"? > + */ > +struct sync_set_deadline { > + __u64 deadline_ns; > + /* Not strictly needed for alignment but gives some possibility > + * for future extension: > + */ > + __u64 pad; > +}; > + > #define SYNC_IOC_MAGIC '>' > > /** > @@ -95,4 +110,11 @@ struct sync_file_info { > */ > #define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) > > +/** > + * DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence > + * > + * Allows userspace to set a deadline on a fence, see dma_fence_set_deadline() Does something here provide doc links to struct sync_set_deadline and to the "DOC: deadline hints"? > + */ > +#define SYNC_IOC_SET_DEADLINE _IOW(SYNC_IOC_MAGIC, 5, struct sync_set_deadline) > + > #endif /* _UAPI_LINUX_SYNC_H */ With all those links added: Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> Thanks, pq
On Tue, Feb 28, 2023 at 1:22 AM Pekka Paalanen <ppaalanen@gmail.com> wrote: > > On Mon, 27 Feb 2023 11:35:11 -0800 > Rob Clark <robdclark@gmail.com> wrote: > > > From: Rob Clark <robdclark@chromium.org> > > > > The initial purpose is for igt tests, but this would also be useful for > > compositors that wait until close to vblank deadline to make decisions > > about which frame to show. > > > > The igt tests can be found at: > > > > https://gitlab.freedesktop.org/robclark/igt-gpu-tools/-/commits/fence-deadline > > > > v2: Clarify the timebase, add link to igt tests > > v3: Use u64 value in ns to express deadline. > > v4: More doc > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/dma-buf/dma-fence.c | 3 ++- > > drivers/dma-buf/sync_file.c | 19 +++++++++++++++++++ > > include/uapi/linux/sync_file.h | 22 ++++++++++++++++++++++ > > 3 files changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > index e103e821d993..7761ceeae620 100644 > > --- a/drivers/dma-buf/dma-fence.c > > +++ b/drivers/dma-buf/dma-fence.c > > @@ -933,7 +933,8 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); > > * the GPU's devfreq to reduce frequency, when in fact the opposite is what is > > * needed. > > * > > - * To this end, deadline hint(s) can be set on a &dma_fence via &dma_fence_set_deadline. > > + * To this end, deadline hint(s) can be set on a &dma_fence via &dma_fence_set_deadline > > + * (or indirectly via userspace facing ioctls like &SYNC_IOC_SET_DEADLINE). > > * The deadline hint provides a way for the waiting driver, or userspace, to > > * convey an appropriate sense of urgency to the signaling driver. > > Hi, > > when the kernel HTML doc is generated, I assume the above becomes a > link to "DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence", right? Heh, kernel docs completely miss the sync_file uABI.. I'll add a patch to correct that in order to make these links work properly. BR, -R > > * > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > > index af57799c86ce..418021cfb87c 100644 > > --- a/drivers/dma-buf/sync_file.c > > +++ b/drivers/dma-buf/sync_file.c > > @@ -350,6 +350,22 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, > > return ret; > > } > > > > +static int sync_file_ioctl_set_deadline(struct sync_file *sync_file, > > + unsigned long arg) > > +{ > > + struct sync_set_deadline ts; > > + > > + if (copy_from_user(&ts, (void __user *)arg, sizeof(ts))) > > + return -EFAULT; > > + > > + if (ts.pad) > > + return -EINVAL; > > + > > + dma_fence_set_deadline(sync_file->fence, ns_to_ktime(ts.deadline_ns)); > > + > > + return 0; > > +} > > + > > static long sync_file_ioctl(struct file *file, unsigned int cmd, > > unsigned long arg) > > { > > @@ -362,6 +378,9 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd, > > case SYNC_IOC_FILE_INFO: > > return sync_file_ioctl_fence_info(sync_file, arg); > > > > + case SYNC_IOC_SET_DEADLINE: > > + return sync_file_ioctl_set_deadline(sync_file, arg); > > + > > default: > > return -ENOTTY; > > } > > diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h > > index ee2dcfb3d660..49325cf6749b 100644 > > --- a/include/uapi/linux/sync_file.h > > +++ b/include/uapi/linux/sync_file.h > > @@ -67,6 +67,21 @@ struct sync_file_info { > > __u64 sync_fence_info; > > }; > > > > +/** > > + * struct sync_set_deadline - set a deadline hint on a fence > > + * @deadline_ns: absolute time of the deadline > > Is it legal to pass zero as deadline_ns? > > > + * @pad: must be zero > > + * > > + * The timebase for the deadline is CLOCK_MONOTONIC (same as vblank) > > Does something here provide doc links to "DOC: SYNC_IOC_SET_DEADLINE - > set a deadline on a fence" and to the "DOC: deadline hints"? > > > + */ > > +struct sync_set_deadline { > > + __u64 deadline_ns; > > + /* Not strictly needed for alignment but gives some possibility > > + * for future extension: > > + */ > > + __u64 pad; > > +}; > > + > > #define SYNC_IOC_MAGIC '>' > > > > /** > > @@ -95,4 +110,11 @@ struct sync_file_info { > > */ > > #define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) > > > > +/** > > + * DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence > > + * > > + * Allows userspace to set a deadline on a fence, see dma_fence_set_deadline() > > Does something here provide doc links to struct sync_set_deadline and > to the "DOC: deadline hints"? > > > + */ > > +#define SYNC_IOC_SET_DEADLINE _IOW(SYNC_IOC_MAGIC, 5, struct sync_set_deadline) > > + > > #endif /* _UAPI_LINUX_SYNC_H */ > > With all those links added: > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> > > > Thanks, > pq
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index e103e821d993..7761ceeae620 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -933,7 +933,8 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout); * the GPU's devfreq to reduce frequency, when in fact the opposite is what is * needed. * - * To this end, deadline hint(s) can be set on a &dma_fence via &dma_fence_set_deadline. + * To this end, deadline hint(s) can be set on a &dma_fence via &dma_fence_set_deadline + * (or indirectly via userspace facing ioctls like &SYNC_IOC_SET_DEADLINE). * The deadline hint provides a way for the waiting driver, or userspace, to * convey an appropriate sense of urgency to the signaling driver. * diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index af57799c86ce..418021cfb87c 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -350,6 +350,22 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, return ret; } +static int sync_file_ioctl_set_deadline(struct sync_file *sync_file, + unsigned long arg) +{ + struct sync_set_deadline ts; + + if (copy_from_user(&ts, (void __user *)arg, sizeof(ts))) + return -EFAULT; + + if (ts.pad) + return -EINVAL; + + dma_fence_set_deadline(sync_file->fence, ns_to_ktime(ts.deadline_ns)); + + return 0; +} + static long sync_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -362,6 +378,9 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd, case SYNC_IOC_FILE_INFO: return sync_file_ioctl_fence_info(sync_file, arg); + case SYNC_IOC_SET_DEADLINE: + return sync_file_ioctl_set_deadline(sync_file, arg); + default: return -ENOTTY; } diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h index ee2dcfb3d660..49325cf6749b 100644 --- a/include/uapi/linux/sync_file.h +++ b/include/uapi/linux/sync_file.h @@ -67,6 +67,21 @@ struct sync_file_info { __u64 sync_fence_info; }; +/** + * struct sync_set_deadline - set a deadline hint on a fence + * @deadline_ns: absolute time of the deadline + * @pad: must be zero + * + * The timebase for the deadline is CLOCK_MONOTONIC (same as vblank) + */ +struct sync_set_deadline { + __u64 deadline_ns; + /* Not strictly needed for alignment but gives some possibility + * for future extension: + */ + __u64 pad; +}; + #define SYNC_IOC_MAGIC '>' /** @@ -95,4 +110,11 @@ struct sync_file_info { */ #define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) +/** + * DOC: SYNC_IOC_SET_DEADLINE - set a deadline on a fence + * + * Allows userspace to set a deadline on a fence, see dma_fence_set_deadline() + */ +#define SYNC_IOC_SET_DEADLINE _IOW(SYNC_IOC_MAGIC, 5, struct sync_set_deadline) + #endif /* _UAPI_LINUX_SYNC_H */