[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 10:53:23 PST 2015
Am 06.01.2015 um 18:54 schrieb Marek Olšák:
> 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.
Yes the sampler_view name is more due to historical reasons.
That said imho the name of that struct isn't really as important as how
they are created. So you'd use either create_surface or
create_sampler_view today. And create_surface() was really meant to
create views for render / depth stencil targets, and
create_sampler_views() was really meant to create views for texture
sampling. But those gallium shader resources (d3d11 UAVs) aren't really
neither though I guess they are indeed closer to surfaces (they can be
read/write, and can't have multiple mip levels). But OTOH I guess with
modern hardware such a pipe_surface is little more than just a means to
pass a bunch of values together with the resource (that is, the driver
doesn't really care in create_surface() if the resource had
BIND_RENDER_TARGET or BIND_SHADER_RESOURCE flag).
I am actually wondering though why are both set_compute_resources() and
set_shader_resources() needed? (I am not sure how d3d11 actually sets
uavs for compute shaders, that setrendertarget stuff seems a bit
inappropriate...) Does it make a difference if you bind these things to
a compute shader or a fragment shader? At least the views for uavs are
the same in d3d11 for these two, as are the resource bind flags if I see
that right (though there's some more flags in the views but only for
buffers - raw/append/counter).
>
> 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.
Oh yes I'm certainly not suggesting we do anything like that (fwiw you
wouldn't actually need 12 slots in d3d11, since as you mentioned pixel
shader outputs and uav slots are shared, but restricted to 8 together).
It just should be reasonably easy to translate from that (and it should
be from what I can tell).
Roland
>
> 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=
>>>
>>
More information about the mesa-dev
mailing list