[Mesa-dev] [PATCH] mesa glthread: allow asynchronous pixel transfer operation when a buffer is bound
Dieter Nützel
Dieter at nuetzel-hh.de
Mon Mar 20 11:51:02 UTC 2017
Tested-by: Dieter Nützel <Dieter at nuetzel-hh.de>
Am 17.03.2017 10:25, schrieb Gregory Hainaut:
> Improve speed on PCSX2
>
> v2:
> Add ppbo/ubpo status in XML file
> Disable variable parameter (as the pointer would be a fixed offset)
>
> Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> ---
> src/mapi/glapi/gen/ARB_direct_state_access.xml | 16 +++++++--------
> src/mapi/glapi/gen/ARB_robustness.xml | 2 +-
> src/mapi/glapi/gen/gl_API.dtd | 10 +++++----
> src/mapi/glapi/gen/gl_API.xml | 28
> +++++++++++++-------------
> src/mapi/glapi/gen/gl_marshal.py | 25
> ++++++++++++++++++++++-
> src/mapi/glapi/gen/marshal_XML.py | 19 ++++++++++++-----
> src/mesa/main/glthread.h | 10 +++++++++
> src/mesa/main/marshal.c | 16 ++++++++++++---
> 8 files changed, 90 insertions(+), 36 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> index 43841bb..8d98c2b 100644
> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> @@ -374,7 +374,7 @@
> <param name="fixedsamplelocations" type="GLboolean" />
> </function>
>
> - <function name="TextureSubImage1D">
> + <function name="TextureSubImage1D" marshal="upbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="xoffset" type="GLint" />
> @@ -384,7 +384,7 @@
> <param name="pixels" type="const GLvoid *" />
> </function>
>
> - <function name="TextureSubImage2D">
> + <function name="TextureSubImage2D" marshal="upbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="xoffset" type="GLint" />
> @@ -396,7 +396,7 @@
> <param name="pixels" type="const GLvoid *" />
> </function>
>
> - <function name="TextureSubImage3D">
> + <function name="TextureSubImage3D" marshal="upbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="xoffset" type="GLint" />
> @@ -410,7 +410,7 @@
> <param name="pixels" type="const GLvoid *" />
> </function>
>
> - <function name="CompressedTextureSubImage1D">
> + <function name="CompressedTextureSubImage1D" marshal="upbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="xoffset" type="GLint" />
> @@ -420,7 +420,7 @@
> <param name="data" type="const GLvoid *" />
> </function>
>
> - <function name="CompressedTextureSubImage2D">
> + <function name="CompressedTextureSubImage2D" marshal="upbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="xoffset" type="GLint" />
> @@ -432,7 +432,7 @@
> <param name="data" type="const GLvoid *" />
> </function>
>
> - <function name="CompressedTextureSubImage3D">
> + <function name="CompressedTextureSubImage3D" marshal="upbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="xoffset" type="GLint" />
> @@ -523,7 +523,7 @@
> <param name="texture" type="GLuint" />
> </function>
>
> - <function name="GetTextureImage">
> + <function name="GetTextureImage" marshal="ppbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="format" type="GLenum" />
> @@ -532,7 +532,7 @@
> <param name="pixels" type="GLvoid *" />
> </function>
>
> - <function name="GetCompressedTextureImage">
> + <function name="GetCompressedTextureImage" marshal="ppbo">
> <param name="texture" type="GLuint" />
> <param name="level" type="GLint" />
> <param name="bufSize" type="GLsizei" />
> diff --git a/src/mapi/glapi/gen/ARB_robustness.xml
> b/src/mapi/glapi/gen/ARB_robustness.xml
> index 9b2f2f0..6e1ac09 100644
> --- a/src/mapi/glapi/gen/ARB_robustness.xml
> +++ b/src/mapi/glapi/gen/ARB_robustness.xml
> @@ -82,7 +82,7 @@
> <param name="img" type="GLvoid *" output="true"/>
> </function>
>
> - <function name="ReadnPixelsARB">
> + <function name="ReadnPixelsARB" marshal="ppbo">
> <param name="x" type="GLint"/>
> <param name="y" type="GLint"/>
> <param name="width" type="GLsizei"/>
> diff --git a/src/mapi/glapi/gen/gl_API.dtd
> b/src/mapi/glapi/gen/gl_API.dtd
> index b464250..447b03a 100644
> --- a/src/mapi/glapi/gen/gl_API.dtd
> +++ b/src/mapi/glapi/gen/gl_API.dtd
> @@ -122,14 +122,16 @@ param:
> offset data should be padded to the next even number of
> dimensions.
> For example, this will insert an empty "height" field after
> the
> "width" field in the protocol for TexImage1D.
> - marshal - One of "sync", "async", "draw", or "custom", defaulting
> to
> - async unless one of the arguments is something we know we
> can't
> - codegen for. If "sync", we finish any queued glthread work
> and call
> + marshal - One of "sync", "async", "draw", "ppbo", "upbo" or
> "custom",
> + defaulting to async unless one of the arguments is something
> we know we
> + can't codegen for. If "sync", we finish any queued glthread
> work and call
> the Mesa implementation directly. If "async", we queue the
> function
> call to be performed by glthread. If "custom", the prototype
> will be
> generated but a custom implementation will be present in
> marshal.c.
> If "draw", it will follow the "async" rules except that
> "indices" are
> - ignored (since they may come from a VBO).
> + ignored (since they may come from a VBO). If "ppbo"/"upbo", it
> will
> + follow the "async" rules when a pack/unpack pixel buffer is
> bound
> + otherwise it will follow the "sync" rules.
> marshal_fail - an expression that, if it evaluates true, causes
> glthread
> to switch back to the Mesa implementation and call it
> directly. Used
> to disable glthread for GL compatibility interactions that we
> don't
> diff --git a/src/mapi/glapi/gen/gl_API.xml
> b/src/mapi/glapi/gen/gl_API.xml
> index 15d7e4f..561f2b5 100644
> --- a/src/mapi/glapi/gen/gl_API.xml
> +++ b/src/mapi/glapi/gen/gl_API.xml
> @@ -2149,7 +2149,7 @@
> <glx rop="108"/>
> </function>
>
> - <function name="TexImage1D">
> + <function name="TexImage1D" marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="internalformat" type="GLint"/>
> @@ -2161,7 +2161,7 @@
> <glx rop="109" large="true"/>
> </function>
>
> - <function name="TexImage2D" es1="1.0" es2="2.0">
> + <function name="TexImage2D" marshal="upbo" es1="1.0" es2="2.0">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="internalformat" type="GLint"/>
> @@ -2640,7 +2640,7 @@
> <glx rop="172"/>
> </function>
>
> - <function name="ReadPixels" es1="1.0" es2="2.0">
> + <function name="ReadPixels" marshal="ppbo" es1="1.0" es2="2.0">
> <param name="x" type="GLint"/>
> <param name="y" type="GLint"/>
> <param name="width" type="GLsizei"/>
> @@ -3293,7 +3293,7 @@
> <glx rop="4122"/>
> </function>
>
> - <function name="TexSubImage1D">
> + <function name="TexSubImage1D" marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="xoffset" type="GLint"/>
> @@ -3305,7 +3305,7 @@
> <glx rop="4099" large="true"/>
> </function>
>
> - <function name="TexSubImage2D" es1="1.0" es2="2.0">
> + <function name="TexSubImage2D" marshal="upbo" es1="1.0" es2="2.0">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="xoffset" type="GLint"/>
> @@ -4005,7 +4005,7 @@
> <glx rop="4113"/>
> </function>
>
> - <function name="TexImage3D" es2="3.0">
> + <function name="TexImage3D" marshal="upbo" es2="3.0">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="internalformat" type="GLint"/>
> @@ -4019,7 +4019,7 @@
> <glx rop="4114" large="true"/>
> </function>
>
> - <function name="TexSubImage3D" es2="3.0">
> + <function name="TexSubImage3D" marshal="upbo" es2="3.0">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="xoffset" type="GLint"/>
> @@ -4501,7 +4501,7 @@
> <glx rop="229"/>
> </function>
>
> - <function name="CompressedTexImage3D" es2="3.0" marshal="sync">
> + <function name="CompressedTexImage3D" es2="3.0" marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="internalformat" type="GLenum"/>
> @@ -4514,7 +4514,7 @@
> <glx rop="216" handcode="client"/>
> </function>
>
> - <function name="CompressedTexImage2D" es1="1.0" es2="2.0"
> marshal="sync">
> + <function name="CompressedTexImage2D" es1="1.0" es2="2.0"
> marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="internalformat" type="GLenum"/>
> @@ -4526,7 +4526,7 @@
> <glx rop="215" handcode="client"/>
> </function>
>
> - <function name="CompressedTexImage1D" marshal="sync">
> + <function name="CompressedTexImage1D" marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="internalformat" type="GLenum"/>
> @@ -4537,7 +4537,7 @@
> <glx rop="214" handcode="client"/>
> </function>
>
> - <function name="CompressedTexSubImage3D" es2="3.0" marshal="sync">
> + <function name="CompressedTexSubImage3D" es2="3.0" marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="xoffset" type="GLint"/>
> @@ -4552,7 +4552,7 @@
> <glx rop="219" handcode="client"/>
> </function>
>
> - <function name="CompressedTexSubImage2D" es1="1.0" es2="2.0"
> marshal="sync">
> + <function name="CompressedTexSubImage2D" es1="1.0" es2="2.0"
> marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="xoffset" type="GLint"/>
> @@ -4565,7 +4565,7 @@
> <glx rop="218" handcode="client"/>
> </function>
>
> - <function name="CompressedTexSubImage1D" marshal="sync">
> + <function name="CompressedTexSubImage1D" marshal="upbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="xoffset" type="GLint"/>
> @@ -4576,7 +4576,7 @@
> <glx rop="217" handcode="client"/>
> </function>
>
> - <function name="GetCompressedTexImage">
> + <function name="GetCompressedTexImage" marshal="ppbo">
> <param name="target" type="GLenum"/>
> <param name="level" type="GLint"/>
> <param name="img" type="GLvoid *" output="true"/>
> diff --git a/src/mapi/glapi/gen/gl_marshal.py
> b/src/mapi/glapi/gen/gl_marshal.py
> index c89d397..5a9a4f2 100644
> --- a/src/mapi/glapi/gen/gl_marshal.py
> +++ b/src/mapi/glapi/gen/gl_marshal.py
> @@ -174,10 +174,16 @@ class PrintCode(gl_XML.gl_print_base):
> 'const struct marshal_cmd_{0} *cmd)').format(func.name))
> out('{')
> with indent():
> + flavor = func.marshal_flavor()
> for p in func.fixed_params:
> if p.count:
> out('const {0} * {1} = cmd->{1};'.format(
> p.get_base_type_string(), p.name))
> + elif p.is_pointer() and flavor == 'ppbo':
> + # Data is written to destination user pointer (if
> no PBO
> + # are bound) therefore the pointer must be
> non-const
> + out('{0} * {1} = cmd->{1};'.format(
> + p.get_base_type_string(), p.name))
> else:
> out('const {0} {1} = cmd->{1};'.format(
> p.type_string(), p.name))
> @@ -246,6 +252,23 @@ class PrintCode(gl_XML.gl_print_base):
> out('return;')
> out('}')
>
> + flavor = func.marshal_flavor()
> + if flavor == "upbo":
> + out('if (!ctx->GLThread->pixel_unpack_buffer_bound)
> {')
> + with indent():
> + out('debug_print_sync("{0}");'.format(func.name))
> + self.print_sync_dispatch(func)
> + out('return;')
> + out('}')
> +
> + if flavor == "ppbo":
> + out('if (!ctx->GLThread->pixel_pack_buffer_bound) {')
> + with indent():
> + out('debug_print_sync("{0}");'.format(func.name))
> + self.print_sync_dispatch(func)
> + out('return;')
> + out('}')
> +
> out('if (cmd_size <= MARSHAL_MAX_CMD_SIZE) {')
> with indent():
> self.print_async_dispatch(func)
> @@ -321,7 +344,7 @@ class PrintCode(gl_XML.gl_print_base):
> flavor = func.marshal_flavor()
> if flavor in ('skip', 'custom'):
> continue
> - elif flavor == 'async':
> + elif flavor == 'async' or flavor == 'ppbo' or flavor ==
> 'upbo':
> self.print_async_body(func)
> async_funcs.append(func)
> elif flavor == 'sync':
> diff --git a/src/mapi/glapi/gen/marshal_XML.py
> b/src/mapi/glapi/gen/marshal_XML.py
> index 80f7f54..528f60f 100644
> --- a/src/mapi/glapi/gen/marshal_XML.py
> +++ b/src/mapi/glapi/gen/marshal_XML.py
> @@ -45,21 +45,30 @@ class marshal_function(gl_XML.gl_function):
> if element.get('name') != self.name:
> return
>
> + # Store the "marshal" attribute, if present.
> + self.marshal = element.get('marshal')
> + self.marshal_fail = element.get('marshal_fail')
> +
> # Classify fixed and variable parameters.
> self.fixed_params = []
> self.variable_params = []
> for p in self.parameters:
> if p.is_padding:
> continue
> - if p.is_variable_length():
> + if self.marshal == "upbo" and p.is_pointer():
> + # Pixel buffer transfer API is tricky. By default it
> contains
> + # a pointer to user memory so a variable length
> parameter.
> + # When a pixel buffer is bound, the pointer becomes an
> offset.
> + #
> + # Non-PBO transfer will be synchronous so parameter
> type isn't
> + # important. PBO transfer will be asynchronous so the
> parameter
> + # must be marked as fixed
> + self.fixed_params.append(p)
> + elif p.is_variable_length():
> self.variable_params.append(p)
> else:
> self.fixed_params.append(p)
>
> - # Store the "marshal" attribute, if present.
> - self.marshal = element.get('marshal')
> - self.marshal_fail = element.get('marshal_fail')
> -
> def marshal_flavor(self):
> """Find out how this function should be marshalled between
> client and server threads."""
> diff --git a/src/mesa/main/glthread.h b/src/mesa/main/glthread.h
> index 50c1db2..f68d6b9 100644
> --- a/src/mesa/main/glthread.h
> +++ b/src/mesa/main/glthread.h
> @@ -92,6 +92,16 @@ struct glthread_state
> * buffer) binding is in a VBO.
> */
> bool element_array_is_vbo;
> +
> + /**
> + * Tracks on the main thread side whether an unpack pixel buffer is
> bound
> + */
> + bool pixel_unpack_buffer_bound;
> +
> + /**
> + * Tracks on the main thread side whether a pack pixel buffer is
> bound
> + */
> + bool pixel_pack_buffer_bound;
> };
>
> /**
> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
> index f8cad30..43a70d4 100644
> --- a/src/mesa/main/marshal.c
> +++ b/src/mesa/main/marshal.c
> @@ -175,7 +175,7 @@ _mesa_unmarshal_BindBufferBase(struct gl_context
> *ctx, const struct marshal_cmd_
> CALL_BindBufferBase(ctx->CurrentServerDispatch, (target, index,
> buffer));
> }
>
> -/** Tracks the current bindings for the vertex array and index array
> buffers.
> +/** Tracks the current bindings of GL buffer targets
> *
> * This is part of what we need to enable glthread on compat-GL
> contexts that
> * happen to use VBOs, without also supporting the full tracking of
> VBO vs
> @@ -197,9 +197,13 @@ _mesa_unmarshal_BindBufferBase(struct gl_context
> *ctx, const struct marshal_cmd_
> * instead of updating the binding. However, compat GL has the
> ridiculous
> * feature that if you pass a bad name, it just gens a buffer object
> for you,
> * so we escape without having to know if things are valid or not.
> + *
> + * Code was extended to track pixel buffers so you know if pixel
> transfer
> + * goes to an user pointer (must be synchronous) or an GL buffer
> (could
> + * be asynchronous)
> */
> static void
> -track_vbo_binding(struct gl_context *ctx, GLenum target, GLuint
> buffer)
> +track_buffers_binding(struct gl_context *ctx, GLenum target, GLuint
> buffer)
> {
> struct glthread_state *glthread = ctx->GLThread;
>
> @@ -214,6 +218,12 @@ track_vbo_binding(struct gl_context *ctx, GLenum
> target, GLuint buffer)
> */
> glthread->element_array_is_vbo = (buffer != 0);
> break;
> + case GL_PIXEL_UNPACK_BUFFER:
> + glthread->pixel_unpack_buffer_bound = (buffer != 0);
> + break;
> + case GL_PIXEL_PACK_BUFFER:
> + glthread->pixel_pack_buffer_bound = (buffer != 0);
> + break;
> }
> }
>
> @@ -245,7 +255,7 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint
> buffer)
> struct marshal_cmd_BindBuffer *cmd;
> debug_print_marshal("BindBuffer");
>
> - track_vbo_binding(ctx, target, buffer);
> + track_buffers_binding(ctx, target, buffer);
>
> if (cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> cmd = _mesa_glthread_allocate_command(ctx,
> DISPATCH_CMD_BindBuffer,
More information about the mesa-dev
mailing list