On Thu, Aug 14, 2014 at 4:11 AM, Iago Toral Quiroga <ito...@igalia.com> wrote: > Hi, > > this series brings support for geometry shaders in Sandy Bridge (gen6) and is > combined work from Samuel and myself. A few notes: > > 1.- Some patches have been based on original work by Ilia Mirkin, specifically > the idea of using arrays to buffer the output of the GS, subclassing the > vec4_gs_visitor for gen6 and generalizing emit_urb_slot(). > > 2.- Geometry shaders were already being used in gen6 to implement transform > feedback support for vertex shaders. We have not changed this. These patches > focus on adding support for user-provided geometry shaders and transform > feedback support for the geometry shader stage. In the future it probably > makes sense to merge transform feedback support for the vertex shader stage > in our implementation so there is only one code path for geometry shaders > in gen6, but it is probably better to tackle that at a later moment, once we > have merged this work. > > 2.- On Ivy Bridge there are no piglit regressions. > > 3.- On Sandy Bridge we get these results after enabling OpenGL 3.2 and > GLSL 1.50 (*1): > > crash: +0 > fail: +15 (*2) > pass: +3265 > skip: -3280
Maybe a list of the failures? Or posting the piglit comparison results might be helpful. For example: http://people.freedesktop.org/~kwg/stuff/bdw-2014-05-13/summary/regressions.html This is not really a big deal, but it would just be nice to quickly see what tests are failing. > (*1) Including Jordan's patches from the series > "Gen6 render surface state changes" since these are required to enable > layered rendering in geometry shaders. The numbers were obtained by comparing > master with Jordan's patches on top (OpenGL 3.1, GLSL 1.40) against master > with these and Jordan's patches on top (OpenGL 3.2, GLSL 1.50) I finally pushed my gen6-layered series to master. (a1dca70) I wonder if you might push these patches to a publicly available branch? Thanks! -Jordan > (*2) These are mostly tests that either fail in Ivy Bridge too, are GS > variants of tests that also fail for the VS/FS stages or relate to other > aspects of OpenGL 3.2 that are not related with geometry shaders. > > 4.- With these patches, the following piglit test hangs: > bin/glsl-1.50-geometry-primitive-id-restart GL_TRIANGLE_STRIP_ADJACENCY > > This problem seems to be unrelated to our implementation, since the hang > happens only for that primitive type, only when using glDrawElements() > (so glDrawArrays works fine), and only in specific cases where the list > of indices provided includes repeated indices with a certain pattern. > Actually, > this test hangs even if we have a geometry shader that does nothing (i.e. an > empty main function), where the code we generate is trivial and works with > any other primitive type. Based on this, I conclude that this is a problem > originating somewhere else, I think probably a hardware bug. Because of this, > piglit runs with these patches should exclude this test by including > "-x primitive-id-restart". The offending piglit test can be trivially reworked > to avoid repeating indices in the call to glDrawElements() too. I'll > develop this issue further in another thread so we can decide what to do about > this problem. > > I'll be on holidays for the next two weeks, starting tomorrow, but Samuel will > be around since Tuesday next week so he can start acting on the review > feedback > we get. > > A quick summary of the patches: > > - Patch 1: is actually about gen7, but since gen6's dispatch mode for geometry > shaders is equivalent to gen7's SINGLE mode it makes sense to do this first. > - Patches 2-4 refactor 3DSTATE_GS to accomodate the code path for > user-provided > geometry shaders while keeping the original code that handles TF support > in vertex shaders. > - Patches 5-13 implement generator opcodes, configure state packets and > handle required URB space. > - Patches 14-15 generalize emit_urb_slot() so we can reuse that code. > - Patches 16-19 are the gen6 geometry shader visitor implementation. > - Patches 20-21 implement gl_PrimitiveIDIn. > - Patch 22 makes sure we compute the right VUE map for user-provided GS. > - Patch 23 enables texture related functions in the GS stage. > - Patches 24-33 mostly implement transform feedback > - Patch 34 handles uploading of ubo and pull constant surfaces > - Patch 35 makes gen6 use this implementation of geometry shaders > - Patches 36-37 enable GLSL 1.5 and OpenGL 3.2 in gen6 > > Iago Toral Quiroga (23): > i965/gs: Use single dispatch mode as fallback to dual object mode when > possible. > i965/gen6/gs: Setup constant push buffers for gen6 geometry shaders. > i965/gen6/gs: Implement GS_OPCODE_FF_SYNC. > i965/gen6/gs: Implement GS_OPCODE_URB_WRITE_ALLOCATE. > i965/gen6/gs: Add instruction URB flags to geometry shaders EOT > message. > i965/gen6/gs: Compute URB entry size for user-provided geometry > shaders. > i965/gen6/gs: Enable URB space for user-provided geometry shaders. > i965/gen6/gs: Upload binding table for user-provided geometry shaders. > i965/gen6/gs: Implement GS_OPCODE_SET_DWORD_2. > i965: Provide means to create registers of a given size. > i965: Generalize emit_urb_slot() to emit to any dst_reg. > i965/gen6/gs: Add initial implementation for a gen6 geometry shader > visitor. > i965/gen6/gs: Implement geometry shaders for outputs other than > points. > i965/gen6/gs: Make sure we complete the last primitive. > i965/gen6/gs: Handle the case where a geometry shader emits no output. > i965/gen6/gs: Implement GS_OPCODE_SET_PRIMITIVE_ID. > i965/gen6/gs: Implement support for gl_PrimitiveIdIn. > i965/gen6/gs: Assign geometry shader VUE map properly. > i965/gen6/gs: Enable texture units and upload sampler state. > i965/gen6/gs: Avoid buffering transform feedback varyings twice. > i965/gen6/gs: Fix binding table clash between TF surfaces and > textures. > i965/gen6/gs: upload ubo and pull constants surfaces. > i965/gen6/gs: Use a specific implementation of geometry shaders for > gen6. > > Samuel Iglesias Gonsalvez (14): > i965/gen6/gs: refactor gen6_gs_state > i965/gen6/gs: use brw_gs_prog atom instead of brw_ff_gs_prog > i965/gen6/gs: Set brw->gs.enabled to FALSE in > gen6_blorp_emit_gs_disable() > i965/gs: Reuse gen6 constant push buffers setup code in gen7+. > i965/gen6/gs: implement GS_OPCODE_SVB_WRITE opcode > i965/gen6/gs: implement GS_OPCODE_SVB_SET_DST_INDEX opcode > i965/gen6/gs: implement GS_OPCODE_FF_SYNC_SET_PRIMITIVES opcode > i965/gen6/gs: Add an additional parameter to the FF_SYNC opcode. > i965/gen6/gs: implement transform feedback support in gen6_gs_visitor > i965/gen6/gs: Setup SOL surfaces for user-provided geometry shaders > i965/gen6/gs: Buffer PSIZ/flags vertex data in gen6_gs_visitor > i965/gen6/gs: Enable transform feedback support in geometry shaders > i965/gen6: enable GLSL 1.50 > i965/gen6: enable OpenGL 3.2 > > src/mesa/drivers/dri/i965/Makefile.sources | 1 + > src/mesa/drivers/dri/i965/brw_binding_tables.c | 5 +- > src/mesa/drivers/dri/i965/brw_context.c | 2 +- > src/mesa/drivers/dri/i965/brw_context.h | 121 ++-- > src/mesa/drivers/dri/i965/brw_defines.h | 86 ++- > src/mesa/drivers/dri/i965/brw_gs.c | 4 + > src/mesa/drivers/dri/i965/brw_gs.h | 1 + > src/mesa/drivers/dri/i965/brw_shader.cpp | 14 + > src/mesa/drivers/dri/i965/brw_state.h | 1 + > src/mesa/drivers/dri/i965/brw_state_upload.c | 10 +- > src/mesa/drivers/dri/i965/brw_vec4.cpp | 9 +- > src/mesa/drivers/dri/i965/brw_vec4.h | 28 +- > src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 199 +++++- > src/mesa/drivers/dri/i965/brw_vec4_gs.c | 102 ++- > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 76 ++- > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h | 2 +- > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 44 +- > src/mesa/drivers/dri/i965/brw_vs.c | 2 +- > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 1 + > src/mesa/drivers/dri/i965/gen6_gs_state.c | 161 ++++- > src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp | 775 > ++++++++++++++++++++++ > src/mesa/drivers/dri/i965/gen6_gs_visitor.h | 83 +++ > src/mesa/drivers/dri/i965/gen6_sampler_state.c | 2 +- > src/mesa/drivers/dri/i965/gen6_sol.c | 155 +++-- > src/mesa/drivers/dri/i965/gen6_urb.c | 30 +- > src/mesa/drivers/dri/i965/gen7_gs_state.c | 37 +- > src/mesa/drivers/dri/i965/gen8_gs_state.c | 4 +- > src/mesa/drivers/dri/i965/intel_extensions.c | 2 +- > src/mesa/drivers/dri/i965/intel_screen.c | 2 +- > 29 files changed, 1714 insertions(+), 245 deletions(-) > create mode 100644 src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp > create mode 100644 src/mesa/drivers/dri/i965/gen6_gs_visitor.h > > -- > 1.9.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev