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 <srol...@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.ol...@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. >>> * >>> >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev