[Mesa-dev] [PATCH] mesa/marshal: add custom BufferData/BufferSubData marshalling
Timothy Arceri
tarceri at itsqueeze.com
Thu Mar 23 12:41:34 UTC 2017
On 23/03/17 23:19, Nicolai Hähnle wrote:
> On 23.03.2017 13:06, Timothy Arceri wrote:
>>
>>
>> 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.
>
> Well, better as a preparatory patch, so that the alignment problem is
> never introduced in the first place, but yes.
This code (and I assume other similar code) is already generated
automatically by gl_marshal.py so there is no regression.
>
> Cheers,
> Nicolai
>
>
>>>
>>> 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