After having to debug down through the kernel to figure out
why my _WAIT calls were always timing out, I realized its
an absolute timeout value instead of the more common relative
timeouts.
This detail should be called out in the documentation, as while
the absolute value makes sense here, its not as common for timeout
values.
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Jason Ekstrand <[email protected]>
Cc: Christian König <[email protected]>
Cc: Lionel Landwerlin <[email protected]>
Cc: Chunming Zhou <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/gpu/drm/drm_syncobj.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 7e48dcd1bee4..b84d842a1c21 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -136,6 +136,10 @@
* requirement is inherited from the wait-before-signal behavior required by
* the Vulkan timeline semaphore API.
*
+ * It should be noted, that both &DRM_IOCTL_SYNCOBJ_WAIT and
+ * &DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT takes an *absolute* CLOCK_MONOTONIC
+ * nanosecond value for the timeout value. Accidentally passing relative time
+ * values will likely result in an immediate -ETIME return.
*
* Import/export of syncobjs
* -------------------------
--
2.37.0.144.g8ac04bfd2-goog
An initial pass at a drm_syncobj API test.
Currently covers trivial use of:
DRM_IOCTL_SYNCOBJ_CREATE
DRM_IOCTL_SYNCOBJ_DESTROY
DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD
DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE
DRM_IOCTL_SYNCOBJ_WAIT
DRM_IOCTL_SYNCOBJ_RESET
DRM_IOCTL_SYNCOBJ_SIGNAL
DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT
DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL
And demonstrates how the userspace API can be used, along with
some fairly simple bad parameter checking.
The patch includes a few helpers taken from libdrm, as at least
on the VM I was testing with, I didn't have a new enough libdrm
to support the *_TIMELINE_* ioctls. Ideally the ioctl-helper bits
can be dropped at a later time.
Feedback would be appreciated!
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Jason Ekstrand <[email protected]>
Cc: Christian König <[email protected]>
Cc: Lionel Landwerlin <[email protected]>
Cc: Chunming Zhou <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
.../drivers/gpu/drm_syncobj/Makefile | 11 +
.../drivers/gpu/drm_syncobj/ioctl-helper.c | 85 ++++
.../drivers/gpu/drm_syncobj/ioctl-helper.h | 74 ++++
.../drivers/gpu/drm_syncobj/syncobj-test.c | 410 ++++++++++++++++++
4 files changed, 580 insertions(+)
create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile
create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c
create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h
create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c
diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile b/tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile
new file mode 100644
index 000000000000..6576d9b2006c
--- /dev/null
+++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -I/usr/include/libdrm/
+LDFLAGS += -pthread -ldrm
+
+TEST_GEN_FILES= syncobj-test
+
+include ../../../lib.mk
+
+$(OUTPUT)/syncobj-test: syncobj-test.c ioctl-helper.c
+EXTRA_CLEAN = $(OUTPUT)/ioctl-helper.o
+
diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c
new file mode 100644
index 000000000000..e5c59c9bed36
--- /dev/null
+++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: MIT
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <time.h>
+#include <errno.h>
+#include <libdrm/drm.h>
+#include <xf86drm.h>
+#include "ioctl-helper.h"
+
+#ifndef DRM_CAP_SYNCOBJ_TIMELINE
+/*
+ * The following is nabbed from libdrm's xf86drm.c as the
+ * installed libdrm doesn't yet include these definitions
+ *
+ *
+ * \author Rickard E. (Rik) Faith <[email protected]>
+ * \author Kevin E. Martin <[email protected]>
+ */
+/*
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+int drmSyncobjTimelineSignal(int fd, const uint32_t *handles,
+ uint64_t *points, uint32_t handle_count)
+{
+ struct drm_syncobj_timeline_array args;
+ int ret;
+
+ memset(&args, 0, sizeof(args));
+ args.handles = (uintptr_t)handles;
+ args.points = (uintptr_t)points;
+ args.count_handles = handle_count;
+
+ ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, &args);
+ return ret;
+}
+
+int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points,
+ unsigned int num_handles,
+ int64_t timeout_nsec, unsigned int flags,
+ uint32_t *first_signaled)
+{
+ struct drm_syncobj_timeline_wait args;
+ int ret;
+
+ memset(&args, 0, sizeof(args));
+ args.handles = (uintptr_t)handles;
+ args.points = (uintptr_t)points;
+ args.timeout_nsec = timeout_nsec;
+ args.count_handles = num_handles;
+ args.flags = flags;
+
+ ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, &args);
+ if (ret < 0)
+ return -errno;
+
+ if (first_signaled)
+ *first_signaled = args.first_signaled;
+ return ret;
+}
+
+#endif
diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h
new file mode 100644
index 000000000000..b0c1025034b5
--- /dev/null
+++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: MIT */
+#ifndef __IOCTL_HELPER_H__
+#define __IOCTL_HELPER_H__
+
+/* Bits pulled from libdrm's include/drm/drm.h */
+#ifndef DRM_CAP_SYNCOBJ_TIMELINE
+/*
+ * Header for the Direct Rendering Manager
+ *
+ * Author: Rickard E. (Rik) Faith <[email protected]>
+ *
+ * Acknowledgments:
+ * Dec 1999, Richard Henderson <[email protected]>, move to generic cmpxchg.
+ */
+
+/*
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+struct drm_syncobj_timeline_wait {
+ __u64 handles;
+ /* wait on specific timeline point for every handles*/
+ __u64 points;
+ /* absolute timeout */
+ __s64 timeout_nsec;
+ __u32 count_handles;
+ __u32 flags;
+ __u32 first_signaled; /* only valid when not waiting all */
+ __u32 pad;
+};
+
+
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0)
+struct drm_syncobj_timeline_array {
+ __u64 handles;
+ __u64 points;
+ __u32 count_handles;
+ __u32 flags;
+};
+
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait)
+#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct drm_syncobj_timeline_array)
+#define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array)
+
+int drmSyncobjTimelineSignal(int fd, const uint32_t *handles,
+ uint64_t *points, uint32_t handle_count);
+int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points,
+ unsigned int num_handles,
+ int64_t timeout_nsec, unsigned int flags,
+ uint32_t *first_signaled);
+#endif
+#endif /*__IOCTL_HELPER_H__*/
+
diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c b/tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c
new file mode 100644
index 000000000000..21474b0d3b9e
--- /dev/null
+++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This test exercises basic syncobj ioctl interfaces from
+ * userland via the libdrm helpers.
+ *
+ * Copyright (C) 2022, Google LLC.
+ *
+ * Currently covers trivial use of:
+ * DRM_IOCTL_SYNCOBJ_CREATE
+ * DRM_IOCTL_SYNCOBJ_DESTROY
+ * DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD
+ * DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE
+ * DRM_IOCTL_SYNCOBJ_WAIT
+ * DRM_IOCTL_SYNCOBJ_RESET
+ * DRM_IOCTL_SYNCOBJ_SIGNAL
+ * DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT
+ * DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL
+ *
+ * TODO: Need coverage for the following ioctls:
+ * DRM_IOCTL_SYNCOBJ_QUERY
+ * DRM_IOCTL_SYNCOBJ_TRANSFER
+ * As well as more complicated use of interface (like
+ * signal/wait with multiple handles, etc), and sync_file
+ * import/export.
+ */
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <time.h>
+#include <poll.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <pthread.h>
+#include <linux/dma-buf.h>
+#include <libdrm/drm.h>
+#include <xf86drm.h>
+
+#include "ioctl-helper.h"
+
+
+#define NSEC_PER_SEC 1000000000ULL
+static uint64_t get_abs_timeout(uint64_t rel_nsec)
+{
+ struct timespec ts;
+ uint64_t ns;
+
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ ns = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
+ ns += rel_nsec;
+ return ns;
+}
+
+struct test_arg {
+ int dev_fd;
+ uint32_t handle;
+ int handle_fd;
+};
+#define TEST_TIMES 5
+
+void *syncobj_signal_reset(void *arg)
+{
+ struct test_arg *d = (struct test_arg *)arg;
+ int ret;
+ int i;
+
+ for (i = 0; i < TEST_TIMES; i++) {
+ sleep(3);
+ printf("%s: sending signal!\n", __func__);
+ ret = drmSyncobjSignal(d->dev_fd, &d->handle, 1);
+ if (ret)
+ printf("Signal failed %i\n", ret);
+ }
+ return NULL;
+}
+
+static int syncobj_wait_reset(struct test_arg *d)
+{
+ uint64_t abs_timeout;
+ int ret;
+ int i;
+
+ for (i = 0; i < TEST_TIMES; i++) {
+ abs_timeout = get_abs_timeout(10*NSEC_PER_SEC);
+ printf("%s calling drmSyncobjWait\n", __func__);
+ ret = drmSyncobjWait(d->dev_fd, &d->handle, 1, abs_timeout,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ NULL);
+ if (ret) {
+ printf("Error: syncobjwait failed %i\n", ret);
+ break;
+ }
+ printf("%s: drmSyncobjWait returned!\n", __func__);
+
+ ret = drmSyncobjReset(d->dev_fd, &d->handle, 1);
+ if (ret) {
+ printf("Error: syncobjreset failed\n");
+ break;
+ }
+ }
+ return ret;
+}
+
+void *syncobj_signal_timeline(void *arg)
+{
+ struct test_arg *d = (struct test_arg *)arg;
+ uint64_t point = 0;
+ int ret;
+
+ for (point = 0; point <= (TEST_TIMES-1)*5; point++) {
+ sleep(1);
+ printf("%s: sending signal %lld!\n", __func__, point);
+ ret = drmSyncobjTimelineSignal(d->dev_fd, &d->handle, &point, 1);
+ if (ret)
+ printf("Signal failed %i\n", ret);
+ }
+ return NULL;
+}
+
+
+int syncobj_timeline_wait(struct test_arg *d)
+{
+ uint64_t abs_timeout;
+ uint64_t point;
+ int ret;
+ int i;
+
+ for (i = 0; i < TEST_TIMES; i++) {
+ abs_timeout = get_abs_timeout(10*NSEC_PER_SEC);
+
+ point = i * 5;
+ printf("%s: drmSyncobjTimelineWait waiting on %lld!\n", __func__, point);
+ ret = drmSyncobjTimelineWait(d->dev_fd, &d->handle, &point, 1,
+ abs_timeout,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ NULL);
+ if (ret) {
+ printf("Error: syncobjwait failed %i\n", ret);
+ return ret;
+ }
+ printf("%s: drmSyncobjTimelineWait got %lld!\n", __func__, point);
+ }
+ return 0;
+}
+
+
+static int test_thread_signal_wait(int devfd, void *(*signal_fn)(void *),
+ int (*wait_fn)(struct test_arg *))
+{
+ uint32_t handle;
+ struct test_arg d;
+ pthread_t pth;
+ int ret;
+
+ ret = drmSyncobjCreate(devfd, 0, &handle);
+ if (ret) {
+ printf("Error: Couldn't create syncobj\n");
+ return ret;
+ }
+
+ d.dev_fd = devfd;
+ d.handle = handle;
+
+ pthread_create(&pth, 0, signal_fn, &d);
+ ret = wait_fn(&d);
+ pthread_join(pth, NULL);
+ drmSyncobjDestroy(devfd, handle);
+
+ return ret;
+}
+
+static int test_fork_signal_wait(int devfd, void *(*signal_fn)(void *),
+ int (*wait_fn)(struct test_arg *))
+{
+ uint32_t handle;
+ struct test_arg p, c;
+ pid_t id;
+ int ret;
+
+ ret = drmSyncobjCreate(devfd, 0, &handle);
+ if (ret) {
+ printf("Error: Couldn't create syncobj\n");
+ return ret;
+ }
+
+ p.dev_fd = devfd;
+ p.handle = 0;
+ p.handle_fd = 0;
+ c = p;
+ p.handle = handle;
+
+ ret = drmSyncobjHandleToFD(devfd, handle, &c.handle_fd);
+ if (ret) {
+ printf("Error: Couldn't convert handle to fd\n");
+ goto out;
+ }
+
+ id = fork();
+ if (id == 0) {
+ ret = drmSyncobjFDToHandle(c.dev_fd, c.handle_fd, &c.handle);
+ if (ret) {
+ printf("Error: Couldn't convert fd to handle\n");
+ exit(-1);
+ }
+ close(c.handle_fd);
+ signal_fn((void *)&c);
+ exit(0);
+ } else {
+ ret = wait_fn(&p);
+ waitpid(id, 0, 0);
+ }
+
+out:
+ if (c.handle_fd)
+ close(c.handle_fd);
+ drmSyncobjDestroy(devfd, handle);
+
+ return ret;
+}
+
+
+static int test_badparameters(int devfd)
+{
+ uint32_t handle1, handle2;
+ int ret, fail = 0;
+
+ /* create bad fd */
+ ret = drmSyncobjCreate(-1, 0, &handle1);
+ if (!ret || errno != EBADF) {
+ printf("drmSyncobjCreate - bad fd fails! (%i != EBADF)\n", errno);
+ fail = 1;
+ }
+ /* destroy bad fd */
+ ret = drmSyncobjDestroy(-1, handle1);
+ if (!ret || errno != EBADF) {
+ printf("drmSyncobjDestroy - bad fd fails! (%i != EBADF)\n", errno);
+ fail = 1;
+ }
+
+ /* TODO: Bad flags */
+
+ ret = drmSyncobjCreate(devfd, 0, &handle1);
+ if (ret) {
+ printf("drmSyncobjCreate - unexpected failure!\n");
+ fail = 1;
+ }
+
+ /* Destroy zeroed handle */
+ handle2 = 0;
+ ret = drmSyncobjDestroy(devfd, handle2);
+ if (!ret || errno != EINVAL) {
+ printf("drmSyncobjDestroy - zero'ed handle! (%i != EINVAL)\n", errno);
+ fail = 1;
+ }
+ /* Destroy invalid handle */
+ handle2 = -1;
+ ret = drmSyncobjDestroy(devfd, handle2);
+ if (!ret || errno != EINVAL) {
+ printf("drmSyncobjDestroy - invalid handle! (%i != EINVAL)\n", errno);
+ fail = 1;
+ }
+
+ /* invalid timeouts */
+ ret = drmSyncobjWait(devfd, &handle1, 1, 1000,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ NULL);
+ if (!ret || errno != ETIME) {
+ printf("drmSyncobjWait - invalid timeout (relative)! (%i != ETIME)\n", errno);
+ fail = 1;
+ }
+
+ ret = drmSyncobjWait(devfd, &handle1, 1, -1,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ NULL);
+ if (!ret || errno != ETIME) {
+ printf("drmSyncobjWait - invalid timeout (-1)! (%i != ETIME)\n", errno);
+ fail = 1;
+ }
+
+ ret = drmSyncobjDestroy(devfd, handle1);
+ if (ret) {
+ printf("drmSyncobjDestroy - unexpected failure!\n");
+ fail = 1;
+ }
+
+
+ return fail;
+}
+
+
+#define NAME_LEN 16
+static int check_device(int fd)
+{
+ drm_version_t version = { 0 };
+ char name[NAME_LEN];
+ uint32_t handle;
+ int ret;
+
+ memset(name, 0, NAME_LEN);
+ version.name_len = NAME_LEN;
+ version.name = name;
+
+ ret = ioctl(fd, DRM_IOCTL_VERSION, &version);
+ if (ret)
+ return -1;
+
+ printf("%s name: %s\n", __func__, name);
+
+ ret = drmSyncobjCreate(fd, 0, &handle);
+ if (!ret) {
+ drmSyncobjDestroy(fd, handle);
+ printf("%s selected: %s\n", __func__, name);
+ }
+ return ret;
+}
+
+static int find_device(void)
+{
+ int i, fd;
+ const char *drmstr = "/dev/dri/card";
+
+ fd = -1;
+ for (i = 0; i < 16; i++) {
+ char name[80];
+
+ snprintf(name, 80, "%s%u", drmstr, i);
+
+ fd = open(name, O_RDWR);
+ if (fd < 0)
+ continue;
+
+ if (check_device(fd)) {
+ close(fd);
+ fd = -1;
+ continue;
+ } else {
+ break;
+ }
+ }
+ return fd;
+}
+
+int main(int argc, char **argv)
+{
+ int devfd = find_device();
+ char *testname;
+ int ret;
+
+ if (devfd < 0) {
+ printf("Error: Couldn't find supported drm device\n");
+ return devfd;
+ }
+
+ testname = "Bad parameters test";
+ printf("\n%s\n", testname);
+ printf("===================\n");
+ ret = test_badparameters(devfd);
+ if (ret)
+ printf("%s: FAILED\n", testname);
+ else
+ printf("%s: PASSED\n", testname);
+
+
+ testname = "Threaded reset test";
+ printf("\n%s\n", testname);
+ printf("===================\n");
+ ret = test_thread_signal_wait(devfd, &syncobj_signal_reset, &syncobj_wait_reset);
+ if (ret)
+ printf("%s: FAILED\n", testname);
+ else
+ printf("%s: PASSED\n", testname);
+
+ testname = "Threaded timeline test";
+ printf("\n%s\n", testname);
+ printf("===================\n");
+ ret = test_thread_signal_wait(devfd, &syncobj_signal_timeline, &syncobj_timeline_wait);
+ if (ret)
+ printf("%s: FAILED\n", testname);
+ else
+ printf("%s: PASSED\n", testname);
+
+
+ testname = "Forked reset test";
+ printf("\n%s\n", testname);
+ printf("===================\n");
+ ret = test_fork_signal_wait(devfd, &syncobj_signal_reset, &syncobj_wait_reset);
+ if (ret)
+ printf("\n%s: FAILED\n", testname);
+ else
+ printf("\n%s: PASSED\n", testname);
+
+ testname = "Forked timeline test";
+ printf("\n%s\n", testname);
+ printf("===================\n");
+ ret = test_fork_signal_wait(devfd, &syncobj_signal_timeline, &syncobj_timeline_wait);
+ if (ret)
+ printf("\n%s: FAILED\n", testname);
+ else
+ printf("\n%s: PASSED\n", testname);
+
+
+ close(devfd);
+ return 0;
+}
--
2.37.0.144.g8ac04bfd2-goog
Allows for basic SYNCOBJ api testing, in environments
like VMs where there may not be a supported drm driver.
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Jason Ekstrand <[email protected]>
Cc: Christian König <[email protected]>
Cc: Lionel Landwerlin <[email protected]>
Cc: Chunming Zhou <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index c5e3e5457737..e5427d7399da 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -109,7 +109,8 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
}
static const struct drm_driver vgem_driver = {
- .driver_features = DRIVER_GEM | DRIVER_RENDER,
+ .driver_features = DRIVER_GEM | DRIVER_RENDER |
+ DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
.open = vgem_open,
.postclose = vgem_postclose,
.ioctls = vgem_ioctls,
--
2.37.0.144.g8ac04bfd2-goog
On Mon, Jul 11, 2022 at 9:23 PM John Stultz <[email protected]> wrote:
>
> After having to debug down through the kernel to figure out
> why my _WAIT calls were always timing out, I realized its
> an absolute timeout value instead of the more common relative
> timeouts.
>
> This detail should be called out in the documentation, as while
> the absolute value makes sense here, its not as common for timeout
> values.
>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: Jason Ekstrand <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Lionel Landwerlin <[email protected]>
> Cc: Chunming Zhou <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/gpu/drm/drm_syncobj.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 7e48dcd1bee4..b84d842a1c21 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -136,6 +136,10 @@
> * requirement is inherited from the wait-before-signal behavior required by
> * the Vulkan timeline semaphore API.
> *
> + * It should be noted, that both &DRM_IOCTL_SYNCOBJ_WAIT and
> + * &DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT takes an *absolute* CLOCK_MONOTONIC
Gah. That should be &DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT. I must have
pasted in the wrong symbol.
Fixed in my tree and I'll share v2 in a few days so I can get
additional feedback.
thanks
-john
Am 12.07.22 um 06:22 schrieb John Stultz:
> An initial pass at a drm_syncobj API test.
>
> Currently covers trivial use of:
> DRM_IOCTL_SYNCOBJ_CREATE
> DRM_IOCTL_SYNCOBJ_DESTROY
> DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD
> DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE
> DRM_IOCTL_SYNCOBJ_WAIT
> DRM_IOCTL_SYNCOBJ_RESET
> DRM_IOCTL_SYNCOBJ_SIGNAL
> DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT
> DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL
>
> And demonstrates how the userspace API can be used, along with
> some fairly simple bad parameter checking.
>
> The patch includes a few helpers taken from libdrm, as at least
> on the VM I was testing with, I didn't have a new enough libdrm
> to support the *_TIMELINE_* ioctls. Ideally the ioctl-helper bits
> can be dropped at a later time.
>
> Feedback would be appreciated!
>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: Jason Ekstrand <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Lionel Landwerlin <[email protected]>
> Cc: Chunming Zhou <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Shuah Khan <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> .../drivers/gpu/drm_syncobj/Makefile | 11 +
> .../drivers/gpu/drm_syncobj/ioctl-helper.c | 85 ++++
> .../drivers/gpu/drm_syncobj/ioctl-helper.h | 74 ++++
> .../drivers/gpu/drm_syncobj/syncobj-test.c | 410 ++++++++++++++++++
DRM userspace selftests usually go either into libdrm or igt and not
into the kernel source.
If you want to make in kernel self tests they should go into
drivers/gpu/drm/selftests/
Regards,
Christian.
> 4 files changed, 580 insertions(+)
> create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile
> create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c
> create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h
> create mode 100644 tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c
>
> diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile b/tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile
> new file mode 100644
> index 000000000000..6576d9b2006c
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0
> +CFLAGS += -I/usr/include/libdrm/
> +LDFLAGS += -pthread -ldrm
> +
> +TEST_GEN_FILES= syncobj-test
> +
> +include ../../../lib.mk
> +
> +$(OUTPUT)/syncobj-test: syncobj-test.c ioctl-helper.c
> +EXTRA_CLEAN = $(OUTPUT)/ioctl-helper.o
> +
> diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c
> new file mode 100644
> index 000000000000..e5c59c9bed36
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: MIT
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <time.h>
> +#include <errno.h>
> +#include <libdrm/drm.h>
> +#include <xf86drm.h>
> +#include "ioctl-helper.h"
> +
> +#ifndef DRM_CAP_SYNCOBJ_TIMELINE
> +/*
> + * The following is nabbed from libdrm's xf86drm.c as the
> + * installed libdrm doesn't yet include these definitions
> + *
> + *
> + * \author Rickard E. (Rik) Faith <[email protected]>
> + * \author Kevin E. Martin <[email protected]>
> + */
> +/*
> + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +int drmSyncobjTimelineSignal(int fd, const uint32_t *handles,
> + uint64_t *points, uint32_t handle_count)
> +{
> + struct drm_syncobj_timeline_array args;
> + int ret;
> +
> + memset(&args, 0, sizeof(args));
> + args.handles = (uintptr_t)handles;
> + args.points = (uintptr_t)points;
> + args.count_handles = handle_count;
> +
> + ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, &args);
> + return ret;
> +}
> +
> +int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points,
> + unsigned int num_handles,
> + int64_t timeout_nsec, unsigned int flags,
> + uint32_t *first_signaled)
> +{
> + struct drm_syncobj_timeline_wait args;
> + int ret;
> +
> + memset(&args, 0, sizeof(args));
> + args.handles = (uintptr_t)handles;
> + args.points = (uintptr_t)points;
> + args.timeout_nsec = timeout_nsec;
> + args.count_handles = num_handles;
> + args.flags = flags;
> +
> + ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, &args);
> + if (ret < 0)
> + return -errno;
> +
> + if (first_signaled)
> + *first_signaled = args.first_signaled;
> + return ret;
> +}
> +
> +#endif
> diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h
> new file mode 100644
> index 000000000000..b0c1025034b5
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/ioctl-helper.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: MIT */
> +#ifndef __IOCTL_HELPER_H__
> +#define __IOCTL_HELPER_H__
> +
> +/* Bits pulled from libdrm's include/drm/drm.h */
> +#ifndef DRM_CAP_SYNCOBJ_TIMELINE
> +/*
> + * Header for the Direct Rendering Manager
> + *
> + * Author: Rickard E. (Rik) Faith <[email protected]>
> + *
> + * Acknowledgments:
> + * Dec 1999, Richard Henderson <[email protected]>, move to generic cmpxchg.
> + */
> +
> +/*
> + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> + * All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +struct drm_syncobj_timeline_wait {
> + __u64 handles;
> + /* wait on specific timeline point for every handles*/
> + __u64 points;
> + /* absolute timeout */
> + __s64 timeout_nsec;
> + __u32 count_handles;
> + __u32 flags;
> + __u32 first_signaled; /* only valid when not waiting all */
> + __u32 pad;
> +};
> +
> +
> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0)
> +struct drm_syncobj_timeline_array {
> + __u64 handles;
> + __u64 points;
> + __u32 count_handles;
> + __u32 flags;
> +};
> +
> +#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait)
> +#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct drm_syncobj_timeline_array)
> +#define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer)
> +#define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array)
> +
> +int drmSyncobjTimelineSignal(int fd, const uint32_t *handles,
> + uint64_t *points, uint32_t handle_count);
> +int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points,
> + unsigned int num_handles,
> + int64_t timeout_nsec, unsigned int flags,
> + uint32_t *first_signaled);
> +#endif
> +#endif /*__IOCTL_HELPER_H__*/
> +
> diff --git a/tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c b/tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c
> new file mode 100644
> index 000000000000..21474b0d3b9e
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/gpu/drm_syncobj/syncobj-test.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This test exercises basic syncobj ioctl interfaces from
> + * userland via the libdrm helpers.
> + *
> + * Copyright (C) 2022, Google LLC.
> + *
> + * Currently covers trivial use of:
> + * DRM_IOCTL_SYNCOBJ_CREATE
> + * DRM_IOCTL_SYNCOBJ_DESTROY
> + * DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD
> + * DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE
> + * DRM_IOCTL_SYNCOBJ_WAIT
> + * DRM_IOCTL_SYNCOBJ_RESET
> + * DRM_IOCTL_SYNCOBJ_SIGNAL
> + * DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT
> + * DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL
> + *
> + * TODO: Need coverage for the following ioctls:
> + * DRM_IOCTL_SYNCOBJ_QUERY
> + * DRM_IOCTL_SYNCOBJ_TRANSFER
> + * As well as more complicated use of interface (like
> + * signal/wait with multiple handles, etc), and sync_file
> + * import/export.
> + */
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <time.h>
> +#include <poll.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <pthread.h>
> +#include <linux/dma-buf.h>
> +#include <libdrm/drm.h>
> +#include <xf86drm.h>
> +
> +#include "ioctl-helper.h"
> +
> +
> +#define NSEC_PER_SEC 1000000000ULL
> +static uint64_t get_abs_timeout(uint64_t rel_nsec)
> +{
> + struct timespec ts;
> + uint64_t ns;
> +
> + clock_gettime(CLOCK_MONOTONIC, &ts);
> + ns = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
> + ns += rel_nsec;
> + return ns;
> +}
> +
> +struct test_arg {
> + int dev_fd;
> + uint32_t handle;
> + int handle_fd;
> +};
> +#define TEST_TIMES 5
> +
> +void *syncobj_signal_reset(void *arg)
> +{
> + struct test_arg *d = (struct test_arg *)arg;
> + int ret;
> + int i;
> +
> + for (i = 0; i < TEST_TIMES; i++) {
> + sleep(3);
> + printf("%s: sending signal!\n", __func__);
> + ret = drmSyncobjSignal(d->dev_fd, &d->handle, 1);
> + if (ret)
> + printf("Signal failed %i\n", ret);
> + }
> + return NULL;
> +}
> +
> +static int syncobj_wait_reset(struct test_arg *d)
> +{
> + uint64_t abs_timeout;
> + int ret;
> + int i;
> +
> + for (i = 0; i < TEST_TIMES; i++) {
> + abs_timeout = get_abs_timeout(10*NSEC_PER_SEC);
> + printf("%s calling drmSyncobjWait\n", __func__);
> + ret = drmSyncobjWait(d->dev_fd, &d->handle, 1, abs_timeout,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + NULL);
> + if (ret) {
> + printf("Error: syncobjwait failed %i\n", ret);
> + break;
> + }
> + printf("%s: drmSyncobjWait returned!\n", __func__);
> +
> + ret = drmSyncobjReset(d->dev_fd, &d->handle, 1);
> + if (ret) {
> + printf("Error: syncobjreset failed\n");
> + break;
> + }
> + }
> + return ret;
> +}
> +
> +void *syncobj_signal_timeline(void *arg)
> +{
> + struct test_arg *d = (struct test_arg *)arg;
> + uint64_t point = 0;
> + int ret;
> +
> + for (point = 0; point <= (TEST_TIMES-1)*5; point++) {
> + sleep(1);
> + printf("%s: sending signal %lld!\n", __func__, point);
> + ret = drmSyncobjTimelineSignal(d->dev_fd, &d->handle, &point, 1);
> + if (ret)
> + printf("Signal failed %i\n", ret);
> + }
> + return NULL;
> +}
> +
> +
> +int syncobj_timeline_wait(struct test_arg *d)
> +{
> + uint64_t abs_timeout;
> + uint64_t point;
> + int ret;
> + int i;
> +
> + for (i = 0; i < TEST_TIMES; i++) {
> + abs_timeout = get_abs_timeout(10*NSEC_PER_SEC);
> +
> + point = i * 5;
> + printf("%s: drmSyncobjTimelineWait waiting on %lld!\n", __func__, point);
> + ret = drmSyncobjTimelineWait(d->dev_fd, &d->handle, &point, 1,
> + abs_timeout,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + NULL);
> + if (ret) {
> + printf("Error: syncobjwait failed %i\n", ret);
> + return ret;
> + }
> + printf("%s: drmSyncobjTimelineWait got %lld!\n", __func__, point);
> + }
> + return 0;
> +}
> +
> +
> +static int test_thread_signal_wait(int devfd, void *(*signal_fn)(void *),
> + int (*wait_fn)(struct test_arg *))
> +{
> + uint32_t handle;
> + struct test_arg d;
> + pthread_t pth;
> + int ret;
> +
> + ret = drmSyncobjCreate(devfd, 0, &handle);
> + if (ret) {
> + printf("Error: Couldn't create syncobj\n");
> + return ret;
> + }
> +
> + d.dev_fd = devfd;
> + d.handle = handle;
> +
> + pthread_create(&pth, 0, signal_fn, &d);
> + ret = wait_fn(&d);
> + pthread_join(pth, NULL);
> + drmSyncobjDestroy(devfd, handle);
> +
> + return ret;
> +}
> +
> +static int test_fork_signal_wait(int devfd, void *(*signal_fn)(void *),
> + int (*wait_fn)(struct test_arg *))
> +{
> + uint32_t handle;
> + struct test_arg p, c;
> + pid_t id;
> + int ret;
> +
> + ret = drmSyncobjCreate(devfd, 0, &handle);
> + if (ret) {
> + printf("Error: Couldn't create syncobj\n");
> + return ret;
> + }
> +
> + p.dev_fd = devfd;
> + p.handle = 0;
> + p.handle_fd = 0;
> + c = p;
> + p.handle = handle;
> +
> + ret = drmSyncobjHandleToFD(devfd, handle, &c.handle_fd);
> + if (ret) {
> + printf("Error: Couldn't convert handle to fd\n");
> + goto out;
> + }
> +
> + id = fork();
> + if (id == 0) {
> + ret = drmSyncobjFDToHandle(c.dev_fd, c.handle_fd, &c.handle);
> + if (ret) {
> + printf("Error: Couldn't convert fd to handle\n");
> + exit(-1);
> + }
> + close(c.handle_fd);
> + signal_fn((void *)&c);
> + exit(0);
> + } else {
> + ret = wait_fn(&p);
> + waitpid(id, 0, 0);
> + }
> +
> +out:
> + if (c.handle_fd)
> + close(c.handle_fd);
> + drmSyncobjDestroy(devfd, handle);
> +
> + return ret;
> +}
> +
> +
> +static int test_badparameters(int devfd)
> +{
> + uint32_t handle1, handle2;
> + int ret, fail = 0;
> +
> + /* create bad fd */
> + ret = drmSyncobjCreate(-1, 0, &handle1);
> + if (!ret || errno != EBADF) {
> + printf("drmSyncobjCreate - bad fd fails! (%i != EBADF)\n", errno);
> + fail = 1;
> + }
> + /* destroy bad fd */
> + ret = drmSyncobjDestroy(-1, handle1);
> + if (!ret || errno != EBADF) {
> + printf("drmSyncobjDestroy - bad fd fails! (%i != EBADF)\n", errno);
> + fail = 1;
> + }
> +
> + /* TODO: Bad flags */
> +
> + ret = drmSyncobjCreate(devfd, 0, &handle1);
> + if (ret) {
> + printf("drmSyncobjCreate - unexpected failure!\n");
> + fail = 1;
> + }
> +
> + /* Destroy zeroed handle */
> + handle2 = 0;
> + ret = drmSyncobjDestroy(devfd, handle2);
> + if (!ret || errno != EINVAL) {
> + printf("drmSyncobjDestroy - zero'ed handle! (%i != EINVAL)\n", errno);
> + fail = 1;
> + }
> + /* Destroy invalid handle */
> + handle2 = -1;
> + ret = drmSyncobjDestroy(devfd, handle2);
> + if (!ret || errno != EINVAL) {
> + printf("drmSyncobjDestroy - invalid handle! (%i != EINVAL)\n", errno);
> + fail = 1;
> + }
> +
> + /* invalid timeouts */
> + ret = drmSyncobjWait(devfd, &handle1, 1, 1000,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + NULL);
> + if (!ret || errno != ETIME) {
> + printf("drmSyncobjWait - invalid timeout (relative)! (%i != ETIME)\n", errno);
> + fail = 1;
> + }
> +
> + ret = drmSyncobjWait(devfd, &handle1, 1, -1,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + NULL);
> + if (!ret || errno != ETIME) {
> + printf("drmSyncobjWait - invalid timeout (-1)! (%i != ETIME)\n", errno);
> + fail = 1;
> + }
> +
> + ret = drmSyncobjDestroy(devfd, handle1);
> + if (ret) {
> + printf("drmSyncobjDestroy - unexpected failure!\n");
> + fail = 1;
> + }
> +
> +
> + return fail;
> +}
> +
> +
> +#define NAME_LEN 16
> +static int check_device(int fd)
> +{
> + drm_version_t version = { 0 };
> + char name[NAME_LEN];
> + uint32_t handle;
> + int ret;
> +
> + memset(name, 0, NAME_LEN);
> + version.name_len = NAME_LEN;
> + version.name = name;
> +
> + ret = ioctl(fd, DRM_IOCTL_VERSION, &version);
> + if (ret)
> + return -1;
> +
> + printf("%s name: %s\n", __func__, name);
> +
> + ret = drmSyncobjCreate(fd, 0, &handle);
> + if (!ret) {
> + drmSyncobjDestroy(fd, handle);
> + printf("%s selected: %s\n", __func__, name);
> + }
> + return ret;
> +}
> +
> +static int find_device(void)
> +{
> + int i, fd;
> + const char *drmstr = "/dev/dri/card";
> +
> + fd = -1;
> + for (i = 0; i < 16; i++) {
> + char name[80];
> +
> + snprintf(name, 80, "%s%u", drmstr, i);
> +
> + fd = open(name, O_RDWR);
> + if (fd < 0)
> + continue;
> +
> + if (check_device(fd)) {
> + close(fd);
> + fd = -1;
> + continue;
> + } else {
> + break;
> + }
> + }
> + return fd;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int devfd = find_device();
> + char *testname;
> + int ret;
> +
> + if (devfd < 0) {
> + printf("Error: Couldn't find supported drm device\n");
> + return devfd;
> + }
> +
> + testname = "Bad parameters test";
> + printf("\n%s\n", testname);
> + printf("===================\n");
> + ret = test_badparameters(devfd);
> + if (ret)
> + printf("%s: FAILED\n", testname);
> + else
> + printf("%s: PASSED\n", testname);
> +
> +
> + testname = "Threaded reset test";
> + printf("\n%s\n", testname);
> + printf("===================\n");
> + ret = test_thread_signal_wait(devfd, &syncobj_signal_reset, &syncobj_wait_reset);
> + if (ret)
> + printf("%s: FAILED\n", testname);
> + else
> + printf("%s: PASSED\n", testname);
> +
> + testname = "Threaded timeline test";
> + printf("\n%s\n", testname);
> + printf("===================\n");
> + ret = test_thread_signal_wait(devfd, &syncobj_signal_timeline, &syncobj_timeline_wait);
> + if (ret)
> + printf("%s: FAILED\n", testname);
> + else
> + printf("%s: PASSED\n", testname);
> +
> +
> + testname = "Forked reset test";
> + printf("\n%s\n", testname);
> + printf("===================\n");
> + ret = test_fork_signal_wait(devfd, &syncobj_signal_reset, &syncobj_wait_reset);
> + if (ret)
> + printf("\n%s: FAILED\n", testname);
> + else
> + printf("\n%s: PASSED\n", testname);
> +
> + testname = "Forked timeline test";
> + printf("\n%s\n", testname);
> + printf("===================\n");
> + ret = test_fork_signal_wait(devfd, &syncobj_signal_timeline, &syncobj_timeline_wait);
> + if (ret)
> + printf("\n%s: FAILED\n", testname);
> + else
> + printf("\n%s: PASSED\n", testname);
> +
> +
> + close(devfd);
> + return 0;
> +}
Am 12.07.22 um 06:22 schrieb John Stultz:
> After having to debug down through the kernel to figure out
> why my _WAIT calls were always timing out, I realized its
> an absolute timeout value instead of the more common relative
> timeouts.
>
> This detail should be called out in the documentation, as while
> the absolute value makes sense here, its not as common for timeout
> values.
Well absolute timeout values are mandatory for making -ERESTARTSYS work
without any additional handling.
So using them is recommended for ~20 years now and IIRC even documented
somewhere.
See here as well https://lwn.net/Articles/17744/ how much trouble system
calls with relative timeouts are.
Regards,
Christian.
>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: Jason Ekstrand <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Lionel Landwerlin <[email protected]>
> Cc: Chunming Zhou <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/gpu/drm/drm_syncobj.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 7e48dcd1bee4..b84d842a1c21 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -136,6 +136,10 @@
> * requirement is inherited from the wait-before-signal behavior required by
> * the Vulkan timeline semaphore API.
> *
> + * It should be noted, that both &DRM_IOCTL_SYNCOBJ_WAIT and
> + * &DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT takes an *absolute* CLOCK_MONOTONIC
> + * nanosecond value for the timeout value. Accidentally passing relative time
> + * values will likely result in an immediate -ETIME return.
> *
> * Import/export of syncobjs
> * -------------------------
Am 12.07.22 um 06:22 schrieb John Stultz:
> Allows for basic SYNCOBJ api testing, in environments
> like VMs where there may not be a supported drm driver.
>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: Jason Ekstrand <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Lionel Landwerlin <[email protected]>
> Cc: Chunming Zhou <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index c5e3e5457737..e5427d7399da 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -109,7 +109,8 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
> }
>
> static const struct drm_driver vgem_driver = {
> - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> + .driver_features = DRIVER_GEM | DRIVER_RENDER |
> + DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
Well that's rather surprising. I'm not an export on VGEM, but AFAIK you
need to adjust the CS interface to support that stuff as well.
Christian.
> .open = vgem_open,
> .postclose = vgem_postclose,
> .ioctls = vgem_ioctls,
On Tue, Jul 12, 2022 at 12:40 AM Christian König
<[email protected]> wrote:
> Am 12.07.22 um 06:22 schrieb John Stultz:
> > After having to debug down through the kernel to figure out
> > why my _WAIT calls were always timing out, I realized its
> > an absolute timeout value instead of the more common relative
> > timeouts.
> >
> > This detail should be called out in the documentation, as while
> > the absolute value makes sense here, its not as common for timeout
> > values.
>
> Well absolute timeout values are mandatory for making -ERESTARTSYS work
> without any additional handling.
Yes! I'm not saying it's wrong to use absolute values, just that
relative values are common enough to create some confusion here.
> So using them is recommended for ~20 years now and IIRC even documented
> somewhere.
So in addition to "somewhere", why not in the interface documentation as well?
> See here as well https://lwn.net/Articles/17744/ how much trouble system
> calls with relative timeouts are.
Yep. Well aware. :)
thanks
-john
Am 12.07.22 um 17:48 schrieb John Stultz:
> On Tue, Jul 12, 2022 at 12:40 AM Christian König
> <[email protected]> wrote:
>> Am 12.07.22 um 06:22 schrieb John Stultz:
>>> After having to debug down through the kernel to figure out
>>> why my _WAIT calls were always timing out, I realized its
>>> an absolute timeout value instead of the more common relative
>>> timeouts.
>>>
>>> This detail should be called out in the documentation, as while
>>> the absolute value makes sense here, its not as common for timeout
>>> values.
>> Well absolute timeout values are mandatory for making -ERESTARTSYS work
>> without any additional handling.
> Yes! I'm not saying it's wrong to use absolute values, just that
> relative values are common enough to create some confusion here.
>
>> So using them is recommended for ~20 years now and IIRC even documented
>> somewhere.
> So in addition to "somewhere", why not in the interface documentation as well?
Because it's the desired default behavior and we shouldn't have to
document that on every instance it is used.
IIRC it's documented centralized (but I need to dig that up as well).
What we should do instead is to have a warning on every relative timeout
that this probably shouldn't be used as example.
Regards,+
Christian.
>
>> See here as well https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F17744%2F&data=05%7C01%7Cchristian.koenig%40amd.com%7C68a13ac3906d4ac4cc4308da641df25c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637932377042931797%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=dM4BkqnO0LrsdKBwKKMvx4zMabWrM%2FY7pPGDsdFO%2BnI%3D&reserved=0 how much trouble system
>> calls with relative timeouts are.
> Yep. Well aware. :)
>
> thanks
> -john
On Tue, Jul 12, 2022 at 12:43 AM Christian König
<[email protected]> wrote:
> Am 12.07.22 um 06:22 schrieb John Stultz:
> > An initial pass at a drm_syncobj API test.
> >
> > Currently covers trivial use of:
> > DRM_IOCTL_SYNCOBJ_CREATE
> > DRM_IOCTL_SYNCOBJ_DESTROY
> > DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD
> > DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE
> > DRM_IOCTL_SYNCOBJ_WAIT
> > DRM_IOCTL_SYNCOBJ_RESET
> > DRM_IOCTL_SYNCOBJ_SIGNAL
> > DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT
> > DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL
> >
> > And demonstrates how the userspace API can be used, along with
> > some fairly simple bad parameter checking.
> >
> > The patch includes a few helpers taken from libdrm, as at least
> > on the VM I was testing with, I didn't have a new enough libdrm
> > to support the *_TIMELINE_* ioctls. Ideally the ioctl-helper bits
> > can be dropped at a later time.
> >
> > Feedback would be appreciated!
>
> DRM userspace selftests usually go either into libdrm or igt and not
> into the kernel source.
Appreciate the pointer, I'll rework and submit to one of those projects.
thanks
-john
On Tue, Jul 12, 2022 at 12:46 AM Christian König
<[email protected]> wrote:
>
> Am 12.07.22 um 06:22 schrieb John Stultz:
> > Allows for basic SYNCOBJ api testing, in environments
> > like VMs where there may not be a supported drm driver.
> >
> > Cc: Maarten Lankhorst <[email protected]>
> > Cc: Maxime Ripard <[email protected]>
> > Cc: Thomas Zimmermann <[email protected]>
> > Cc: Jason Ekstrand <[email protected]>
> > Cc: Christian König <[email protected]>
> > Cc: Lionel Landwerlin <[email protected]>
> > Cc: Chunming Zhou <[email protected]>
> > Cc: David Airlie <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: John Stultz <[email protected]>
> > ---
> > drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> > index c5e3e5457737..e5427d7399da 100644
> > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > @@ -109,7 +109,8 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
> > }
> >
> > static const struct drm_driver vgem_driver = {
> > - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> > + .driver_features = DRIVER_GEM | DRIVER_RENDER |
> > + DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
>
> Well that's rather surprising. I'm not an export on VGEM, but AFAIK you
> need to adjust the CS interface to support that stuff as well.
Apologies, could you clarify a bit more what you mean here? This was
just helpful to enable the generic userland ioctls for the example
test tool in this series.
Are you proposing to add interfaces so the vgem driver can
attach/signal syncobjs similar to the
DRM_IOCTL_VGEM_FENCE_ATTACH/DRM_IOCTL_VGEM_FENCE_SIGNAL calls?
thanks
-john
Am 12.07.22 um 18:50 schrieb John Stultz:
> On Tue, Jul 12, 2022 at 12:46 AM Christian König
> <[email protected]> wrote:
>> Am 12.07.22 um 06:22 schrieb John Stultz:
>>> Allows for basic SYNCOBJ api testing, in environments
>>> like VMs where there may not be a supported drm driver.
>>>
>>> Cc: Maarten Lankhorst <[email protected]>
>>> Cc: Maxime Ripard <[email protected]>
>>> Cc: Thomas Zimmermann <[email protected]>
>>> Cc: Jason Ekstrand <[email protected]>
>>> Cc: Christian König <[email protected]>
>>> Cc: Lionel Landwerlin <[email protected]>
>>> Cc: Chunming Zhou <[email protected]>
>>> Cc: David Airlie <[email protected]>
>>> Cc: Daniel Vetter <[email protected]>
>>> Cc: [email protected]
>>> Signed-off-by: John Stultz <[email protected]>
>>> ---
>>> drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
>>> index c5e3e5457737..e5427d7399da 100644
>>> --- a/drivers/gpu/drm/vgem/vgem_drv.c
>>> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
>>> @@ -109,7 +109,8 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
>>> }
>>>
>>> static const struct drm_driver vgem_driver = {
>>> - .driver_features = DRIVER_GEM | DRIVER_RENDER,
>>> + .driver_features = DRIVER_GEM | DRIVER_RENDER |
>>> + DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
>> Well that's rather surprising. I'm not an export on VGEM, but AFAIK you
>> need to adjust the CS interface to support that stuff as well.
> Apologies, could you clarify a bit more what you mean here? This was
> just helpful to enable the generic userland ioctls for the example
> test tool in this series.
>
> Are you proposing to add interfaces so the vgem driver can
> attach/signal syncobjs similar to the
> DRM_IOCTL_VGEM_FENCE_ATTACH/DRM_IOCTL_VGEM_FENCE_SIGNAL calls?
Yes, exactly that. I don't see how it would be useful otherwise.
Christian.
>
> thanks
> -john
On Tue, Jul 12, 2022 at 08:52:53AM -0700, John Stultz wrote:
> On Tue, Jul 12, 2022 at 12:43 AM Christian K?nig
> <[email protected]> wrote:
> > Am 12.07.22 um 06:22 schrieb John Stultz:
> > > An initial pass at a drm_syncobj API test.
> > >
> > > Currently covers trivial use of:
> > > DRM_IOCTL_SYNCOBJ_CREATE
> > > DRM_IOCTL_SYNCOBJ_DESTROY
> > > DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD
> > > DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE
> > > DRM_IOCTL_SYNCOBJ_WAIT
> > > DRM_IOCTL_SYNCOBJ_RESET
> > > DRM_IOCTL_SYNCOBJ_SIGNAL
> > > DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT
> > > DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL
> > >
> > > And demonstrates how the userspace API can be used, along with
> > > some fairly simple bad parameter checking.
> > >
> > > The patch includes a few helpers taken from libdrm, as at least
> > > on the VM I was testing with, I didn't have a new enough libdrm
> > > to support the *_TIMELINE_* ioctls. Ideally the ioctl-helper bits
> > > can be dropped at a later time.
> > >
> > > Feedback would be appreciated!
> >
> > DRM userspace selftests usually go either into libdrm or igt and not
> > into the kernel source.
>
> Appreciate the pointer, I'll rework and submit to one of those projects.
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation
There should be already a ton of syncobj tests, so probably more just work
needed to make them work on vgem so we can test them all without a
suitable hw driver loaded.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch