[PATCH] drm/etnaviv: correct timeout calculation

[Russell King - ARM Linux]

Hi Lucas,

Please retain my authorship of my patch, which was sent on 23 Oct 2017.
The patch you have below is 100% identical to that which I sent.

You should also point out, as per the follow-on discussion, that using
clock_gettime() on 32-bit systems will not work once the time it
reports wraps - so something like the comment I suggested in a follow
up patch:

+ * Etnaviv timeouts are specified wrt CLOCK_MONOTONIC, not jiffies.
+ * We need to calculate the timeout in terms of number of jiffies
+ * between the specified timeout and the current CLOCK_MONOTONIC time.
+ * Note: clock_gettime() is 32-bit on 32-bit arch.  Using 64-bit
+ * timespec math here just means that when a wrap occurs, the
+ * specified timeout goes into the past and we can't request a
+ * timeout in the future: IOW, the code breaks.

would be sensible either in the commit message or the code.

On Fri, Mar 09, 2018 at 12:21:49PM +0100, Lucas Stach wrote:
> The old way did clamp the jiffy conversion and thus caused the timeouts
> to become negative after some time. Also it didn't work with userspace
> which actually fills the upper 32bits of the 64bit timestamp value.
> 
> Fix this by using the solution developed and tested by Russell.
> 
> Suggested-by: Russell King <linux at armlinux.org.uk>
> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index ddb17ee565e9..17a43da98fb9 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -26,6 +26,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  #include <linux/list.h>
> +#include <linux/time64.h>
>  #include <linux/types.h>
>  #include <linux/sizes.h>
>  
> @@ -132,19 +133,27 @@ static inline bool fence_after_eq(u32 a, u32 b)
>  	return (s32)(a - b) >= 0;
>  }
>  
> +/*
> + * Etnaviv timeouts are specified wrt CLOCK_MONOTONIC, not jiffies.
> + * We need to calculate the timeout in terms of number of jiffies
> + * between the specified timeout and the current CLOCK_MONOTONIC time.
> + */
>  static inline unsigned long etnaviv_timeout_to_jiffies(
>  	const struct timespec *timeout)
>  {
> -	unsigned long timeout_jiffies = timespec_to_jiffies(timeout);
> -	unsigned long start_jiffies = jiffies;
> -	unsigned long remaining_jiffies;
> +	struct timespec64 ts, to;
> +
> +	to = timespec_to_timespec64(*timeout);
> +
> +	ktime_get_ts64(&ts);
> +
> +	/* timeouts before "now" have already expired */
> +	if (timespec64_compare(&to, &ts) <= 0)
> +		return 0;
>  
> -	if (time_after(start_jiffies, timeout_jiffies))
> -		remaining_jiffies = 0;
> -	else
> -		remaining_jiffies = timeout_jiffies - start_jiffies;
> +	ts = timespec64_sub(to, ts);
>  
> -	return remaining_jiffies;
> +	return timespec64_to_jiffies(&ts);
>  }
>  
>  #endif /* __ETNAVIV_DRV_H__ */
> -- 
> 2.16.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up