[Mesa-dev] [PATCH 12/20] mesa: implement buffer/texture barriers for semaphore signal/wait v2

[Marek Olšák]

On Mon, Jan 29, 2018 at 7:53 PM, Marek Olšák <maraeo at gmail.com> wrote:
> On Tue, Jan 23, 2018 at 7:05 PM, Andres Rodriguez <andresx7 at gmail.com> wrote:
>> Make sure memory is accessible to the external client, for the specified
>> memory object, before the signal/after the wait.
>>
>> v2: fixed flush order with respect to wait/signal emission
>>
>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
>> ---
>>  src/mesa/main/dd.h                              | 14 ++++++-
>>  src/mesa/main/externalobjects.c                 | 38 +++++++++++++++---
>>  src/mesa/state_tracker/st_cb_semaphoreobjects.c | 53 +++++++++++++++++++++++--
>>  3 files changed, 95 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>> index abbc4d5f5c..b56dd176aa 100644
>> --- a/src/mesa/main/dd.h
>> +++ b/src/mesa/main/dd.h
>> @@ -1156,14 +1156,24 @@ struct dd_function_table {
>>      * server's command stream
>>      */
>>     void (*ServerWaitSemaphoreObject)(struct gl_context *ctx,
>> -                                     struct gl_semaphore_object *semObj);
>> +                                     struct gl_semaphore_object *semObj,
>> +                                     GLuint numBufferBarriers,
>> +                                     struct gl_buffer_object **bufObjs,
>> +                                     GLuint numTextureBarriers,
>> +                                     struct gl_texture_object **texObjs,
>> +                                     const GLenum *srcLayouts);
>>
>>     /**
>>      * Introduce an operation to signal the semaphore object in the GL
>>      * server's command stream
>>      */
>>     void (*ServerSignalSemaphoreObject)(struct gl_context *ctx,
>> -                                       struct gl_semaphore_object *semObj);
>> +                                       struct gl_semaphore_object *semObj,
>> +                                       GLuint numBufferBarriers,
>> +                                       struct gl_buffer_object **bufObjs,
>> +                                       GLuint numTextureBarriers,
>> +                                       struct gl_texture_object **texObjs,
>> +                                       const GLenum *dstLayouts);
>>     /*@}*/
>>
>>     /**
>> diff --git a/src/mesa/main/externalobjects.c b/src/mesa/main/externalobjects.c
>> index 4fb3ca07a9..c070d7a28d 100644
>> --- a/src/mesa/main/externalobjects.c
>> +++ b/src/mesa/main/externalobjects.c
>> @@ -23,6 +23,7 @@
>>
>>  #include "macros.h"
>>  #include "mtypes.h"
>> +#include "bufferobj.h"
>>  #include "context.h"
>>  #include "externalobjects.h"
>>  #include "teximage.h"
>> @@ -716,7 +717,8 @@ _mesa_WaitSemaphoreEXT(GLuint semaphore,
>>  {
>>     GET_CURRENT_CONTEXT(ctx);
>>     struct gl_semaphore_object *semObj;
>> -
>> +   struct gl_buffer_object **bufObjs;
>> +   struct gl_texture_object **texObjs;
>>
>>     if (!ctx->Extensions.EXT_semaphore) {
>>        _mesa_error(ctx, GL_INVALID_OPERATION, "glWaitSemaphoreEXT(unsupported)");
>> @@ -732,8 +734,20 @@ _mesa_WaitSemaphoreEXT(GLuint semaphore,
>>     FLUSH_VERTICES( ctx, 0 );
>>     FLUSH_CURRENT( ctx, 0 );
>>
>> -   /* TODO: memory barriers and layout transitions */
>> -   ctx->Driver.ServerWaitSemaphoreObject(ctx, semObj);
>> +   bufObjs = alloca(sizeof(struct gl_buffer_object **) * numBufferBarriers);
>> +   for (unsigned i = 0; i < numBufferBarriers; i++) {
>> +      bufObjs[i] = _mesa_lookup_bufferobj(ctx, buffers[i]);
>> +   }
>> +
>> +   texObjs = alloca(sizeof(struct gl_texture_object **) * numTextureBarriers);
>> +   for (unsigned i = 0; i < numTextureBarriers; i++) {
>> +      texObjs[i] = _mesa_lookup_texture(ctx, textures[i]);
>> +   }
>> +
>> +   ctx->Driver.ServerWaitSemaphoreObject(ctx, semObj,
>> +                                         numBufferBarriers, bufObjs,
>> +                                         numTextureBarriers, texObjs,
>> +                                         srcLayouts);
>>  }
>>
>>  void GLAPIENTRY
>> @@ -746,6 +760,8 @@ _mesa_SignalSemaphoreEXT(GLuint semaphore,
>>  {
>>     GET_CURRENT_CONTEXT(ctx);
>>     struct gl_semaphore_object *semObj;
>> +   struct gl_buffer_object **bufObjs;
>> +   struct gl_texture_object **texObjs;
>>
>>     if (!ctx->Extensions.EXT_semaphore) {
>>        _mesa_error(ctx, GL_INVALID_OPERATION, "glSignalSemaphoreEXT(unsupported)");
>> @@ -761,8 +777,20 @@ _mesa_SignalSemaphoreEXT(GLuint semaphore,
>>     FLUSH_VERTICES( ctx, 0 );
>>     FLUSH_CURRENT( ctx, 0 );
>>
>> -   /* TODO: memory barriers and layout transitions */
>> -   ctx->Driver.ServerSignalSemaphoreObject(ctx, semObj);
>> +   bufObjs = alloca(sizeof(struct gl_buffer_object **) * numBufferBarriers);
>> +   for (unsigned i = 0; i < numBufferBarriers; i++) {
>> +      bufObjs[i] = _mesa_lookup_bufferobj(ctx, buffers[i]);
>> +   }
>> +
>> +   texObjs = alloca(sizeof(struct gl_texture_object **) * numTextureBarriers);
>> +   for (unsigned i = 0; i < numTextureBarriers; i++) {
>> +      texObjs[i] = _mesa_lookup_texture(ctx, textures[i]);
>> +   }
>> +
>> +   ctx->Driver.ServerSignalSemaphoreObject(ctx, semObj,
>> +                                           numBufferBarriers, bufObjs,
>> +                                           numTextureBarriers, texObjs,
>> +                                           dstLayouts);
>>  }
>>
>>  void GLAPIENTRY
>> diff --git a/src/mesa/state_tracker/st_cb_semaphoreobjects.c b/src/mesa/state_tracker/st_cb_semaphoreobjects.c
>> index 5510102635..e0fc5704b6 100644
>> --- a/src/mesa/state_tracker/st_cb_semaphoreobjects.c
>> +++ b/src/mesa/state_tracker/st_cb_semaphoreobjects.c
>> @@ -6,6 +6,8 @@
>>  #include "main/externalobjects.h"
>>
>>  #include "st_context.h"
>> +#include "st_texture.h"
>> +#include "st_cb_bufferobjects.h"
>>  #include "st_cb_semaphoreobjects.h"
>>
>>  #include "state_tracker/drm_driver.h"
>> @@ -50,24 +52,69 @@ st_import_semaphoreobj_fd(struct gl_context *ctx,
>>
>>  static void
>>  st_server_wait_semaphore(struct gl_context *ctx,
>> -                         struct gl_semaphore_object *semObj)
>> +                         struct gl_semaphore_object *semObj,
>> +                         GLuint numBufferBarriers,
>> +                         struct gl_buffer_object **bufObjs,
>> +                         GLuint numTextureBarriers,
>> +                         struct gl_texture_object **texObjs,
>> +                         const GLenum *srcLayouts)
>>  {
>>     struct st_semaphore_object *st_obj = st_semaphore_object(semObj);
>>     struct st_context *st = st_context(ctx);
>>     struct pipe_context *pipe = st->pipe;
>> +   struct st_buffer_object *bufObj;
>> +   struct st_texture_object *texObj;
>>
>>     pipe->fence_server_sync(pipe, st_obj->fence);
>> +
>> +   for (unsigned i = 0; i < numBufferBarriers; i++) {
>> +      if (!bufObjs[i])
>> +         continue;
>> +
>> +      bufObj = st_buffer_object(bufObjs[i]);
>> +      pipe->flush_resource(pipe, bufObj->buffer);
>> +   }
>> +
>> +   for (unsigned i = 0; i < numTextureBarriers; i++) {
>> +      if (!texObjs[i])
>> +         continue;
>> +
>> +      texObj = st_texture_object(texObjs[i]);
>> +      pipe->flush_resource(pipe, texObj->pt);
>> +   }
>
> Can you add a comment and explain why the flushing takes place after
> fence_server_sync and not before?

If you apply all my feedback, the series is:

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek