[Mesa-dev] [PATCH] mesa/marshal: add custom marshallingforglNamedBuffer(Sub)Data
Grigori Goronzy
greg at chown.ath.cx
Sun Jul 9 23:59:05 UTC 2017
On 2017-06-26 15:51, Marc Dietrich wrote:
> Am Montag, 26. Juni 2017, 15:35:15 CEST schrieb Grigori Goronzy:
>> On 2017-06-26 15:11, Marc Dietrich wrote:
>> > unfortunately, this change broke vmware/vmplayer here (bisected).
>> > Windows
>> > guest on linux host. Sig 11 in SVGA driver. All good if
>> > mesa_glthread=false.
>>
>> Can you provide instructions how to reproduce this problem? A
>> backtrace
>> might help, too.
>
> well, this is all proprietary software, so the backtrace doesn't really
> tell
> something.
>
>> I don't really get it, by the way. Isn't the SVGA driver for Linux
>> guests?
>
> I think the windows driver is named the same. Here is a paste of
> vmware.log:
>
> https://pastebin.com/X3CS7rCP
>
> I also have core dump, maybe only useful for VMWARE staff...
>
Hey Marc,
does the attached patch fix the crash?
Grigori
>
>>
>> Best regards
>> Grigori
>>
>> >> > Best regards
>> >> > Grigori
>> >> >
>> >> >> [1]
>> >> >> https://lists.freedesktop.org/archives/mesa-dev/2017-June/160329.html
>> >> >>
>> >> >> On 25/06/17 02:59, Grigori Goronzy wrote:
>> >> >>> These entry points are used by Alien Isolation and caused
>> >> >>> synchronization with glthread. The async marshalling implementation
>> >> >>> is similar to glBuffer(Sub)Data.
>> >> >>>
>> >> >>> Results in an approximately 6x drop in glthread synchronizations and
>> >> >>> a
>> >> >>> ~30% FPS jump in Alien Isolation (Medium preset, Athlon 860K, RX
>> >> >>> 480).
>> >> >>>
>> >> >>> This does not care about the EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD
>> >> >>> special
>> >> >>> case like the Buffer(Sub)Data marshalling functions.
>> >> >>> ---
>> >> >>> I'm not a fan of the code duplication and I'll try to address that in
>> >> >>> further changes to glthread/marshalling, but the improvement is so
>> >> >>> noticeable that I'd like to share it. Alien Isolation is now
>> >> >>> playable on
>> >> >>> my system while it wasn't before.
>> >> >>>
>> >> >>> src/mapi/glapi/gen/ARB_direct_state_access.xml | 4 +-
>> >> >>> src/mesa/main/marshal.c | 108
>> >> >>>
>> >> >>> +++++++++++++++++++++++++
>> >> >>>
>> >> >>> src/mesa/main/marshal.h | 18 +++++
>> >> >>> 3 files changed, 128 insertions(+), 2 deletions(-)
>> >> >>>
>> >> >>> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >> >>> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >> >>> index cb24d79..d3d2246 100644
>> >> >>> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >> >>> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
>> >> >>> @@ -61,14 +61,14 @@
>> >> >>>
>> >> >>> <param name="flags" type="GLbitfield" />
>> >> >>>
>> >> >>> </function>
>> >> >>>
>> >> >>> - <function name="NamedBufferData">
>> >> >>>
>> >> >>> + <function name="NamedBufferData" marshal="custom">
>> >> >>>
>> >> >>> <param name="buffer" type="GLuint" />
>> >> >>> <param name="size" type="GLsizeiptr" />
>> >> >>> <param name="data" type="const GLvoid *" />
>> >> >>> <param name="usage" type="GLenum" />
>> >> >>>
>> >> >>> </function>
>> >> >>>
>> >> >>> - <function name="NamedBufferSubData" no_error="true">
>> >> >>>
>> >> >>> + <function name="NamedBufferSubData" no_error="true"
>> >> >>> marshal="custom">
>> >> >>>
>> >> >>> <param name="buffer" type="GLuint" />
>> >> >>> <param name="offset" type="GLintptr" />
>> >> >>> <param name="size" type="GLsizeiptr" />
>> >> >>>
>> >> >>> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
>> >> >>> index 4840f32..1fddf8e 100644
>> >> >>> --- a/src/mesa/main/marshal.c
>> >> >>> +++ b/src/mesa/main/marshal.c
>> >> >>> @@ -408,6 +408,114 @@ _mesa_marshal_BufferSubData(GLenum target,
>> >> >>> GLintptr offset, GLsizeiptr size,
>> >> >>>
>> >> >>> }
>> >> >>>
>> >> >>> }
>> >> >>> +/* NamedBufferData: marshalled asynchronously */
>> >> >>>
>> >> >>> +struct marshal_cmd_NamedBufferData
>> >> >>> +{
>> >> >>> + struct marshal_cmd_base cmd_base;
>> >> >>> + GLuint name;
>> >> >>> + GLsizei size;
>> >> >>> + GLenum usage;
>> >> >>> + /* Next size bytes are GLubyte data[size] */
>> >> >>> +};
>> >> >>> +
>> >> >>> +void
>> >> >>> +_mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
>> >> >>> + const struct
>> >> >>> marshal_cmd_NamedBufferData *cmd)
>> >> >>> +{
>> >> >>> + const GLuint name = cmd->name;
>> >> >>> + const GLsizei size = cmd->size;
>> >> >>> + const GLenum usage = cmd->usage;
>> >> >>> + const void *data = (const void *) (cmd + 1);
>> >> >>> +
>> >> >>> + CALL_NamedBufferData(ctx->CurrentServerDispatch,
>> >> >>> + (name, size, data, usage));
>> >> >>> +}
>> >> >>> +
>> >> >>> +void GLAPIENTRY
>> >> >>> +_mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
>> >> >>> + const GLvoid * data, GLenum usage)
>> >> >>> +{
>> >> >>> + GET_CURRENT_CONTEXT(ctx);
>> >> >>> + size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferData) +
>> >> >>> size;
>> >> >>> +
>> >> >>> + debug_print_marshal("NamedBufferData");
>> >> >>> + if (unlikely(size < 0)) {
>> >> >>> + _mesa_glthread_finish(ctx);
>> >> >>> + _mesa_error(ctx, GL_INVALID_VALUE, "NamedBufferData(size <
>> >> >>> 0)");
>> >> >>> + return;
>> >> >>> + }
>> >> >>> +
>> >> >>> + if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>> >> >>> + struct marshal_cmd_NamedBufferData *cmd =
>> >> >>> + _mesa_glthread_allocate_command(ctx,
>> >> >>> DISPATCH_CMD_NamedBufferData,
>> >> >>> + cmd_size);
>> >> >>> + cmd->name = buffer;
>> >> >>> + cmd->size = size;
>> >> >>> + cmd->usage = usage;
>> >> >>> + char *variable_data = (char *) (cmd + 1);
>> >> >>> + memcpy(variable_data, data, size);
>> >> >>> + _mesa_post_marshal_hook(ctx);
>> >> >>> + } else {
>> >> >>> + _mesa_glthread_finish(ctx);
>> >> >>> + CALL_NamedBufferData(ctx->CurrentServerDispatch,
>> >> >>> + (buffer, size, data, usage));
>> >> >>> + }
>> >> >>> +}
>> >> >>> +
>> >> >>> +/* NamedBufferSubData: marshalled asynchronously */
>> >> >>> +struct marshal_cmd_NamedBufferSubData
>> >> >>> +{
>> >> >>> + struct marshal_cmd_base cmd_base;
>> >> >>> + GLuint name;
>> >> >>> + GLintptr offset;
>> >> >>> + GLsizei size;
>> >> >>> + /* Next size bytes are GLubyte data[size] */
>> >> >>> +};
>> >> >>> +
>> >> >>> +void
>> >> >>> +_mesa_unmarshal_NamedBufferSubData(struct gl_context *ctx,
>> >> >>> + const struct
>> >> >>> marshal_cmd_NamedBufferSubData *cmd)
>> >> >>> +{
>> >> >>> + const GLuint name = cmd->name;
>> >> >>> + const GLintptr offset = cmd->offset;
>> >> >>> + const GLsizei size = cmd->size;
>> >> >>> + const void *data = (const void *) (cmd + 1);
>> >> >>> +
>> >> >>> + CALL_NamedBufferSubData(ctx->CurrentServerDispatch,
>> >> >>> + (name, offset, size, data));
>> >> >>> +}
>> >> >>> +
>> >> >>> +void GLAPIENTRY
>> >> >>> +_mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset,
>> >> >>> + GLsizeiptr size, const GLvoid *
>> >> >>> data)
>> >> >>> +{
>> >> >>> + GET_CURRENT_CONTEXT(ctx);
>> >> >>> + size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferSubData)
>> >> >>> + size;
>> >> >>> +
>> >> >>> + debug_print_marshal("NamedBufferSubData");
>> >> >>> + if (unlikely(size < 0)) {
>> >> >>> + _mesa_glthread_finish(ctx);
>> >> >>> + _mesa_error(ctx, GL_INVALID_VALUE, "NamedBufferSubData(size <
>> >> >>> 0)");
>> >> >>> + return;
>> >> >>> + }
>> >> >>> +
>> >> >>> + if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
>> >> >>> + struct marshal_cmd_NamedBufferSubData *cmd =
>> >> >>> + _mesa_glthread_allocate_command(ctx,
>> >> >>> DISPATCH_CMD_NamedBufferSubData,
>> >> >>> + cmd_size);
>> >> >>> + cmd->name = buffer;
>> >> >>> + cmd->offset = offset;
>> >> >>> + cmd->size = size;
>> >> >>> + char *variable_data = (char *) (cmd + 1);
>> >> >>> + memcpy(variable_data, data, size);
>> >> >>> + _mesa_post_marshal_hook(ctx);
>> >> >>> + } else {
>> >> >>> + _mesa_glthread_finish(ctx);
>> >> >>> + CALL_NamedBufferSubData(ctx->CurrentServerDispatch,
>> >> >>> + (buffer, offset, size, data));
>> >> >>> + }
>> >> >>> +}
>> >> >>> +
>> >> >>>
>> >> >>> /* ClearBufferfv: marshalled asynchronously */
>> >> >>> struct marshal_cmd_ClearBufferfv
>> >> >>> {
>> >> >>>
>> >> >>> diff --git a/src/mesa/main/marshal.h b/src/mesa/main/marshal.h
>> >> >>> index ddfa834..999c75e 100644
>> >> >>> --- a/src/mesa/main/marshal.h
>> >> >>> +++ b/src/mesa/main/marshal.h
>> >> >>> @@ -180,6 +180,8 @@ struct marshal_cmd_Flush;
>> >> >>>
>> >> >>> struct marshal_cmd_BindBuffer;
>> >> >>> struct marshal_cmd_BufferData;
>> >> >>> struct marshal_cmd_BufferSubData;
>> >> >>>
>> >> >>> +struct marshal_cmd_NamedBufferData;
>> >> >>> +struct marshal_cmd_NamedBufferSubData;
>> >> >>>
>> >> >>> struct marshal_cmd_ClearBufferfv;
>> >> >>>
>> >> >>> void
>> >> >>>
>> >> >>> @@ -228,6 +230,22 @@ _mesa_marshal_BufferSubData(GLenum target,
>> >> >>> GLintptr offset, GLsizeiptr size,
>> >> >>>
>> >> >>> const GLvoid * data);
>> >> >>>
>> >> >>> void
>> >> >>>
>> >> >>> +_mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
>> >> >>> + const struct
>> >> >>> marshal_cmd_NamedBufferData *cmd);
>> >> >>> +
>> >> >>> +void GLAPIENTRY
>> >> >>> +_mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
>> >> >>> + const GLvoid * data, GLenum usage);
>> >> >>> +
>> >> >>> +void
>> >> >>> +_mesa_unmarshal_NamedBufferSubData(struct gl_context *ctx,
>> >> >>> + const struct
>> >> >>> marshal_cmd_NamedBufferSubData *cmd);
>> >> >>> +
>> >> >>> +void GLAPIENTRY
>> >> >>> +_mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset,
>> >> >>> GLsizeiptr size,
>> >> >>> + const GLvoid * data);
>> >> >>> +
>> >> >>> +void
>> >> >>>
>> >> >>> _mesa_unmarshal_ClearBufferfv(struct gl_context *ctx,
>> >> >>>
>> >> >>> const struct
>> >> >>>
>> >> >>> marshal_cmd_ClearBufferfv *cmd);
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-mesa-marshal-fix-glNamedBufferData-with-NULL-data.patch
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170710/a9f80360/attachment-0001.ksh>
More information about the mesa-dev
mailing list