On Mon, Oct 31, 2016 at 10:38 AM, Chris Wilson chris@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@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