[Mesa-dev] [PATCH] gallium: remove set_shader_resources, add set_shader_buffers for untyped buffers

[Marek Olšák]

On Wed, Jan 7, 2015 at 2:41 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Wed, Jan 7, 2015 at 5:56 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> set_shader_resources is unused.
>>
>> set_shader_buffers should support shader atomic counter buffers and shader
>> storage buffers from OpenGL.
>>
>> The plan is to use slots 0..15 for atomic counters and slots 16..31
>> for storage buffers. Atomic counters are planned to be supported first.
>>
>> This doesn't add any interface for images. The documentation is added
>> for future reference.
>> ---
>>
>> This is the interface only. I don't plan to do anything else for now.
>> Comments welcome.
>
> Can you clarify how this is better than the set_shader_resources
> interface, which can also be shared for images (which will need to
> support texture buffers...)?

1) You don't need to create any views for these. Creating,
initializing, referencing, and destroying views is work that should be
avoided if it's unnecessary.

2) It saves space for resource descriptions on SI (both memory and
cache). A buffer slot needs 4 dwords, but a texture (image) slot needs
8 dwords.

Original DX11 AMD hardware (Evergreen) will have to merge
set_shader_buffers, set_shader_images, and set_framebuffer_state
anyway. One less function won't make it much easier. Post-DX11
hardware (SI) can do pretty much anything, but this solution is more
efficient for that hardware.

>
> FWIW, there's already an impl for nve4 images using
> set_shader_resources (not sure how Christoph had tested it, I think
> using some preliminary OpenCL C -> TGSI converter with image support).
>
> Are these buffers fundamentally different than images? We'll still
> need atomic support for images as well...

The main difference is:
- shader buffers don't have a view and format. pipe_resources are set directly.
- shader images have a view and format, this also includes buffers
that have a format.

>
> Also how do you anticipate this will be integrated into TGSI? Right
> now there's a TGSI_FILE_RESOURCE -- will there be a new
> TGSI_FILE_BUFFER and TGSI_FILE_IMAGE?

Yes, this needs to be changed as well.

Opinions?

Marek