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

2017-04-13 Thread Nicolai Hähnle 

On 13.04.2017 18:39, gregory hainaut wrote:

On Thu, 13 Apr 2017 18:31:06 +0200
Nicolai Hähnle  wrote:


On 05.04.2017 12:30, Gregory Hainaut wrote:

 # 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


Hello Nicolai,

It isn't symmetrical. In case of UPBO data ought to be copied from app
thread to gl thread. You can see variable_length parameter as input
pointer. Variable length will generate the memcpy code.

However PPBO will copy from GPU to user pointer. There is no data
associated with the pointer so the pointer isn't "used" by glthread,
only transferred to GL.

I think the code would love an extra comment.


Okay, I get it now. But the comment could maybe use some re-wording,
especially because this whole issue only applies to some functions:
those where the marshaling code theoretically has a size, such as with
glCompressedTexImage2D. It would be good to clarify that.


Yes. I added this comment. Though I'm not sure it enough.

+ # variable_params are meaningful when pointers are associated
+ # with a payload hence it only impacts upbo but not ppbo.


I'd rewrite the thing. Here's a suggestion:

  # There are some texture upload functions (the CompressedTexImage
  # family) which provide an imageSize together with the image
  # pointer. The default marshaling behavior is to copy the contents
  # of image to a variable length section, which is incorrect when
  # image refers to a pixel buffer.
  #
  # This ensures that the pointer is marshaled as is. The non-PBO
  # case is always synchronous and so does not use marshaling anyway.

Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-04-13 Thread gregory hainaut 
On Thu, 13 Apr 2017 18:31:06 +0200
Nicolai Hähnle  wrote:

> On 05.04.2017 12:30, Gregory Hainaut wrote:
> >  # 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
> >
> > Hello Nicolai,
> >
> > It isn't symmetrical. In case of UPBO data ought to be copied from app
> > thread to gl thread. You can see variable_length parameter as input
> > pointer. Variable length will generate the memcpy code.
> >
> > However PPBO will copy from GPU to user pointer. There is no data
> > associated with the pointer so the pointer isn't "used" by glthread,
> > only transferred to GL.
> >
> > I think the code would love an extra comment.
> 
> Okay, I get it now. But the comment could maybe use some re-wording, 
> especially because this whole issue only applies to some functions: 
> those where the marshaling code theoretically has a size, such as with 
> glCompressedTexImage2D. It would be good to clarify that.

Yes. I added this comment. Though I'm not sure it enough.

+ # variable_params are meaningful when pointers are associated
+ # with a payload hence it only impacts upbo but not ppbo.

Cheers,
Gregory

> 
> Thanks,
> Nicolai
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-04-13 Thread Nicolai Hähnle 

On 05.04.2017 12:30, Gregory Hainaut wrote:

 # 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


Hello Nicolai,

It isn't symmetrical. In case of UPBO data ought to be copied from app
thread to gl thread. You can see variable_length parameter as input
pointer. Variable length will generate the memcpy code.

However PPBO will copy from GPU to user pointer. There is no data
associated with the pointer so the pointer isn't "used" by glthread,
only transferred to GL.

I think the code would love an extra comment.


Okay, I get it now. But the comment could maybe use some re-wording, 
especially because this whole issue only applies to some functions: 
those where the marshaling code theoretically has a size, such as with 
glCompressedTexImage2D. It would be good to clarify that.


Thanks,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-04-05 Thread Gregory Hainaut 
 # 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

Hello Nicolai,

It isn't symmetrical. In case of UPBO data ought to be copied from app
thread to gl thread. You can see variable_length parameter as input
pointer. Variable length will generate the memcpy code.

However PPBO will copy from GPU to user pointer. There is no data
associated with the pointer so the pointer isn't "used" by glthread,
only transferred to GL.

I think the code would love an extra comment.

Cheers,
Gregory
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-04-05 Thread Nicolai Hähnle 

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 
---
 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 @@
   


-   
+   
   
   
   
@@ -384,7 +384,7 @@
   


-   
+   
   
   
   
@@ -396,7 +396,7 @@
   


-   
+   
   
   
   
@@ -410,7 +410,7 @@
   


-   
+   
   
   
   
@@ -420,7 +420,7 @@
   


-   
+   
   
   
   
@@ -432,7 +432,7 @@
   


-   
+   
   
   
   
@@ -523,7 +523,7 @@
   


-   
+   
   
   
   
@@ -532,7 +532,7 @@
   


-   
+   
   
   
   
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 @@
 
 

-
+
 
 
 
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 @@
 
 

-
+
 
 
 
@@ -2161,7 +2161,7 @@
 
 

-
+
 
 
 
@@ -2640,7 +2640,7 @@
 
 

-
+
 
 
 
@@ -3293,7 +3293,7 @@
 
 

-
+
 
 
 
@@ -3305,7 +3305,7 @@
 
 

-
+
 
 
 
@@ -4005,7 +4005,7 @@
 
 

-
+
 
 
 
@@ -4019,7 +4019,7 @@
 
 

-
+
 
 
 
@@ -4501,7 +4501,7 @@
 
 

-
+
 
 
 
@@ -4514,7 +4514,7 @@
 
 

-
+
 
 
 
@@ -4526,7 +4526,7 @@
 
 

-
+
 
 
 
@@ -4537,7 +4537,7 @@
 
 

-
+
 
 
 
@@ -4552,7 +4552,7 @@
 
 

-
+
 
 
 
@@ -4565,7 +4565,7 @@
 
 

-
+
 
 
 
@@ -4576,7 +4576,7 @@
 
 

-
+
 
 
 
diff --git a/src/mapi/glapi/gen/gl_marshal.py b/src/mapi/glapi/gen/gl_marshal.py
index

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

2017-04-04 Thread 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)

v3:
split buffer tracking into separate patches.
use 'goto fallback_to_sync' when asynchronous transfer isn't supported

Signed-off-by: Gregory Hainaut 
---
 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 @@
   

 
-   
+   
   
   
   
@@ -384,7 +384,7 @@
   

 
-   
+   
   
   
   
@@ -396,7 +396,7 @@
   

 
-   
+   
   
   
   
@@ -410,7 +410,7 @@
   

 
-   
+   
   
   
   
@@ -420,7 +420,7 @@
   

 
-   
+   
   
   
   
@@ -432,7 +432,7 @@
   

 
-   
+   
   
   
   
@@ -523,7 +523,7 @@
   

 
-   
+   
   
   
   
@@ -532,7 +532,7 @@
   

 
-   
+   
   
   
   
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 @@
 
 
 
-
+
 
 
 
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 @@
 
 
 
-
+
 
 
 
@@ -2161,7 +2161,7 @@
 
 
 
-
+
 
 
 
@@ -2640,7 +2640,7 @@
 
 
 
-
+
 
 
 
@@ -3293,7 +3293,7 @@
 
 
 
-
+
 
 
 
@@ -3305,7 +3305,7 @@
 
 
 
-
+
 
 
 
@@ -4005,7 +4005,7 @@
 
 
 
-
+
 
 
 
@@ -4019,7 +4019,7 @@
 
 
 
-
+
 
 
 
@@ -4501,7 +4501,7 @@
 
 
 
-
+
 
 
 
@@ -4514,7 +4514,7 @@
 
 
 
-
+
 
 
 
@@ -4526,7 +4526,7 @@
 
 
 
-
+
 
 
 
@@ -4537,7 +4537,7 @@
 
 
 
-
+
 
 
 
@@ -4552,7 +4552,7 @@
 
 
 
-
+
 
 
 
@@ -4565,7 +4565,7 @@
 
 
 
-
+
 
 
 
@@ -4576,7 +4576,7 @@
 
 
 
-
+
 
 
 
diff --git a/src/mapi/glapi/gen/gl_marshal.py b/src/mapi/glapi/gen/gl_marshal.py
index 51475e1..713ea02 100644
---