[Mesa-dev] [PATCH v2 09/82] mesa: Add shader storage buffer support to struct gl_context
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Tue Jun 9 00:30:45 PDT 2015
On 09/06/15 08:18, Samuel Iglesias Gonsálvez wrote:
> On 08/06/15 23:36, Jordan Justen wrote:
>> On 2015-06-03 00:00:59, Iago Toral Quiroga wrote:
>>> This includes the array of bindings, the current buffer bound to the
>>> GL_SHADER_STORAGE_BUFFER target and a set of general limits and default
>>> values for shader storage buffers.
>>> ---
>>> src/mesa/main/bufferobj.c | 5 +++++
>>> src/mesa/main/config.h | 2 ++
>>> src/mesa/main/context.c | 6 ++++++
>>> src/mesa/main/mtypes.h | 38 ++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 51 insertions(+)
>>>
>>> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
>>> index 66dee68..c5d4ada 100644
>>> --- a/src/mesa/main/bufferobj.c
>>> +++ b/src/mesa/main/bufferobj.c
>>> @@ -112,6 +112,11 @@ get_buffer_target(struct gl_context *ctx, GLenum target)
>>> return &ctx->UniformBuffer;
>>> }
>>> break;
>>> + case GL_SHADER_STORAGE_BUFFER:
>>> + if (ctx->Extensions.ARB_shader_storage_buffer_object) {
>>> + return &ctx->ShaderStorageBuffer;
>>> + }
>>> + break;
>>> case GL_ATOMIC_COUNTER_BUFFER:
>>> if (ctx->Extensions.ARB_shader_atomic_counters) {
>>> return &ctx->AtomicBuffer;
>>> diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h
>>> index 5a66a4e..551aad1 100644
>>> --- a/src/mesa/main/config.h
>>> +++ b/src/mesa/main/config.h
>>> @@ -171,8 +171,10 @@
>>> #define MAX_PROGRAM_LOCAL_PARAMS 4096
>>> #define MAX_UNIFORMS 4096
>>> #define MAX_UNIFORM_BUFFERS 15 /* + 1 default uniform buffer */
>>> +#define MAX_SHADER_STORAGE_BUFFERS 15
>>
>> This is just to copy the ubo value?
>>
>
> Yes, it is. We started copying the values from UBOs, like
> consts->MaxShaderStorageBufferBindings (copied from
> consts->MaxUniformBufferBindings) and similarly for others.
>
> We explained that in the cover letter of the first version of the patch
> series [0]:
>
> "Both Mesa and i965 need to give default values for certain things
> like the maximum allowed size of a shader storage buffer, the maximum
> number of buffer bindings, the maximum number of combined shader storage
> blocks, etc. We are not sure about what default values we should use
> for these in all cases and except in a few cases we generally copied the
> default values from uniforms. That may be fine or not, so let us know if
> we should use different values for any of these setting at the Mesa or
> i965 levels."
>
> [0] http://lists.freedesktop.org/archives/mesa-dev/2015-May/084196.html
>
>>> /* 6 is for vertex, hull, domain, geometry, fragment, and compute shader. */
>>> #define MAX_COMBINED_UNIFORM_BUFFERS (MAX_UNIFORM_BUFFERS * 6)
>>> +#define MAX_COMBINED_SHADER_STORAGE_BUFFERS (MAX_SHADER_STORAGE_BUFFERS * 6)
>>> #define MAX_ATOMIC_COUNTERS 4096
>>> /* 6 is for vertex, hull, domain, geometry, fragment, and compute shader. */
>>> #define MAX_COMBINED_ATOMIC_BUFFERS (MAX_UNIFORM_BUFFERS * 6)
>>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>>> index 8a59b5e..8577e43 100644
>>> --- a/src/mesa/main/context.c
>>> +++ b/src/mesa/main/context.c
>>> @@ -615,6 +615,12 @@ _mesa_init_constants(struct gl_constants *consts, gl_api api)
>>> consts->MaxUniformBlockSize = 16384;
>>> consts->UniformBufferOffsetAlignment = 1;
>>>
>>> + /** GL_ARB_shader_storage_buffer_object */
>>> + consts->MaxCombinedShaderStorageBlocks = 36;
>>
>> Spec has 8. I guess 36 > 8, so this may be okay, but where is 36 from?
>
> As explained before, we picked it from uniforms:
> consts->MaxCombinedUniformBlocks = 36;
>
>>
>>> + consts->MaxShaderStorageBufferBindings = 36;
>>
>> Spec has 8. Same comment/question as above.
>
> Copied from consts->MaxUniformBufferBindings = 36;
>>
>>> + consts->MaxShaderStorageBlockSize = 16 * 1024 * 1024;
>>> + consts->ShaderStorageBufferOffsetAlignment = 1;
>>
>> Spec has 256.
>>
>
> Oh, right. We picked it from consts->UniformBufferOffsetAlignment = 1
> but, according to the spec, this is a mistake. We will fix it!
>
> So, just to confirm, we will pick the spec values (if available) for all
> cases. If you (or somebody else) think a better default value should be
> chosen for any constant, please say so :-)
>
By the way, next patch "mesa: add MaxShaderStorageBlocks to struct
gl_program_constants" [0] sets prog->MaxShaderStorageBlocks = 12, which
is also copied from prog->MaxUniformBlocks = 12.
Because of that, one piglit test [1] starts to fail because
MaxCombinedUniformBlocks is now set to 8 but we have more shader storage
block per shader.
I plan to modify patch [0] to set prog->MaxShaderStorageBlocks = 8 and
write a separate patch that makes i965 driver to set the following
constant values:
* MaxShaderStorageBlocks = 12
* MaxCombinedUniformBlocks = 36 (12 * 3 different shader types: vertex,
geometry and fragment)
* MaxShaderStorageBufferBindings = 36.
Those values are arbitrary (picked from uniforms) but our tests seem to
work with them. As it was said in last email, if you think we should
change the values or this is not right, just say so.
What do you think?
Sam
[0] http://lists.freedesktop.org/archives/mesa-dev/2015-June/085570.html
[1] bin/arb_shader_storage_buffer_object-maxblocks -auto -fbo
> Thanks!
>
> Sam
>
>> -Jordan
>>
>>> +
>>> /* GL_ARB_explicit_uniform_location, GL_MAX_UNIFORM_LOCATIONS */
>>> consts->MaxUserAssignableUniformLocations =
>>> 4 * MESA_SHADER_STAGES * MAX_UNIFORMS;
>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>> index 03b8e48..741930d 100644
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -3370,6 +3370,15 @@ struct gl_constants
>>> GLuint UniformBufferOffsetAlignment;
>>> /** @} */
>>>
>>> + /** @{
>>> + * GL_ARB_shader_storage_buffer_object
>>> + */
>>> + GLuint MaxCombinedShaderStorageBlocks;
>>> + GLuint MaxShaderStorageBufferBindings;
>>> + GLuint MaxShaderStorageBlockSize;
>>> + GLuint ShaderStorageBufferOffsetAlignment;
>>> + /** @} */
>>> +
>>> /**
>>> * GL_ARB_explicit_uniform_location
>>> */
>>> @@ -4015,6 +4024,20 @@ struct gl_uniform_buffer_binding
>>> GLboolean AutomaticSize;
>>> };
>>>
>>> +struct gl_shader_storage_buffer_binding
>>> +{
>>> + struct gl_buffer_object *BufferObject;
>>> + /** Start of shader storage block data in the buffer */
>>> + GLintptr Offset;
>>> + /** Size of data allowed to be referenced from the buffer (in bytes) */
>>> + GLsizeiptr Size;
>>> + /**
>>> + * glBindBufferBase() indicates that the Size should be ignored and only
>>> + * limited by the current size of the BufferObject.
>>> + */
>>> + GLboolean AutomaticSize;
>>> +};
>>> +
>>> /**
>>> * ARB_shader_image_load_store image unit.
>>> */
>>> @@ -4262,6 +4285,12 @@ struct gl_context
>>> struct gl_buffer_object *UniformBuffer;
>>>
>>> /**
>>> + * Current GL_ARB_shader_storage_buffer_object binding referenced by
>>> + * GL_SHADER_STORAGE_BUFFER target for glBufferData, glMapBuffer, etc.
>>> + */
>>> + struct gl_buffer_object *ShaderStorageBuffer;
>>> +
>>> + /**
>>> * Array of uniform buffers for GL_ARB_uniform_buffer_object and GL 3.1.
>>> * This is set up using glBindBufferRange() or glBindBufferBase(). They are
>>> * associated with uniform blocks by glUniformBlockBinding()'s state in the
>>> @@ -4271,6 +4300,15 @@ struct gl_context
>>> UniformBufferBindings[MAX_COMBINED_UNIFORM_BUFFERS];
>>>
>>> /**
>>> + * Array of shader storage buffers for ARB_shader_storage_buffer_object
>>> + * and GL 4.3. This is set up using glBindBufferRange() or
>>> + * glBindBufferBase(). They are associated with shader storage blocks by
>>> + * glShaderStorageBlockBinding()'s state in the shader program.
>>> + */
>>> + struct gl_shader_storage_buffer_binding
>>> + ShaderStorageBufferBindings[MAX_COMBINED_SHADER_STORAGE_BUFFERS];
>>> +
>>> + /**
>>> * Object currently associated with the GL_ATOMIC_COUNTER_BUFFER
>>> * target.
>>> */
>>> --
>>> 1.9.1
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list