[Mesa-dev] [PATCH] mesa: stop abstracting buffer object hashtable locking
Samuel Pitoiset
samuel.pitoiset at gmail.com
Thu Apr 6 07:38:58 UTC 2017
Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
On 04/06/2017 03:00 AM, Timothy Arceri wrote:
> This doesn't do anything useful so just remove it.
> ---
> src/mesa/main/bufferobj.c | 33 ++++++++++-----------------------
> src/mesa/main/bufferobj.h | 6 ------
> src/mesa/main/varray.c | 4 ++--
> 3 files changed, 12 insertions(+), 31 deletions(-)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 139efd1..58e0a50 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -1101,49 +1101,36 @@ _mesa_lookup_bufferobj_err(struct gl_context *ctx, GLuint buffer,
> if (!bufObj || bufObj == &DummyBufferObject) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "%s(non-existent buffer object %u)", caller, buffer);
> return NULL;
> }
>
> return bufObj;
> }
>
>
> -void
> -_mesa_begin_bufferobj_lookups(struct gl_context *ctx)
> -{
> - _mesa_HashLockMutex(ctx->Shared->BufferObjects);
> -}
> -
> -
> -void
> -_mesa_end_bufferobj_lookups(struct gl_context *ctx)
> -{
> - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> -}
> -
> -
> /**
> * Look up a buffer object for a multi-bind function.
> *
> * Unlike _mesa_lookup_bufferobj(), this function also takes care
> * of generating an error if the buffer ID is not zero or the name
> * of an existing buffer object.
> *
> * If the buffer ID refers to an existing buffer object, a pointer
> * to the buffer object is returned. If the ID is zero, a pointer
> * to the shared NullBufferObj is returned. If the ID is not zero
> * and does not refer to a valid buffer object, this function
> * returns NULL.
> *
> * This function assumes that the caller has already locked the
> - * hash table mutex by calling _mesa_begin_bufferobj_lookups().
> + * hash table mutex by calling
> + * _mesa_HashLockMutex(ctx->Shared->BufferObjects).
> */
> struct gl_buffer_object *
> _mesa_multi_bind_lookup_bufferobj(struct gl_context *ctx,
> const GLuint *buffers,
> GLuint index, const char *caller)
> {
> struct gl_buffer_object *bufObj;
>
> if (buffers[index] != 0) {
> bufObj = _mesa_lookup_bufferobj_locked(ctx, buffers[index]);
> @@ -3238,21 +3225,21 @@ bind_uniform_buffers(struct gl_context *ctx, GLuint first, GLsizei count,
> * and then a second pass to actually perform the bindings.
> * Should we have different error semantics?
> *
> * RESOLVED: Yes. In this specification, when the parameters for
> * one of the <count> binding points are invalid, that binding point
> * is not updated and an error will be generated. However, other
> * binding points in the same command will be updated if their
> * parameters are valid and no other error occurs."
> */
>
> - _mesa_begin_bufferobj_lookups(ctx);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> for (i = 0; i < count; i++) {
> struct gl_uniform_buffer_binding *binding =
> &ctx->UniformBufferBindings[first + i];
> struct gl_buffer_object *bufObj;
> GLintptr offset = 0;
> GLsizeiptr size = 0;
>
> if (range) {
> if (!bind_buffers_check_offset_and_size(ctx, i, offsets, sizes))
> @@ -3298,21 +3285,21 @@ bind_uniform_buffers(struct gl_context *ctx, GLuint first, GLsizei count,
> bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, caller);
>
> if (bufObj) {
> if (bufObj == ctx->Shared->NullBufferObj)
> set_ubo_binding(ctx, binding, bufObj, -1, -1, !range);
> else
> set_ubo_binding(ctx, binding, bufObj, offset, size, !range);
> }
> }
>
> - _mesa_end_bufferobj_lookups(ctx);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
> static void
> bind_shader_storage_buffers(struct gl_context *ctx, GLuint first,
> GLsizei count, const GLuint *buffers,
> bool range,
> const GLintptr *offsets,
> const GLsizeiptr *sizes,
> const char *caller)
> {
> @@ -3350,21 +3337,21 @@ bind_shader_storage_buffers(struct gl_context *ctx, GLuint first,
> * and then a second pass to actually perform the bindings.
> * Should we have different error semantics?
> *
> * RESOLVED: Yes. In this specification, when the parameters for
> * one of the <count> binding points are invalid, that binding point
> * is not updated and an error will be generated. However, other
> * binding points in the same command will be updated if their
> * parameters are valid and no other error occurs."
> */
>
> - _mesa_begin_bufferobj_lookups(ctx);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> for (i = 0; i < count; i++) {
> struct gl_shader_storage_buffer_binding *binding =
> &ctx->ShaderStorageBufferBindings[first + i];
> struct gl_buffer_object *bufObj;
> GLintptr offset = 0;
> GLsizeiptr size = 0;
>
> if (range) {
> if (!bind_buffers_check_offset_and_size(ctx, i, offsets, sizes))
> @@ -3410,21 +3397,21 @@ bind_shader_storage_buffers(struct gl_context *ctx, GLuint first,
> bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, caller);
>
> if (bufObj) {
> if (bufObj == ctx->Shared->NullBufferObj)
> set_ssbo_binding(ctx, binding, bufObj, -1, -1, !range);
> else
> set_ssbo_binding(ctx, binding, bufObj, offset, size, !range);
> }
> }
>
> - _mesa_end_bufferobj_lookups(ctx);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
> static bool
> error_check_bind_xfb_buffers(struct gl_context *ctx,
> struct gl_transform_feedback_object *tfObj,
> GLuint first, GLsizei count, const char *caller)
> {
> if (!ctx->Extensions.EXT_transform_feedback) {
> _mesa_error(ctx, GL_INVALID_ENUM,
> "%s(target=GL_TRANSFORM_FEEDBACK_BUFFER)", caller);
> @@ -3529,21 +3516,21 @@ bind_xfb_buffers(struct gl_context *ctx,
> * and then a second pass to actually perform the bindings.
> * Should we have different error semantics?
> *
> * RESOLVED: Yes. In this specification, when the parameters for
> * one of the <count> binding points are invalid, that binding point
> * is not updated and an error will be generated. However, other
> * binding points in the same command will be updated if their
> * parameters are valid and no other error occurs."
> */
>
> - _mesa_begin_bufferobj_lookups(ctx);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> for (i = 0; i < count; i++) {
> const GLuint index = first + i;
> struct gl_buffer_object * const boundBufObj = tfObj->Buffers[index];
> struct gl_buffer_object *bufObj;
> GLintptr offset = 0;
> GLsizeiptr size = 0;
>
> if (range) {
> offset = offsets[i];
> @@ -3595,21 +3582,21 @@ bind_xfb_buffers(struct gl_context *ctx,
> if (boundBufObj && boundBufObj->Name == buffers[i])
> bufObj = boundBufObj;
> else
> bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, caller);
>
> if (bufObj)
> _mesa_set_transform_feedback_binding(ctx, tfObj, index, bufObj,
> offset, size);
> }
>
> - _mesa_end_bufferobj_lookups(ctx);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
> static bool
> error_check_bind_atomic_buffers(struct gl_context *ctx,
> GLuint first, GLsizei count,
> const char *caller)
> {
> if (!ctx->Extensions.ARB_shader_atomic_counters) {
> _mesa_error(ctx, GL_INVALID_ENUM,
> "%s(target=GL_ATOMIC_COUNTER_BUFFER)", caller);
> @@ -3692,21 +3679,21 @@ bind_atomic_buffers(struct gl_context *ctx,
> * and then a second pass to actually perform the bindings.
> * Should we have different error semantics?
> *
> * RESOLVED: Yes. In this specification, when the parameters for
> * one of the <count> binding points are invalid, that binding point
> * is not updated and an error will be generated. However, other
> * binding points in the same command will be updated if their
> * parameters are valid and no other error occurs."
> */
>
> - _mesa_begin_bufferobj_lookups(ctx);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> for (i = 0; i < count; i++) {
> struct gl_atomic_buffer_binding *binding =
> &ctx->AtomicBufferBindings[first + i];
> struct gl_buffer_object *bufObj;
> GLintptr offset = 0;
> GLsizeiptr size = 0;
>
> if (range) {
> if (!bind_buffers_check_offset_and_size(ctx, i, offsets, sizes))
> @@ -3745,21 +3732,21 @@ bind_atomic_buffers(struct gl_context *ctx,
>
> if (binding->BufferObject && binding->BufferObject->Name == buffers[i])
> bufObj = binding->BufferObject;
> else
> bufObj = _mesa_multi_bind_lookup_bufferobj(ctx, buffers, i, caller);
>
> if (bufObj)
> set_atomic_buffer_binding(ctx, binding, bufObj, offset, size);
> }
>
> - _mesa_end_bufferobj_lookups(ctx);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
> void GLAPIENTRY
> _mesa_BindBufferRange(GLenum target, GLuint index,
> GLuint buffer, GLintptr offset, GLsizeiptr size)
> {
> GET_CURRENT_CONTEXT(ctx);
> struct gl_buffer_object *bufObj;
>
> if (MESA_VERBOSE & VERBOSE_API) {
> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> index cd0df93..259de94 100644
> --- a/src/mesa/main/bufferobj.h
> +++ b/src/mesa/main/bufferobj.h
> @@ -91,26 +91,20 @@ _mesa_update_default_objects_buffer_objects(struct gl_context *ctx);
> extern struct gl_buffer_object *
> _mesa_lookup_bufferobj(struct gl_context *ctx, GLuint buffer);
>
> extern struct gl_buffer_object *
> _mesa_lookup_bufferobj_locked(struct gl_context *ctx, GLuint buffer);
>
> extern struct gl_buffer_object *
> _mesa_lookup_bufferobj_err(struct gl_context *ctx, GLuint buffer,
> const char *caller);
>
> -extern void
> -_mesa_begin_bufferobj_lookups(struct gl_context *ctx);
> -
> -extern void
> -_mesa_end_bufferobj_lookups(struct gl_context *ctx);
> -
> extern struct gl_buffer_object *
> _mesa_multi_bind_lookup_bufferobj(struct gl_context *ctx,
> const GLuint *buffers,
> GLuint index, const char *caller);
>
> extern void
> _mesa_initialize_buffer_object(struct gl_context *ctx,
> struct gl_buffer_object *obj,
> GLuint name);
>
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index 8139466..a3836a1 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -2035,21 +2035,21 @@ vertex_array_vertex_buffers(struct gl_context *ctx,
> * errors and then a second pass to actually perform the
> * bindings. Should we have different error semantics?
> *
> * RESOLVED: Yes. In this specification, when the parameters for
> * one of the <count> binding points are invalid, that binding
> * point is not updated and an error will be generated. However,
> * other binding points in the same command will be updated if
> * their parameters are valid and no other error occurs."
> */
>
> - _mesa_begin_bufferobj_lookups(ctx);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> for (i = 0; i < count; i++) {
> struct gl_buffer_object *vbo;
>
> /* The ARB_multi_bind spec says:
> *
> * "An INVALID_VALUE error is generated if any value in
> * <offsets> or <strides> is negative (per binding)."
> */
> if (offsets[i] < 0) {
> @@ -2086,21 +2086,21 @@ vertex_array_vertex_buffers(struct gl_context *ctx,
> if (!vbo)
> continue;
> } else {
> vbo = ctx->Shared->NullBufferObj;
> }
>
> _mesa_bind_vertex_buffer(ctx, vao, VERT_ATTRIB_GENERIC(first + i),
> vbo, offsets[i], strides[i]);
> }
>
> - _mesa_end_bufferobj_lookups(ctx);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
>
> void GLAPIENTRY
> _mesa_BindVertexBuffers(GLuint first, GLsizei count, const GLuint *buffers,
> const GLintptr *offsets, const GLsizei *strides)
> {
> GET_CURRENT_CONTEXT(ctx);
>
> /* The ARB_vertex_attrib_binding spec says:
>
More information about the mesa-dev
mailing list