[PATCH libdrm] add libsync.h helper

[Rob Clark]

On Mon, Oct 31, 2016 at 10:38 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Mon, Oct 31, 2016 at 10:30:23AM -0400, Rob Clark wrote:
>> On Mon, Oct 31, 2016 at 9:55 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>> > What I liked was doing
>> >
>> > if (fd2 < 0)
>> >         return dup(fd1);
>> >
>> > if (fd1 < 0)
>> >         return dup(fd2);
>> >
>> > That makes accumulating the fences in the caller much easier (i.e. they
>> > start with
>> >         batch.fence_in = -1;
>> > then
>> >         batch.fence_in = sync_merge(batch.fence_in, fence);
>>
>> note that if you don't want to leak fd's you'd have to do something more like:
>>
>>   int new_fence = sync_merge(batch->fence_in, fence);
>>   if (batch->fence_in != -1)
>>      close(batch->fence_in);
>>   batch->fence_in = new_fence;
>
> Hmm. so I thought the ioctl was closing the input.

hmm, the new ioctl is not.. I guess I need to dig up the old code to
confirm it's behaviour was the same.

In reality I think most things would want to close() the old fence,
but I don't want to change the behaviour from an existing android
libsync function of the same name.  So probably will keep the existing
behaviour but add a new function (sync_merge_if() or something like
that.. feel free to suggest a better name) which does the dup/close
dance.

>> so it isn't *that* much better..  I guess you could do the close()
>> unconditionally and ignore the error if batch->fence_in==-1..
>
> Yup, realised after writing that a bit more on the input side is
> required.
>
>
> if (fd2 < 0)
>         return fd1;
>
> if (fd1 < 0)
>         return dup(fd2);
>
> ret = ioctl();
> if (ret < 0)
>         return fd1;
>
> close(fd1);
> return result.fence;
>
> Which discards the synchronisation on the new fence if there's an error,
> are we meant to flag a GL_ERROR?

The error checking should already be done at the egl level.  The same
eglCreateSync() has two modes, one where you pass in -1 and are asking
the driver to create a fence, and one where you pass in a valid fd
which you want to sync on.  For at least the gallium bits, these turn
into different code paths into the driver.  So the fd2<0 case would be
an assert().  I'm not entirely sure what behaviour you'd want for
i965.

BR,
-R

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre