[Mesa-dev] [PATCH 13/17] mesa: implement buffer/texture barriers for semaphore wait/signal
Andres Rodriguez
andresx7 at gmail.com
Fri Nov 3 17:43:34 UTC 2017
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 <andresx7 at 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);
>> }
>>
>
>
More information about the mesa-dev
mailing list