[Mesa-dev] Mesa (master): mesa/marshal: add custom BufferData/ BufferSubData marshalling
Timothy Arceri
tarceri at itsqueeze.com
Fri Mar 24 22:23:43 UTC 2017
On 25/03/17 02:49, Brian Paul wrote:
> 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.
This code is mostly a copy from the generated code, which adds these for
some reason (possibly a bug in generation, but it may be used by other
functions and just excess here).
>
> I'm going to post a patch which simplifies some of this code...
Thanks, will review. Sorry for the trouble, I noticed these while
writing the patch but forgot to remove them.
>
> -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