[Mesa-dev] [PATCH 02/16] radeonsi: Initial geometry shader support

[Michel Dänzer]

On Sam, 2014-01-25 at 02:21 +0100, Marek Olšák wrote:
> On Fri, Jan 24, 2014 at 10:19 PM, Marek Olšák <maraeo at gmail.com> wrote:
> > 1) Do we really have to have so much code inside #if...#endif? Can we
> > always compile everything and just not set the pipe_context functions
> > if LLVM < 3.5 and the same for get_(shader_)param? You can even do it
> > without the preprocessor, e.g. if (HAVE_LLVM >= 0x0305) ...

Okay, dropped most of them in the v2 patches.


> > 2) Sooner or later we'll have GL_ARB_shader_storage_buffer_object,
> > which adds shader read-write resources. Maybe it would be better to
> > put the ring buffers in a new array of descriptors and leave the
> > constant buffers with the READ-only usage. The new array of
> > descriptors could later be used for OpenGL read-write resources in
> > addition to being used for ring buffers.
> 
> Actually, my original plan was to put stream-out buffers after OpenGL
> shader-storage buffers. We'll probably want to use the same array for
> OpenGL shader atomic counters as well.

Makes sense, added patch 17 which puts the ring buffer descriptors
together with the streamout buffer descriptors.


> 3) In si_init_gs_rings:
> - could you please use readable decimal numbers for specifying the
> sizes? Like 1024 * 1024 * ...
[...]
> - isn't 64 MB too many for a ring buffer?

I can write the numbers any way you like. :) But I just copied them from
the corresponding r600g patches; I don't know yet how these numbers were
derived, or what the constraints are for the ring buffer sizes. I'm
trying to find out more about this.


> - could you please add an assertion to make sure the function is not
> called twice to prevent potential memory leaks

Done in v2.


> 4) If the GS is disabled, GS_OUT_PRIM_TYPE should be compatible with
> the draw_info primitive for transform feedback to work properly. It
> was a requirement on R600-Evergreen. Not sure if it applies to SI too.

Doesn't seem to make a difference, but I guess it can't hurt, done in v2.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer