[Mesa-dev] [PATCH 8/8] i965: Flush pipeline on EndTransformFeedback.

[Kenneth Graunke]

On 12/13/2011 03:35 PM, Paul Berry wrote:
> A common use case for transform feedback is to perform one draw
> operation that writes transform feedback output to a buffer, followed
> by a second draw operation that consumes that buffer as vertex input.
> Since vertex input is consumed at an earlier pipeline stage than
> writing transform feedback output, we need to flush the pipeline to
> ensure that the transform feedback output is completely written before
> the data is consumed.
> 
> In an ideal world, we would do some dependency tracking, so that we
> would only flush the pipeline if the next draw call was about to
> consume data generated by a previous draw call in the same batch.
> However, since we don't have that sort of dependency tracking
> infrastructure right now, we just unconditionally flush the buffer
> every time glEndTransformFeedback() is called.  This will cause a
> performance hit compared to the ideal case (since we will sometimes
> flush the pipeline unnecessarily), but fortunately the performance hit
> will be confined to circumstances where transform feedback is in use.
> ---
>  src/mesa/drivers/dri/i965/brw_context.c |    8 ++++++--
>  src/mesa/drivers/dri/i965/brw_context.h |    5 +++++
>  src/mesa/drivers/dri/i965/gen6_sol.c    |   16 ++++++++++++++++
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index eb68bf4..f7b88c3 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -52,6 +52,7 @@
>  
>  #include "tnl/t_pipeline.h"
>  #include "glsl/ralloc.h"
> +#include "intel_batchbuffer.h"
>  
>  /***************************************
>   * Mesa's Driver Functions
> @@ -109,7 +110,8 @@ static void brwPrepareExecBegin(struct gl_context *ctx)
>     }
>  }
>  
> -static void brwInitDriverFunctions( struct dd_function_table *functions )
> +static void brwInitDriverFunctions(struct intel_context *intel,
> +                                   struct dd_function_table *functions)
>  {
>     intelInitDriverFunctions( functions );
>  
> @@ -117,6 +119,8 @@ static void brwInitDriverFunctions( struct dd_function_table *functions )
>     brw_init_queryobj_functions(functions);
>  
>     functions->PrepareExecBegin = brwPrepareExecBegin;
> +   if (intel->gen == 6)
> +      functions->EndTransformFeedback = gen6_end_transform_feedback;

Unfortunately, this doesn't work: brwInitDriverFunctions is called
before intelInitContext, so intel->gen isn't set yet.  So this function
pointer never gets set, and gen6_end_transform_feedback never happens.

I blithely tried moving the brwInitDriverFunctions call after
intelInitContext, but that crashes.  Also, setting anything in
"functions" after intelInitContext simply doesn't take; something
must've copied the table (remapping code most likely).  In other words,
a generation check just isn't going to happen.

I think the best solution is to set the function pointer unconditionally
and add

   if (intel->gen < 6)
      return;

to the top of gen6_end_transform_feedback so it becomes a no-op on
earlier platforms.

Sorry for suggesting the generation check earlier; I didn't realize it
was impossible.

>  }
>  
>  bool
> @@ -136,7 +140,7 @@ brwCreateContext(int api,
>        return false;
>     }
>  
> -   brwInitDriverFunctions( &functions );
> +   brwInitDriverFunctions(intel, &functions);
>  
>     if (!intelInitContext( intel, api, mesaVis, driContextPriv,
>  			  sharedContextPrivate, &functions )) {
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index da1de02..fa6c883 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1071,6 +1071,11 @@ void brw_init_surface_formats(struct brw_context *brw);
>  bool
>  brw_fprog_uses_noperspective(const struct gl_fragment_program *fprog);
>  
> +/* gen6_sol.c */
> +void
> +gen6_end_transform_feedback(struct gl_context *ctx,
> +                            struct gl_transform_feedback_object *obj);
> +
>  
>  
>  /*======================================================================
> diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c b/src/mesa/drivers/dri/i965/gen6_sol.c
> index af372c1..53e3325 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sol.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sol.c
> @@ -28,6 +28,7 @@
>  
>  #include "brw_context.h"
>  #include "intel_buffer_objects.h"
> +#include "intel_batchbuffer.h"
>  #include "brw_defines.h"
>  
>  static void
> @@ -100,3 +101,18 @@ const struct brw_tracked_state gen6_sol_surface = {
>     },
>     .emit = brw_update_sol_surfaces,
>  };
> +
> +void
> +gen6_end_transform_feedback(struct gl_context *ctx,
> +                            struct gl_transform_feedback_object *obj)
> +{
> +   /* After EndTransformFeedback, it's likely that the client program will try
> +    * to draw using the contents of the transform feedback buffer as vertex
> +    * input.  In order for this to work, we need to flush the data through at
> +    * least the GS stage of the pipeline, and flush out the render cache.  For
> +    * simplicity, just do a full flush.
> +    */
> +   struct brw_context *brw = brw_context(ctx);
> +   struct intel_context *intel = &brw->intel;
> +   intel_batchbuffer_emit_mi_flush(intel);
> +}