[Mesa-dev] [PATCH 1/2] gallium/include/pipe: Added interface for atomic counter buffers in pipe
Aditya Avinash
adityaavinash1 at gmail.com
Tue Jan 6 14:51:28 PST 2015
On Tue, Jan 6, 2015 at 4:23 PM, Marek Olšák <maraeo at gmail.com> wrote:
> Having a different view type for writable shader resources sounds good.
>
> An alternative solution would be to have separate functions for images
> and buffers:
>
> - set_image_views - image views only, pipe_image_view should look
> similar to pipe_surface
>
> - set_shader_buffers - buffers only, untyped, no views, each slot has
> only 3 parameters: resource, offset, size
>
I am not expert on this but, with my experience reading the code.
I think using a generic one with flags set in it could be better. Using
"union" can also be a better design.
As textures and buffers can be used as resources from shaders, using a
generic one and setting flags can be useful.
> That would be even better from the radeonsi design point of view, and
> it would avoid creating a useless view for writable buffers, which are
> always untyped in GL.
>
> Marek
>
> On Tue, Jan 6, 2015 at 10:09 PM, Roland Scheidegger <sroland at vmware.com>
> wrote:
> > Actually initially pipe_surface really was for "traditional" render
> > surface. There were even stand-alone surfaces (i.e. not based on
> > resources) for the non-fbo (system window) surfaces - this is why the
> > width/height fields are actually still in there in a pipe_surface (yes
> > with a XXX comment)...
> > So, the functionality was really all restricted to GL2 / d3d9 level,
> > hence it didn't really make a difference how you interpreted it, as
> > there weren't any different bind points.
> > I'm not sure if we really need a different view for uavs or whatever you
> > want to call them in gallium. d3d does that but for instance we didn't
> > do different views for depth stencil / rt neither as it seemed quite
> > redundant (but nonetheless, create_surface was indeed intended to serve
> > the role of the create depth stencil and render target views, at least
> > once we cleaned it up to really rely on resources). If it can be reused
> > for uavs in a reasonably clean way that's ok - as I said the biggest
> > problem I see is that when you create the view the driver won't know for
> > which bind point this view is and it must be able to bind the same view
> > to several different bind points later. It is, however, probably
> > suboptimal if you'd have a driver which needs things closer to d3d
> > semantics (as we do in the svga driver) - at least if you'd have
> > specified different bind points in the resource the driver is going to
> > need to generate multiple views on its own internally. In that way it
> > may be cleaner if each different bind point would have its own view.
> >
> > Roland
> >
> >
> >
> > Am 06.01.2015 um 20:14 schrieb Ilia Mirkin:
> >> Ah, OK. I was interpreting sampler view as "all the things you need to
> >> be able to sample from a resource", and surface as "all the things you
> >> need to know where to write in a resource", including a render target.
> >> But you guys have all been playing this game for much much longer than
> >> me...
> >>
> >> On Tue, Jan 6, 2015 at 1:37 PM, Marek Olšák <maraeo at gmail.com> wrote:
> >>> I thought pipe_surface was a render target view.
> >>>
> >>> From the r600 point of view, writable resources are pretty much
> >>> "colorbuffers", because they share slots with colorbuffers and are set
> >>> up exactly like colorbuffers. So pipe_surface maps well to r600, and
> >>> using set_framebuffer_state for setting UAVs would be perfect for that
> >>> hardware, but it would be ugly.
> >>>
> >>> From the radeonsi point of view, writable resources are no different
> >>> from textures and texture buffers. So pipe_sampler_view maps well to
> >>> radeonsi.
> >>>
> >>> Marek
> >>>
> >>> On Tue, Jan 6, 2015 at 7:17 PM, Ilia Mirkin <imirkin at alum.mit.edu>
> wrote:
> >>>> I thought that a surface was basically a writable view into a
> >>>> resource. What does sampler view have that surface doesn't (that
> >>>> matters)? The only thing I can think of is levels, but does anything
> >>>> actually support multi-level stuff? (ARB_shader_image_load_store
> >>>> doesn't seem to.)
> >>>>
> >>>> On Tue, Jan 6, 2015 at 12:54 PM, Marek Olšák <maraeo at gmail.com>
> wrote:
> >>>>> I would just rename pipe_sampler_view to pipe_resource_view. I don't
> >>>>> think "pipe_sampler_view" has a lot to do with samplers. It's just a
> >>>>> universal view into textures and yes, buffers too. Of course, some
> >>>>> fields don't apply to certain types and use cases.
> >>>>>
> >>>>> OMSetRenderTargetsAndUnorderedAccessViews looks like a design fail.
> It
> >>>>> exactly matches Evergreen/Cayman design, which is now deprecated.
> >>>>> Evergreen shares UAV slots with colorbuffers. There are 12 slots, the
> >>>>> first 8 can be colorbuffers, those that are unused can be UAVs, so
> you
> >>>>> have at least 4. If you bind only one colorbuffer, you can bind 11
> >>>>> UAVs, etc. The other problem with this design is that UAVs can only
> be
> >>>>> used in the pixel shader. It would be a bad idea to follow this
> design
> >>>>> precisely in Gallium. We should have something more generic and let
> >>>>> drivers describe any limitations with CAPs.
> >>>>>
> >>>>> Marek
> >>>>>
> >>>>>
> >>>>> On Tue, Jan 6, 2015 at 6:14 PM, Roland Scheidegger <
> sroland at vmware.com> wrote:
> >>>>>> You are quite right using pipe_surface seems like a bit of an abuse.
> >>>>>> pipe_sampler_view wouldn't be quite right though neither, no
> samplers
> >>>>>> are involved. Plus, the views here cannot have multiple mip levels
> >>>>>> (which is presumably why pipe_surface was used - nevertheless
> >>>>>> pipe_surface was intended only for render / depth stencil target).
> >>>>>> I guess an alternative would be to use a new view type altogether
> (like
> >>>>>> d3d does).
> >>>>>> I admit I don't quite get how the same stuff is done with d3d11 (as
> we
> >>>>>> should have an interface which works for that as well). It looks
> like
> >>>>>> what's called shader resource in gallium (as in
> set_shader_resources) is
> >>>>>> really UAVs in d3d11 (though I'm not quite sure how these are
> actually
> >>>>>> set in d3d11 in the ddi - at the api level these are interestingly
> >>>>>> enough set together with render targets
> >>>>>> (OMSetRenderTargetsAndUnorderedAccessViews()) though the parameters
> are
> >>>>>> all separate).
> >>>>>> These atomics here more look like the structured buffers which are
> also
> >>>>>> set the same way (maybe?), at least there's mention of
> append/consume
> >>>>>> buffers too there.
> >>>>>>
> >>>>>> Roland
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Am 06.01.2015 um 16:27 schrieb Marek Olšák:
> >>>>>>> Using set_shader_resources is preferable. I'd rather it used
> >>>>>>> pipe_sampler_view, not pipe_surface.
> >>>>>>>
> >>>>>>> GL has a lot of shader resource types though:
> >>>>>>> - uniform buffers (load only)
> >>>>>>> - textures and texture buffers (sample+load only)
> >>>>>>> - storage buffers (load+store)
> >>>>>>> - atomic counter buffers (atomic only)
> >>>>>>> - images (load+store+atomic)
> >>>>>>>
> >>>>>>> Hardware resource categories on r600:
> >>>>>>> - textures and read-only buffers (sample+load only)
> >>>>>>> - constant buffers (load only)
> >>>>>>> - writable buffers and images (load+store+atomic)
> >>>>>>>
> >>>>>>> Hardware resource categories on radeonsi:
> >>>>>>> - none, everything is considered a generic shader resource and
> >>>>>>> supports load+store+atomic+sample
> >>>>>>>
> >>>>>>> Thus, it is preferable to use set_shader_resources for all writable
> >>>>>>> resources and keep the current interfaces for sampler views and
> >>>>>>> constant buffers intact.
> >>>>>>>
> >>>>>>> Marek
> >>>>>>>
> >>>>>>>
> >>>>>>> On Tue, Jan 6, 2015 at 2:54 PM, Jose Fonseca <jfonseca at vmware.com>
> wrote:
> >>>>>>>> Do we really need a new pipe_context::set_counter_buffer method?
> Shouldn't
> >>>>>>>> this case be covered by pipe_context::set_shader_resources ?
> >>>>>>>>
> >>>>>>>> If we really need new method, I'd like we have better names for
> them, so we
> >>>>>>>> can clearly distinguish them.
> >>>>>>>>
> >>>>>>>> IIUC GL_ARB_shader_atomic_counters correctly, this roughly
> matches D3D11s
> >>>>>>>> "unordered access views" [1], though D3D11's UAVs can by typed
> [2], or raw
> >>>>>>>> [3].
> >>>>>>>>
> >>>>>>>> Also, are counter buffers in user memory really supported? I
> can't spot any
> >>>>>>>> mention of that in GL_ARB_shader_atomic_counters.
> >>>>>>>>
> >>>>>>>> I think that rather than mimicking pipe_constant_buffer, this
> feature should
> >>>>>>>> more closely to sampler views.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> I confess I'm not familiar with counter buffers / UAVs, but I
> think this
> >>>>>>>> needs a bit more thought to avoid being completely redone in the
> medium
> >>>>>>>> term.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Jose
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> [1]
> >>>>>>>>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__msdn.microsoft.com_en-2Dgb_library_windows_desktop_ff476335.aspx-23Unordered-5FAccess&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=UKHfRUDlIPUSvsUpW3toE2gMpzd1CNUNRIK4CjGrCHo&s=qMm6xJyygPBy-qluPFLHmzH_39o-dSlbG87meenKyq8&e=
> >>>>>>>> [2]
> https://urldefense.proofpoint.com/v2/url?u=http-3A__msdn.microsoft.com_en-2Dgb_library_windows_desktop_ff476258.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=UKHfRUDlIPUSvsUpW3toE2gMpzd1CNUNRIK4CjGrCHo&s=f2PAAZTT1Eu1wCjexvk_Hx7otgD3wAOz_yFMUN583UM&e=
> >>>>>>>> [3]
> >>>>>>>>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__msdn.microsoft.com_en-2Dgb_library_windows_desktop_ff476900.aspx-23Raw-5FBuffer-5FViews&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=UKHfRUDlIPUSvsUpW3toE2gMpzd1CNUNRIK4CjGrCHo&s=CcRwntJiBsA-k0qo9UAxZrLbluEyp3huZlvFtJCJNms&e=
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 04/01/15 21:44, adityaatluri 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
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> _______________________________________________
> >>>>>>>> mesa-dev mailing list
> >>>>>>>> mesa-dev at lists.freedesktop.org
> >>>>>>>>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=UKHfRUDlIPUSvsUpW3toE2gMpzd1CNUNRIK4CjGrCHo&s=yeOKjr9Wd2qQqv6PMmrwzZbQJK_BqiA4FnHRgd5FD8E&e=
> >>>>>>> _______________________________________________
> >>>>>>> mesa-dev mailing list
> >>>>>>> mesa-dev at lists.freedesktop.org
> >>>>>>>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=UKHfRUDlIPUSvsUpW3toE2gMpzd1CNUNRIK4CjGrCHo&s=yeOKjr9Wd2qQqv6PMmrwzZbQJK_BqiA4FnHRgd5FD8E&e=
> >>>>>>>
> >>>>>>
> >>>>> _______________________________________________
> >>>>> mesa-dev mailing list
> >>>>> mesa-dev at lists.freedesktop.org
> >>>>>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=hQbv4jSynGDz9IHC35vlhKe264zk5k2-iZtkYaB-sqo&s=txzEnwI5iS1RZd-XAWViYdOP_HjkhgCgqpLgXD4LPYc&e=
> >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
--
Regards,
*Aditya Atluri,*
*USA.*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150106/af757305/attachment-0001.html>
More information about the mesa-dev
mailing list