<div dir="ltr"><div><div><div>Hi, </div>I agree with the solution. Considering "images" as "buffers" or "buffers" as "images" (as they are operated inside shaders but not texture units) makes sense. I think we can make atomic buffers as a part of ARB_shader_image_load_store. Because, images are 2D array of buffers with atomic operations run on them. </div>You have mentioned in our previous conversation about the un-necessity of store. The spec for ARB_shader_image_load_store says imageStore(). If we are implementing the same for atomics, then we have to use store for it too. </div>Thank you!! </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Nov 17, 2014 at 11:28 AM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Because shader resources were already specified as pipe_surfaces (in the existing, albeit presently unused API). A pipe_surface is a wrapper around a resource that specifies what "view" of the resource should be writable, and attaches an optional format to that resource. Normally pipe_surfaces are used for render targets, but it applies just as well here. It applies especially well with a view towards ARB_shader_image_load_store. It's conceivable to make a totally separate thing for atomic buffers, but there's no good reason to -- they will have to be handled in essentially the same way as regular "image" surfaces will for ARB_shader_image_load_store. Cheers, -ilia On Mon, Nov 17, 2014 at 11:19 AM, Aditya Avinash <div class="HOEnZb"><div class="h5"><<a href="mailto:adityaavinash1@gmail.com">adityaavinash1@gmail.com</a>> wrote: > Hi, > I am having difficulty in understanding why you implemented pipe_surface in > st_atom_atomicbuf.c. > > On Mon, Nov 17, 2014 at 2:24 AM, Aditya Avinash <<a href="mailto:adityaavinash1@gmail.com">adityaavinash1@gmail.com</a>> > wrote: >> >> Hi, >> >> On Sunday, November 16, 2014, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> wrote: >>> >>> The direction I went in was by adapting the shader resources interface >>> for this. I believe it will be possible to use for >>> shader_image_load_store as well. >> >> >> I asked some questions on mailing list about the implementation. I took >> the same path as uniform buffers. But, I realized that it's not efficient to >> do that. >> >>> >>> See <a href="https://github.com/imirkin/mesa/commits/atomic" target="_blank">https://github.com/imirkin/mesa/commits/atomic</a> >>> >> >> The commits are similar to my previous patch. I was doing atomics similar >> to uniform buffers, Now I was changing cso_context. >> >>> >>> I believe that makes a lot more sense than creating a special counter >>> buffer type only to be used for this. pipe_surface has the requisite >>> offset/etc options. >> >> >> I thought of using uniform buffer data structure with certain flags which >> differentiate between atomics, uniforms, index. Like a generic buffer. >> >>> >>> I feel like I've mentioned this before when you asked questions, but >>> I'd highly recommend taking my work and improving on it (or at least >>> understanding it). It's at the point where a driver can implement the >>> backend logic, although some of the mesa/st bits are going to be >>> subject to discussion (and need fixing, it messes up the DCE tgsi >>> pass, so I just disabled it). >>> >>> On Thu, Nov 13, 2014 at 12:18 AM, adityaatluri <<a href="mailto:adityaavinash1@gmail.com">adityaavinash1@gmail.com</a>> >>> wrote: >>> > --- >>> > src/gallium/include/pipe/p_context.h | 5 +++++ >>> > src/gallium/include/pipe/p_defines.h | 7 ++++++- >>> > src/gallium/include/pipe/p_state.h | 10 ++++++++++ >>> > 3 files changed, 21 insertions(+), 1 deletion(-) >>> > >>> > diff --git a/src/gallium/include/pipe/p_context.h >>> > b/src/gallium/include/pipe/p_context.h >>> > index af5674f..bf3be31 100644 >>> > --- a/src/gallium/include/pipe/p_context.h >>> > +++ b/src/gallium/include/pipe/p_context.h >>> > @@ -44,6 +44,7 @@ struct pipe_blit_info; >>> > struct pipe_box; >>> > struct pipe_clip_state; >>> > struct pipe_constant_buffer; >>> > +struct pipe_counter_buffer; >>> > struct pipe_depth_stencil_alpha_state; >>> > struct pipe_draw_info; >>> > struct pipe_fence_handle; >>> > @@ -201,6 +202,10 @@ struct pipe_context { >>> > uint shader, uint index, >>> > struct pipe_constant_buffer *buf ); >>> > >>> > + void (*set_counter_buffer)( struct pipe_context *, >>> > + uint shader, uint index, >>> > + struct pipe_counter_buffer *buf ); >>> > + >>> > void (*set_framebuffer_state)( struct pipe_context *, >>> > const struct pipe_framebuffer_state >>> > * ); >>> > >>> > diff --git a/src/gallium/include/pipe/p_defines.h >>> > b/src/gallium/include/pipe/p_defines.h >>> > index 8c4e415..717ab6a 100644 >>> > --- a/src/gallium/include/pipe/p_defines.h >>> > +++ b/src/gallium/include/pipe/p_defines.h >>> > @@ -341,6 +341,7 @@ enum pipe_flush_flags { >>> > #define PIPE_BIND_VERTEX_BUFFER (1 << 4) /* set_vertex_buffers >>> > */ >>> > #define PIPE_BIND_INDEX_BUFFER (1 << 5) /* draw_elements */ >>> > #define PIPE_BIND_CONSTANT_BUFFER (1 << 6) /* set_constant_buffer >>> > */ >>> > +#define PIPE_BIND_COUNTER_BUFFER (1 << 7) /* set_counter_buffer >>> > */ >>> > #define PIPE_BIND_DISPLAY_TARGET (1 << 8) /* flush_front_buffer >>> > */ >>> > #define PIPE_BIND_TRANSFER_WRITE (1 << 9) /* transfer_map */ >>> > #define PIPE_BIND_TRANSFER_READ (1 << 10) /* transfer_map */ >>> > @@ -572,6 +573,8 @@ enum pipe_cap { >>> > PIPE_CAP_MAX_VERTEX_ATTRIB_STRIDE = 109, >>> > PIPE_CAP_SAMPLER_VIEW_TARGET = 110, >>> > PIPE_CAP_CLIP_HALFZ = 111, >>> > + PIPE_CAP_USER_COUNTER_BUFFERS = 112, >>> > + PIPE_CAP_COUNTER_BUFFER_OFFSET_ALIGNMENT = 113, >>> > }; >>> > >>> > #define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0) >>> > @@ -631,7 +634,9 @@ enum pipe_shader_cap >>> > PIPE_SHADER_CAP_PREFERRED_IR, >>> > PIPE_SHADER_CAP_TGSI_SQRT_SUPPORTED, >>> > PIPE_SHADER_CAP_MAX_SAMPLER_VIEWS, >>> > - PIPE_SHADER_CAP_DOUBLES >>> > + PIPE_SHADER_CAP_DOUBLES, >>> > + PIPE_SHADER_CAP_MAX_COUNTER_BUFFER_SIZE, >>> > + PIPE_SHADER_CAP_MAX_COUNTER_BUFFERS >>> > }; >>> > >>> > /** >>> > diff --git a/src/gallium/include/pipe/p_state.h >>> > b/src/gallium/include/pipe/p_state.h >>> > index 43bc48b..49fae5d 100644 >>> > --- a/src/gallium/include/pipe/p_state.h >>> > +++ b/src/gallium/include/pipe/p_state.h >>> > @@ -57,6 +57,7 @@ extern "C" { >>> > #define PIPE_MAX_CLIP_PLANES 8 >>> > #define PIPE_MAX_COLOR_BUFS 8 >>> > #define PIPE_MAX_CONSTANT_BUFFERS 32 >>> > +#define PIPE_MAX_COUNTER_BUFFERS 32 >>> > #define PIPE_MAX_SAMPLERS 16 >>> > #define PIPE_MAX_SHADER_INPUTS 32 >>> > #define PIPE_MAX_SHADER_OUTPUTS 48 /* 32 GENERICs + POS, PSIZE, FOG, >>> > etc. */ >>> > @@ -462,6 +463,15 @@ struct pipe_constant_buffer { >>> > const void *user_buffer; /**< pointer to a user buffer if buffer >>> > == NULL */ >>> > }; >>> > >>> > +/** >>> > + * A Counter buffer. A new buffer is set everytime a variable with >>> > + * atomic_uint is defined. >>> > + */ >>> > +struct pipe_counter_buffer{ >>> > + struct pipe_resource *buffer; /**< The actual buffer */ >>> > + unsigned buffer_offset; /**< The offset to start of data in buffer >>> > in bytes */ >>> > + const void *user_buffer; /**< The buffer which is created by the >>> > compiler */ >>> > +}; >>> > >>> > /** >>> > * A stream output target. The structure specifies the range vertices >>> > can >>> > -- >>> > 1.9.1 >>> > >>> > _______________________________________________ >>> > mesa-dev mailing list >>> > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >>> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >> >> >> >> -- >> Regards, >> Aditya Atluri, >> USA. >> >> > > > > -- > Regards, > Aditya Atluri, > USA. > </div></div></blockquote></div> -- <div class="gmail_signature"><div dir="ltr"><div><div>Regards, </div>Aditya Atluri, </div><div>USA. </div> </div></div> </div>