Re: [Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
Fixed in my branch: http://cgit.freedesktop.org/~mareko/mesa/log/?h=buffer-storage Marek On Sat, Feb 8, 2014 at 6:32 PM, Fredrik Höglund fred...@kde.org wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com --- src/mesa/main/api_validate.c | 2 +- src/mesa/main/bufferobj.c | 12 src/mesa/main/bufferobj.h | 8 src/mesa/main/drawpix.c | 4 ++-- src/mesa/main/pbo.c | 4 ++-- src/mesa/main/readpix.c | 2 +- src/mesa/main/texgetimage.c | 4 ++-- src/mesa/vbo/vbo_exec_array.c | 4 ++-- 8 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index 6945584..50fd757 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -865,7 +865,7 @@ valid_draw_indirect(struct gl_context *ctx, return GL_FALSE; } - if (_mesa_bufferobj_mapped(ctx-DrawIndirectBuffer)) { + if (_mesa_check_disallowed_mapping(ctx-DrawIndirectBuffer)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s(DRAW_INDIRECT_BUFFER is mapped), name); return GL_FALSE; diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index ca0b5dd..2a642fe 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -269,6 +269,9 @@ buffer_object_subdata_range_good(struct gl_context * ctx, GLenum target, return NULL; } + if (bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + return bufObj; + if (mappedRange) { if (bufferobj_range_mapped(bufObj, offset, size)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s, caller); @@ -1452,7 +1455,7 @@ _mesa_ClearBufferData(GLenum target, GLenum internalformat, GLenum format, return; } - if (_mesa_bufferobj_mapped(bufObj)) { + if (_mesa_check_disallowed_mapping(bufObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, glClearBufferData(buffer currently mapped)); return; @@ -1861,13 +1864,13 @@ _mesa_CopyBufferSubData(GLenum readTarget, GLenum writeTarget, if (!dst) return; - if (_mesa_bufferobj_mapped(src)) { + if (_mesa_check_disallowed_mapping(src)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(readBuffer is mapped)); return; } - if (_mesa_bufferobj_mapped(dst)) { + if (_mesa_check_disallowed_mapping(dst)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(writeBuffer is mapped)); return; @@ -2793,7 +2796,8 @@ _mesa_InvalidateBufferSubData(GLuint buffer, GLintptr offset, * mapped by MapBuffer, or if the invalidate range intersects the range * currently mapped by MapBufferRange. */ - if (bufferobj_range_mapped(bufObj, offset, length)) { + if (!(bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + bufferobj_range_mapped(bufObj, offset, length)) { _mesa_error(ctx, GL_INVALID_OPERATION, glInvalidateBufferSubData(intersection with mapped range)); The mesa_bufferobj_mapped() call in _mesa_InvalidateBufferData() should also be changed to _mesa_check_disallowed_mapping(). I would also replace the ARB_invalidate_subdata quotations with quotations from the GL 4.4 specification. diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h index 174fd60..43795a9 100644 --- a/src/mesa/main/bufferobj.h +++ b/src/mesa/main/bufferobj.h @@ -44,6 +44,14 @@ _mesa_bufferobj_mapped(const struct gl_buffer_object *obj) return obj-Pointer != NULL; } +/** Cannot we use this buffer while mapped? */ Can we not +static inline GLboolean +_mesa_check_disallowed_mapping(const struct gl_buffer_object *obj) +{ + return _mesa_bufferobj_mapped(obj) + !(obj-AccessFlags GL_MAP_PERSISTENT_BIT); +} + /** * Is the given buffer object a user-created buffer object? * Mesa uses default buffer objects in several places. Default buffers diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c index 096615c..63e5870 100644 --- a/src/mesa/main/drawpix.c +++ b/src/mesa/main/drawpix.c @@ -151,7 +151,7 @@ _mesa_DrawPixels( GLsizei width, GLsizei height, glDrawPixels(invalid PBO access)); goto end; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped - that's an error */ _mesa_error(ctx, GL_INVALID_OPERATION, glDrawPixels(PBO is mapped)); @@ -335,7 +335,7 @@ _mesa_Bitmap( GLsizei width, GLsizei height, glBitmap(invalid PBO access)); return; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if
Re: [Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
On Saturday 15 February 2014, Marek Olšák wrote: Fixed in my branch: http://cgit.freedesktop.org/~mareko/mesa/log/?h=buffer-storage Marek LGTM. Reviewed-by: Fredrik Höglund fred...@kde.org ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com --- src/mesa/main/api_validate.c | 2 +- src/mesa/main/bufferobj.c | 12 src/mesa/main/bufferobj.h | 8 src/mesa/main/drawpix.c | 4 ++-- src/mesa/main/pbo.c | 4 ++-- src/mesa/main/readpix.c | 2 +- src/mesa/main/texgetimage.c | 4 ++-- src/mesa/vbo/vbo_exec_array.c | 4 ++-- 8 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index 6945584..50fd757 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -865,7 +865,7 @@ valid_draw_indirect(struct gl_context *ctx, return GL_FALSE; } - if (_mesa_bufferobj_mapped(ctx-DrawIndirectBuffer)) { + if (_mesa_check_disallowed_mapping(ctx-DrawIndirectBuffer)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s(DRAW_INDIRECT_BUFFER is mapped), name); return GL_FALSE; diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index ca0b5dd..2a642fe 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -269,6 +269,9 @@ buffer_object_subdata_range_good(struct gl_context * ctx, GLenum target, return NULL; } + if (bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + return bufObj; + if (mappedRange) { if (bufferobj_range_mapped(bufObj, offset, size)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s, caller); @@ -1452,7 +1455,7 @@ _mesa_ClearBufferData(GLenum target, GLenum internalformat, GLenum format, return; } - if (_mesa_bufferobj_mapped(bufObj)) { + if (_mesa_check_disallowed_mapping(bufObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, glClearBufferData(buffer currently mapped)); return; @@ -1861,13 +1864,13 @@ _mesa_CopyBufferSubData(GLenum readTarget, GLenum writeTarget, if (!dst) return; - if (_mesa_bufferobj_mapped(src)) { + if (_mesa_check_disallowed_mapping(src)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(readBuffer is mapped)); return; } - if (_mesa_bufferobj_mapped(dst)) { + if (_mesa_check_disallowed_mapping(dst)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(writeBuffer is mapped)); return; @@ -2793,7 +2796,8 @@ _mesa_InvalidateBufferSubData(GLuint buffer, GLintptr offset, * mapped by MapBuffer, or if the invalidate range intersects the range * currently mapped by MapBufferRange. */ - if (bufferobj_range_mapped(bufObj, offset, length)) { + if (!(bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + bufferobj_range_mapped(bufObj, offset, length)) { _mesa_error(ctx, GL_INVALID_OPERATION, glInvalidateBufferSubData(intersection with mapped range)); The mesa_bufferobj_mapped() call in _mesa_InvalidateBufferData() should also be changed to _mesa_check_disallowed_mapping(). I would also replace the ARB_invalidate_subdata quotations with quotations from the GL 4.4 specification. diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h index 174fd60..43795a9 100644 --- a/src/mesa/main/bufferobj.h +++ b/src/mesa/main/bufferobj.h @@ -44,6 +44,14 @@ _mesa_bufferobj_mapped(const struct gl_buffer_object *obj) return obj-Pointer != NULL; } +/** Cannot we use this buffer while mapped? */ Can we not +static inline GLboolean +_mesa_check_disallowed_mapping(const struct gl_buffer_object *obj) +{ + return _mesa_bufferobj_mapped(obj) + !(obj-AccessFlags GL_MAP_PERSISTENT_BIT); +} + /** * Is the given buffer object a user-created buffer object? * Mesa uses default buffer objects in several places. Default buffers diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c index 096615c..63e5870 100644 --- a/src/mesa/main/drawpix.c +++ b/src/mesa/main/drawpix.c @@ -151,7 +151,7 @@ _mesa_DrawPixels( GLsizei width, GLsizei height, glDrawPixels(invalid PBO access)); goto end; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped - that's an error */ _mesa_error(ctx, GL_INVALID_OPERATION, glDrawPixels(PBO is mapped)); @@ -335,7 +335,7 @@ _mesa_Bitmap( GLsizei width, GLsizei height, glBitmap(invalid PBO access)); return; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped - that's an error */ _mesa_error(ctx,
Re: [Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
On Thursday 30 January 2014, Chris Forbes wrote: Marek, I think there's an interaction with software primitive restart here. The primitive restart path maps the index buffer (and the indirect buffer, for indirect draws), and relies on these checks to guarantee that's possible. That may not be an issue for your driver, though -- I don't know. This is also an issue with display list compilation and PBO fallbacks, which is relevant for the gallium state tracker. I'm not sure how to best deal with it. Maybe we need another driver hook that can create a private mapping of an already mapped buffer object. Fredrik ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
You are right. We need to support at least 2 buffer mappings: one for the user and the other one for internal purposes. Marek On Thu, Jan 30, 2014 at 11:33 PM, Fredrik Höglund fred...@kde.org wrote: On Thursday 30 January 2014, Chris Forbes wrote: Marek, I think there's an interaction with software primitive restart here. The primitive restart path maps the index buffer (and the indirect buffer, for indirect draws), and relies on these checks to guarantee that's possible. That may not be an issue for your driver, though -- I don't know. This is also an issue with display list compilation and PBO fallbacks, which is relevant for the gallium state tracker. I'm not sure how to best deal with it. Maybe we need another driver hook that can create a private mapping of an already mapped buffer object. Fredrik ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
From: Marek Olšák marek.ol...@amd.com --- src/mesa/main/api_validate.c | 2 +- src/mesa/main/bufferobj.c | 12 src/mesa/main/bufferobj.h | 8 src/mesa/main/drawpix.c | 4 ++-- src/mesa/main/pbo.c | 4 ++-- src/mesa/main/readpix.c | 2 +- src/mesa/main/texgetimage.c | 4 ++-- src/mesa/vbo/vbo_exec_array.c | 4 ++-- 8 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index 6945584..50fd757 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -865,7 +865,7 @@ valid_draw_indirect(struct gl_context *ctx, return GL_FALSE; } - if (_mesa_bufferobj_mapped(ctx-DrawIndirectBuffer)) { + if (_mesa_check_disallowed_mapping(ctx-DrawIndirectBuffer)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s(DRAW_INDIRECT_BUFFER is mapped), name); return GL_FALSE; diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index ca0b5dd..2a642fe 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -269,6 +269,9 @@ buffer_object_subdata_range_good(struct gl_context * ctx, GLenum target, return NULL; } + if (bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + return bufObj; + if (mappedRange) { if (bufferobj_range_mapped(bufObj, offset, size)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s, caller); @@ -1452,7 +1455,7 @@ _mesa_ClearBufferData(GLenum target, GLenum internalformat, GLenum format, return; } - if (_mesa_bufferobj_mapped(bufObj)) { + if (_mesa_check_disallowed_mapping(bufObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, glClearBufferData(buffer currently mapped)); return; @@ -1861,13 +1864,13 @@ _mesa_CopyBufferSubData(GLenum readTarget, GLenum writeTarget, if (!dst) return; - if (_mesa_bufferobj_mapped(src)) { + if (_mesa_check_disallowed_mapping(src)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(readBuffer is mapped)); return; } - if (_mesa_bufferobj_mapped(dst)) { + if (_mesa_check_disallowed_mapping(dst)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(writeBuffer is mapped)); return; @@ -2793,7 +2796,8 @@ _mesa_InvalidateBufferSubData(GLuint buffer, GLintptr offset, * mapped by MapBuffer, or if the invalidate range intersects the range * currently mapped by MapBufferRange. */ - if (bufferobj_range_mapped(bufObj, offset, length)) { + if (!(bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + bufferobj_range_mapped(bufObj, offset, length)) { _mesa_error(ctx, GL_INVALID_OPERATION, glInvalidateBufferSubData(intersection with mapped range)); diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h index 174fd60..43795a9 100644 --- a/src/mesa/main/bufferobj.h +++ b/src/mesa/main/bufferobj.h @@ -44,6 +44,14 @@ _mesa_bufferobj_mapped(const struct gl_buffer_object *obj) return obj-Pointer != NULL; } +/** Cannot we use this buffer while mapped? */ +static inline GLboolean +_mesa_check_disallowed_mapping(const struct gl_buffer_object *obj) +{ + return _mesa_bufferobj_mapped(obj) + !(obj-AccessFlags GL_MAP_PERSISTENT_BIT); +} + /** * Is the given buffer object a user-created buffer object? * Mesa uses default buffer objects in several places. Default buffers diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c index 096615c..63e5870 100644 --- a/src/mesa/main/drawpix.c +++ b/src/mesa/main/drawpix.c @@ -151,7 +151,7 @@ _mesa_DrawPixels( GLsizei width, GLsizei height, glDrawPixels(invalid PBO access)); goto end; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped - that's an error */ _mesa_error(ctx, GL_INVALID_OPERATION, glDrawPixels(PBO is mapped)); @@ -335,7 +335,7 @@ _mesa_Bitmap( GLsizei width, GLsizei height, glBitmap(invalid PBO access)); return; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped - that's an error */ _mesa_error(ctx, GL_INVALID_OPERATION, glBitmap(PBO is mapped)); diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c index 400cec3..4a39404 100644 --- a/src/mesa/main/pbo.c +++ b/src/mesa/main/pbo.c @@ -201,7 +201,7 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx, return ptr; } - if (_mesa_bufferobj_mapped(unpack-BufferObj)) { + if (_mesa_check_disallowed_mapping(unpack-BufferObj)) { /*
Re: [Mesa-dev] [PATCH 06/13] mesa: allow buffers mapped with the persistent flag to be used by the GPU
Marek, I think there's an interaction with software primitive restart here. The primitive restart path maps the index buffer (and the indirect buffer, for indirect draws), and relies on these checks to guarantee that's possible. That may not be an issue for your driver, though -- I don't know. -- Chris On Thu, Jan 30, 2014 at 2:20 PM, Marek Olšák mar...@gmail.com wrote: From: Marek Olšák marek.ol...@amd.com --- src/mesa/main/api_validate.c | 2 +- src/mesa/main/bufferobj.c | 12 src/mesa/main/bufferobj.h | 8 src/mesa/main/drawpix.c | 4 ++-- src/mesa/main/pbo.c | 4 ++-- src/mesa/main/readpix.c | 2 +- src/mesa/main/texgetimage.c | 4 ++-- src/mesa/vbo/vbo_exec_array.c | 4 ++-- 8 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index 6945584..50fd757 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -865,7 +865,7 @@ valid_draw_indirect(struct gl_context *ctx, return GL_FALSE; } - if (_mesa_bufferobj_mapped(ctx-DrawIndirectBuffer)) { + if (_mesa_check_disallowed_mapping(ctx-DrawIndirectBuffer)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s(DRAW_INDIRECT_BUFFER is mapped), name); return GL_FALSE; diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index ca0b5dd..2a642fe 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -269,6 +269,9 @@ buffer_object_subdata_range_good(struct gl_context * ctx, GLenum target, return NULL; } + if (bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + return bufObj; + if (mappedRange) { if (bufferobj_range_mapped(bufObj, offset, size)) { _mesa_error(ctx, GL_INVALID_OPERATION, %s, caller); @@ -1452,7 +1455,7 @@ _mesa_ClearBufferData(GLenum target, GLenum internalformat, GLenum format, return; } - if (_mesa_bufferobj_mapped(bufObj)) { + if (_mesa_check_disallowed_mapping(bufObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, glClearBufferData(buffer currently mapped)); return; @@ -1861,13 +1864,13 @@ _mesa_CopyBufferSubData(GLenum readTarget, GLenum writeTarget, if (!dst) return; - if (_mesa_bufferobj_mapped(src)) { + if (_mesa_check_disallowed_mapping(src)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(readBuffer is mapped)); return; } - if (_mesa_bufferobj_mapped(dst)) { + if (_mesa_check_disallowed_mapping(dst)) { _mesa_error(ctx, GL_INVALID_OPERATION, glCopyBufferSubData(writeBuffer is mapped)); return; @@ -2793,7 +2796,8 @@ _mesa_InvalidateBufferSubData(GLuint buffer, GLintptr offset, * mapped by MapBuffer, or if the invalidate range intersects the range * currently mapped by MapBufferRange. */ - if (bufferobj_range_mapped(bufObj, offset, length)) { + if (!(bufObj-AccessFlags GL_MAP_PERSISTENT_BIT) + bufferobj_range_mapped(bufObj, offset, length)) { _mesa_error(ctx, GL_INVALID_OPERATION, glInvalidateBufferSubData(intersection with mapped range)); diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h index 174fd60..43795a9 100644 --- a/src/mesa/main/bufferobj.h +++ b/src/mesa/main/bufferobj.h @@ -44,6 +44,14 @@ _mesa_bufferobj_mapped(const struct gl_buffer_object *obj) return obj-Pointer != NULL; } +/** Cannot we use this buffer while mapped? */ +static inline GLboolean +_mesa_check_disallowed_mapping(const struct gl_buffer_object *obj) +{ + return _mesa_bufferobj_mapped(obj) + !(obj-AccessFlags GL_MAP_PERSISTENT_BIT); +} + /** * Is the given buffer object a user-created buffer object? * Mesa uses default buffer objects in several places. Default buffers diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c index 096615c..63e5870 100644 --- a/src/mesa/main/drawpix.c +++ b/src/mesa/main/drawpix.c @@ -151,7 +151,7 @@ _mesa_DrawPixels( GLsizei width, GLsizei height, glDrawPixels(invalid PBO access)); goto end; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped - that's an error */ _mesa_error(ctx, GL_INVALID_OPERATION, glDrawPixels(PBO is mapped)); @@ -335,7 +335,7 @@ _mesa_Bitmap( GLsizei width, GLsizei height, glBitmap(invalid PBO access)); return; } -if (_mesa_bufferobj_mapped(ctx-Unpack.BufferObj)) { +if (_mesa_check_disallowed_mapping(ctx-Unpack.BufferObj)) { /* buffer is mapped -