On Sun, Jul 5, 2015 at 9:25 AM, Marek Olšák <mar...@gmail.com> wrote:
> From: Marek Olšák <marek.ol...@amd.com>
>
> Other approaches are being considered:
>
> 1) Don't use resource wrappers (views) and pass all view parameters
> (format, layer range, level) to set_shader_images just like
> set_vertex_buffers, set_constant_buffer, or even glBindImageTexture do.
>
> 2) Use pipe_sampler_view instead of pipe_image_view,
> and maybe even use set_sampler_views instead of set_shader_images.
> set_sampler_views would have to use start_slot >= PIPE_MAX_SAMPLERS for
> all writable images to allow for OpenGL textures in the lower slots.
>
> I wouldn't like to go back to using pipe_surface, because it doesn't fit
> into the DX12 hw design.
>
> Please discuss.
> ---
> diff --git a/src/gallium/include/pipe/p_context.h
> b/src/gallium/include/pipe/p_context.h
> index c2eedf8..13f3938 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -48,6 +48,7 @@ struct pipe_depth_stencil_alpha_state;
> struct pipe_draw_info;
> struct pipe_fence_handle;
> struct pipe_framebuffer_state;
> +struct pipe_image_view;
> struct pipe_index_buffer;
> struct pipe_query;
> struct pipe_poly_stipple;
> @@ -236,20 +237,21 @@ struct pipe_context {
> const float default_inner_level[2]);
>
> /**
> - * 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 images that will be used by a shader.
> + * Any imagse 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 images will be bound.
> + * \param start_slot first iamage slot to bind.
> + * \param count number of consecutive images to bind.
> + * \param buffers array of pointers to the images to bind, it
> * should contain at least \a count elements
> - * unless it's NULL, in which case no new
> - * resources will be bound.
> + * unless it's NULL, in which case no images will
> + * be bound.
> */
> - void (*set_shader_resources)(struct pipe_context *,
> - unsigned start, unsigned count,
> - struct pipe_surface **resources);
> + void (*set_shader_images)(struct pipe_context *, unsigned shader,
> + unsigned start_slot, unsigned count,
> + struct pipe_image_view *images);
On Fermi, there is only one set of 8 images that can be bound,
irrespective of stage. There are a different set of 8 bindings for 3d
and compute pipelines though. Kepler+ is all bindless so it can do
whatever.
Even though I originally advocated for the stage argument, it appears
to neither match Fermi hardware nor the GL API where I believe images
are also in a global namespace.
>
> void (*set_vertex_buffers)( struct pipe_context *,
> unsigned start_slot,
> @@ -392,6 +394,17 @@ struct pipe_context {
> struct pipe_surface *);
>
> /**
> + * Create an image view into a buffer or texture to be used with load,
> + * store, and atomic instructions by a shader stage.
> + */
> + struct pipe_image_view * (*create_image_view)(struct pipe_context *ctx,
> + struct pipe_resource
> *texture,
> + const struct
> pipe_image_view *templat);
> +
> + void (*image_view_destroy)(struct pipe_context *ctx,
> + struct pipe_image_view *view);
> +
> + /**
> * Map a resource.
> *
> * Transfers are (by default) context-private and allow uploads to be
> diff --git a/src/gallium/include/pipe/p_state.h
> b/src/gallium/include/pipe/p_state.h
> index dc8f550..f655dda 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -386,6 +386,31 @@ struct pipe_sampler_view
>
>
> /**
> + * A view into a writable buffer or texture that can be bound to a shader
> + * stage.
> + */
> +struct pipe_image_view
> +{
> + struct pipe_reference reference;
> + struct pipe_resource *resource; /**< resource into which this is a view
> */
> + struct pipe_context *context; /**< context this view belongs to */
> + enum pipe_format format; /**< typed PIPE_FORMAT_x */
> +
> + union {
> + struct {
> + unsigned first_layer:16; /**< first layer to use for array
> textures */
> + unsigned last_layer:16; /**< last layer to use for array
> textures */
> + unsigned level:8; /**< mipmap level to use */
> + } tex;
> + struct {
> + unsigned first_element;
> + unsigned last_element;
> + } buf;
> + } u;
> +};
This is an exact copy of pipe_surface, except for the
supposedly-removable width/height and "writable" bit, which should
probably make a comeback here.
Can you remind me what was wrong with pipe_surface? I seem to recall
that you were offended by its refcountedness, but that appears to have
stayed in this version.
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
