[Mesa-dev] [PATCH 4/6] gallium: add way for drivers to create fences without flushing

[Rob Clark]

I did toy with the idea of adding a
DONT_REALLY_FLUSH_JUST_CREATE_A_FENCE flag to existing pctx->flush()..
I'm not really sure I could call that better.  And either way, we want
the ctx ptr in fence_finish(), otherwise we are hiding the ctx ptr
internally in the driver's pipe_fence struct so that it can
potentially flush in screen->fence_finish() (and that doesn't make the
threading explicit.. when the fence_finish() is associated w/ the ctx
it is clear that it might have to do ctx related stuff).

I'm open to better suggestions if anyone has a better idea.  But right
now this, plus maybe eventually ripping out the old way, seems like
the best plan.

BR,
-R

On Fri, Apr 1, 2016 at 8:03 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> I'll admit I'm not an expert on this but I got a bad feeling on it.
> Do you really need another per-context fence_finish function?
> This looks to me like rather than improving the existing api, it throws
> another one at the same problem, which is to be sort of used in parallel
> making things confusing as hell.
>
> Roland
>
> Am 01.04.2016 um 22:29 schrieb Rob Clark:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Since current thing is kinda horrible for tilers.  And that issue will
>> be even worse with EGL_ANDROID_native_fence_sync.
>>
>> Not wired up yet for gl syncobj, which can come later.  For now we just
>> need this with EGL.
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  src/gallium/include/pipe/p_context.h  | 24 ++++++++++++++++++++++++
>>  src/gallium/state_trackers/dri/dri2.c | 29 ++++++++++++++++++++---------
>>  2 files changed, 44 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>> index 1c97e82..02a946b 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -457,6 +457,30 @@ struct pipe_context {
>>                   unsigned flags);
>>
>>     /**
>> +    * Create a fence without necessarily flushing rendering.  Note
>> +    * that if the driver implements this, it must also implement
>> +    * ctx->fence_finish() which will be used instead of
>> +    * screen->fence_finish() to give the driver an opportunity to
>> +    * flush.
>> +    *
>> +    * This allows drivers, in particular tilers, to defer flush
>> +    * until someone actually wants to wait on a fence.
>> +    *
>> +    * \param fence  if not NULL, an old fence to unref and transfer a
>> +    *    new fence reference to
>> +    */
>> +   void (*create_fence)(struct pipe_context *pipe,
>> +                        struct pipe_fence_handle **fence);
>> +
>> +   /**
>> +    * Wait for the fence to finish.
>> +    * \param timeout in nanoseconds (may be PIPE_TIMEOUT_INFINITE).
>> +    */
>> +   boolean (*fence_finish)(struct pipe_context *pipe,
>> +                           struct pipe_fence_handle *fence,
>> +                           uint64_t timeout);
>> +
>> +   /**
>>      * Create a view on a texture to be used by a shader stage.
>>      */
>>     struct pipe_sampler_view * (*create_sampler_view)(struct pipe_context *ctx,
>> diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
>> index fb0a180..b66d885 100644
>> --- a/src/gallium/state_trackers/dri/dri2.c
>> +++ b/src/gallium/state_trackers/dri/dri2.c
>> @@ -1320,7 +1320,12 @@ dri2_create_fence(__DRIcontext *_ctx)
>>     if (!fence)
>>        return NULL;
>>
>> -   ctx->flush(ctx, &fence->pipe_fence, 0);
>> +   if (ctx->create_fence) {
>> +      debug_assert(ctx->fence_finish);
>> +      ctx->create_fence(ctx, &fence->pipe_fence);
>> +   } else {
>> +      ctx->flush(ctx, &fence->pipe_fence, 0);
>> +   }
>>
>>     if (!fence->pipe_fence) {
>>        FREE(fence);
>> @@ -1376,27 +1381,33 @@ static GLboolean
>>  dri2_client_wait_sync(__DRIcontext *_ctx, void *_fence, unsigned flags,
>>                        uint64_t timeout)
>>  {
>> +   struct pipe_context *ctx = dri_context(_ctx)->st->pipe;
>>     struct dri2_fence *fence = (struct dri2_fence*)_fence;
>>     struct dri_screen *driscreen = fence->driscreen;
>>     struct pipe_screen *screen = driscreen->base.screen;
>> +   struct pipe_fence_handle *pipe_fence = NULL;
>>
>> -   /* No need to flush. The context was flushed when the fence was created. */
>> +   /* No need to flush. The context was flushed when the fence was created,
>> +    * or the ctx implements ctx->fence_finish() which will take care of
>> +    * flushing if required
>> +    */
>>
>>     if (fence->pipe_fence)
>> -      return screen->fence_finish(screen, fence->pipe_fence, timeout);
>> +      pipe_fence = fence->pipe_fence;
>>     else if (fence->cl_event) {
>> -      struct pipe_fence_handle *pipe_fence =
>> -         driscreen->opencl_dri_event_get_fence(fence->cl_event);
>> -
>> -      if (pipe_fence)
>> -         return screen->fence_finish(screen, pipe_fence, timeout);
>> -      else
>> +      pipe_fence = driscreen->opencl_dri_event_get_fence(fence->cl_event);
>> +      if (!pipe_fence)
>>           return driscreen->opencl_dri_event_wait(fence->cl_event, timeout);
>>     }
>>     else {
>>        assert(0);
>>        return false;
>>     }
>> +
>> +   if (ctx->fence_finish)
>> +      return ctx->fence_finish(ctx, pipe_fence, timeout);
>> +
>> +   return screen->fence_finish(screen, pipe_fence, timeout);
>>  }
>>
>>  static void
>>
>