Re: [PATCH v7 05/15] dma-buf/sync_file: Add SET_DEADLINE ioctl

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4492 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]