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@xxxxxxxxxxxxxxx>
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
