Re: [PATCH 3/6] drm/amdgpu: Use drm_timeout_abs_to_jiffies()

On 8/9/26 21:23, Maíra Canal wrote:
> amdgpu_gem_timeout() converts an absolute deadline in ns into jiffies,
> which is what drm_timeout_abs_to_jiffies() already does for the other
> drivers whose wait UAPI takes a deadline. Use the shared helper and keep
> only the part that is specific to amdgpu.
> 
> Two details change along this conversion: the helper rounds up rather than
> truncating, so a deadline less than a tick away now waits for one jiffy
> instead of returning 0. It also uses nsecs_to_jiffies64(), so the
> conversion no longer truncates on 32-bit, where a large deadline could
> previously be reduced to an arbitrary shorter one.
> 
> Signed-off-by: Maíra Canal <[email protected]>
> 
> ---
> 
> As a note, this patch can be merged independently to the AMD tree
> without any dependencies.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6a0699746fbc..84b509a484b0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -25,7 +25,6 @@
>   *          Alex Deucher
>   *          Jerome Glisse
>   */
> -#include <linux/ktime.h>
>  #include <linux/module.h>
>  #include <linux/overflow.h>
>  #include <linux/pagemap.h>
> @@ -40,6 +39,7 @@
>  #include <drm/drm_gem_ttm_helper.h>
>  #include <drm/ttm/ttm_tt.h>
>  #include <drm/drm_syncobj.h>
> +#include <drm/drm_utils.h>
>  
>  #include "amdgpu.h"
>  #include "amdgpu_display.h"
> @@ -622,23 +622,11 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   */
>  unsigned long amdgpu_gem_timeout(uint64_t timeout_ns)

Please completely nuke that function and replace it with calls to drm_timeout_abs_to_jiffies().

>  {
> -	unsigned long timeout_jiffies;
> -	ktime_t timeout;
> -
>  	/* clamp timeout if it's to large */
>  	if (((int64_t)timeout_ns) < 0)
>  		return MAX_SCHEDULE_TIMEOUT;

That check was actually never correct at all as far as I can see.

We need to make sure that when timeout_ns is larger than represent-able in long jiffies (especially on 32bit systems) then MAX_SCHEDULE_TIMEOUT is returned by drm_timeout_abs_to_jiffies().

And I hope that drm_timeout_abs_to_jiffies() does that correctly already, if not this seriously needs fixing anyway.

Regards,
Christian.

>  
> -	timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
> -	if (ktime_to_ns(timeout) < 0)
> -		return 0;
> -
> -	timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
> -	/*  clamp timeout to avoid unsigned-> signed overflow */
> -	if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT)
> -		return MAX_SCHEDULE_TIMEOUT - 1;
> -
> -	return timeout_jiffies;
> +	return drm_timeout_abs_to_jiffies(timeout_ns);
>  }
>  
>  int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
>