[Intel-gfx] [PATCH i-g-t] syncobj: Add some wait and reset tests
Chris Wilson
chris at chris-wilson.co.uk
Wed Aug 9 17:28:10 UTC 2017
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
More information about the Intel-gfx
mailing list