[Mesa-dev] [PATCH] mesa/marshal: add custom marshallingforglNamedBuffer(Sub)Data

Marc Dietrich marvin24 at gmx.de
Mon Jun 26 14:28:46 UTC 2017


Hi Roland,

Am Montag, 26. Juni 2017, 15:51:12 CEST schrieb Marc Dietrich:
> 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...

can you help?

Marc

> > Best regards
> > Grigori
> > 
> > >> > Best regards
> > >> > Grigori
> > >> > 
> > >> >> [1]
> > >> >> https://lists.freedesktop.org/archives/mesa-dev/2017-June/160329.htm
> > >> >> l
> > >> >> 
> > >> >> 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 --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170626/fdce859d/attachment.sig>


More information about the mesa-dev mailing list