drm/xe: Use nanoseconds instead of jiffies in uapi for user fence
authorZbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Wed, 28 Jun 2023 05:51:41 +0000 (07:51 +0200)
committerRodrigo Vivi <rodrigo.vivi@intel.com>
Thu, 21 Dec 2023 16:35:06 +0000 (11:35 -0500)
Using jiffies as a timeout from userspace is weird even if
theoretically exists possiblity of acquiring jiffies via getconf.
Unfortunately this method is unreliable and the returned
value may vary from the one configured in the kernel config.

Now timeout is expressed in nanoseconds and its interpretation depends
on setting DRM_XE_UFENCE_WAIT_ABSTIME flag. Relative timeout (flag
is not set) means fence expire at now() + timeout. Absolute timeout
(flag is set) means that the fence expires at exact point of time.
Passing negative timeout means we will wait "forever" by setting
wait time to MAX_SCHEDULE_TIMEOUT.

Cc: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Link: https://lore.kernel.org/r/20230628055141.398036-2-zbigniew.kempczynski@intel.com
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
drivers/gpu/drm/xe/xe_wait_user_fence.c
include/uapi/drm/xe_drm.h

index 098e2a4cff3fb7eb6ddb36bad4cb4637c710ef31..c4420c0dbf9c2e906fb6b6f2dbf0ec2280e669a5 100644 (file)
@@ -7,6 +7,7 @@
 
 #include <drm/drm_device.h>
 #include <drm/drm_file.h>
+#include <drm/drm_utils.h>
 #include <drm/xe_drm.h>
 
 #include "xe_device.h"
@@ -84,6 +85,21 @@ static int check_hw_engines(struct xe_device *xe,
                         DRM_XE_UFENCE_WAIT_VM_ERROR)
 #define MAX_OP         DRM_XE_UFENCE_WAIT_LTE
 
+static unsigned long to_jiffies_timeout(struct drm_xe_wait_user_fence *args)
+{
+       unsigned long timeout;
+
+       if (args->flags & DRM_XE_UFENCE_WAIT_ABSTIME)
+               return drm_timeout_abs_to_jiffies(args->timeout);
+
+       if (args->timeout == MAX_SCHEDULE_TIMEOUT || args->timeout == 0)
+               return args->timeout;
+
+       timeout = nsecs_to_jiffies(args->timeout);
+
+       return timeout ?: 1;
+}
+
 int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
                             struct drm_file *file)
 {
@@ -98,7 +114,8 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
        int err;
        bool no_engines = args->flags & DRM_XE_UFENCE_WAIT_SOFT_OP ||
                args->flags & DRM_XE_UFENCE_WAIT_VM_ERROR;
-       unsigned long timeout = args->timeout;
+       unsigned long timeout;
+       ktime_t start;
 
        if (XE_IOCTL_ERR(xe, args->extensions) || XE_IOCTL_ERR(xe, args->pad) ||
            XE_IOCTL_ERR(xe, args->reserved[0] || args->reserved[1]))
@@ -152,8 +169,18 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
                addr = vm->async_ops.error_capture.addr;
        }
 
-       if (XE_IOCTL_ERR(xe, timeout > MAX_SCHEDULE_TIMEOUT))
-               return -EINVAL;
+       /*
+        * For negative timeout we want to wait "forever" by setting
+        * MAX_SCHEDULE_TIMEOUT. But we have to assign this value also
+        * to args->timeout to avoid being zeroed on the signal delivery
+        * (see arithmetics after wait).
+        */
+       if (args->timeout < 0)
+               args->timeout = MAX_SCHEDULE_TIMEOUT;
+
+       timeout = to_jiffies_timeout(args);
+
+       start = ktime_get();
 
        /*
         * FIXME: Very simple implementation at the moment, single wait queue
@@ -192,17 +219,17 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
        } else {
                remove_wait_queue(&xe->ufence_wq, &w_wait);
        }
+
+       if (!(args->flags & DRM_XE_UFENCE_WAIT_ABSTIME)) {
+               args->timeout -= ktime_to_ns(ktime_sub(ktime_get(), start));
+               if (args->timeout < 0)
+                       args->timeout = 0;
+       }
+
        if (XE_IOCTL_ERR(xe, err < 0))
                return err;
        else if (XE_IOCTL_ERR(xe, !timeout))
                return -ETIME;
 
-       /*
-        * Again very simple, return the time in jiffies that has past, may need
-        * a more precision
-        */
-       if (args->flags & DRM_XE_UFENCE_WAIT_ABSTIME)
-               args->timeout = args->timeout - timeout;
-
        return 0;
 }
index 8e7be155133329e5f538587e2ce678e53c27259a..347351a8f618488d19b040b82a3b080b486eaae5 100644 (file)
@@ -904,8 +904,20 @@ struct drm_xe_wait_user_fence {
 #define DRM_XE_UFENCE_WAIT_U64         0xffffffffffffffffu
        /** @mask: comparison mask */
        __u64 mask;
-
-       /** @timeout: how long to wait before bailing, value in jiffies */
+       /**
+        * @timeout: how long to wait before bailing, value in nanoseconds.
+        * Without DRM_XE_UFENCE_WAIT_ABSTIME flag set (relative timeout)
+        * it contains timeout expressed in nanoseconds to wait (fence will
+        * expire at now() + timeout).
+        * When DRM_XE_UFENCE_WAIT_ABSTIME flat is set (absolute timeout) wait
+        * will end at timeout (uses system MONOTONIC_CLOCK).
+        * Passing negative timeout leads to neverending wait.
+        *
+        * On relative timeout this value is updated with timeout left
+        * (for restarting the call in case of signal delivery).
+        * On absolute timeout this value stays intact (restarted call still
+        * expire at the same point of time).
+        */
        __s64 timeout;
 
        /**