Re: [Mesa-dev] [PATCH 13/17] mesa: implement buffer/texture barriers for semaphore wait/signal

Fri, 03 Nov 2017 10:43:58 -0700 


On 2017-11-03 05:18 AM, Nicolai Hähnle wrote:

On 02.11.2017 04:57, Andres Rodriguez wrote:

Make sure memory is accessible to the external client, for the specified
memory object, before the signal/after the wait.

Signed-off-by: Andres Rodriguez <andre...@gmail.com>
---
  src/mesa/main/dd.h                              | 14 ++++++-
  src/mesa/main/externalobjects.c                 | 38 ++++++++++++++---

  src/mesa/state_tracker/st_cb_semaphoreobjects.c | 55 
++++++++++++++++++++++++-

  3 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
index ec9ed8e..5089c86 100644
--- a/src/mesa/main/dd.h
+++ b/src/mesa/main/dd.h
@@ -1149,14 +1149,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 67912dd..a7e42a9 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"
@@ -744,7 +745,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)");

@@ -757,8 +759,20 @@ _mesa_WaitSemaphoreEXT(GLuint semaphore,
     if (!semObj)
        return;
-   /* 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
@@ -771,6 +785,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)");

@@ -783,8 +799,20 @@ _mesa_SignalSemaphoreEXT(GLuint semaphore,
     if (!semObj)
        return;
-   /* 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 4cff3fd..f6242c7 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"
@@ -51,25 +53,74 @@ 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;
+   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);
+   }



What's the justifications of these flushes? pipe_context::flush_resource 
is described as


     * Flush the resource cache, so that the resource can be used
     * by an external client. Possible usage:


... but WaitSemaphore is about *importing* resources from an external 
client (acquire semantics).





You are right I need to take a look at this again.


+
+   /* TODO: layout transition */
     _mesa_flush(ctx);
     pipe->semobj_wait(pipe, st_obj->semaphore);
  }
  static void
  st_server_signal_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 *dstLayouts)
  {
     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->semobj_signal(pipe, st_obj->semaphore);
+
+   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);
+   }



These flushes must go before the call to semobj_signal: Logically, we 
can only signal the semaphore *after* the resources have been flushed.



In practice, the code can indeed go wrong as-is, because each of the 
flush_resource call can trigger an actually flush, so the semaphore can 
really be signalled by a command submission that does not contain 
flush_resource for the later resources.





Correct, this is wrong and I just got lucky that my signals and 
submissions aligned correctly. I'll get the order fixed.


Regards,
Andres


Cheers,
Nicolai



+
+   /* TODO: layout transition */
     _mesa_flush(ctx);
  }





_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev























					Reply via email to