[Mesa-dev] [PATCH] mesa/marshal: add custom BufferData/BufferSubData marshalling

Timothy Arceri tarceri at itsqueeze.com
Thu Mar 23 12:06:24 UTC 2017



On 23/03/17 22:52, Nicolai Hähnle wrote:
> On 23.03.2017 07:32, Timothy Arceri wrote:
>> GL_AMD_pinned_memory requires memory to be aligned correctly, so
>> we skip marshalling in this case. Also copying the data defeats
>> the purpose of EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD.
>>
>> Fixes GL_AMD_pinned_memory piglit tests when glthread is enabled.
>> ---
>>  src/mapi/glapi/gen/gl_API.xml |   4 +-
>>  src/mesa/main/marshal.c       | 125
>> ++++++++++++++++++++++++++++++++++++++++++
>>  src/mesa/main/marshal.h       |  18 ++++++
>>  3 files changed, 145 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mapi/glapi/gen/gl_API.xml
>> b/src/mapi/glapi/gen/gl_API.xml
>> index c1f0f8f..dfaeaaf 100644
>> --- a/src/mapi/glapi/gen/gl_API.xml
>> +++ b/src/mapi/glapi/gen/gl_API.xml
>> @@ -5026,29 +5026,29 @@
>>
>>      <type name="intptr"   size="4"                  glx_name="CARD32"/>
>>      <type name="sizeiptr" size="4"  unsigned="true" glx_name="CARD32"/>
>>
>>      <function name="BindBuffer" es1="1.1" es2="2.0" marshal="custom">
>>          <param name="target" type="GLenum"/>
>>          <param name="buffer" type="GLuint"/>
>>          <glx ignore="true"/>
>>      </function>
>>
>> -    <function name="BufferData" es1="1.1" es2="2.0">
>> +    <function name="BufferData" es1="1.1" es2="2.0" marshal="custom">
>>          <param name="target" type="GLenum"/>
>>          <param name="size" type="GLsizeiptr" counter="true"/>
>>          <param name="data" type="const GLvoid *" count="size"
>> img_null_flag="true"/>
>>          <param name="usage" type="GLenum"/>
>>          <glx ignore="true"/>
>>      </function>
>>
>> -    <function name="BufferSubData" es1="1.1" es2="2.0">
>> +    <function name="BufferSubData" es1="1.1" es2="2.0" marshal="custom">
>>          <param name="target" type="GLenum"/>
>>          <param name="offset" type="GLintptr"/>
>>          <param name="size" type="GLsizeiptr" counter="true"/>
>>          <param name="data" type="const GLvoid *" count="size"/>
>>          <glx ignore="true"/>
>>      </function>
>>
>>      <function name="DeleteBuffers" es1="1.1" es2="2.0">
>>          <param name="n" type="GLsizei" counter="true"/>
>>          <param name="buffer" type="const GLuint *" count="n"/>
>> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
>> index f8cad30..35b5314 100644
>> --- a/src/mesa/main/marshal.c
>> +++ b/src/mesa/main/marshal.c
>> @@ -252,11 +252,136 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint
>> buffer)
>>                                              cmd_size);
>>        cmd->target = target;
>>        cmd->buffer = buffer;
>>        _mesa_post_marshal_hook(ctx);
>>     } else {
>>        _mesa_glthread_finish(ctx);
>>        CALL_BindBuffer(ctx->CurrentServerDispatch, (target, buffer));
>>     }
>>  }
>>
>> +/* BufferData: marshalled asynchronously */
>> +struct marshal_cmd_BufferData
>> +{
>> +   struct marshal_cmd_base cmd_base;
>> +   GLenum target;
>> +   GLsizeiptr size;
>> +   GLenum usage;
>> +   bool data_null; /* If set, no data follows for "data" */
>> +   /* Next size bytes are GLvoid data[size] */
>> +};
>> +
>> +void
>> +_mesa_unmarshal_BufferData(struct gl_context *ctx,
>> +                           const struct marshal_cmd_BufferData *cmd)
>> +{
>> +   const GLenum target = cmd->target;
>> +   const GLsizeiptr size = cmd->size;
>> +   const GLenum usage = cmd->usage;
>> +   const char *variable_data = (const char *) (cmd + 1);
>> +   const GLvoid *data = (const GLvoid *) variable_data;
>> +
>> +   if (cmd->data_null)
>> +      data = NULL;
>> +   else
>> +      variable_data += size;
>> +
>> +   CALL_BufferData(ctx->CurrentServerDispatch, (target, size, data,
>> usage));
>> +}
>> +
>> +void GLAPIENTRY
>> +_mesa_marshal_BufferData(GLenum target, GLsizeiptr size, const GLvoid
>> * data,
>> +                         GLenum usage)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +   size_t cmd_size =
>> +      sizeof(struct marshal_cmd_BufferData) + (data ? size *2 : 0);
>
> Why the *2?

That's a a typo I thought, I fixed it before sending.

>
> Also, I didn't notice that before, but I'm concerned that this can break
> the alignment of subsequent marshal_cmd structs. It's probably best to
> force the alignment in _mesa_glthread_allocate_command, so that this
> problem cannot be missed later on in similar situations.

You mean as a follow up patch right? We don't want to make a copy here.

>
> Cheers,
> Nicolai
>
>> +   debug_print_marshal("BufferData");
>> +
>> +   if (unlikely(size < 0)) {
>> +      _mesa_glthread_finish(ctx);
>> +      _mesa_error(ctx, GL_INVALID_VALUE, "BufferData(size < 0)");
>> +      return;
>> +   }
>> +
>> +   if (target != GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD &&
>> +       cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>> +      struct marshal_cmd_BufferData *cmd =
>> +         _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BufferData,
>> +                                         cmd_size);
>> +
>> +      cmd->target = target;
>> +      cmd->size = size;
>> +      cmd->usage = usage;
>> +      char *variable_data = (char *) (cmd + 1);
>> +      cmd->data_null = !data;
>> +      if (!cmd->data_null) {
>> +         memcpy(variable_data, data, size);
>> +         variable_data += size;
>> +      }
>> +      _mesa_post_marshal_hook(ctx);
>> +   } else {
>> +      _mesa_glthread_finish(ctx);
>> +      CALL_BufferData(ctx->CurrentServerDispatch,
>> +                      (target, size, data, usage));
>> +   }
>> +}
>> +
>> +/* BufferSubData: marshalled asynchronously */
>> +struct marshal_cmd_BufferSubData
>> +{
>> +   struct marshal_cmd_base cmd_base;
>> +   GLenum target;
>> +   GLintptr offset;
>> +   GLsizeiptr size;
>> +   /* Next size bytes are GLvoid data[size] */
>> +};
>> +
>> +void
>> +_mesa_unmarshal_BufferSubData(struct gl_context *ctx,
>> +                              const struct marshal_cmd_BufferSubData
>> *cmd)
>> +{
>> +   const GLenum target = cmd->target;
>> +   const GLintptr offset = cmd->offset;
>> +   const GLsizeiptr size = cmd->size;
>> +   const char *variable_data = (const char *) (cmd + 1);
>> +   const GLvoid *data = (const GLvoid *) variable_data;
>> +
>> +   variable_data += size;
>> +   CALL_BufferSubData(ctx->CurrentServerDispatch,
>> +                      (target, offset, size, data));
>> +}
>> +
>> +void GLAPIENTRY
>> +_mesa_marshal_BufferSubData(GLenum target, GLintptr offset,
>> GLsizeiptr size,
>> +                            const GLvoid * data)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +   size_t cmd_size = sizeof(struct marshal_cmd_BufferSubData) + size;
>> +
>> +   debug_print_marshal("BufferSubData");
>> +   if (unlikely(size < 0)) {
>> +      _mesa_glthread_finish(ctx);
>> +      _mesa_error(ctx, GL_INVALID_VALUE, "BufferSubData(size < 0)");
>> +      return;
>> +   }
>> +
>> +   if (target != GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD &&
>> +       cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>> +      struct marshal_cmd_BufferSubData *cmd =
>> +         _mesa_glthread_allocate_command(ctx,
>> DISPATCH_CMD_BufferSubData,
>> +                                         cmd_size);
>> +      cmd->target = target;
>> +      cmd->offset = offset;
>> +      cmd->size = size;
>> +      char *variable_data = (char *) (cmd + 1);
>> +      memcpy(variable_data, data, size);
>> +      variable_data += size;
>> +      _mesa_post_marshal_hook(ctx);
>> +   } else {
>> +      _mesa_glthread_finish(ctx);
>> +      CALL_BufferSubData(ctx->CurrentServerDispatch,
>> +                         (target, offset, size, data));
>> +   }
>> +}
>> +
>>  #endif
>> diff --git a/src/mesa/main/marshal.h b/src/mesa/main/marshal.h
>> index 1b4fd51..4e9a665 100644
>> --- a/src/mesa/main/marshal.h
>> +++ b/src/mesa/main/marshal.h
>> @@ -180,20 +180,22 @@ _mesa_post_marshal_hook(struct gl_context *ctx)
>>   */
>>  static inline bool
>>  _mesa_glthread_is_compat_bind_vertex_array(const struct gl_context *ctx)
>>  {
>>     return ctx->API != API_OPENGL_CORE;
>>  }
>>
>>  struct marshal_cmd_ShaderSource;
>>  struct marshal_cmd_Flush;
>>  struct marshal_cmd_BindBuffer;
>> +struct marshal_cmd_BufferData;
>> +struct marshal_cmd_BufferSubData;
>>
>>  void GLAPIENTRY
>>  _mesa_marshal_ShaderSource(GLuint shader, GLsizei count,
>>                             const GLchar * const *string, const GLint
>> *length);
>>
>>  void
>>  _mesa_unmarshal_ShaderSource(struct gl_context *ctx,
>>                               const struct marshal_cmd_ShaderSource
>> *cmd);
>>
>>  void GLAPIENTRY
>> @@ -203,11 +205,27 @@ void
>>  _mesa_unmarshal_Flush(struct gl_context *ctx,
>>                        const struct marshal_cmd_Flush *cmd);
>>
>>  void GLAPIENTRY
>>  _mesa_marshal_BindBuffer(GLenum target, GLuint buffer);
>>
>>  void
>>  _mesa_unmarshal_BindBuffer(struct gl_context *ctx,
>>                             const struct marshal_cmd_BindBuffer *cmd);
>>
>> +void
>> +_mesa_unmarshal_BufferData(struct gl_context *ctx,
>> +                           const struct marshal_cmd_BufferData *cmd);
>> +
>> +void GLAPIENTRY
>> +_mesa_marshal_BufferData(GLenum target, GLsizeiptr size, const GLvoid
>> * data,
>> +                         GLenum usage);
>> +
>> +void
>> +_mesa_unmarshal_BufferSubData(struct gl_context *ctx,
>> +                              const struct marshal_cmd_BufferSubData
>> *cmd);
>> +
>> +void GLAPIENTRY
>> +_mesa_marshal_BufferSubData(GLenum target, GLintptr offset,
>> GLsizeiptr size,
>> +                            const GLvoid * data);
>> +
>>  #endif /* MARSHAL_H */
>>
>
>


More information about the mesa-dev mailing list