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

[Roland Scheidegger]

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
>