[Mesa-dev] [PATCH] gallium: remove set_shader_resources, add set_shader_buffers for untyped buffers
Ilia Mirkin
imirkin at alum.mit.edu
Wed Jan 7 05:41:31 PST 2015
On Wed, Jan 7, 2015 at 5:56 AM, Marek Olšák <maraeo at gmail.com> wrote:
> 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.
Can you clarify how this is better than the set_shader_resources
interface, which can also be shared for images (which will need to
support texture buffers...)?
FWIW, there's already an impl for nve4 images using
set_shader_resources (not sure how Christoph had tested it, I think
using some preliminary OpenCL C -> TGSI converter with image support).
Are these buffers fundamentally different than images? We'll still
need atomic support for images as well...
Also how do you anticipate this will be integrated into TGSI? Right
now there's a TGSI_FILE_RESOURCE -- will there be a new
TGSI_FILE_BUFFER and TGSI_FILE_IMAGE?
>
> 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.
> *
> --
> 2.1.0
>
> _______________________________________________
> 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