Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Dieter Nützel]

Hello Gregory,

time for an update/rebase (after Timothy's GREAT work on 'no error').
Your Patchwork series (v5-x-y) didn't apply anylonger after commit 
f96edf7.


Greetings,
Dieter

Am 19.04.2017 18:54, schrieb Gregory Hainaut:

Hello,

Please find the latest version that include a small fix for hash 
deletion. I

think the series is good now. Please review/ack it.

Allow to handle this kind of case:
   genBuffer();
   BindBuffer(pbo)
   DeleteBuffer(pbo);
   BindBuffer(rand_pbo)
   TexSubImage2D(user_memory_pointer); // Data transfer will be 
synchronous


There are various subtely to handle multi threaded shared context. In 
order to
keep the code sane, I've considered a buffer invalid when it is deleted 
by a
context even it is still bound to others contexts. It will force a 
synchronous

transfer which is always safe.

An example could be
   Ctx A: glGenBuffers(1, );
   Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo);
   Ctx B: glDeleteBuffers(1, );
   Ctx A: glTexSubImage2D(...); // will be synchronous, even though it
   _could_ be asynchronous (because the PBO that was generated first is
   still bound!)

V3: I mixed up the number so I jumped right away to v4...
V4: improve commments based on Nicolai feedback
V5: Properly delete element of the new hash (first patch)

Best regards,

Gregory Hainaut (3):
  mesa/glthread: track buffer creation/destruction
  mesa/glthread: add tracking of PBO binding
  mapi/glthread: generate asynchronous code for PBO transfer

 src/mapi/glapi/gen/ARB_direct_state_access.xml |  18 +--
 src/mapi/glapi/gen/ARB_robustness.xml  |   2 +-
 src/mapi/glapi/gen/gl_API.dtd  |  10 +-
 src/mapi/glapi/gen/gl_API.xml  |  32 +++---
 src/mapi/glapi/gen/gl_marshal.py   |  23 +++-
 src/mapi/glapi/gen/marshal_XML.py  |  21 +++-
 src/mesa/main/glthread.h   |  10 ++
 src/mesa/main/marshal.c| 149 
-

 src/mesa/main/marshal.h|  24 
 src/mesa/main/mtypes.h |   5 +
 src/mesa/main/shared.c |  14 +++
 11 files changed, 269 insertions(+), 39 deletions(-)

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

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[gregory hainaut]

On Wed, 26 Apr 2017 11:03:11 +0900
Michel Dänzer  wrote:

> On 25/04/17 06:14 PM, Gregory Hainaut wrote:
> > Hello,
> > 
> > I did more tests on my side. DRI3 + recent stack is fine. Older
> > (Debian Jessie, ~2y old) XCB hangs/deadlock. So all DRI3 drivers
> > should be fine (typically AMD). And all applications that called
> > XInitThread soon enough. So yeah I think we can merge it. Note: I
> > don't have commit access.  
> 
> Okay, I'm not blocking somebody else from pushing the series.
> 
> 
> > Nouveau doesn't use DRI3 by default. So I'm looking to use XCB for
> > dri2GetBuffer* operations. I copied the code from EGL/X11. So far it
> > isn't a success. The dri2_drawable pdraw object is correctly populated
> > with good value but I have tons of piglit failure. Debug is on-going
> > but I'm afraid that it might not be possible to use XCB.  
> 
> Can you share the patch you're testing?
> 
> 

Hello Michel,

I found my issue, I forgot to set the *width, *height pointer. Piglit status is 
good.

You can found the patch here 
https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html

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

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Michel Dänzer]

On 25/04/17 06:14 PM, Gregory Hainaut wrote:
> Hello,
> 
> I did more tests on my side. DRI3 + recent stack is fine. Older
> (Debian Jessie, ~2y old) XCB hangs/deadlock. So all DRI3 drivers
> should be fine (typically AMD). And all applications that called
> XInitThread soon enough. So yeah I think we can merge it. Note: I
> don't have commit access.

Okay, I'm not blocking somebody else from pushing the series.


> Nouveau doesn't use DRI3 by default. So I'm looking to use XCB for
> dri2GetBuffer* operations. I copied the code from EGL/X11. So far it
> isn't a success. The dri2_drawable pdraw object is correctly populated
> with good value but I have tons of piglit failure. Debug is on-going
> but I'm afraid that it might not be possible to use XCB.

Can you share the patch you're testing?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Gregory Hainaut]

Hello,

I did more tests on my side. DRI3 + recent stack is fine. Older
(Debian Jessie, ~2y old) XCB hangs/deadlock. So all DRI3 drivers
should be fine (typically AMD). And all applications that called
XInitThread soon enough. So yeah I think we can merge it. Note: I
don't have commit access.

Nouveau doesn't use DRI3 by default. So I'm looking to use XCB for
dri2GetBuffer* operations. I copied the code from EGL/X11. So far it
isn't a success. The dri2_drawable pdraw object is correctly populated
with good value but I have tons of piglit failure. Debug is on-going
but I'm afraid that it might not be possible to use XCB.

Best Regads,
Gregory


On 4/25/17, Dieter Nützel  wrote:
> Am 21.04.2017 12:11, schrieb Marek Olšák:
>> FWIW, I think this series can land, because glthread is not enabled by
>> default, and the libX11 issue is unrelated.
>>
>> Marek
>
>
> Gregory?
>
> For the series:
>
> Tested-by: Dieter Nützel 
>
> On Turks XT (6670).
>
> Dieter
>
>> On Apr 21, 2017 4:22 AM, "Michel Dänzer"  wrote:
>>
>>> On 21/04/17 09:01 AM, Marek Olšák wrote:
 On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
  wrote:
> On Thu, 20 Apr 2017 20:01:00 +0200
> Marek Olšák  wrote:
>
>> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>>  wrote:
>>> On Thu, 20 Apr 2017 11:57:08 +0200
>>> Marek Olšák  wrote:
>>>
 On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
  wrote:
> On Thu, 20 Apr 2017 12:29:11 +0900
> Michel Dänzer  wrote:
>
>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>>> Hello,
>>>
>>> Please find the latest version that include a small fix for
>>> hash deletion. I
>>> think the series is good now. Please review/ack it.
>>
>> I'm afraid I have to NACK it. As discussed in the v4 cover
>>> letter
>> thread, Mesa's glthread cannot make any libX11 API calls.
>>
>>
>
> Hello Michel,
>
> Just to be sure we are on the same line, let's me do a
>>> summary.
>
> PCSX2 does the following bad pattern
> 1/ no call to XInitThread
> 2/ XGetGeometry to get the size of the surface
> 3/ glDrawArray into the default framebuffer 0 (the window,
>>> I'm not sure how to call it)
> => which seem to call DRI2GetBuffersWithFormat under the
>>> hood. I guess to get the
> associated buffer of the window.
>
>
> So far it was (kind of) working fine because PCSX2 does tons
>>> of PBO transfer. So glthread
> was mostly synchronous.
>
> This series removes the (useless) PBO transfer
>>> synchronization. So now glthread is really
> asynchronous and the above bad pattern crash as expected.
>
> I didn't add any libX11 API call on the patches. And I don't
>>> think we can remvove the DRI stuff.
>
> Hum, I don't know what will be the impact on the perf but
>>> potentially we can force a synchronization
> when there is a draw to framebuffer 0.

 Can you send us the backtrace when DRI2GetBuffersWithFormat is
>>> called
 by glDrawArrays?

 Marek
>>>
>>> Hello Marek, Michel,
>>>
>>> Here the full backtrace.
>>>
>>> #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48,
>>> drawable=104857784, width=0xdef348c0, height=0xdef348c4,
>>> attachments=0xdcc15c88, count=1, outCount=0xdcc15c74) at dri2.c:464
>>> #1  0xf05bac45 in dri2GetBuffersWithFormat
>>> (driDrawable=0xdef348a8, width=0xdef348c0, height=0xdef348c4,
>>> attachments=0xdcc15c88, count=1, out_count=0xdcc15c74,
>>> loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
>>> #2  0xe3ec3111 in dri2_drawable_get_buffers (count=>> pointer>, atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
>>> #3  dri2_allocate_textures (ctx=0xdef15928,
>>> drawable=0xdefc7b08, statts=0xdef252f8, statts_count=2) at
>>> dri2.c:480
>>> #4  0xe3ebcbb0 in dri_st_framebuffer_validate
>>> (stctx=0xdef4ab10, stfbi=0xdefc7b08, statts=0xdef252f8, count=2,
>>> out=0xdcc15d78) at dri_drawable.c:83
>>> #5  0xe3d37afc in st_framebuffer_validate
>>> (stfb=stfb@entry=0xdef24f58, st=st@entry=0xdef4ab10) at
>>> state_tracker/st_manager.c:189
>>> Note "print stfb->Base->Name" give me 0
>>> #6  0xe3d38649 in st_manager_validate_framebuffers
>>> (st=0xdef4ab10) at state_tracker/st_manager.c:869
>>> #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10,
>>> pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
>>> #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8,
>>> prims=0xdcc15f40, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001',
>>> min_index=39911, max_index=39914, tfb_vertcount=0x0, stream=0,
>>> indirect=0x0) at 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Dieter Nützel]

Am 21.04.2017 12:11, schrieb Marek Olšák:

FWIW, I think this series can land, because glthread is not enabled by
default, and the libX11 issue is unrelated.

Marek



Gregory?

For the series:

Tested-by: Dieter Nützel 

On Turks XT (6670).

Dieter


On Apr 21, 2017 4:22 AM, "Michel Dänzer"  wrote:


On 21/04/17 09:01 AM, Marek Olšák wrote:

On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
 wrote:

On Thu, 20 Apr 2017 20:01:00 +0200
Marek Olšák  wrote:


On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
 wrote:

On Thu, 20 Apr 2017 11:57:08 +0200
Marek Olšák  wrote:


On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
 wrote:

On Thu, 20 Apr 2017 12:29:11 +0900
Michel Dänzer  wrote:


On 20/04/17 01:54 AM, Gregory Hainaut wrote:

Hello,

Please find the latest version that include a small fix for

hash deletion. I

think the series is good now. Please review/ack it.


I'm afraid I have to NACK it. As discussed in the v4 cover

letter

thread, Mesa's glthread cannot make any libX11 API calls.




Hello Michel,

Just to be sure we are on the same line, let's me do a

summary.


PCSX2 does the following bad pattern
1/ no call to XInitThread
2/ XGetGeometry to get the size of the surface
3/ glDrawArray into the default framebuffer 0 (the window,

I'm not sure how to call it)

=> which seem to call DRI2GetBuffersWithFormat under the

hood. I guess to get the

associated buffer of the window.


So far it was (kind of) working fine because PCSX2 does tons

of PBO transfer. So glthread

was mostly synchronous.

This series removes the (useless) PBO transfer

synchronization. So now glthread is really

asynchronous and the above bad pattern crash as expected.

I didn't add any libX11 API call on the patches. And I don't

think we can remvove the DRI stuff.


Hum, I don't know what will be the impact on the perf but

potentially we can force a synchronization

when there is a draw to framebuffer 0.


Can you send us the backtrace when DRI2GetBuffersWithFormat is

called

by glDrawArrays?

Marek


Hello Marek, Michel,

Here the full backtrace.

#0  DRI2GetBuffersWithFormat (dpy=0xc6307e48,

drawable=104857784, width=0xdef348c0, height=0xdef348c4,
attachments=0xdcc15c88, count=1, outCount=0xdcc15c74) at dri2.c:464

#1  0xf05bac45 in dri2GetBuffersWithFormat

(driDrawable=0xdef348a8, width=0xdef348c0, height=0xdef348c4,
attachments=0xdcc15c88, count=1, out_count=0xdcc15c74,
loaderPrivate=0xdefc6ef0) at dri2_glx.c:894

#2  0xe3ec3111 in dri2_drawable_get_buffers (count=
pointer>, atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285

#3  dri2_allocate_textures (ctx=0xdef15928,

drawable=0xdefc7b08, statts=0xdef252f8, statts_count=2) at
dri2.c:480

#4  0xe3ebcbb0 in dri_st_framebuffer_validate

(stctx=0xdef4ab10, stfbi=0xdefc7b08, statts=0xdef252f8, count=2,
out=0xdcc15d78) at dri_drawable.c:83

#5  0xe3d37afc in st_framebuffer_validate

(stfb=stfb@entry=0xdef24f58, st=st@entry=0xdef4ab10) at
state_tracker/st_manager.c:189

Note "print stfb->Base->Name" give me 0
#6  0xe3d38649 in st_manager_validate_framebuffers

(st=0xdef4ab10) at state_tracker/st_manager.c:869

#7  0xe3cd0580 in st_validate_state (st=0xdef4ab10,

pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174

#8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8,

prims=0xdcc15f40, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001',
min_index=39911, max_index=39914, tfb_vertcount=0x0, stream=0,
indirect=0x0) at state_tracker/st_draw.c:191

#9  0xe3caf35e in vbo_draw_arrays (baseInstance=0,

numInstances=1, count=4, start=39911, mode=5, ctx=0xdef7f8f8) at
vbo/vbo_exec_array.c:427

#10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at

vbo/vbo_exec_array.c:575

#11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8,

cmd=0xc41574b0) at main/marshal_generated.c:26644

#12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8,

cmd=0xc41574b0) at main/marshal_generated.c:42457

#13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true,

batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64

#14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110



If this DRI2GetBuffersWithFormat call results in libX11 API

calls on the

glthread, that's a bug which needs to be fixed, either by

moving the

DRI2GetBuffersWithFormat call to the main thread or (if

possible) by

changing DRI2GetBuffersWithFormat to use XCB directly.


First, we need to understand why it's happening. Does the app

use the

front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
glReadBuffer(GL_FRONT)?


Hello Marek,

No, I don't use the front buffer. I don't call glDrawBuffer for

the FB 0. So I think,

it should use GL_BACK.
Hum except if some 3rparty libs do something in my back.

In case it have any impact, I'm using either glXSwapIntervalEXT

or glXSwapIntervalMESA (if the former wasn't found)

to control 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Marek Olšák]

FWIW, I think this series can land, because glthread is not enabled by
default, and the libX11 issue is unrelated.

Marek


On Apr 21, 2017 4:22 AM, "Michel Dänzer"  wrote:

On 21/04/17 09:01 AM, Marek Olšák wrote:
> On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
>  wrote:
>> On Thu, 20 Apr 2017 20:01:00 +0200
>> Marek Olšák  wrote:
>>
>>> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>>>  wrote:
 On Thu, 20 Apr 2017 11:57:08 +0200
 Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>  wrote:
>> On Thu, 20 Apr 2017 12:29:11 +0900
>> Michel Dänzer  wrote:
>>
>>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
 Hello,

 Please find the latest version that include a small fix for hash
deletion. I
 think the series is good now. Please review/ack it.
>>>
>>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>>> thread, Mesa's glthread cannot make any libX11 API calls.
>>>
>>>
>>
>> Hello Michel,
>>
>> Just to be sure we are on the same line, let's me do a summary.
>>
>> PCSX2 does the following bad pattern
>> 1/ no call to XInitThread
>> 2/ XGetGeometry to get the size of the surface
>> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not
sure how to call it)
>>=> which seem to call DRI2GetBuffersWithFormat under the hood. I
guess to get the
>>   associated buffer of the window.
>>
>>
>> So far it was (kind of) working fine because PCSX2 does tons of PBO
transfer. So glthread
>> was mostly synchronous.
>>
>> This series removes the (useless) PBO transfer synchronization. So
now glthread is really
>> asynchronous and the above bad pattern crash as expected.
>>
>> I didn't add any libX11 API call on the patches. And I don't think
we can remvove the DRI stuff.
>>
>> Hum, I don't know what will be the impact on the perf but
potentially we can force a synchronization
>> when there is a draw to framebuffer 0.
>
> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> by glDrawArrays?
>
> Marek

 Hello Marek, Michel,

 Here the full backtrace.

 #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784,
width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1,
outCount=0xdcc15c74) at dri2.c:464
 #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8,
width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1,
out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
 #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
 #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08,
statts=0xdef252f8, statts_count=2) at dri2.c:480
 #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10,
stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at
dri_drawable.c:83
 #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58,
st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
 Note "print stfb->Base->Name" give me 0
 #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at
state_tracker/st_manager.c:869
 #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10,
pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
 #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40,
nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911,
max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at
state_tracker/st_draw.c:191
 #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1,
count=4, start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
 #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at
vbo/vbo_exec_array.c:575
 #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8,
cmd=0xc41574b0) at main/marshal_generated.c:26644
 #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at
main/marshal_generated.c:42457
 #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true,
batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
 #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110


> If this DRI2GetBuffersWithFormat call results in libX11 API calls on
the
> glthread, that's a bug which needs to be fixed, either by moving the
> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> changing DRI2GetBuffersWithFormat to use XCB directly.
>>>
>>> First, we need to understand why it's happening. Does the app use the
>>> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
>>> glReadBuffer(GL_FRONT)?
>>
>> Hello Marek,
>>
>> No, I don't use the front buffer. I 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Michel Dänzer]

On 21/04/17 09:01 AM, Marek Olšák wrote:
> On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
>  wrote:
>> On Thu, 20 Apr 2017 20:01:00 +0200
>> Marek Olšák  wrote:
>>
>>> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>>>  wrote:
 On Thu, 20 Apr 2017 11:57:08 +0200
 Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>  wrote:
>> On Thu, 20 Apr 2017 12:29:11 +0900
>> Michel Dänzer  wrote:
>>
>>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
 Hello,

 Please find the latest version that include a small fix for hash 
 deletion. I
 think the series is good now. Please review/ack it.
>>>
>>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>>> thread, Mesa's glthread cannot make any libX11 API calls.
>>>
>>>
>>
>> Hello Michel,
>>
>> Just to be sure we are on the same line, let's me do a summary.
>>
>> PCSX2 does the following bad pattern
>> 1/ no call to XInitThread
>> 2/ XGetGeometry to get the size of the surface
>> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
>> how to call it)
>>=> which seem to call DRI2GetBuffersWithFormat under the hood. I 
>> guess to get the
>>   associated buffer of the window.
>>
>>
>> So far it was (kind of) working fine because PCSX2 does tons of PBO 
>> transfer. So glthread
>> was mostly synchronous.
>>
>> This series removes the (useless) PBO transfer synchronization. So now 
>> glthread is really
>> asynchronous and the above bad pattern crash as expected.
>>
>> I didn't add any libX11 API call on the patches. And I don't think we 
>> can remvove the DRI stuff.
>>
>> Hum, I don't know what will be the impact on the perf but potentially we 
>> can force a synchronization
>> when there is a draw to framebuffer 0.
>
> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> by glDrawArrays?
>
> Marek

 Hello Marek, Michel,

 Here the full backtrace.

 #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
 width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
 outCount=0xdcc15c74) at dri2.c:464
 #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
 width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
 out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
 #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
 atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
 #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
 statts=0xdef252f8, statts_count=2) at dri2.c:480
 #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
 stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
 dri_drawable.c:83
 #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
 st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
 Note "print stfb->Base->Name" give me 0
 #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
 state_tracker/st_manager.c:869
 #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
 pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
 #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, 
 nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911, 
 max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at 
 state_tracker/st_draw.c:191
 #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, 
 count=4, start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
 #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
 vbo/vbo_exec_array.c:575
 #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, 
 cmd=0xc41574b0) at main/marshal_generated.c:26644
 #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
 main/marshal_generated.c:42457
 #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
 batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
 #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110


> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
> glthread, that's a bug which needs to be fixed, either by moving the
> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> changing DRI2GetBuffersWithFormat to use XCB directly.
>>>
>>> First, we need to understand why it's happening. Does the app use the
>>> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
>>> glReadBuffer(GL_FRONT)?
>>
>> Hello Marek,
>>
>> No, I don't use the front buffer. I don't call 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Michel Dänzer]

On 20/04/17 05:28 PM, gregory hainaut wrote:
> On Thu, 20 Apr 2017 12:29:11 +0900
> Michel Dänzer  wrote:
> 
>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>>> Hello,
>>>
>>> Please find the latest version that include a small fix for hash deletion. I
>>> think the series is good now. Please review/ack it.
>>
>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> thread, Mesa's glthread cannot make any libX11 API calls.
>>
>>
> 
> Hello Michel,
> 
> Just to be sure we are on the same line, let's me do a summary.
> 
> PCSX2 does the following bad pattern
> 1/ no call to XInitThread
> 2/ XGetGeometry to get the size of the surface
> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how 
> to call it)
>=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess to 
> get the
>   associated buffer of the window.

If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
glthread, that's a bug which needs to be fixed, either by moving the
DRI2GetBuffersWithFormat call to the main thread or (if possible) by
changing DRI2GetBuffersWithFormat to use XCB directly.


> So far it was (kind of) working fine because PCSX2 does tons of PBO transfer. 
> So glthread
> was mostly synchronous.
> 
> This series removes the (useless) PBO transfer synchronization. So now 
> glthread is really
> asynchronous and the above bad pattern crash as expected. 
> 
> I didn't add any libX11 API call on the patches.

It sounds like the glthread bug above exists independently from this
patch series, which might just make it more likely to result in a crash.
I think it would still be better to fix the bug before landing this series.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Marek Olšák]

On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
 wrote:
> On Thu, 20 Apr 2017 20:01:00 +0200
> Marek Olšák  wrote:
>
>> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>>  wrote:
>> > On Thu, 20 Apr 2017 11:57:08 +0200
>> > Marek Olšák  wrote:
>> >
>> >> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>> >>  wrote:
>> >> > On Thu, 20 Apr 2017 12:29:11 +0900
>> >> > Michel Dänzer  wrote:
>> >> >
>> >> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>> >> >> > Hello,
>> >> >> >
>> >> >> > Please find the latest version that include a small fix for hash 
>> >> >> > deletion. I
>> >> >> > think the series is good now. Please review/ack it.
>> >> >>
>> >> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> >> >> thread, Mesa's glthread cannot make any libX11 API calls.
>> >> >>
>> >> >>
>> >> >
>> >> > Hello Michel,
>> >> >
>> >> > Just to be sure we are on the same line, let's me do a summary.
>> >> >
>> >> > PCSX2 does the following bad pattern
>> >> > 1/ no call to XInitThread
>> >> > 2/ XGetGeometry to get the size of the surface
>> >> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
>> >> > how to call it)
>> >> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I 
>> >> > guess to get the
>> >> >   associated buffer of the window.
>> >> >
>> >> >
>> >> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
>> >> > transfer. So glthread
>> >> > was mostly synchronous.
>> >> >
>> >> > This series removes the (useless) PBO transfer synchronization. So now 
>> >> > glthread is really
>> >> > asynchronous and the above bad pattern crash as expected.
>> >> >
>> >> > I didn't add any libX11 API call on the patches. And I don't think we 
>> >> > can remvove the DRI stuff.
>> >> >
>> >> > Hum, I don't know what will be the impact on the perf but potentially 
>> >> > we can force a synchronization
>> >> > when there is a draw to framebuffer 0.
>> >>
>> >> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
>> >> by glDrawArrays?
>> >>
>> >> Marek
>> >
>> > Hello Marek, Michel,
>> >
>> > Here the full backtrace.
>> >
>> > #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
>> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
>> > outCount=0xdcc15c74) at dri2.c:464
>> > #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
>> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
>> > out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
>> > #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
>> > atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
>> > #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
>> > statts=0xdef252f8, statts_count=2) at dri2.c:480
>> > #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
>> > stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
>> > dri_drawable.c:83
>> > #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
>> > st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
>> > Note "print stfb->Base->Name" give me 0
>> > #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
>> > state_tracker/st_manager.c:869
>> > #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
>> > pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
>> > #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, 
>> > nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911, 
>> > max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at 
>> > state_tracker/st_draw.c:191
>> > #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, 
>> > count=4, start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
>> > #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
>> > vbo/vbo_exec_array.c:575
>> > #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, 
>> > cmd=0xc41574b0) at main/marshal_generated.c:26644
>> > #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
>> > main/marshal_generated.c:42457
>> > #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
>> > batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
>> > #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110
>> >
>> >
>> >> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
>> >> glthread, that's a bug which needs to be fixed, either by moving the
>> >> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
>> >> changing DRI2GetBuffersWithFormat to use XCB directly.
>>
>> First, we need to understand why it's happening. Does the app use the
>> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
>> glReadBuffer(GL_FRONT)?
>
> Hello Marek,
>
> No, I don't use the front buffer. I don't call glDrawBuffer for the 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[gregory hainaut]

On Thu, 20 Apr 2017 20:01:00 +0200
Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>  wrote:
> > On Thu, 20 Apr 2017 11:57:08 +0200
> > Marek Olšák  wrote:
> >
> >> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
> >>  wrote:
> >> > On Thu, 20 Apr 2017 12:29:11 +0900
> >> > Michel Dänzer  wrote:
> >> >
> >> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> >> >> > Hello,
> >> >> >
> >> >> > Please find the latest version that include a small fix for hash 
> >> >> > deletion. I
> >> >> > think the series is good now. Please review/ack it.
> >> >>
> >> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
> >> >> thread, Mesa's glthread cannot make any libX11 API calls.
> >> >>
> >> >>
> >> >
> >> > Hello Michel,
> >> >
> >> > Just to be sure we are on the same line, let's me do a summary.
> >> >
> >> > PCSX2 does the following bad pattern
> >> > 1/ no call to XInitThread
> >> > 2/ XGetGeometry to get the size of the surface
> >> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
> >> > how to call it)
> >> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I 
> >> > guess to get the
> >> >   associated buffer of the window.
> >> >
> >> >
> >> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
> >> > transfer. So glthread
> >> > was mostly synchronous.
> >> >
> >> > This series removes the (useless) PBO transfer synchronization. So now 
> >> > glthread is really
> >> > asynchronous and the above bad pattern crash as expected.
> >> >
> >> > I didn't add any libX11 API call on the patches. And I don't think we 
> >> > can remvove the DRI stuff.
> >> >
> >> > Hum, I don't know what will be the impact on the perf but potentially we 
> >> > can force a synchronization
> >> > when there is a draw to framebuffer 0.
> >>
> >> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> >> by glDrawArrays?
> >>
> >> Marek
> >
> > Hello Marek, Michel,
> >
> > Here the full backtrace.
> >
> > #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> > outCount=0xdcc15c74) at dri2.c:464
> > #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> > out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
> > #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
> > atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
> > #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
> > statts=0xdef252f8, statts_count=2) at dri2.c:480
> > #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
> > stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
> > dri_drawable.c:83
> > #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
> > st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
> > Note "print stfb->Base->Name" give me 0
> > #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
> > state_tracker/st_manager.c:869
> > #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
> > pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
> > #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, 
> > nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911, 
> > max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at 
> > state_tracker/st_draw.c:191
> > #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, count=4, 
> > start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
> > #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
> > vbo/vbo_exec_array.c:575
> > #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, 
> > cmd=0xc41574b0) at main/marshal_generated.c:26644
> > #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
> > main/marshal_generated.c:42457
> > #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
> > batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
> > #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110
> >
> >
> >> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
> >> glthread, that's a bug which needs to be fixed, either by moving the
> >> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> >> changing DRI2GetBuffersWithFormat to use XCB directly.
> 
> First, we need to understand why it's happening. Does the app use the
> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
> glReadBuffer(GL_FRONT)?

Hello Marek,

No, I don't use the front buffer. I don't call glDrawBuffer for the FB 0. So I 
think,
it should use GL_BACK.
Hum except if some 3rparty libs do something in my back.

In case it have any impact, I'm using either glXSwapIntervalEXT or 
glXSwapIntervalMESA (if the 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Marek Olšák]

On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
 wrote:
> On Thu, 20 Apr 2017 11:57:08 +0200
> Marek Olšák  wrote:
>
>> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>>  wrote:
>> > On Thu, 20 Apr 2017 12:29:11 +0900
>> > Michel Dänzer  wrote:
>> >
>> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>> >> > Hello,
>> >> >
>> >> > Please find the latest version that include a small fix for hash 
>> >> > deletion. I
>> >> > think the series is good now. Please review/ack it.
>> >>
>> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> >> thread, Mesa's glthread cannot make any libX11 API calls.
>> >>
>> >>
>> >
>> > Hello Michel,
>> >
>> > Just to be sure we are on the same line, let's me do a summary.
>> >
>> > PCSX2 does the following bad pattern
>> > 1/ no call to XInitThread
>> > 2/ XGetGeometry to get the size of the surface
>> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
>> > how to call it)
>> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess 
>> > to get the
>> >   associated buffer of the window.
>> >
>> >
>> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
>> > transfer. So glthread
>> > was mostly synchronous.
>> >
>> > This series removes the (useless) PBO transfer synchronization. So now 
>> > glthread is really
>> > asynchronous and the above bad pattern crash as expected.
>> >
>> > I didn't add any libX11 API call on the patches. And I don't think we can 
>> > remvove the DRI stuff.
>> >
>> > Hum, I don't know what will be the impact on the perf but potentially we 
>> > can force a synchronization
>> > when there is a draw to framebuffer 0.
>>
>> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
>> by glDrawArrays?
>>
>> Marek
>
> Hello Marek, Michel,
>
> Here the full backtrace.
>
> #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
> width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> outCount=0xdcc15c74) at dri2.c:464
> #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
> width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
> #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
> atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
> #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
> statts=0xdef252f8, statts_count=2) at dri2.c:480
> #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
> stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
> dri_drawable.c:83
> #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
> st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
> Note "print stfb->Base->Name" give me 0
> #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
> state_tracker/st_manager.c:869
> #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
> pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
> #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, nr_prims=1, 
> ib=0x0, index_bounds_valid=1 '\001', min_index=39911, max_index=39914, 
> tfb_vertcount=0x0, stream=0, indirect=0x0) at state_tracker/st_draw.c:191
> #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, count=4, 
> start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
> #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
> vbo/vbo_exec_array.c:575
> #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, cmd=0xc41574b0) 
> at main/marshal_generated.c:26644
> #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
> main/marshal_generated.c:42457
> #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
> batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
> #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110
>
>
>> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
>> glthread, that's a bug which needs to be fixed, either by moving the
>> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
>> changing DRI2GetBuffersWithFormat to use XCB directly.

First, we need to understand why it's happening. Does the app use the
front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
glReadBuffer(GL_FRONT)?

>
> Ok. On one hand, moving DRI2GetBuffersWithFormat to main thread won't be 
> easy. I think we need to
> keep track of the current bound framebuffer on the app thread. So we can 
> force a sync
> on gl operation that will access the framebuffer 0.
>
> On the other hand, GLX/XLIB/XCB interactions are  quite foggy for me. It seem 
> there already some xcb codes
> in various place typically EGL and the xcb_dri2 extension. So maybe there is 
> a reason that code wasn't ported here.
> In particular, I saw this option in configure
>   

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[gregory hainaut]

On Thu, 20 Apr 2017 11:57:08 +0200
Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>  wrote:
> > On Thu, 20 Apr 2017 12:29:11 +0900
> > Michel Dänzer  wrote:
> >
> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> >> > Hello,
> >> >
> >> > Please find the latest version that include a small fix for hash 
> >> > deletion. I
> >> > think the series is good now. Please review/ack it.
> >>
> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
> >> thread, Mesa's glthread cannot make any libX11 API calls.
> >>
> >>
> >
> > Hello Michel,
> >
> > Just to be sure we are on the same line, let's me do a summary.
> >
> > PCSX2 does the following bad pattern
> > 1/ no call to XInitThread
> > 2/ XGetGeometry to get the size of the surface
> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how 
> > to call it)
> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess 
> > to get the
> >   associated buffer of the window.
> >
> >
> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
> > transfer. So glthread
> > was mostly synchronous.
> >
> > This series removes the (useless) PBO transfer synchronization. So now 
> > glthread is really
> > asynchronous and the above bad pattern crash as expected.
> >
> > I didn't add any libX11 API call on the patches. And I don't think we can 
> > remvove the DRI stuff.
> >
> > Hum, I don't know what will be the impact on the perf but potentially we 
> > can force a synchronization
> > when there is a draw to framebuffer 0.
> 
> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> by glDrawArrays?
> 
> Marek

Hello Marek, Michel,

Here the full backtrace.

#0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
outCount=0xdcc15c74) at dri2.c:464
#1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
#2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
#3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
statts=0xdef252f8, statts_count=2) at dri2.c:480
#4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
dri_drawable.c:83
#5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
Note "print stfb->Base->Name" give me 0
#6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
state_tracker/st_manager.c:869
#7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
#8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, nr_prims=1, 
ib=0x0, index_bounds_valid=1 '\001', min_index=39911, max_index=39914, 
tfb_vertcount=0x0, stream=0, indirect=0x0) at state_tracker/st_draw.c:191
#9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, count=4, 
start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
#10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
vbo/vbo_exec_array.c:575
#11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, cmd=0xc41574b0) 
at main/marshal_generated.c:26644
#12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
main/marshal_generated.c:42457
#13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
#14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110


> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
> glthread, that's a bug which needs to be fixed, either by moving the
> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> changing DRI2GetBuffersWithFormat to use XCB directly.

Ok. On one hand, moving DRI2GetBuffersWithFormat to main thread won't be easy. 
I think we need to 
keep track of the current bound framebuffer on the app thread. So we can force 
a sync
on gl operation that will access the framebuffer 0.

On the other hand, GLX/XLIB/XCB interactions are  quite foggy for me. It seem 
there already some xcb codes
in various place typically EGL and the xcb_dri2 extension. So maybe there is a 
reason that code wasn't ported here.
In particular, I saw this option in configure
  --enable-glx[=dri|xlib|gallium-xlib] enable the GLX library and choose an 
implementation
  [default=auto]


> It sounds like the glthread bug above exists independently from this
> patch series, which might just make it more likely to result in a crash.
> I think it would still be better to fix the bug before landing this series.
Yes it increases the probability to trigger the bug.


Cheers,
Gregory

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Marek Olšák]

On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
 wrote:
> On Thu, 20 Apr 2017 12:29:11 +0900
> Michel Dänzer  wrote:
>
>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>> > Hello,
>> >
>> > Please find the latest version that include a small fix for hash deletion. 
>> > I
>> > think the series is good now. Please review/ack it.
>>
>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> thread, Mesa's glthread cannot make any libX11 API calls.
>>
>>
>
> Hello Michel,
>
> Just to be sure we are on the same line, let's me do a summary.
>
> PCSX2 does the following bad pattern
> 1/ no call to XInitThread
> 2/ XGetGeometry to get the size of the surface
> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how 
> to call it)
>=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess to 
> get the
>   associated buffer of the window.
>
>
> So far it was (kind of) working fine because PCSX2 does tons of PBO transfer. 
> So glthread
> was mostly synchronous.
>
> This series removes the (useless) PBO transfer synchronization. So now 
> glthread is really
> asynchronous and the above bad pattern crash as expected.
>
> I didn't add any libX11 API call on the patches. And I don't think we can 
> remvove the DRI stuff.
>
> Hum, I don't know what will be the impact on the perf but potentially we can 
> force a synchronization
> when there is a draw to framebuffer 0.

Can you send us the backtrace when DRI2GetBuffersWithFormat is called
by glDrawArrays?

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

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[gregory hainaut]

On Thu, 20 Apr 2017 12:29:11 +0900
Michel Dänzer  wrote:

> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> > Hello,
> > 
> > Please find the latest version that include a small fix for hash deletion. I
> > think the series is good now. Please review/ack it.
> 
> I'm afraid I have to NACK it. As discussed in the v4 cover letter
> thread, Mesa's glthread cannot make any libX11 API calls.
> 
> 

Hello Michel,

Just to be sure we are on the same line, let's me do a summary.

PCSX2 does the following bad pattern
1/ no call to XInitThread
2/ XGetGeometry to get the size of the surface
3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how to 
call it)
   => which seem to call DRI2GetBuffersWithFormat under the hood. I guess to 
get the
  associated buffer of the window.


So far it was (kind of) working fine because PCSX2 does tons of PBO transfer. 
So glthread
was mostly synchronous.

This series removes the (useless) PBO transfer synchronization. So now glthread 
is really
asynchronous and the above bad pattern crash as expected. 

I didn't add any libX11 API call on the patches. And I don't think we can 
remvove the DRI stuff.

Hum, I don't know what will be the impact on the perf but potentially we can 
force a synchronization
when there is a draw to framebuffer 0.

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

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Michel Dänzer]

On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> Hello,
> 
> Please find the latest version that include a small fix for hash deletion. I
> think the series is good now. Please review/ack it.

I'm afraid I have to NACK it. As discussed in the v4 cover letter
thread, Mesa's glthread cannot make any libX11 API calls.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

[Gregory Hainaut]

Hello,

Please find the latest version that include a small fix for hash deletion. I
think the series is good now. Please review/ack it.

Allow to handle this kind of case:
   genBuffer();
   BindBuffer(pbo)
   DeleteBuffer(pbo);
   BindBuffer(rand_pbo)
   TexSubImage2D(user_memory_pointer); // Data transfer will be synchronous

There are various subtely to handle multi threaded shared context. In order to
keep the code sane, I've considered a buffer invalid when it is deleted by a
context even it is still bound to others contexts. It will force a synchronous
transfer which is always safe.

An example could be
   Ctx A: glGenBuffers(1, );
   Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo);
   Ctx B: glDeleteBuffers(1, );
   Ctx A: glTexSubImage2D(...); // will be synchronous, even though it
   _could_ be asynchronous (because the PBO that was generated first is
   still bound!)

V3: I mixed up the number so I jumped right away to v4...
V4: improve commments based on Nicolai feedback
V5: Properly delete element of the new hash (first patch)

Best regards,

Gregory Hainaut (3):
  mesa/glthread: track buffer creation/destruction
  mesa/glthread: add tracking of PBO binding
  mapi/glthread: generate asynchronous code for PBO transfer

 src/mapi/glapi/gen/ARB_direct_state_access.xml |  18 +--
 src/mapi/glapi/gen/ARB_robustness.xml  |   2 +-
 src/mapi/glapi/gen/gl_API.dtd  |  10 +-
 src/mapi/glapi/gen/gl_API.xml  |  32 +++---
 src/mapi/glapi/gen/gl_marshal.py   |  23 +++-
 src/mapi/glapi/gen/marshal_XML.py  |  21 +++-
 src/mesa/main/glthread.h   |  10 ++
 src/mesa/main/marshal.c| 149 -
 src/mesa/main/marshal.h|  24 
 src/mesa/main/mtypes.h |   5 +
 src/mesa/main/shared.c |  14 +++
 11 files changed, 269 insertions(+), 39 deletions(-)

-- 
2.1.4

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