[Mesa-dev] [PATCH 7/8] i965 gen6+: Make intel_batchbuffer_emit_mi_flush() actually flush.

[Kenneth Graunke]

On 12/13/2011 03:35 PM, Paul Berry wrote:
> Previous to this patch, the function intel_batchbuffer_emit_mi_flush()
> was a bit of a misnomer.  On Gen4+, when not using the blit engine, it
> didn't actually flush the pipeline--it simply generated a
> _3DSTATE_PIPE_CONTROL command with the necessary bits set to flush GPU

It's actually just called "PIPE_CONTROL", never 3DSTATE_PIPE_CONTROL.

> caches.  This was usually sufficient, since in most situations where
> intel_batchbuffer_emit_mi_flush() wass called, all we really care

"... was called, all we really cared" (typo, tense)

This makes a lot of sense.  At some point, I think we ought to convert
some of the callers of this function to emit the proper PIPE_CONTROL
directly (with only the necessary bits and workarounds).  But this is
the right way to start.  Nice work figuring this out.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> about was ensuring cache coherency.
> 
> However, with the advent of OpenGL 3.0, there are two cases in which
> data output by one stage of the pipeline might be consumed, in a later
> draw operation, by an earlier stage of the pipeline:
> 
> (a) When using textures in the vertex shader.
> 
> (b) When using drawing with a vertex buffer that was previously
>     generated using transform feedback.
> 
> This patch addresses case (a) by changing
> intel_batchbuffer_emit_mi_flush() so that on Gen6+, it sets the
> PIPE_CONTROL_CS_STALL bit (this forces the pipeline to actually
> flush).  (Case (b) will be addressed by the next patch in the series).
> 
> This is not an ideal solution--in a perfect world, the driver would
> have some buffer dependency tracking so that we would only have to
> flush the pipeline in the two cases above.  Until that dependency
> tracking is implemented, however, it seems prudent to have
> intel_batchbuffer_emit_mi_flush() actually flush the pipeline, so that
> we get correct rendering, at the expense of a (hopefully small)
> performance hit.
> 
> The change is only applied to Gen6+, since at the moment only Gen6+
> supports the OpenGL 3.0 features that make a full pipeline flush
> necessary.
> ---
>  src/mesa/drivers/dri/intel/intel_batchbuffer.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> index 6991db8..4ff098a 100644
> --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> @@ -461,7 +461,8 @@ intel_batchbuffer_emit_mi_flush(struct intel_context *intel)
>  		   PIPE_CONTROL_WRITE_FLUSH |
>  		   PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>  		   PIPE_CONTROL_TC_FLUSH |
> -		   PIPE_CONTROL_NO_WRITE);
> +		   PIPE_CONTROL_NO_WRITE |
> +                   PIPE_CONTROL_CS_STALL);
>  	 OUT_BATCH(0); /* write address */
>  	 OUT_BATCH(0); /* write data */
>  	 ADVANCE_BATCH();