[Mesa-dev] [PATCH 1/2] gallium/include/pipe: Added interface for atomic counter buffers in pipe
Roland Scheidegger
sroland at vmware.com
Tue Jan 6 13:09:58 PST 2015
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=
More information about the mesa-dev
mailing list