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

[Ilia Mirkin]

On Wed, Jan 7, 2015 at 9:35 AM, Marek Olšák <maraeo at gmail.com> wrote:
> 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.

I guess you mean surfaces? You still have to bind a reference to the
backing buffer _somewhere_...

>
> 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?

OK, well, this interface also seems workable. From what I can tell,
nve0 (kepler) is more similar to radeonsi in this regard, and nv50
isn't realistically going to gain support for this (blob driver
doesn't either). The wildcard is nvc0, which I haven't really traced
for image stuff yet. I guess instructions like LOAD/STORE/ATOM* would
be able to take either IMAGE or BUFFER things? Or separate instruction
variants?

This does, however, present an asymmetry to the compute interface,
which currently just has

   void (*set_compute_resources)(struct pipe_context *,
                                 unsigned start, unsigned count,
                                 struct pipe_surface **resources);

Should that be changed over to the buffer/image interface as well?

  -ilia