[Mesa-dev] [PATCH 3/6] i965/gen6-7: Implement stall and flushes required prior to switching pipelines.

[Kenneth Graunke]

On Saturday, January 2, 2016 10:48:02 PM PST Francisco Jerez wrote:
> Switching the current pipeline while it's not completely idle or the
> read and write caches aren't flushed can lead to corruption.  Fixes
> misrendering of at least the following Khronos CTS test:
> 
>  ES31-CTS.shader_image_load_store.basic-allTargets-store-fs
> 
> The stall and flushes are no longer required on Gen8+.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93323
> ---
>  src/mesa/drivers/dri/i965/brw_misc_state.c | 28 +++++++++++++++++++++++++++
+
>  1 file changed, 28 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/
dri/i965/brw_misc_state.c
> index 7d53d18..75540c1 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -886,6 +886,34 @@ brw_emit_select_pipeline(struct brw_context *brw, enum 
brw_pipeline pipeline)
>  
>           brw->ctx.NewDriverState |= BRW_NEW_CC_STATE;
>        }
> +
> +   } else if (brw->gen >= 6) {
> +      /* From "BXML » GT » MI » vol1a GPU Overview » [Instruction]
> +       * PIPELINE_SELECT [DevBWR+]":

Can we cite the public docs?

> +       *
> +       *   Project: DEVSNB+
> +       *
> +       *   Software must ensure all the write caches are flushed through a
> +       *   stalling PIPE_CONTROL command followed by another PIPE_CONTROL
> +       *   command to invalidate read only caches prior to programming
> +       *   MI_PIPELINE_SELECT command to change the Pipeline Select Mode.
> +       */
> +      const unsigned dc_flush =
> +         brw->gen >= 7 ? PIPE_CONTROL_DATA_CACHE_INVALIDATE : 0;

I was going to suggest doing a brw_emit_post_sync_nonzero_flush first
on Sandybridge, but I forgot that we now just emit that at the start
of every state upload.  Fairly moot anyway since we don't do GPGPU on
Sandybridge anyway.

> +
> +      brw_emit_pipe_control_flush(brw,
> +                                  PIPE_CONTROL_RENDER_TARGET_FLUSH |
> +                                  PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +                                  dc_flush |
> +                                  PIPE_CONTROL_NO_WRITE |
> +                                  PIPE_CONTROL_CS_STALL);

Why RENDER_TARGET_FLUSH, DEPTH_CACHE_FLUSH, DATA_CACHE_INVALIDATE,
and NO_WRITE?  The cited workaround explains a CS Stall and the RO
invalidations below, but I'm not seeing why the others are needed.

> +
> +      brw_emit_pipe_control_flush(brw,
> +                                  PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE |
> +                                  PIPE_CONTROL_CONST_CACHE_INVALIDATE |
> +                                  PIPE_CONTROL_STATE_CACHE_INVALIDATE |
> +                                  PIPE_CONTROL_INSTRUCTION_INVALIDATE |
> +                                  PIPE_CONTROL_NO_WRITE);
>     }
>  
>     /* Select the pipeline */
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160103/242cabb8/attachment.sig>