[Mesa-dev] Mesa (master): mesa/marshal: add custom BufferData/ BufferSubData marshalling

Brian Paul brianp at vmware.com
Fri Mar 24 15:49:12 UTC 2017


Hi Timothy,

Some late comments below.

On 03/23/2017 06:28 PM, Timothy Arceri wrote:
> Module: Mesa
> Branch: master
> Commit: adced4a2f9d017ae126a438f97eb305fa0ca3bd0
> URL:    https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3Dadced4a2f9d017ae126a438f97eb305fa0ca3bd0&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA&m=euA0jTcoAmQ2Zvy_d-Fv7gEc7REbDj19MbIRrYVfXoc&s=RM6-RjO08ktuyTV6AhhjU7t-LGPw-SgL78pFH2QXiQs&e=
>
> Author: Timothy Arceri <tarceri at itsqueeze.com>
> Date:   Thu Mar 23 17:19:36 2017 +1100
>
> mesa/marshal: add custom BufferData/BufferSubData marshalling
>
> 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.
>
> Acked-by: Edward O'Callaghan <funfunctor at folklore1984.net>
>
> ---
>
>   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 c1f0f8fe92..dfaeaafa03 100644
> --- a/src/mapi/glapi/gen/gl_API.xml
> +++ b/src/mapi/glapi/gen/gl_API.xml
> @@ -5033,7 +5033,7 @@
>           <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"/>
> @@ -5041,7 +5041,7 @@
>           <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"/>
> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
> index f8cad30e20..cdc7fed8a5 100644
> --- a/src/mesa/main/marshal.c
> +++ b/src/mesa/main/marshal.c
> @@ -259,4 +259,129 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint 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;

I don't understand why this var is incremented if it's never used again.

> +
> +   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 : 0);
> +   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;

Same here.

> +      }
> +      _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;

And here, and below.

I'm going to post a patch which simplifies some of this code...

-Brian



> +   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 1b4fd51505..4e9a6653b4 100644
> --- a/src/mesa/main/marshal.h
> +++ b/src/mesa/main/marshal.h
> @@ -187,6 +187,8 @@ _mesa_glthread_is_compat_bind_vertex_array(const struct gl_context *ctx)
>   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,
> @@ -210,4 +212,20 @@ 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 */
>
> _______________________________________________
> mesa-commit mailing list
> mesa-commit at lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA&m=euA0jTcoAmQ2Zvy_d-Fv7gEc7REbDj19MbIRrYVfXoc&s=6n85d_RjkNcVeS-KpcQMv-lTHZM8nOqGUK0Aj-hnFcY&e=
>



More information about the mesa-dev mailing list