On Mon, Oct 31, 2016 at 10:38 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
