Re: [PATCH i-g-t] syncobj: Add some wait and reset tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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, &current);
> +   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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux