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

Lionel Landwerlin
2019-08-02T09:04:02Z

On 31/07/2019 23:33, Chris Wilson wrote:
> 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);

Not quite sure this ends up blocking on engine=1.

>
> /* 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]);
>> + }
>> +}