[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 <maraeo at gmail.com> wrote:

> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
> <gregory.hainaut at gmail.com> wrote:
> > On Thu, 20 Apr 2017 11:57:08 +0200
> > Marek Olšák <maraeo at gmail.com> wrote:
> >
> >> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
> >> <gregory.hainaut at gmail.com> wrote:
> >> > On Thu, 20 Apr 2017 12:29:11 +0900
> >> > Michel Dänzer <michel at daenzer.net> 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=<synthetic 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 at entry=0xdef24f58, st=st at 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 Vsync. 
 
If I understand you, you expect that DRI2GetBuffersWithFormat should haven't be called in the first place ?

> >
> > 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]
> 
> glx means libGL. dri means libGL loads _dri.so files. xlib means that
> libGL itself contains swrast and doesn't load any drivers.
> gallium-xlib means that libGL itself contains softpipe/llvmpipe and
> doesn't load any drivers either.

Thanks for the info.

> Marek

Cheers,
Gregory