[Mesa-dev] [PATCH v2 3/3] mapi/glthread: generate asynchronous code for PBO transfer

Nicolai Hähnle nhaehnle at gmail.com
Wed Apr 5 10:18:13 UTC 2017


On 01.04.2017 11:42, Gregory Hainaut wrote:
> Improve speed on PCSX2
>
> v2:
> Add ppbo/ubpo status in XML file
> Disable variable parameter (as the pointer would be a fixed offset)
>
> v3:
> split buffer tracking into separate patches.
> use 'goto fallback_to_sync' when asynchronous transfer isn't supported
>
> 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               | 23 ++++++++++++++++++++-
>  src/mapi/glapi/gen/marshal_XML.py              | 19 ++++++++++++-----
>  6 files changed, 65 insertions(+), 33 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> index 566f157..d842ebd 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 ce904a5..6e29d3f 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 51475e1..713ea02 100644
> --- a/src/mapi/glapi/gen/gl_marshal.py
> +++ b/src/mapi/glapi/gen/gl_marshal.py
> @@ -175,10 +175,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))
> @@ -249,6 +255,21 @@ class PrintCode(gl_XML.gl_print_base):
>                      out('return;')
>                  out('}')
>
> +            flavor = func.marshal_flavor()
> +            if flavor == "upbo":
> +                need_fallback_sync = True
> +                out('if (!ctx->GLThread->pixel_unpack_buffer_bound) {')
> +                with indent():
> +                    out('goto fallback_to_sync;')
> +                out('}')
> +
> +            if flavor == "ppbo":
> +                need_fallback_sync = True
> +                out('if (!ctx->GLThread->pixel_pack_buffer_bound) {')
> +                with indent():
> +                    out('goto fallback_to_sync;')
> +                out('}')
> +
>              out('if (cmd_size <= MARSHAL_MAX_CMD_SIZE) {')
>              with indent():
>                  self.print_async_dispatch(func)
> @@ -328,7 +349,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)

If this is needed for upbo, shouldn't it also be needed for ppbo?

Cheers,
Nicolai


> +            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."""
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.


More information about the mesa-dev mailing list