[Mesa-dev] [PATCH 2/6] dri: extend fence extension to support native fd fences

[Marek Olšák]

On Tue, Apr 5, 2016 at 4:08 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Tue, Apr 5, 2016 at 8:55 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Mon, Apr 4, 2016 at 2:25 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> On Mon, Apr 4, 2016 at 7:49 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>>> On Fri, Apr 1, 2016 at 10:29 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>>
>>>>> Required to implement EGL_ANDROID_native_fence_sync.
>>>>>
>>>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>>>> ---
>>>>>  include/GL/internal/dri_interface.h | 44 ++++++++++++++++++++++++++++++++++++-
>>>>>  1 file changed, 43 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
>>>>> index 2b49a29..8b0dadc 100644
>>>>> --- a/include/GL/internal/dri_interface.h
>>>>> +++ b/include/GL/internal/dri_interface.h
>>>>> @@ -339,12 +339,19 @@ struct __DRI2throttleExtensionRec {
>>>>>   */
>>>>>
>>>>>  #define __DRI2_FENCE "DRI2_Fence"
>>>>> -#define __DRI2_FENCE_VERSION 1
>>>>> +#define __DRI2_FENCE_VERSION 2
>>>>>
>>>>>  #define __DRI2_FENCE_TIMEOUT_INFINITE     0xffffffffffffffffllu
>>>>>
>>>>>  #define __DRI2_FENCE_FLAG_FLUSH_COMMANDS  (1 << 0)
>>>>>
>>>>> +/**
>>>>> + * \name Capabilities that might be returned by __DRI2fenceExtensionRec::get_capabilities
>>>>> + */
>>>>> +/*@{*/
>>>>> +#define __DRI_FENCE_CAP_NATIVE_FD 1
>>>>> +/*@}*/
>>>>> +
>>>>>  struct __DRI2fenceExtensionRec {
>>>>>     __DRIextension base;
>>>>>
>>>>> @@ -389,6 +396,41 @@ struct __DRI2fenceExtensionRec {
>>>>>      *                sense with this function (right now there are none)
>>>>>      */
>>>>>     void (*server_wait_sync)(__DRIcontext *ctx, void *fence, unsigned flags);
>>>>> +
>>>>> +   /**
>>>>> +    * Query for general capabilities of the driver that concern fences.
>>>>> +    * Returns a bitmask of __DRI_FENCE_CAP_x
>>>>> +    *
>>>>> +    * \since 2
>>>>> +    */
>>>>> +   unsigned (*get_capabilities)(__DRIscreen *screen);
>>>>> +
>>>>> +   /**
>>>>> +    * Create an fd (file descriptor) associated fence.  If the fence fd
>>>>> +    * is -1, this behaves similarly to create_fence() except that when
>>>>> +    * rendering is flushed the driver creates a fence fd.  Otherwise,
>>>>> +    * the driver wraps an existing fence fd.
>>>>> +    *
>>>>> +    * This is used to implement the EGL_ANDROID_native_fence_sync extension.
>>>>> +    *
>>>>> +    * \since 2
>>>>> +    *
>>>>> +    * \param ctx     the context associated with the fence
>>>>> +    * \param fd      the fence fd or -1
>>>>> +    */
>>>>> +   void *(*create_fence_fd)(__DRIcontext *ctx, int fd);
>>>>> +
>>>>> +   /**
>>>>> +    * For fences created with create_fence_fd(), after rendering is flushed,
>>>>> +    * this retrieves the native fence fd.  Caller takes ownership of the
>>>>> +    * fd and will close() it when it is no longer needed.
>>>>> +    *
>>>>> +    * \since 2
>>>>> +    *
>>>>> +    * \param screen  the screen associated with the fence
>>>>> +    * \param fence   the fence
>>>>> +    */
>>>>> +   int (*get_fence_fd)(__DRIscreen *screen, void *fence);
>>>>
>>>> This can be even more generic. If you add a requirement that
>>>> get_fence_fd must work with any fence, create_fence_fd() essentially
>>>> becomes import_fence_fd() and create_fence() can be used if fd is -1.
>>>>
>>>> The reason is that we may be interested in adding support for
>>>> cl_khr_gl_event in the future (MesaGL + HSA/OpenCL), which allows
>>>> sharing any GL fence with CL, and we will most likely use fd fences.
>>>
>>> Hmm, if you read the EGL_ANDROID_native_fence_sync extension, it
>>> explicitly makes it clear that not *all* fences would need to be
>>> convertible to a fd, to avoid needing to create a fd for every fence.
>>> (See Issues #1.)
>>>
>>> I'm still going back and forth on the kernel interface for this on the
>>> driver side, but it could get awkward if you allow multiple 'struct
>>> fence' objects created for the same seqno, so the interface I'll
>>> probably end up going with makes your suggested change difficult to
>>> implement.
>>>
>>> Could you not just use the existing ANDROID_native_fence_sync
>>> extension for GL + HSL/CL interop?
>>
>> We will need a way to pass *any OpenGL sync object* to OpenCL.  We
>> won't need inter-process fence sharing though. Maybe we'll have to
>> invent a different mechanism for that.
>
> and I *assume* you wouldn't need inter-device sharing either?  If that
> is the case, probably makes more sense to have something where the
> driver on the other side could unwrap the fence and get at the
> ring-id+seqno (or however you identify fences currently within the
> gallium driver)?  That would avoid forcing to create a file object for
> each submit/execlist..

I don't know if we'll need inter-device sharing. Let's cross that
bridge when we get there.

Marek