[Mesa-dev] [PATCH] gallium: remove set_shader_resources, add set_shader_buffers for untyped buffers

Marek Olšák maraeo at gmail.com
Wed Jan 7 15:26:34 PST 2015


Hardware doesn't work like that actually, but it's flexible enough
that it can do anything. However, some solutions work better than
others. We also have to take OpenGL into account, which has per-shader
slots.

RadeonSI shaders don't have any fixed slots for resources and sampler
states. Instead, they get pointers to arrays of slots (resource
descriptions), which are in memory. The hardware has to fetch the
description from memory first (e.g. constant buffer #1) and use that
description as a parameter of a load instruction. Therefore, we can
support as many slots as we want. We can share them between shaders,
etc. (you want to fetch vertex buffers in a pixel shader? Sure I can
do that) However, we are constrained by what OpenGL requires, so we
have to design things around that too, especially when it gets ahead
of Direct3D in terms of features.

Marek

On Wed, Jan 7, 2015 at 6:44 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Hmm I'm not quite sure what to think of it. Apparently
> set_shader_resources was a closer match to what d3d does (seems to use
> UAVs for everything as far as I can tell, plus they are global and not
> per stage - I guess another reason why they were set together with RTs).
> I guess set_shader_buffers would then cover what you can access in d3d
> as StructuredBuffer and RWStructuredBuffer whereas set_shader_images
> would cover what can be accessed declared as RWBuffer/RWTextureXD? In
> that case I guess this would be manageable for translation, though a
> driver would need to figure out what to set with
> set_shader_buffers/set_shader_images on their own based on the actual
> shaders. But if hardware works like that I don't really oppose that.
> As for the slot numbers though d3d11.1 seems to support 64 UAVs -
> globally but no further restrictions as far as I can tell (so all could
> be atomic counters, for instance).
> d3d also has some initial count feature to set the current offset (or
> set to -1 to keep current value). I guess this works similarly to how
> setting offset for streamout buffers work (a major pita to deal with).
>
> And I think too that the compute interface should match - I guess though
> since you now can set this per shader stage you don't need a separate
> interface at all (for the record I found how you set that with d3d in a
> compute shader, instead of OMSetRenderTargetsAndUnorderedAccessViews()
> you use CSSetUnorderedAccessViews()).
>
> Roland
>
>
> Am 07.01.2015 um 11:56 schrieb Marek Olšák:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> set_shader_resources is unused.
>>
>> set_shader_buffers should support shader atomic counter buffers and shader
>> storage buffers from OpenGL.
>>
>> The plan is to use slots 0..15 for atomic counters and slots 16..31
>> for storage buffers. Atomic counters are planned to be supported first.
>>
>> This doesn't add any interface for images. The documentation is added
>> for future reference.
>> ---
>>
>> This is the interface only. I don't plan to do anything else for now.
>> Comments welcome.
>>
>>  src/gallium/docs/source/context.rst             | 16 ++++++++--------
>>  src/gallium/docs/source/screen.rst              |  4 ++--
>>  src/gallium/drivers/galahad/glhd_context.c      |  2 +-
>>  src/gallium/drivers/ilo/ilo_state.c             |  2 +-
>>  src/gallium/drivers/nouveau/nouveau_buffer.c    |  2 +-
>>  src/gallium/drivers/nouveau/nouveau_screen.c    |  2 +-
>>  src/gallium/drivers/nouveau/nv50/nv50_formats.c |  2 +-
>>  src/gallium/drivers/nouveau/nvc0/nvc0_state.c   |  2 +-
>>  src/gallium/include/pipe/p_context.h            | 20 +++++++++++---------
>>  src/gallium/include/pipe/p_defines.h            |  2 +-
>>  src/gallium/include/pipe/p_state.h              | 10 ++++++++++
>>  11 files changed, 38 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
>> index 5861f46..73fd35f 100644
>> --- a/src/gallium/docs/source/context.rst
>> +++ b/src/gallium/docs/source/context.rst
>> @@ -126,14 +126,14 @@ from a shader without an associated sampler.  This means that they
>>  have no support for floating point coordinates, address wrap modes or
>>  filtering.
>>
>> -Shader resources are specified for all the shader stages at once using
>> -the ``set_shader_resources`` method.  When binding texture resources,
>> -the ``level``, ``first_layer`` and ``last_layer`` pipe_surface fields
>> -specify the mipmap level and the range of layers the texture will be
>> -constrained to.  In the case of buffers, ``first_element`` and
>> -``last_element`` specify the range within the buffer that will be used
>> -by the shader resource.  Writes to a shader resource are only allowed
>> -when the ``writable`` flag is set.
>> +There are 2 types of shader resources: buffers and images.
>> +
>> +Buffers are specified using the ``set_shader_buffers`` method.
>> +
>> +Images are specified using the ``set_shader_images`` method. When binding
>> +images, the ``level``, ``first_layer`` and ``last_layer`` pipe_image_view
>> +fields specify the mipmap level and the range of layers the image will be
>> +constrained to.
>>
>>  Surfaces
>>  ^^^^^^^^
>> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
>> index 55d114c..c81ad66 100644
>> --- a/src/gallium/docs/source/screen.rst
>> +++ b/src/gallium/docs/source/screen.rst
>> @@ -403,8 +403,8 @@ resources might be created and handled quite differently.
>>    process.
>>  * ``PIPE_BIND_GLOBAL``: A buffer that can be mapped into the global
>>    address space of a compute program.
>> -* ``PIPE_BIND_SHADER_RESOURCE``: A buffer or texture that can be
>> -  bound to the graphics pipeline as a shader resource.
>> +* ``PIPE_BIND_SHADER_BUFFER``: A buffer that can be bound to a shader where
>> +  it should support reads, writes, and atomics.
>>  * ``PIPE_BIND_COMPUTE_RESOURCE``: A buffer or texture that can be
>>    bound to the compute program as a shader resource.
>>  * ``PIPE_BIND_COMMAND_ARGS_BUFFER``: A buffer that may be sourced by the
>> diff --git a/src/gallium/drivers/galahad/glhd_context.c b/src/gallium/drivers/galahad/glhd_context.c
>> index 37ea170..383d76c 100644
>> --- a/src/gallium/drivers/galahad/glhd_context.c
>> +++ b/src/gallium/drivers/galahad/glhd_context.c
>> @@ -1017,7 +1017,7 @@ galahad_context_create(struct pipe_screen *_screen, struct pipe_context *pipe)
>>     GLHD_PIPE_INIT(set_scissor_states);
>>     GLHD_PIPE_INIT(set_viewport_states);
>>     GLHD_PIPE_INIT(set_sampler_views);
>> -   //GLHD_PIPE_INIT(set_shader_resources);
>> +   //GLHD_PIPE_INIT(set_shader_buffers);
>>     GLHD_PIPE_INIT(set_vertex_buffers);
>>     GLHD_PIPE_INIT(set_index_buffer);
>>     GLHD_PIPE_INIT(create_stream_output_target);
>> diff --git a/src/gallium/drivers/ilo/ilo_state.c b/src/gallium/drivers/ilo/ilo_state.c
>> index b852f9f..09209ec 100644
>> --- a/src/gallium/drivers/ilo/ilo_state.c
>> +++ b/src/gallium/drivers/ilo/ilo_state.c
>> @@ -1267,7 +1267,7 @@ ilo_init_state_functions(struct ilo_context *ilo)
>>     ilo->base.set_scissor_states = ilo_set_scissor_states;
>>     ilo->base.set_viewport_states = ilo_set_viewport_states;
>>     ilo->base.set_sampler_views = ilo_set_sampler_views;
>> -   ilo->base.set_shader_resources = ilo_set_shader_resources;
>> +   //ilo->base.set_shader_resources = ilo_set_shader_resources;
>>     ilo->base.set_vertex_buffers = ilo_set_vertex_buffers;
>>     ilo->base.set_index_buffer = ilo_set_index_buffer;
>>
>> diff --git a/src/gallium/drivers/nouveau/nouveau_buffer.c b/src/gallium/drivers/nouveau/nouveau_buffer.c
>> index 49ff100..722c516 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_buffer.c
>> +++ b/src/gallium/drivers/nouveau/nouveau_buffer.c
>> @@ -44,7 +44,7 @@ nouveau_buffer_allocate(struct nouveau_screen *screen,
>>
>>     if (buf->base.bind & (PIPE_BIND_CONSTANT_BUFFER |
>>                           PIPE_BIND_COMPUTE_RESOURCE |
>> -                         PIPE_BIND_SHADER_RESOURCE))
>> +                         PIPE_BIND_SHADER_BUFFER))
>>        size = align(size, 0x100);
>>
>>     if (domain == NOUVEAU_BO_VRAM) {
>> diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c
>> index 517978d..68ab672 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_screen.c
>> +++ b/src/gallium/drivers/nouveau/nouveau_screen.c
>> @@ -197,7 +197,7 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev)
>>               PIPE_BIND_DISPLAY_TARGET | PIPE_BIND_SCANOUT |
>>               PIPE_BIND_CURSOR |
>>               PIPE_BIND_SAMPLER_VIEW |
>> -             PIPE_BIND_SHADER_RESOURCE | PIPE_BIND_COMPUTE_RESOURCE |
>> +             PIPE_BIND_SHADER_BUFFER | PIPE_BIND_COMPUTE_RESOURCE |
>>               PIPE_BIND_GLOBAL;
>>       screen->sysmem_bindings =
>>               PIPE_BIND_SAMPLER_VIEW | PIPE_BIND_STREAM_OUTPUT |
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_formats.c b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> index d394015..4fc8380 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> @@ -44,7 +44,7 @@
>>   */
>>  #define U_V   PIPE_BIND_VERTEX_BUFFER
>>  #define U_T   PIPE_BIND_SAMPLER_VIEW
>> -#define U_I   PIPE_BIND_SHADER_RESOURCE | PIPE_BIND_COMPUTE_RESOURCE
>> +#define U_I   PIPE_BIND_SHADER_BUFFER | PIPE_BIND_COMPUTE_RESOURCE
>>  #define U_TR  PIPE_BIND_RENDER_TARGET | U_T
>>  #define U_IR  U_TR | U_I
>>  #define U_TB  PIPE_BIND_BLENDABLE | U_TR
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>> index 728618f..f8fb955 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>> @@ -1269,7 +1269,7 @@ nvc0_init_state_functions(struct nvc0_context *nvc0)
>>
>>     pipe->set_global_binding = nvc0_set_global_bindings;
>>     pipe->set_compute_resources = nvc0_set_compute_resources;
>> -   pipe->set_shader_resources = nvc0_set_shader_resources;
>> +   //pipe->set_shader_resources = nvc0_set_shader_resources;
>>
>>     nvc0->sample_mask = ~0;
>>     nvc0->min_samples = 1;
>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>> index af5674f..641b93a 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -57,6 +57,7 @@ struct pipe_resource;
>>  struct pipe_sampler_state;
>>  struct pipe_sampler_view;
>>  struct pipe_scissor_state;
>> +struct pipe_shader_buffer;
>>  struct pipe_shader_state;
>>  struct pipe_stencil_ref;
>>  struct pipe_stream_output_target;
>> @@ -222,20 +223,21 @@ struct pipe_context {
>>                               struct pipe_sampler_view **);
>>
>>     /**
>> -    * Bind an array of shader resources that will be used by the
>> -    * graphics pipeline.  Any resources that were previously bound to
>> -    * the specified range will be unbound after this call.
>> +    * Bind an array of shader buffers that will be used by a shader.
>> +    * Any resources that were previously bound to the specified range
>> +    * will be unbound.
>>      *
>> -    * \param start      first resource to bind.
>> -    * \param count      number of consecutive resources to bind.
>> -    * \param resources  array of pointers to the resources to bind, it
>> +    * \param shader     shader stage where the buffers will be bound.
>> +    * \param start_slot first buffer slot to bind.
>> +    * \param count      number of consecutive buffers to bind.
>> +    * \param buffers    array of pointers to the buffers to bind, it
>>      *                   should contain at least \a count elements
>>      *                   unless it's NULL, in which case no new
>>      *                   resources will be bound.
>>      */
>> -   void (*set_shader_resources)(struct pipe_context *,
>> -                                unsigned start, unsigned count,
>> -                                struct pipe_surface **resources);
>> +   void (*set_shader_buffers)(struct pipe_context *, unsigned shader,
>> +                              unsigned start_slot, unsigned count,
>> +                              struct pipe_shader_buffer *buffers);
>>
>>     void (*set_vertex_buffers)( struct pipe_context *,
>>                                 unsigned start_slot,
>> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
>> index 6c5703a..b964fd6 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -348,7 +348,7 @@ enum pipe_flush_flags {
>>  #define PIPE_BIND_CURSOR               (1 << 16) /* mouse cursor */
>>  #define PIPE_BIND_CUSTOM               (1 << 17) /* state-tracker/winsys usages */
>>  #define PIPE_BIND_GLOBAL               (1 << 18) /* set_global_binding */
>> -#define PIPE_BIND_SHADER_RESOURCE      (1 << 19) /* set_shader_resources */
>> +#define PIPE_BIND_SHADER_BUFFER        (1 << 19) /* set_shader_buffers */
>>  #define PIPE_BIND_COMPUTE_RESOURCE     (1 << 20) /* set_compute_resources */
>>  #define PIPE_BIND_COMMAND_ARGS_BUFFER  (1 << 21) /* pipe_draw_info.indirect */
>>
>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>> index 43bc48b..450a270 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -464,6 +464,16 @@ struct pipe_constant_buffer {
>>
>>
>>  /**
>> + * An untyped shader buffer supporting loads, stores, and atomics.
>> + */
>> +struct pipe_shader_buffer {
>> +   struct pipe_resource *buffer; /**< the actual buffer */
>> +   unsigned buffer_offset; /**< offset to start of data in buffer, in bytes */
>> +   unsigned buffer_size;   /**< how much data can be read in shader */
>> +};
>> +
>> +
>> +/**
>>   * A stream output target. The structure specifies the range vertices can
>>   * be written to.
>>   *
>>
>


More information about the mesa-dev mailing list