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

[Rob Clark]

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?

BR,
-R