[Mesa-dev] [PATCH 1/4] i965/gen6: Factor the PIPE_CONTROL workaround to a separate function.

[Kenneth Graunke]

On 06/17/2011 06:46 PM, Eric Anholt wrote:
> We're need this workaorund a lot more than we're currently doing, so
> let's reuse it.
> ---
>   src/mesa/drivers/dri/intel/intel_batchbuffer.c |   29 +++++++++++++++++------
>   1 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> index 77edc3a..250102e 100644
> --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
> @@ -276,6 +276,17 @@ emit:
>      item->header = intel->batch.emit;
>   }
>
> +static void
> +intel_emit_post_sync_nonzero_flush(struct intel_context *intel)
> +{
> +   BEGIN_BATCH(4);
> +   OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> +   OUT_BATCH(PIPE_CONTROL_WRITE_IMMEDIATE);
> +   OUT_BATCH(0); /* write address */
> +   OUT_BATCH(0); /* write data */
> +   ADVANCE_BATCH();
> +}
> +
>   /* Emit a pipelined flush to either flush render and texture cache for
>    * reading from a FBO-drawn texture, or flush so that frontbuffer
>    * render appears on the screen in DRI1.
> @@ -294,15 +305,17 @@ intel_batchbuffer_emit_mi_flush(struct intel_context *intel)
>   	 OUT_BATCH(0);
>   	 ADVANCE_BATCH();
>         } else {
> -	 BEGIN_BATCH(8);
> -	 /* XXX workaround: issue any post sync != 0 before write
> -	  * cache flush = 1
> -	  */
> -	 OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> -	 OUT_BATCH(PIPE_CONTROL_WRITE_IMMEDIATE);
> -	 OUT_BATCH(0); /* write address */
> -	 OUT_BATCH(0); /* write data */
> +	 if (intel->gen == 6) {

Before this patch, the workaround was applied on Ivybridge as well.  I 
haven't tested to see whether it's necessary, but since it's documented 
as a Sandybridge workaround, this seems sensible.

> +	    /* Hardware workaround: SNB B-Spec says:

Just for the record, this is in Volume 2, Part 1, Section 1.7.4.1 (page 
63) of the public Sandybridge PRM.  Though pdfgrep "Before a 
PIPE_CONTROL" finds it just fine.  I'd prefer to start citing the public 
documentation where possible instead of the BSpec.

> +	     *
> +	     * [Dev-SNB{W/A}]: Before a PIPE_CONTROL with Write Cache
> +	     * Flush Enable =1, a PIPE_CONTROL with any non-zero
> +	     * post-sync-op is required.
> +	     */
> +	    intel_emit_post_sync_nonzero_flush(intel);
> +	 }
>
> +	 BEGIN_BATCH(4);
>   	 OUT_BATCH(_3DSTATE_PIPE_CONTROL);
>   	 OUT_BATCH(PIPE_CONTROL_INSTRUCTION_FLUSH |
>   		   PIPE_CONTROL_WRITE_FLUSH |

Thanks for citing the spec and removing the confusing BEGIN_BATCH(8).

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