| Message ID | 1502298282-12425-1-git-send-email-jason.ekstrand@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Jason Ekstrand (2017-08-09 18:04:42) > This adds both trivial error-checking tests as well as more complex > tests which actually test whether or not waits do what they're supposed > to do. They only currently work on i915 but it should be simple to hook > them up for other drivers by simply implementing the little function > pointer hook provided at the top for triggering a syncobj. Note that this requires a libdrm version more recent than is requested. > --- > tests/Makefile.sources | 1 + > tests/syncobj_wait.c | 624 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 625 insertions(+) > create mode 100644 tests/syncobj_wait.c > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index bb013c7..430b637 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -230,6 +230,7 @@ TESTS_progs = \ > prime_vgem \ > sw_sync \ > syncobj_basic \ > + syncobj_wait \ > template \ > tools_test \ > vgem_basic \ > diff --git a/tests/syncobj_wait.c b/tests/syncobj_wait.c > new file mode 100644 > index 0000000..6689d34 > --- /dev/null > +++ b/tests/syncobj_wait.c > @@ -0,0 +1,624 @@ > +/* > + * Copyright © 2017 Intel Corporation > + * > + * 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 > + * THE AUTHORS OR COPYRIGHT HOLDERS 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. > + */ > + > +#include "igt.h" > +#include <unistd.h> > +#include <time.h> > +#include <sys/ioctl.h> > +#include "drm.h" > + > +IGT_TEST_DESCRIPTION("Tests for the drm sync object wait API"); > + > +/* One one tenth of a second */ > +#define SHORT_TIME_NSEC 100000000ull > + > +/** A per-platform function which triggers a set of sync objects > + * > + * If wait is set, the function should wait for the work to complete so > + * that an immediate call to SYNCOBJ_WAIT will return success. If wait is > + * not set, then the function should try to submit enough work that an > + * immediate call to SYNCOBJ_WAIT with a timeout of 0 will time out. > + */ > +void (*trigger_syncobj)(int fd, uint32_t *syncobjs, int count, bool wait); > + > +#define NSECS_PER_SEC 1000000000ull > + > +static uint64_t > +gettime_ns(void) > +{ > + struct timespec current; > + clock_gettime(CLOCK_MONOTONIC, ¤t); > + return (uint64_t)current.tv_sec * NSECS_PER_SEC + current.tv_nsec; > +} > + > +static uint64_t > +short_timeout(void) > +{ > + return gettime_ns() + SHORT_TIME_NSEC; > +} > + > +static uint32_t > +syncobj_create(int fd) > +{ > + struct drm_syncobj_create create = { 0 }; > + int ret; > + > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_CREATE, &create); > + igt_assert(ret == 0); > + igt_assert(create.handle > 0); > + > + return create.handle; > +} > + > +static void > +syncobj_destroy(int fd, uint32_t handle) > +{ > + struct drm_syncobj_destroy destroy = { 0 }; > + int ret; > + > + destroy.handle = handle; > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_DESTROY, &destroy); > + igt_assert(ret == 0); > +} > + > +struct delayed_trigger { > + int fd; > + uint32_t *syncobjs; > + int count; > + uint64_t nsec; > +}; > + > +static void * > +trigger_syncobj_delayed_func(void *data) > +{ > + struct delayed_trigger *trigger = data; > + struct timespec time; > + > + time.tv_sec = trigger->nsec / NSECS_PER_SEC; > + time.tv_nsec = trigger->nsec % NSECS_PER_SEC; > + > + nanosleep(&time, NULL); > + trigger_syncobj(trigger->fd, trigger->syncobjs, trigger->count, true); > + free(data); > + > + return NULL; > +} > + > +static pthread_t > +trigger_syncobj_delayed(int fd, uint32_t *syncobjs, int count, uint64_t nsec) > +{ > + struct delayed_trigger *trigger; > + pthread_t thread; > + int ret; > + > + trigger = malloc(sizeof(*trigger)); > + trigger->fd = fd; > + trigger->syncobjs = syncobjs; > + trigger->count = count; > + trigger->nsec = nsec; > + > + ret = pthread_create(&thread, NULL, > + trigger_syncobj_delayed_func, trigger); This is just a timer: timer_t timer; struct sigevent sev; struct itimerspec its; memset(&sev, 0, sizeof(sev)); sev.sigev_notify = SIGEV_THREAD; sev.sigev_value.sival_ptr = tigger; sev.sigev_notify_function = trigger_syncobj_delayed_func; igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0); memset(&its, 0, sizeof(its)); its.it_value.tv_sec = nsec / NSEC_PER_SEC; its.it_value.tv_nsec = nsec % NSEC_PER_SEC; igt_assert(timer_settime(timer, 0, &its, NULL) == 0); > + igt_assert(ret == 0); > + > + return thread; > +} [snip] Key tests missing here are signal (SIGINT) handling, especially how the timeout parameters is handled on repeats, see igt_interruptible(), though you must use igt_ioctl(). Polling multiple handles is not checked, especially combinations of signaled/unsignaled syncojbs. > + > +/******** i915 specific bits ******/ > +struct { > + uint32_t batch; > +} i915; > + > +#define I915_BATCH_COUNT 128 Is overrunning the ring/guc-wq and blocking a concern? Do 128 provide any more significance than 2, one for wait, one for signal? Do 2 provide any more than 1? Does this provide any more value than the coverage (of execbuf + wait/signaling) in gem_exec_fence? > +static void > +i915_trigger_syncobj(int fd, uint32_t *syncobjs, int count, bool wait) > +{ > + struct drm_i915_gem_exec_object2 exec_obj = { 0 }; > + struct drm_i915_gem_exec_fence *fence_array; > + struct drm_i915_gem_execbuffer2 execbuf = { 0 }; > + struct drm_i915_gem_wait gem_wait; > + int i, ret; > + > + fence_array = calloc(count, sizeof(*fence_array)); > + for (i = 0; i < count; i++) { > + fence_array[i].handle = syncobjs[i]; > + fence_array[i].flags = I915_EXEC_FENCE_SIGNAL; > + } > + > + exec_obj.handle = i915.batch; Is there any advantage in keeping the handle around in a global? > + > + execbuf.buffers_ptr = to_user_pointer(&exec_obj); > + execbuf.buffer_count = 1; > + execbuf.batch_start_offset = 0; > + execbuf.batch_len = 8; > + execbuf.flags = I915_EXEC_RENDER | I915_EXEC_FENCE_ARRAY; > + > + for (i = 0; i < I915_BATCH_COUNT; i++) { > + if (i == I915_BATCH_COUNT - 1) { > + execbuf.cliprects_ptr = to_user_pointer(fence_array); > + execbuf.num_cliprects = count; > + } > + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf); gem_execbuf > + igt_assert(ret == 0); > + } > + > + free(fence_array); > + > + if (wait) { This is just gem_sync() (actually just gem_wait, but you since you just want a sync, you can use gem_sync) > + gem_wait.bo_handle = i915.batch; > + gem_wait.flags = 0; > + gem_wait.timeout_ns = INT64_MAX; > + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait); > + igt_assert(ret == 0); > + } > +} > + > +static void > +i915_init(int fd) > +{ > + uint32_t batch_data[2] = {0, MI_BATCH_BUFFER_END}; > + > + i915.batch = gem_create(fd, 4096); > + gem_write(fd, i915.batch, 0, batch_data, sizeof(batch_data)); > + trigger_syncobj = i915_trigger_syncobj; > +} > +/******** end of i915 bits ******/ If this used either vgem or sw_sync, you wouldn't need i915 specific details. > + > +static bool > +has_syncobj_wait(int fd) > +{ > + struct drm_syncobj_wait wait = { 0 }; > + uint64_t value; > + int ret; > + > + if (drmGetCap(fd, DRM_CAP_SYNCOBJ, &value)) > + return false; > + if (!value) > + return false; > + > + /* Try waiting for zero sync objects should fail with EINVAL */ > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > + return ret == -1 && errno == EINVAL; > +} > + > +igt_main > +{ > + int fd; > + > + igt_fixture { > + fd = drm_open_driver_render(DRIVER_INTEL); > + igt_require_gem(fd); > + igt_require(has_syncobj_wait(fd)); > + > + if (is_i915_device(fd)) > + i915_init(fd); You asked for i915-only anyway, see DRIVER_INTEL. You meant DRIVER_ANY, but maybe just using DRIVER_VGEM and avoiding driver details would be better. -Chris
Hi Jason, On 9 August 2017 at 18:04, Jason Ekstrand <jason@jlekstrand.net> wrote: > +/* One one tenth of a second */ > +#define SHORT_TIME_NSEC 100000000ull Er, a hundredth? Or only one, one tenth? > +static void > +test_wait_illegal_handle(int fd) > +{ > + struct drm_syncobj_wait wait = { 0 }; > + uint32_t handle = 2; Use 0. > +static void > +test_wait_for_submit_unsignaled(int fd) > +{ > + uint32_t syncobj = syncobj_create(fd); > + struct drm_syncobj_wait wait = { 0 }; > + int ret; > + > + wait.handles = to_user_pointer(&syncobj); > + wait.count_handles = 1; > + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; > + wait.timeout_nsec = short_timeout(); > + > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > + igt_assert(ret == -1 && errno == ETIME); There's do_ioctl_err() for this pattern BTW, and I think that takes care of EINTR as well. > +static void > +test_wait_signaled(int fd) > +{ > + uint32_t syncobj = syncobj_create(fd); > + struct drm_syncobj_wait wait = { 0 }; > + int ret; > + > + wait.handles = to_user_pointer(&syncobj); > + wait.count_handles = 1; > + > + trigger_syncobj(fd, &syncobj, 1, false); > + > + wait.timeout_nsec = 0; > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > + igt_warn_on(ret != -1 || errno != ETIME); > + > + wait.timeout_nsec = short_timeout(); > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > + igt_assert(ret == 0); ... and do_ioctl() for this pattern. > +static bool > +has_syncobj_wait(int fd) > +{ > + struct drm_syncobj_wait wait = { 0 }; This probably needs a local_ definition. > + uint64_t value; > + int ret; > + > + if (drmGetCap(fd, DRM_CAP_SYNCOBJ, &value)) > + return false; > + if (!value) > + return false; > + > + /* Try waiting for zero sync objects should fail with EINVAL */ > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > + return ret == -1 && errno == EINVAL; Unfortunately an unrecognised ioctl also leads to a failure with EINVAL. Try another test for ioctl presence, e.g. do you get ENOENT if you pass one handle to wait for, but that handle is 0 (invalid GEM object ID)? I couldn't see much else obvious, and it seems like a decent enough workout of the wait API, so, with these and what Chris suggested: Acked-by: Daniel Stone <daniels@collabora.com> Cheers, Daniel
On Wed, Aug 9, 2017 at 10:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Jason Ekstrand (2017-08-09 18:04:42) > > This adds both trivial error-checking tests as well as more complex > > tests which actually test whether or not waits do what they're supposed > > to do. They only currently work on i915 but it should be simple to hook > > them up for other drivers by simply implementing the little function > > pointer hook provided at the top for triggering a syncobj. > > Note that this requires a libdrm version more recent than is requested. > > > --- > > tests/Makefile.sources | 1 + > > tests/syncobj_wait.c | 624 ++++++++++++++++++++++++++++++ > +++++++++++++++++++ > > 2 files changed, 625 insertions(+) > > create mode 100644 tests/syncobj_wait.c > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > > index bb013c7..430b637 100644 > > --- a/tests/Makefile.sources > > +++ b/tests/Makefile.sources > > @@ -230,6 +230,7 @@ TESTS_progs = \ > > prime_vgem \ > > sw_sync \ > > syncobj_basic \ > > + syncobj_wait \ > > template \ > > tools_test \ > > vgem_basic \ > > diff --git a/tests/syncobj_wait.c b/tests/syncobj_wait.c > > new file mode 100644 > > index 0000000..6689d34 > > --- /dev/null > > +++ b/tests/syncobj_wait.c > > @@ -0,0 +1,624 @@ > > +/* > > + * Copyright © 2017 Intel Corporation > > + * > > + * 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 > > + * THE AUTHORS OR COPYRIGHT HOLDERS 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. > > + */ > > + > > +#include "igt.h" > > +#include <unistd.h> > > +#include <time.h> > > +#include <sys/ioctl.h> > > +#include "drm.h" > > + > > +IGT_TEST_DESCRIPTION("Tests for the drm sync object wait API"); > > + > > +/* One one tenth of a second */ > > +#define SHORT_TIME_NSEC 100000000ull > > + > > +/** A per-platform function which triggers a set of sync objects > > + * > > + * If wait is set, the function should wait for the work to complete so > > + * that an immediate call to SYNCOBJ_WAIT will return success. If wait > is > > + * not set, then the function should try to submit enough work that an > > + * immediate call to SYNCOBJ_WAIT with a timeout of 0 will time out. > > + */ > > +void (*trigger_syncobj)(int fd, uint32_t *syncobjs, int count, bool > wait); > > + > > +#define NSECS_PER_SEC 1000000000ull > > + > > +static uint64_t > > +gettime_ns(void) > > +{ > > + struct timespec current; > > + clock_gettime(CLOCK_MONOTONIC, ¤t); > > + return (uint64_t)current.tv_sec * NSECS_PER_SEC + current.tv_nsec; > > +} > > + > > +static uint64_t > > +short_timeout(void) > > +{ > > + return gettime_ns() + SHORT_TIME_NSEC; > > +} > > + > > +static uint32_t > > +syncobj_create(int fd) > > +{ > > + struct drm_syncobj_create create = { 0 }; > > + int ret; > > + > > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_CREATE, &create); > > + igt_assert(ret == 0); > > + igt_assert(create.handle > 0); > > + > > + return create.handle; > > +} > > + > > +static void > > +syncobj_destroy(int fd, uint32_t handle) > > +{ > > + struct drm_syncobj_destroy destroy = { 0 }; > > + int ret; > > + > > + destroy.handle = handle; > > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_DESTROY, &destroy); > > + igt_assert(ret == 0); > > +} > > + > > +struct delayed_trigger { > > + int fd; > > + uint32_t *syncobjs; > > + int count; > > + uint64_t nsec; > > +}; > > + > > +static void * > > +trigger_syncobj_delayed_func(void *data) > > +{ > > + struct delayed_trigger *trigger = data; > > + struct timespec time; > > + > > + time.tv_sec = trigger->nsec / NSECS_PER_SEC; > > + time.tv_nsec = trigger->nsec % NSECS_PER_SEC; > > + > > + nanosleep(&time, NULL); > > + trigger_syncobj(trigger->fd, trigger->syncobjs, trigger->count, > true); > > + free(data); > > + > > + return NULL; > > +} > > + > > +static pthread_t > > +trigger_syncobj_delayed(int fd, uint32_t *syncobjs, int count, uint64_t > nsec) > > +{ > > + struct delayed_trigger *trigger; > > + pthread_t thread; > > + int ret; > > + > > + trigger = malloc(sizeof(*trigger)); > > + trigger->fd = fd; > > + trigger->syncobjs = syncobjs; > > + trigger->count = count; > > + trigger->nsec = nsec; > > + > > + ret = pthread_create(&thread, NULL, > > + trigger_syncobj_delayed_func, trigger); > > This is just a timer: > > timer_t timer; > struct sigevent sev; > struct itimerspec its; > > memset(&sev, 0, sizeof(sev)); > sev.sigev_notify = SIGEV_THREAD; > sev.sigev_value.sival_ptr = tigger; > sev.sigev_notify_function = trigger_syncobj_delayed_func; > igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0); > > memset(&its, 0, sizeof(its)); > its.it_value.tv_sec = nsec / NSEC_PER_SEC; > its.it_value.tv_nsec = nsec % NSEC_PER_SEC; > igt_assert(timer_settime(timer, 0, &its, NULL) == 0); > How do I do the equivalent of pthread_join() on said timer? > > + igt_assert(ret == 0); > > + > > + return thread; > > +} > > [snip] > > Key tests missing here are signal (SIGINT) handling, especially how the > timeout parameters is handled on repeats, see igt_interruptible(), though > you must use igt_ioctl(). Polling multiple handles is not > checked, especially combinations of signaled/unsignaled syncojbs. > > > + > > +/******** i915 specific bits ******/ > > +struct { > > + uint32_t batch; > > +} i915; > > + > > +#define I915_BATCH_COUNT 128 > > Is overrunning the ring/guc-wq and blocking a concern? > > Do 128 provide any more significance than 2, one for wait, one for signal? > Do 2 provide any more than 1? > 1 appears to work reliably. I just wanted to make sure. I'll drop it to one. It should only produce warnings at worst. > Does this provide any more value than the coverage (of execbuf + > wait/signaling) in gem_exec_fence? > They're different wait mechanisms... > > +static void > > +i915_trigger_syncobj(int fd, uint32_t *syncobjs, int count, bool wait) > > +{ > > + struct drm_i915_gem_exec_object2 exec_obj = { 0 }; > > + struct drm_i915_gem_exec_fence *fence_array; > > + struct drm_i915_gem_execbuffer2 execbuf = { 0 }; > > + struct drm_i915_gem_wait gem_wait; > > + int i, ret; > > + > > + fence_array = calloc(count, sizeof(*fence_array)); > > + for (i = 0; i < count; i++) { > > + fence_array[i].handle = syncobjs[i]; > > + fence_array[i].flags = I915_EXEC_FENCE_SIGNAL; > > + } > > + > > + exec_obj.handle = i915.batch; > > Is there any advantage in keeping the handle around in a global? > Less work at trigger_syncobj time but that's all. I can create and destroy it here if you'd rather. > > + > > + execbuf.buffers_ptr = to_user_pointer(&exec_obj); > > + execbuf.buffer_count = 1; > > + execbuf.batch_start_offset = 0; > > + execbuf.batch_len = 8; > > + execbuf.flags = I915_EXEC_RENDER | I915_EXEC_FENCE_ARRAY; > > + > > + for (i = 0; i < I915_BATCH_COUNT; i++) { > > + if (i == I915_BATCH_COUNT - 1) { > > + execbuf.cliprects_ptr = > to_user_pointer(fence_array); > > + execbuf.num_cliprects = count; > > + } > > + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, > &execbuf); > > gem_execbuf > ? > > + igt_assert(ret == 0); > > + } > > + > > + free(fence_array); > > + > > + if (wait) { > > This is just gem_sync() (actually just gem_wait, but you since you just > want a sync, you can use gem_sync) > What's the difference between wait and sync? > > + gem_wait.bo_handle = i915.batch; > > + gem_wait.flags = 0; > > + gem_wait.timeout_ns = INT64_MAX; > > + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait); > > + igt_assert(ret == 0); > > + } > > +} > > + > > +static void > > +i915_init(int fd) > > +{ > > + uint32_t batch_data[2] = {0, MI_BATCH_BUFFER_END}; > > + > > + i915.batch = gem_create(fd, 4096); > > + gem_write(fd, i915.batch, 0, batch_data, sizeof(batch_data)); > > + trigger_syncobj = i915_trigger_syncobj; > > +} > > +/******** end of i915 bits ******/ > > If this used either vgem or sw_sync, you wouldn't need i915 specific > details. > Those paths might be worth implementing as well but I wanted to make sure it worked for syncobjs triggered by the actual driver. See also our discussion about default waits. Also, that requires adding syncobj support to vgem. Maybe a worth endeavor but I don't really see the point atm. > > + > > +static bool > > +has_syncobj_wait(int fd) > > +{ > > + struct drm_syncobj_wait wait = { 0 }; > > + uint64_t value; > > + int ret; > > + > > + if (drmGetCap(fd, DRM_CAP_SYNCOBJ, &value)) > > + return false; > > + if (!value) > > + return false; > > + > > + /* Try waiting for zero sync objects should fail with EINVAL */ > > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > > + return ret == -1 && errno == EINVAL; > > +} > > + > > +igt_main > > +{ > > + int fd; > > + > > + igt_fixture { > > + fd = drm_open_driver_render(DRIVER_INTEL); > > + igt_require_gem(fd); > > + igt_require(has_syncobj_wait(fd)); > > + > > + if (is_i915_device(fd)) > > + i915_init(fd); > > You asked for i915-only anyway, see DRIVER_INTEL. You meant DRIVER_ANY, > but maybe just using DRIVER_VGEM and avoiding driver details would be > better. > I was hoping that Dave would come along and add amdgpu bits and then we would need it. But maybe the extra check here is overkill.
On Wed, Aug 9, 2017 at 10:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > [snip] > > Key tests missing here are signal (SIGINT) handling, especially how the > timeout parameters is handled on repeats, see igt_interruptible(), though > you must use igt_ioctl(). I'll see what I can do > Polling multiple handles is not > checked, especially combinations of signaled/unsignaled syncojbs. Uh... that should be covered (maybe not well enough) by the wait_any and wait_all tests. --Jason
Quoting Jason Ekstrand (2017-08-09 18:46:49) > On Wed, Aug 9, 2017 at 10:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Jason Ekstrand (2017-08-09 18:04:42) > > This adds both trivial error-checking tests as well as more complex > > tests which actually test whether or not waits do what they're supposed > > to do. They only currently work on i915 but it should be simple to hook > > them up for other drivers by simply implementing the little function > > pointer hook provided at the top for triggering a syncobj. > > Note that this requires a libdrm version more recent than is requested. > > > --- > > tests/Makefile.sources | 1 + > > tests/syncobj_wait.c | 624 ++++++++++++++++++++++++++++++ > +++++++++++++++++++ > > 2 files changed, 625 insertions(+) > > create mode 100644 tests/syncobj_wait.c > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > > index bb013c7..430b637 100644 > > --- a/tests/Makefile.sources > > +++ b/tests/Makefile.sources > > @@ -230,6 +230,7 @@ TESTS_progs = \ > > prime_vgem \ > > sw_sync \ > > syncobj_basic \ > > + syncobj_wait \ > > template \ > > tools_test \ > > vgem_basic \ > > diff --git a/tests/syncobj_wait.c b/tests/syncobj_wait.c > > new file mode 100644 > > index 0000000..6689d34 > > --- /dev/null > > +++ b/tests/syncobj_wait.c > > @@ -0,0 +1,624 @@ > > +/* > > + * Copyright © 2017 Intel Corporation > > + * > > + * 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 > > + * THE AUTHORS OR COPYRIGHT HOLDERS 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. > > + */ > > + > > +#include "igt.h" > > +#include <unistd.h> > > +#include <time.h> > > +#include <sys/ioctl.h> > > +#include "drm.h" > > + > > +IGT_TEST_DESCRIPTION("Tests for the drm sync object wait API"); > > + > > +/* One one tenth of a second */ > > +#define SHORT_TIME_NSEC 100000000ull > > + > > +/** A per-platform function which triggers a set of sync objects > > + * > > + * If wait is set, the function should wait for the work to complete so > > + * that an immediate call to SYNCOBJ_WAIT will return success. If wait > is > > + * not set, then the function should try to submit enough work that an > > + * immediate call to SYNCOBJ_WAIT with a timeout of 0 will time out. > > + */ > > +void (*trigger_syncobj)(int fd, uint32_t *syncobjs, int count, bool > wait); > > + > > +#define NSECS_PER_SEC 1000000000ull > > + > > +static uint64_t > > +gettime_ns(void) > > +{ > > + struct timespec current; > > + clock_gettime(CLOCK_MONOTONIC, ¤t); > > + return (uint64_t)current.tv_sec * NSECS_PER_SEC + current.tv_nsec; > > +} > > + > > +static uint64_t > > +short_timeout(void) > > +{ > > + return gettime_ns() + SHORT_TIME_NSEC; > > +} > > + > > +static uint32_t > > +syncobj_create(int fd) > > +{ > > + struct drm_syncobj_create create = { 0 }; > > + int ret; > > + > > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_CREATE, &create); > > + igt_assert(ret == 0); > > + igt_assert(create.handle > 0); > > + > > + return create.handle; > > +} > > + > > +static void > > +syncobj_destroy(int fd, uint32_t handle) > > +{ > > + struct drm_syncobj_destroy destroy = { 0 }; > > + int ret; > > + > > + destroy.handle = handle; > > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_DESTROY, &destroy); > > + igt_assert(ret == 0); > > +} > > + > > +struct delayed_trigger { > > + int fd; > > + uint32_t *syncobjs; > > + int count; > > + uint64_t nsec; > > +}; > > + > > +static void * > > +trigger_syncobj_delayed_func(void *data) > > +{ > > + struct delayed_trigger *trigger = data; > > + struct timespec time; > > + > > + time.tv_sec = trigger->nsec / NSECS_PER_SEC; > > + time.tv_nsec = trigger->nsec % NSECS_PER_SEC; > > + > > + nanosleep(&time, NULL); > > + trigger_syncobj(trigger->fd, trigger->syncobjs, trigger->count, > true); > > + free(data); > > + > > + return NULL; > > +} > > + > > +static pthread_t > > +trigger_syncobj_delayed(int fd, uint32_t *syncobjs, int count, uint64_t > nsec) > > +{ > > + struct delayed_trigger *trigger; > > + pthread_t thread; > > + int ret; > > + > > + trigger = malloc(sizeof(*trigger)); > > + trigger->fd = fd; > > + trigger->syncobjs = syncobjs; > > + trigger->count = count; > > + trigger->nsec = nsec; > > + > > + ret = pthread_create(&thread, NULL, > > + trigger_syncobj_delayed_func, trigger); > > This is just a timer: > > timer_t timer; > struct sigevent sev; > struct itimerspec its; > > memset(&sev, 0, sizeof(sev)); > sev.sigev_notify = SIGEV_THREAD; > sev.sigev_value.sival_ptr = tigger; > sev.sigev_notify_function = trigger_syncobj_delayed_func; > igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0); > > memset(&its, 0, sizeof(its)); > its.it_value.tv_sec = nsec / NSEC_PER_SEC; > its.it_value.tv_nsec = nsec % NSEC_PER_SEC; > igt_assert(timer_settime(timer, 0, &its, NULL) == 0); > > > How do I do the equivalent of pthread_join() on said timer? You aren't doing anything more complicated than deleting the thread, so timer_delete(). > > + igt_assert(ret == 0); > > + > > + return thread; > > +} > > [snip] > > Key tests missing here are signal (SIGINT) handling, especially how the > timeout parameters is handled on repeats, see igt_interruptible(), though > you must use igt_ioctl(). Polling multiple handles is not > checked, especially combinations of signaled/unsignaled syncojbs. > > > + > > +/******** i915 specific bits ******/ > > +struct { > > + uint32_t batch; > > +} i915; > > + > > +#define I915_BATCH_COUNT 128 > > Is overrunning the ring/guc-wq and blocking a concern? > > Do 128 provide any more significance than 2, one for wait, one for signal? > Do 2 provide any more than 1? > > > 1 appears to work reliably. I just wanted to make sure. I'll drop it to one. > It should only produce warnings at worst. > > > Does this provide any more value than the coverage (of execbuf + > wait/signaling) in gem_exec_fence? > > > They're different wait mechanisms... I know you are testing the syncobj wait-ioctl, I am questioning whether this pattern of i915<->syncobj is interesting at all as it should already be covered elsewhere. If we only need to use i915 as a plain and simple trigger mechanism (i.e. not the split submit/signal), we can just replace i915 with vgem. I want to try and avoid having i915/gem dependencies in what should be a driver agnostic test. > > +static void > > +i915_trigger_syncobj(int fd, uint32_t *syncobjs, int count, bool wait) > > +{ > > + struct drm_i915_gem_exec_object2 exec_obj = { 0 }; > > + struct drm_i915_gem_exec_fence *fence_array; > > + struct drm_i915_gem_execbuffer2 execbuf = { 0 }; > > + struct drm_i915_gem_wait gem_wait; > > + int i, ret; > > + > > + fence_array = calloc(count, sizeof(*fence_array)); > > + for (i = 0; i < count; i++) { > > + fence_array[i].handle = syncobjs[i]; > > + fence_array[i].flags = I915_EXEC_FENCE_SIGNAL; > > + } > > + > > + exec_obj.handle = i915.batch; > > Is there any advantage in keeping the handle around in a global? > > > Less work at trigger_syncobj time but that's all. I can create and destroy it > here if you'd rather. Keep it all local, definitely. The cost of allocating, clflushing, and binding 1 page for each trigger should be immaterial in the test runtime. If you were concerned you would not have set .batch_len=8 and just left it as 0 as per normal. > > + > > + execbuf.buffers_ptr = to_user_pointer(&exec_obj); > > + execbuf.buffer_count = 1; > > + execbuf.batch_start_offset = 0; > > + execbuf.batch_len = 8; > > + execbuf.flags = I915_EXEC_RENDER | I915_EXEC_FENCE_ARRAY; > > + > > + for (i = 0; i < I915_BATCH_COUNT; i++) { > > + if (i == I915_BATCH_COUNT - 1) { > > + execbuf.cliprects_ptr = to_user_pointer > (fence_array); > > + execbuf.num_cliprects = count; > > + } > > + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, & > execbuf); > > gem_execbuf > > ? This (igt_assert(drmIoctl(EXECBUFFER2)) is open-coding gem_execbuf(). Anytime you use a raw ioctl/drmIoctl, it is a sign that we are missing a wrapper. > > + igt_assert(ret == 0); > > + } > > + > > + free(fence_array); > > + > > + if (wait) { > > This is just gem_sync() (actually just gem_wait, but you since you just > want a sync, you can use gem_sync) > > > What's the difference between wait and sync? gem_sync() is a gem_wait(fd, bo, NULL [== infinity]) > > > > + gem_wait.bo_handle = i915.batch; > > + gem_wait.flags = 0; > > + gem_wait.timeout_ns = INT64_MAX; > > + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait); > > + igt_assert(ret == 0); > > + } > > +} > > + > > +static void > > +i915_init(int fd) > > +{ > > + uint32_t batch_data[2] = {0, MI_BATCH_BUFFER_END}; > > + > > + i915.batch = gem_create(fd, 4096); > > + gem_write(fd, i915.batch, 0, batch_data, sizeof(batch_data)); > > + trigger_syncobj = i915_trigger_syncobj; > > +} > > +/******** end of i915 bits ******/ > > If this used either vgem or sw_sync, you wouldn't need i915 specific > details. > > > Those paths might be worth implementing as well but I wanted to make sure it > worked for syncobjs triggered by the actual driver. See also our discussion > about default waits. Also, that requires adding syncobj support to vgem. > Maybe a worth endeavor but I don't really see the point atm. > > > > + > > +static bool > > +has_syncobj_wait(int fd) > > +{ > > + struct drm_syncobj_wait wait = { 0 }; > > + uint64_t value; > > + int ret; > > + > > + if (drmGetCap(fd, DRM_CAP_SYNCOBJ, &value)) > > + return false; > > + if (!value) > > + return false; > > + > > + /* Try waiting for zero sync objects should fail with EINVAL */ > > + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); > > + return ret == -1 && errno == EINVAL; > > +} > > + > > +igt_main > > +{ > > + int fd; > > + > > + igt_fixture { > > + fd = drm_open_driver_render(DRIVER_INTEL); > > + igt_require_gem(fd); > > + igt_require(has_syncobj_wait(fd)); > > + > > + if (is_i915_device(fd)) > > + i915_init(fd); > > You asked for i915-only anyway, see DRIVER_INTEL. You meant DRIVER_ANY, > but maybe just using DRIVER_VGEM and avoiding driver details would be > better. > > > I was hoping that Dave would come along and add amdgpu bits and then we would > need it. But maybe the extra check here is overkill. We shouldn't need driver specific tests of the syncobj wait-ioctl though. Given driver coverage of fence signaling (required elsewhere) and syncobj interaction, that should then give us coverage of all edges that lead to the wait-ioctl, and so we only need a single central tester for the wait-ioctl ABI. -Chris
diff --git a/tests/Makefile.sources b/tests/Makefile.sources index bb013c7..430b637 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -230,6 +230,7 @@ TESTS_progs = \ prime_vgem \ sw_sync \ syncobj_basic \ + syncobj_wait \ template \ tools_test \ vgem_basic \ diff --git a/tests/syncobj_wait.c b/tests/syncobj_wait.c new file mode 100644 index 0000000..6689d34 --- /dev/null +++ b/tests/syncobj_wait.c @@ -0,0 +1,624 @@ +/* + * Copyright © 2017 Intel Corporation + * + * 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 + * THE AUTHORS OR COPYRIGHT HOLDERS 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. + */ + +#include "igt.h" +#include <unistd.h> +#include <time.h> +#include <sys/ioctl.h> +#include "drm.h" + +IGT_TEST_DESCRIPTION("Tests for the drm sync object wait API"); + +/* One one tenth of a second */ +#define SHORT_TIME_NSEC 100000000ull + +/** A per-platform function which triggers a set of sync objects + * + * If wait is set, the function should wait for the work to complete so + * that an immediate call to SYNCOBJ_WAIT will return success. If wait is + * not set, then the function should try to submit enough work that an + * immediate call to SYNCOBJ_WAIT with a timeout of 0 will time out. + */ +void (*trigger_syncobj)(int fd, uint32_t *syncobjs, int count, bool wait); + +#define NSECS_PER_SEC 1000000000ull + +static uint64_t +gettime_ns(void) +{ + struct timespec current; + clock_gettime(CLOCK_MONOTONIC, ¤t); + return (uint64_t)current.tv_sec * NSECS_PER_SEC + current.tv_nsec; +} + +static uint64_t +short_timeout(void) +{ + return gettime_ns() + SHORT_TIME_NSEC; +} + +static uint32_t +syncobj_create(int fd) +{ + struct drm_syncobj_create create = { 0 }; + int ret; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_CREATE, &create); + igt_assert(ret == 0); + igt_assert(create.handle > 0); + + return create.handle; +} + +static void +syncobj_destroy(int fd, uint32_t handle) +{ + struct drm_syncobj_destroy destroy = { 0 }; + int ret; + + destroy.handle = handle; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_DESTROY, &destroy); + igt_assert(ret == 0); +} + +struct delayed_trigger { + int fd; + uint32_t *syncobjs; + int count; + uint64_t nsec; +}; + +static void * +trigger_syncobj_delayed_func(void *data) +{ + struct delayed_trigger *trigger = data; + struct timespec time; + + time.tv_sec = trigger->nsec / NSECS_PER_SEC; + time.tv_nsec = trigger->nsec % NSECS_PER_SEC; + + nanosleep(&time, NULL); + trigger_syncobj(trigger->fd, trigger->syncobjs, trigger->count, true); + free(data); + + return NULL; +} + +static pthread_t +trigger_syncobj_delayed(int fd, uint32_t *syncobjs, int count, uint64_t nsec) +{ + struct delayed_trigger *trigger; + pthread_t thread; + int ret; + + trigger = malloc(sizeof(*trigger)); + trigger->fd = fd; + trigger->syncobjs = syncobjs; + trigger->count = count; + trigger->nsec = nsec; + + ret = pthread_create(&thread, NULL, + trigger_syncobj_delayed_func, trigger); + igt_assert(ret == 0); + + return thread; +} + +static void +test_wait_bad_flags(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + int ret; + + wait.flags = 0xdeadbeef; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == EINVAL); +} + +static void +test_wait_zero_handles(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + int ret; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == EINVAL); +} + +static void +test_wait_illegal_handle(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t handle = 2; + int ret; + + wait.count_handles = 1; + wait.handles = (unsigned long)(void *)&handle; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == ENOENT); +} + +static void +test_wait_unsignaled(int fd) +{ + uint32_t syncobj = syncobj_create(fd); + struct drm_syncobj_wait wait = { 0 }; + int ret; + + wait.handles = to_user_pointer(&syncobj); + wait.count_handles = 1; + wait.timeout_nsec = short_timeout(); + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == EINVAL); + + syncobj_destroy(fd, syncobj); +} + +static void +test_wait_for_submit_unsignaled(int fd) +{ + uint32_t syncobj = syncobj_create(fd); + struct drm_syncobj_wait wait = { 0 }; + int ret; + + wait.handles = to_user_pointer(&syncobj); + wait.count_handles = 1; + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + wait.timeout_nsec = short_timeout(); + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == ETIME); + + syncobj_destroy(fd, syncobj); +} + +static void +test_wait_signaled(int fd) +{ + uint32_t syncobj = syncobj_create(fd); + struct drm_syncobj_wait wait = { 0 }; + int ret; + + wait.handles = to_user_pointer(&syncobj); + wait.count_handles = 1; + + trigger_syncobj(fd, &syncobj, 1, false); + + wait.timeout_nsec = 0; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_warn_on(ret != -1 || errno != ETIME); + + wait.timeout_nsec = short_timeout(); + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0); + + syncobj_destroy(fd, syncobj); +} + +static void +test_wait_for_submit_signaled(int fd) +{ + uint32_t syncobj = syncobj_create(fd); + struct drm_syncobj_wait wait = { 0 }; + int ret; + + wait.handles = to_user_pointer(&syncobj); + wait.count_handles = 1; + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + + trigger_syncobj(fd, &syncobj, 1, false); + + wait.timeout_nsec = 0; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_warn_on(ret != -1 || errno != ETIME); + + wait.timeout_nsec = short_timeout(); + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0); + + syncobj_destroy(fd, syncobj); +} + +static void +test_wait_for_submit_delayed_signal(int fd) +{ + uint32_t syncobj = syncobj_create(fd); + struct drm_syncobj_wait wait = { 0 }; + int ret; + + wait.handles = to_user_pointer(&syncobj); + wait.count_handles = 1; + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + + trigger_syncobj_delayed(fd, &syncobj, 1, SHORT_TIME_NSEC); + + wait.timeout_nsec = 0; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == ETIME); + + wait.timeout_nsec = gettime_ns() + SHORT_TIME_NSEC * 2; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0); + + syncobj_destroy(fd, syncobj); +} + +static void +test_wait_all_signaled(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 2, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_all_some_signaled(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 1, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == EINVAL); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_any_signaled(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 2, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_any_some_signaled(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 1, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + + /* Even though we're waiting for anything, the kernel should still + * reject the unsignaled syncobj. + */ + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == EINVAL); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_for_submit_all_some_signaled(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 1, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL | + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == ETIME); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_for_submit_any_some_signaled(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t tmp, syncobjs[2]; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 1, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0 && wait.first_signaled == 0); + + /* Swap and try again */ + tmp = syncobjs[0]; + syncobjs[0] = syncobjs[1]; + syncobjs[1] = tmp; + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0 && wait.first_signaled == 1); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_all_for_submit_some_delayed_signal(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + pthread_t thread; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + trigger_syncobj(fd, syncobjs, 1, true); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL | + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + + thread = trigger_syncobj_delayed(fd, &syncobjs[1], 1, SHORT_TIME_NSEC); + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == ETIME); + + wait.timeout_nsec = gettime_ns() + 2 * SHORT_TIME_NSEC; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0); + + pthread_join(thread, NULL); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +static void +test_wait_any_for_submit_some_delayed_signal(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint32_t syncobjs[2]; + pthread_t thread; + int ret; + + syncobjs[0] = syncobj_create(fd); + syncobjs[1] = syncobj_create(fd); + + wait.handles = to_user_pointer(&syncobjs); + wait.count_handles = 2; + wait.timeout_nsec = short_timeout(); + wait.flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT; + + thread = trigger_syncobj_delayed(fd, &syncobjs[1], 1, SHORT_TIME_NSEC); + + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == -1 && errno == ETIME); + + wait.timeout_nsec = gettime_ns() + 2 * SHORT_TIME_NSEC; + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + igt_assert(ret == 0 && wait.first_signaled == 1); + + pthread_join(thread, NULL); + + syncobj_destroy(fd, syncobjs[0]); + syncobj_destroy(fd, syncobjs[1]); +} + +/******** i915 specific bits ******/ +struct { + uint32_t batch; +} i915; + +#define I915_BATCH_COUNT 128 + +static void +i915_trigger_syncobj(int fd, uint32_t *syncobjs, int count, bool wait) +{ + struct drm_i915_gem_exec_object2 exec_obj = { 0 }; + struct drm_i915_gem_exec_fence *fence_array; + struct drm_i915_gem_execbuffer2 execbuf = { 0 }; + struct drm_i915_gem_wait gem_wait; + int i, ret; + + fence_array = calloc(count, sizeof(*fence_array)); + for (i = 0; i < count; i++) { + fence_array[i].handle = syncobjs[i]; + fence_array[i].flags = I915_EXEC_FENCE_SIGNAL; + } + + exec_obj.handle = i915.batch; + + execbuf.buffers_ptr = to_user_pointer(&exec_obj); + execbuf.buffer_count = 1; + execbuf.batch_start_offset = 0; + execbuf.batch_len = 8; + execbuf.flags = I915_EXEC_RENDER | I915_EXEC_FENCE_ARRAY; + + for (i = 0; i < I915_BATCH_COUNT; i++) { + if (i == I915_BATCH_COUNT - 1) { + execbuf.cliprects_ptr = to_user_pointer(fence_array); + execbuf.num_cliprects = count; + } + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf); + igt_assert(ret == 0); + } + + free(fence_array); + + if (wait) { + gem_wait.bo_handle = i915.batch; + gem_wait.flags = 0; + gem_wait.timeout_ns = INT64_MAX; + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait); + igt_assert(ret == 0); + } +} + +static void +i915_init(int fd) +{ + uint32_t batch_data[2] = {0, MI_BATCH_BUFFER_END}; + + i915.batch = gem_create(fd, 4096); + gem_write(fd, i915.batch, 0, batch_data, sizeof(batch_data)); + trigger_syncobj = i915_trigger_syncobj; +} +/******** end of i915 bits ******/ + +static bool +has_syncobj_wait(int fd) +{ + struct drm_syncobj_wait wait = { 0 }; + uint64_t value; + int ret; + + if (drmGetCap(fd, DRM_CAP_SYNCOBJ, &value)) + return false; + if (!value) + return false; + + /* Try waiting for zero sync objects should fail with EINVAL */ + ret = ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + return ret == -1 && errno == EINVAL; +} + +igt_main +{ + int fd; + + igt_fixture { + fd = drm_open_driver_render(DRIVER_INTEL); + igt_require_gem(fd); + igt_require(has_syncobj_wait(fd)); + + if (is_i915_device(fd)) + i915_init(fd); + } + + igt_subtest("wait-bad-flags") + test_wait_bad_flags(fd); + + igt_subtest("wait-zero-handles") + test_wait_zero_handles(fd); + + igt_subtest("wait-illegal-handle") + test_wait_illegal_handle(fd); + + igt_subtest("wait-unsignaled") + test_wait_unsignaled(fd); + + igt_subtest("wait-for-submit-unsignaled") + test_wait_for_submit_unsignaled(fd); + + igt_subtest("wait-signaled") + test_wait_signaled(fd); + + igt_subtest("wait-for-submit-signaled") + test_wait_for_submit_signaled(fd); + + igt_subtest("wait-for-submit-delayed-signal") + test_wait_for_submit_delayed_signal(fd); + + igt_subtest("wait-all-signaled") + test_wait_all_signaled(fd); + + igt_subtest("wait-all-some-signaled") + test_wait_all_some_signaled(fd); + + igt_subtest("wait-any-signaled") + test_wait_any_signaled(fd); + + igt_subtest("wait-any-some-signaled") + test_wait_any_some_signaled(fd); + + igt_subtest("wait-for-submit-all-some-signaled") + test_wait_for_submit_all_some_signaled(fd); + + igt_subtest("wait-for-submit-any-some-signaled") + test_wait_for_submit_any_some_signaled(fd); + + igt_subtest("wait-all-for-submit-some-delayed-signal") + test_wait_all_for_submit_some_delayed_signal(fd); + + igt_subtest("wait-any-for-submit-some-delayed-signal") + test_wait_any_for_submit_some_delayed_signal(fd); +}