[Mesa-dev] [PATCH] gallium: remove set_shader_resources, add set_shader_buffers for untyped buffers
Marek Olšák
maraeo at gmail.com
Thu Jan 8 10:43:19 PST 2015
DX11 can be supported very well with this interface. OpenGL has these
queries (from shader storage buffers
objects):
MAX_VERTEX_SHADER_STORAGE_BLOCKS (min: 0)
MAX_GEOMETRY_SHADER_STORAGE_BLOCKS (min: 0)
MAX_FRAGMENT_SHADER_STORAGE_BLOCKS (min: 8)
I guess the minimum values are DX11 requirements, so original DX11 hw
is allowed to support writable resources in only a pixel shader. There
is also:
MAX_COMBINED_SHADER_OUTPUT_RESOURCES
which is an alias of
MAX_COMBINED_IMAGE_UNITS_AND_FRAGMENT_OUTPUTS
This explicitly allows writable resources to share slots with colorbuffers.
If we copy these "caps" from OpenGL, I think any DX11.0 implementation
will be just fine.
Marek
On Thu, Jan 8, 2015 at 6:45 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Yes, I'm just trying to figure out if there's a design which easily
> works both for d3d and opengl. Apparently, being per stage or global is
> a difference which we just need to live with, so I don't have any
> objections of making this more suited to GL. Using different types for
> images and buffers is also something which helps GL I guess.
> (The initial count issue is probably something which can be dealt with
> later.)
>
> Roland
>
>
> Am 08.01.2015 um 00:26 schrieb Marek Olšák:
>> 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