[Mesa-dev] i965 gen6: Pass-through GS program for future use by transform feedback

[Paul Berry]

On 7 December 2011 00:50, Kenneth Graunke <kenneth at whitecape.org> wrote:

> On 12/05/2011 09:40 AM, Paul Berry wrote:
> > This patch series introduces a geometry shader program for i965 Gen6
> > (Sandy Bridge) that does nothing--it simply passes vertices through to
> > later stages of the graphics pipeline.  This is a preliminary step
> > towards implementing transform feedback, since on Gen6, transform
> > feedback is accomplished by having the geometry shader write vertices
> > to memory.
> >
> > Patch 01/10 is a core mesa patch--it introduces a new bit to the
> > ctx->NewState bitfield to track when changes to the transform feedback
> > setup have occurred.  This allows the i965 back-end to avoid
> > unnecessary recomputations when the transform feedback setup is
> > unchanged.
> >
> > Patches 02/10-09/10 refactor the i965 driver in preparation for adding
> > the new geometry shader program.  In particular, they clean up the old
> > (Gen4-Gen5) geometry shader code so that it can be expanded, and fix
> > some code generation bugs that had previously been benign.  Note:
> > patches 06/10 and 07/10 are from Kenneth Graunke's "gs2" branch, which
> > I used as the starting point for my development.  They ensure that URB
> > space is allocated for the GS unit so that the geometry shader program
> > can run.  Down the road, we may want to tweak the URB space allocation
> > to improve performance, but that seems like it should wait until
> > transform feedback functionality works.
> >
> > Patch 10/10 adds the pass-through program itself.  At the moment it is
> > not used (since transform feedback support isn't enabled by default
> > yet), however it can be tested by setting the "INTEL_FORCE_GS"
> > environment variable.
> >
> > [PATCH 01/10] mesa: Track changes to transform feedback state.
>
> I agree with Eric's assessment about FLUSH_VERTICES.  With that change,
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> > [PATCH 02/10] i965 gs: Reduce information in key to avoid unnecessary
> recompiles.
>
> No comment here; sounds like you and Eric have it figured out.
>
> > [PATCH 03/10] i965 gs: Remove unnecessary mapping of key->primitive.
> > [PATCH 04/10] i965: Don't convert if/else to conditional adds on Gen6.
> > [PATCH 05/10] i965: Fix convert_IF_ELSE_to_ADD for gen7.
>
> These three are
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>

With regard to patch 04/10, I assume your review includes the change that
extends this patch to Gen>=6 rather than Gen==6, right?

Patch 05/10 is moot once patch 04/10 is extended to Gen>=6 so I'm just
going to drop it.


>
> > [PATCH 06/10] i965: Set the maximum number of GS URB entries on
> Sandybridge.
> > [PATCH 07/10] i965: Initial stab at GS URB space allocation.
>
> No comment here.
>
> > [PATCH 08/10] i965 gs: Clean up dodgy register re-use, at the cost of a
> few MOVs.
>
> Had comments/ideas; would love to see feedback from others.  Still, it
> looks correct, so you get a Reviewed-by.
>
> > [PATCH 09/10] i965: Clean up misleading defines for DWORD 2 of URB_WRITE
> header.
>
> Made a small suggestion but either way you get a Reviewed-by.
>
> > [PATCH 10/10] i965 gen6: Implement pass-through GS for transform
> feedback.
>
> Aside from the scary lack of GEN6_GS_RENDERING_ENABLE, you get a R-b.
>
> Thanks, Paul.  This looks like a great series.
>

Thanks, Ken.  With your review I believe everything is covered except for
the compromise Eric and I arrived at for patch 07/10.  Since I'd like to
get some review on that, I'm going to go ahead and post a v2 of the series
just so people can see it in context.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111207/f87e4d6c/attachment-0001.htm>