[Mesa-dev] [PATCH 3/3] i965 Gen6+: Invalidate VF address-based cache on flush

[Kenneth Graunke]

On 12/22/2011 02:06 PM, Paul Berry wrote:
> Although there is not much documentation of this fact, there are in
> fact two separate VF caches:
> 
> - an "index-based" cache (described in the Sandy Bridge PRM, vol 2
>   part 1, section 2.1.2 "Vertex Cache").  This cache stores URB
>   handles of vertex shader outputs; its purpose is to avoid redundant
>   invocations of the vertex shader when drawing in random access mode
>   (e.g. glDrawElements()), and the same vertex index is specified
>   multiple times.  It is automatically invalidated between
>   3D_PRIMITIVE commands and between instances within a single
>   3D_PRIMITIVE command.
> 
> - an "address-based" cache (mentioned briefly in vol 2 part 1, section
>   1.7.4 "PIPE_CONTROL Command").  This cache stores the data read from
>   vertex buffers; its purpose is to avoid redundant memory accesses
>   when doing instanced drawing or when multiple 3D_PRIMITIVE commands
>   access the same vertex data.  It needs to be manually invalidated
>   whenever new data is written to a buffer that is used for vertex
>   data.
> 
> Previous to this patch, it was not necessary for Mesa to explicitly
> invalidate the address-based cache, because there were no reasonable
> use cases in which the GPU would write to a vertex data buffer during
> a batch, and inter-batch flushing was taken care of by the kernel.
> 
> However, with transform feedback, there is now a reasonable use case:
> vertex data is written to a buffer using transform feedback, and then
> that data is immediately re-used as vertex input in the next drawing
> operation.  To make this use case work, we need to flush the
> address-based VF cache between transform feedback and the next draw
> operation.  Since we are already calling
> intel_batchbuffer_emit_mi_flush() when transform feedback completes,
> and intel_batchbuffer_emit_mi_flush() is intended to invalidate all
> caches, it seems reasonable to add VF cache invalidation to this
> function.
> 
> As with commit 63cf7fad13fc9cfdd2ae7b031426f79107000300 (i965: Flush
> pipeline on EndTransformFeedback), this is not an ideal solution.  It
> would be preferable to only invalidate the VF cache 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 the necessary dependency
> tracking infrastructure to figure that out right now, we have to
> overzealously invalidate the cache.
> 
> Fixes Piglit test "EXT_transform_feedback/immediate-reuse".
> ---
>  src/mesa/drivers/dri/intel/intel_batchbuffer.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> index 4ff098a..cb23dbc 100644
> --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> @@ -460,6 +460,7 @@ intel_batchbuffer_emit_mi_flush(struct intel_context *intel)
>  	 OUT_BATCH(PIPE_CONTROL_INSTRUCTION_FLUSH |
>  		   PIPE_CONTROL_WRITE_FLUSH |
>  		   PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +                   PIPE_CONTROL_VF_CACHE_INVALIDATE |
>  		   PIPE_CONTROL_TC_FLUSH |
>  		   PIPE_CONTROL_NO_WRITE |
>                     PIPE_CONTROL_CS_STALL);

I checked the workaround list, and it doesn't look like there are any
workarounds needed for VF (address based) Cache invalidation.  Plus, we
now do that in the kernel inbetween every batch, so I'm not concerned
about adding it here.

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