Re: [igt-dev] [PATCH i-g-t 10/10] tests/i915/exec_fence: add timeline fence tests

Chris Wilson
2019-07-31T20:33:50Z

Quoting Lionel Landwerlin (2019-07-25 11:30:34)
> +static void test_syncobj_timeline_wait(int fd)
> +{
> + const uint32_t bbe = MI_BATCH_BUFFER_END;
> + struct drm_i915_gem_exec_object2 obj;
> + struct drm_i915_gem_execbuffer2 execbuf;
> + struct drm_i915_gem_execbuffer_ext_timeline_fences timeline_fences;
> + struct drm_i915_gem_exec_fence fence = {
> + .handle = syncobj_create(fd, 0),
> + };
> + uint64_t value = 1;
> + igt_spin_t *spin;
> + unsigned engine;
> + unsigned handle[16];
> + int n;
> +
> + /* Check that we can use the syncobj to asynchronous wait prior to
> + * execution.
> + */
> +
> + gem_quiescent_gpu(fd);
> +
> + spin = igt_spin_new(fd);
> +
> + memset(&timeline_fences, 0, sizeof(timeline_fences));
> + timeline_fences.base.name = DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES;
> + timeline_fences.fence_count = 1;
> + timeline_fences.handles_ptr = to_user_pointer(&fence);
> + timeline_fences.values_ptr = to_user_pointer(&value);
> +
> + memset(&execbuf, 0, sizeof(execbuf));
> + execbuf.buffers_ptr = to_user_pointer(&obj);
> + execbuf.buffer_count = 1;
> +
> + memset(&obj, 0, sizeof(obj));
> + obj.handle = gem_create(fd, 4096);
> + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
> +
> + /* Queue a signaler from the blocked engine */
> + execbuf.flags = I915_EXEC_EXT;
> + execbuf.cliprects_ptr = to_user_pointer(&timeline_fences);
> + execbuf.num_cliprects = 0;
> + fence.flags = I915_EXEC_FENCE_SIGNAL;
> + gem_execbuf(fd, &execbuf);
> + igt_assert(gem_bo_busy(fd, spin->handle));
> +
> + gem_close(fd, obj.handle);
> + obj.handle = gem_create(fd, 4096);
> + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));

This handle is leaked. Instead of recreating a handle every time, you
could export a sync-file.

> + n = 0;
> + for_each_engine(fd, engine) {
> + obj.handle = gem_create(fd, 4096);
> + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
> +
> + /* No inter-engine synchronisation, will complete */
> + if (engine == I915_EXEC_BLT) {
> + execbuf.flags = engine;
> + execbuf.cliprects_ptr = 0;
> + execbuf.num_cliprects = 0;
> + gem_execbuf(fd, &execbuf);
> + gem_sync(fd, obj.handle);
> + igt_assert(gem_bo_busy(fd, spin->handle));
> + }
> + igt_assert(gem_bo_busy(fd, spin->handle));
> +
> + /* Now wait upon the blocked engine */
> + execbuf.flags = I915_EXEC_EXT | engine;
> + execbuf.cliprects_ptr = to_user_pointer(&timeline_fences);
> + execbuf.num_cliprects = 0;
> + fence.flags = I915_EXEC_FENCE_WAIT;
> + gem_execbuf(fd, &execbuf);
> +
> + igt_assert(gem_bo_busy(fd, obj.handle));
> + handle[n++] = obj.handle;
> + }

You could move this to a second context and avoid the differentiation
between engines:

fence.flags = I915_EXEC_FENCE_WAIT;
execbuf.rsvd1 = gem_context_create(fd);

n = 0;
for_each_engine(fd, engine) {
obj.handle = gem_create(fd, 4096);
gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));

/* Unsynchronised will execute ahead of the blockage */
execbuf.flags = engine;
execbuf.cliprects_ptr = 0;
gem_execbuf(fd, &execbuf);
gem_sync(fd, obj.handle);

/* Now wait upon the blocked engine */
execbuf.flags |= I915_EXEC_EXT;
execbuf.cliprects_ptr = to_user_pointer(&timeline_fences);
gem_execbuf(fd, &execbuf);
handle[n++] = obj.handle;
}
igt_assert(gem_bo_busy(fd, spin->handle));
gem_context_destroy(fd, execbuf.rsvd1);

> + syncobj_destroy(fd, fence.handle);
> +
> + for (int i = 0; i < n; i++)
> + igt_assert(gem_bo_busy(fd, handle[i]));
> +
> + igt_spin_free(fd, spin);
> +
> + for (int i = 0; i < n; i++) {
> + gem_sync(fd, handle[i]);
> + gem_close(fd, handle[i]);
> + }
> +}