Re: [PATCH v2 1/6] drm: Move drm_timeout_abs_to_jiffies() to drm_timeout.c

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 17.08.26 um 23:05 schrieb Maíra Canal:
> drm_timeout_abs_to_jiffies() began as a static helper inside the syncobj
> wait implementation and was later exported in place, so that it could be
> called by multiple drivers (e.g. panfrost, lima, tegra and accel
> drivers). All of them reach it through drm_utils.h.
>
> Give it a file of its own, so that timeout conversion helpers have a
> home, which will be useful when we add new timeout handlers. The
> declaration stays in drm_utils.h, so no caller changes.
>
> No functional change.
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>   drivers/gpu/drm/Makefile      |  1 +
>   drivers/gpu/drm/drm_syncobj.c | 33 ------------------------------
>   drivers/gpu/drm/drm_timeout.c | 47 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 48 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index e97faabcd783..0a0d7ea08347 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -68,6 +68,7 @@ drm-y := \
>   	drm_rect.o \
>   	drm_syncobj.o \
>   	drm_sysfs.o \
> +	drm_timeout.o \
>   	drm_trace_points.o \
>   	drm_vblank.o \
>   	drm_vblank_work.o \
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 2fa170a29a62..cf03681de6f7 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1193,39 +1193,6 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   	return timeout;
>   }
>   
> -/**
> - * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value
> - *
> - * @timeout_nsec: timeout nsec component in ns, 0 for poll
> - *
> - * Calculate the timeout in jiffies from an absolute time in sec/nsec.
> - */
> -signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
> -{
> -	ktime_t abs_timeout, now;
> -	u64 timeout_ns, timeout_jiffies64;
> -
> -	/* make 0 timeout means poll - absolute 0 doesn't seem valid */
> -	if (timeout_nsec == 0)
> -		return 0;
> -
> -	abs_timeout = ns_to_ktime(timeout_nsec);
> -	now = ktime_get();
> -
> -	if (!ktime_after(abs_timeout, now))
> -		return 0;
> -
> -	timeout_ns = ktime_to_ns(ktime_sub(abs_timeout, now));
> -
> -	timeout_jiffies64 = nsecs_to_jiffies64(timeout_ns);
> -	/*  clamp timeout to avoid infinite timeout */
> -	if (timeout_jiffies64 >= MAX_SCHEDULE_TIMEOUT - 1)
> -		return MAX_SCHEDULE_TIMEOUT - 1;
> -
> -	return timeout_jiffies64 + 1;
> -}
> -EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
> -
>   static int drm_syncobj_array_wait(struct drm_device *dev,
>   				  struct drm_file *file_private,
>   				  struct drm_syncobj_wait *wait,
> diff --git a/drivers/gpu/drm/drm_timeout.c b/drivers/gpu/drm/drm_timeout.c
> new file mode 100644
> index 000000000000..78e9f65e5477
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_timeout.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Timeout conversion helpers for wait ioctls.
> + *
> + * Copyright 2017 Red Hat
> + * Copyright 2016 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/export.h>
> +#include <linux/jiffies.h>
> +#include <linux/ktime.h>
> +#include <linux/sched.h>
> +
> +#include <drm/drm_utils.h>
> +
> +/**
> + * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value
> + *

No empty line here

> + * @timeout_nsec: timeout nsec component in ns, 0 for poll
> + *
> + * Calculate the timeout in jiffies from an absolute time in sec/nsec.

Just 'nsec'.

The documentation and commit  description should also be clear that the 
result is the number of jiffies until the given timeout (right?) and not 
some absolute value in jiffies.

> + */

Needs to document the returned value

   Returns:
   The number of jiffies until the given timeout, or some upper limit

Best regards
Thomas

> +signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
> +{
> +	ktime_t abs_timeout, now;
> +	u64 timeout_ns, timeout_jiffies64;
> +
> +	/* make 0 timeout means poll - absolute 0 doesn't seem valid */
> +	if (timeout_nsec == 0)
> +		return 0;
> +
> +	abs_timeout = ns_to_ktime(timeout_nsec);
> +	now = ktime_get();
> +
> +	if (!ktime_after(abs_timeout, now))
> +		return 0;
> +
> +	timeout_ns = ktime_to_ns(ktime_sub(abs_timeout, now));
> +
> +	timeout_jiffies64 = nsecs_to_jiffies64(timeout_ns);
> +	/*  clamp timeout to avoid infinite timeout */
> +	if (timeout_jiffies64 >= MAX_SCHEDULE_TIMEOUT - 1)
> +		return MAX_SCHEDULE_TIMEOUT - 1;
> +
> +	return timeout_jiffies64 + 1;
> +}
> +EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)