[Mesa-dev] [PATCH] mesa glthread: allow asynchronous pixel transfer operation when a buffer is bound

Gregory Hainaut gregory.hainaut at gmail.com
Fri Mar 17 09:25:37 UTC 2017


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,
-- 
2.1.4



More information about the mesa-dev mailing list