Re: [Mesa-dev] [PATCH v2 0/3] asynchronous pbo transfer with glthread
Hello Gregory, do you have the v3 'ready'? v2 do NOT apply any longer since Samuel Pitoiset's changes for 'bind less'. I'm running with this on Turks XT since you've sent it. Greetings, Dieter Am 13.04.2017 17:49, schrieb gregory hainaut: On Wed, 5 Apr 2017 12:52:03 +0200 Gregory Hainaut wrote: >> 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 | 19 +++- >> 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 | 4 + >> 11 files changed, 257 insertions(+), 39 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/3] asynchronous pbo transfer with glthread
Hi Gregory, Sorry, this got dropped somehow. On 13.04.2017 17:49, gregory hainaut wrote: On Wed, 5 Apr 2017 12:52:03 +0200 Gregory Hainaut wrote: Still, I believe there is the following bug in the series: glGenBuffers(1, &pbo); glBindBuffer(..., pbo); glDeleteBuffers(1, &pbo); glTexSubImage2D(...); // will be asynchronous, but refers to user memory because the pbo was implicitly unbound I unbound the buffer when it is deleted in the track_buffers_destruction function. See code chunk below. So I think it must work. Or did I miss something ? + if (buffers[i] == glthread->pixel_pack_buffer_bound) + glthread->pixel_pack_buffer_bound = 0; + + if (buffers[i] == glthread->pixel_unpack_buffer_bound) + glthread->pixel_unpack_buffer_bound = 0; Hello Nicolai, Do we agree that above pattern will work with my patches ? Yes, I'm convinced now. Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name You can't have the same name because name is still bound in context A. Actually I'm wrong here. glDeleteBuffers makes the name available again. Which lead to interesting question if the buffer is bound again after the name is reused by a glGenBuffers call. Anyway it isn't important. Also agreed. There's still the comment on patch 2&3. Cheers, Nicolai Chapter 5.1.3 of 4.5 core spec "Since the name is marked unused, binding the name will create a new object with the same name, and attaching the name will generate an error." Cheers, Gregory So maybe you mean something instead for your example ? Ctx A: glGenBuffers(1, &pbo); Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); Ctx B: glDeleteBuffers(1, &pbo); ... Ctx A: glTexSubImage2D(...); // will be synchronous, even though it _could_ be asynchronous (because the PBO that was generated first is still bound!) And yes I confirm, it isn't optimal. I did it on purpose because it is much safer/easier and as you said it is a crazy case. Cheers, Gregory On 4/5/17, Nicolai Hähnle wrote: On 01.04.2017 11:42, Gregory Hainaut wrote: Hello, Please find a new version to handle invalid buffer handles. Allow to handle this kind of case: genBuffer(&pbo); DeleteBuffer(pbo); BindBuffer(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. Thanks! I think you're right that tracking pointers is not actually needed as long as Create/Delete are synchronous. It's less accurate, but the inaccuracy doesn't necessarily lead to correctness problems. Still, I believe there is the following bug in the series: glGenBuffers(1, &pbo); glBindBuffer(..., pbo); glDeleteBuffers(1, &pbo); glTexSubImage2D(...); // will be asynchronous, but refers to user memory because the pbo was implicitly unbound Once you fix this bug by updating the current context's bindings in glDeleteBuffers, you'll have the following multi-context situation, which is sub-optimal but not a bug -- and applications which do that are crazy anyway, so let's not worry about them. I'm just mentioning it for completeness: Ctx A: glGenBuffers(1, &pbo); Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); Ctx B: glDeleteBuffers(1, &pbo); Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name Ctx A: glDeleteBuffers(1, &pbo); Ctx A: glTexSubImage2D(...); // will be synchronous, even though it _could_ be asynchronous (because the PBO that was generated first is still bound!) Cheers, Nicolai 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 | 19 +++- 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 | 4 + 11 files changed, 257 insertions(+), 39 deletions(-) -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. -- 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 0/3] asynchronous pbo transfer with glthread
On Wed, 5 Apr 2017 12:52:03 +0200 Gregory Hainaut wrote: > > Still, I believe there is the following bug in the series: > > > glGenBuffers(1, &pbo); > > glBindBuffer(..., pbo); > > glDeleteBuffers(1, &pbo); > > glTexSubImage2D(...); // will be asynchronous, but refers to user memory > > because the pbo was implicitly unbound > > I unbound the buffer when it is deleted in the > track_buffers_destruction function. See code chunk below. So I think > it must work. Or did I miss something ? > > > + if (buffers[i] == glthread->pixel_pack_buffer_bound) > > + glthread->pixel_pack_buffer_bound = 0; > > + > > + if (buffers[i] == glthread->pixel_unpack_buffer_bound) > > + glthread->pixel_unpack_buffer_bound = 0; > Hello Nicolai, Do we agree that above pattern will work with my patches ? > > > Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name > You can't have the same name because name is still bound in context A. Actually I'm wrong here. glDeleteBuffers makes the name available again. Which lead to interesting question if the buffer is bound again after the name is reused by a glGenBuffers call. Anyway it isn't important. Chapter 5.1.3 of 4.5 core spec "Since the name is marked unused, binding the name will create a new object with the same name, and attaching the name will generate an error." Cheers, Gregory > So maybe you mean something instead for your example ? > > > Ctx A: glGenBuffers(1, &pbo); > > Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); > > Ctx B: glDeleteBuffers(1, &pbo); > ... > > Ctx A: glTexSubImage2D(...); // will be synchronous, even though it _could_ > > be asynchronous (because the PBO that was generated first is still bound!) > > And yes I confirm, it isn't optimal. I did it on purpose because it is > much safer/easier and as you said it is a crazy case. > > > Cheers, > Gregory > > On 4/5/17, Nicolai Hähnle wrote: > > On 01.04.2017 11:42, Gregory Hainaut wrote: > >> Hello, > >> > >> Please find a new version to handle invalid buffer handles. > >> > >> Allow to handle this kind of case: > >>genBuffer(&pbo); > >>DeleteBuffer(pbo); > >>BindBuffer(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. > > > > Thanks! I think you're right that tracking pointers is not actually > > needed as long as Create/Delete are synchronous. It's less accurate, but > > the inaccuracy doesn't necessarily lead to correctness problems. > > > > Still, I believe there is the following bug in the series: > > > > glGenBuffers(1, &pbo); > > glBindBuffer(..., pbo); > > glDeleteBuffers(1, &pbo); > > glTexSubImage2D(...); // will be asynchronous, but refers to user memory > > because the pbo was implicitly unbound > > > > Once you fix this bug by updating the current context's bindings in > > glDeleteBuffers, you'll have the following multi-context situation, > > which is sub-optimal but not a bug -- and applications which do that are > > crazy anyway, so let's not worry about them. I'm just mentioning it for > > completeness: > > > > Ctx A: glGenBuffers(1, &pbo); > > Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); > > Ctx B: glDeleteBuffers(1, &pbo); > > Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name > > Ctx A: glDeleteBuffers(1, &pbo); > > Ctx A: glTexSubImage2D(...); // will be synchronous, even though it > > _could_ be asynchronous (because the PBO that was generated first is > > still bound!) > > > > Cheers, > > Nicolai > > > > > >> > >> 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 | 19 +++- > >> 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 | 4 + > >> 11 files changed, 257 insertions(+), 39 deletions(-) > >> > > > > > > -- > > Lerne, wie die Welt wirklich ist, > > Aber vergiss niemals, wie sie sein sollte. > > ___ mesa-dev mailing li
Re: [Mesa-dev] [PATCH v2 0/3] asynchronous pbo transfer with glthread
> Still, I believe there is the following bug in the series: > glGenBuffers(1, &pbo); > glBindBuffer(..., pbo); > glDeleteBuffers(1, &pbo); > glTexSubImage2D(...); // will be asynchronous, but refers to user memory > because the pbo was implicitly unbound I unbound the buffer when it is deleted in the track_buffers_destruction function. See code chunk below. So I think it must work. Or did I miss something ? > + if (buffers[i] == glthread->pixel_pack_buffer_bound) > + glthread->pixel_pack_buffer_bound = 0; > + > + if (buffers[i] == glthread->pixel_unpack_buffer_bound) > + glthread->pixel_unpack_buffer_bound = 0; > Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name You can't have the same name because name is still bound in context A. So maybe you mean something instead for your example ? > Ctx A: glGenBuffers(1, &pbo); > Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); > Ctx B: glDeleteBuffers(1, &pbo); ... > Ctx A: glTexSubImage2D(...); // will be synchronous, even though it _could_ > be asynchronous (because the PBO that was generated first is still bound!) And yes I confirm, it isn't optimal. I did it on purpose because it is much safer/easier and as you said it is a crazy case. Cheers, Gregory On 4/5/17, Nicolai Hähnle wrote: > On 01.04.2017 11:42, Gregory Hainaut wrote: >> Hello, >> >> Please find a new version to handle invalid buffer handles. >> >> Allow to handle this kind of case: >>genBuffer(&pbo); >>DeleteBuffer(pbo); >>BindBuffer(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. > > Thanks! I think you're right that tracking pointers is not actually > needed as long as Create/Delete are synchronous. It's less accurate, but > the inaccuracy doesn't necessarily lead to correctness problems. > > Still, I believe there is the following bug in the series: > > glGenBuffers(1, &pbo); > glBindBuffer(..., pbo); > glDeleteBuffers(1, &pbo); > glTexSubImage2D(...); // will be asynchronous, but refers to user memory > because the pbo was implicitly unbound > > Once you fix this bug by updating the current context's bindings in > glDeleteBuffers, you'll have the following multi-context situation, > which is sub-optimal but not a bug -- and applications which do that are > crazy anyway, so let's not worry about them. I'm just mentioning it for > completeness: > > Ctx A: glGenBuffers(1, &pbo); > Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); > Ctx B: glDeleteBuffers(1, &pbo); > Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name > Ctx A: glDeleteBuffers(1, &pbo); > Ctx A: glTexSubImage2D(...); // will be synchronous, even though it > _could_ be asynchronous (because the PBO that was generated first is > still bound!) > > Cheers, > Nicolai > > >> >> 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 | 19 +++- >> 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 | 4 + >> 11 files changed, 257 insertions(+), 39 deletions(-) >> > > > -- > 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 0/3] asynchronous pbo transfer with glthread
On 01.04.2017 11:42, Gregory Hainaut wrote: Hello, Please find a new version to handle invalid buffer handles. Allow to handle this kind of case: genBuffer(&pbo); DeleteBuffer(pbo); BindBuffer(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. Thanks! I think you're right that tracking pointers is not actually needed as long as Create/Delete are synchronous. It's less accurate, but the inaccuracy doesn't necessarily lead to correctness problems. Still, I believe there is the following bug in the series: glGenBuffers(1, &pbo); glBindBuffer(..., pbo); glDeleteBuffers(1, &pbo); glTexSubImage2D(...); // will be asynchronous, but refers to user memory because the pbo was implicitly unbound Once you fix this bug by updating the current context's bindings in glDeleteBuffers, you'll have the following multi-context situation, which is sub-optimal but not a bug -- and applications which do that are crazy anyway, so let's not worry about them. I'm just mentioning it for completeness: Ctx A: glGenBuffers(1, &pbo); Ctx A: glBindBuffer(PIXEL_UNPACK_BUFFER, pbo); Ctx B: glDeleteBuffers(1, &pbo); Ctx A: glGenBuffers(1, &pbo); // assume that this gets the same buffer name Ctx A: glDeleteBuffers(1, &pbo); Ctx A: glTexSubImage2D(...); // will be synchronous, even though it _could_ be asynchronous (because the PBO that was generated first is still bound!) Cheers, Nicolai 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 | 19 +++- 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 | 4 + 11 files changed, 257 insertions(+), 39 deletions(-) -- 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
[Mesa-dev] [PATCH v2 0/3] asynchronous pbo transfer with glthread
Hello, Please find a new version to handle invalid buffer handles. Allow to handle this kind of case: genBuffer(&pbo); DeleteBuffer(pbo); BindBuffer(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. 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 | 19 +++- 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 | 4 + 11 files changed, 257 insertions(+), 39 deletions(-) -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev