[Mesa-dev] [PATCH 03/10] i965: Create a helper function for emitting PIPE_CONTROL flushes.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> These days, we need to emit PIPE_CONTROL flushes all over the place.
> Being able to do that via a single function call seems convenient.
>
> Broadwell will also increase the length of these packets by 1; with the
> refactoring, we should have to do this in substantially fewer places.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>

> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index d9b6c15..d2f0e90 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -475,6 +475,32 @@ emit:
>  }
>  
>  /**
> + * Emit a PIPE_CONTROL with various flushing flags.
> + *
> + * The caller is responsible for deciding what flags are appropriate for the
> + * given generation.
> + */
> +void
> +brw_emit_pipe_control_flush(struct brw_context *brw, uint32_t flags)
> +{
> +   if (brw->gen >= 6) {
> +      BEGIN_BATCH(4);
> +      OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> +      OUT_BATCH(flags);
> +      OUT_BATCH(0);
> +      OUT_BATCH(0);
> +      ADVANCE_BATCH();
> +   } else {
> +      BEGIN_BATCH(4);
> +      OUT_BATCH(_3DSTATE_PIPE_CONTROL | flags | (4 - 2));
> +      OUT_BATCH(0);
> +      OUT_BATCH(0);
> +      OUT_BATCH(0);
> +      ADVANCE_BATCH();
> +   }
> +}
> +
> +/**
>   * Restriction [DevSNB, DevIVB]:
>   *
>   * Prior to changing Depth/Stencil Buffer state (i.e. any combination of
> @@ -491,26 +517,9 @@ intel_emit_depth_stall_flushes(struct brw_context *brw)
>  {
>     assert(brw->gen >= 6 && brw->gen <= 7);
>  
> -   BEGIN_BATCH(4);
> -   OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> -   OUT_BATCH(PIPE_CONTROL_DEPTH_STALL);
> -   OUT_BATCH(0); /* address */
> -   OUT_BATCH(0); /* write data */
> -   ADVANCE_BATCH()
> -
> -   BEGIN_BATCH(4);
> -   OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> -   OUT_BATCH(PIPE_CONTROL_DEPTH_CACHE_FLUSH);
> -   OUT_BATCH(0); /* address */
> -   OUT_BATCH(0); /* write data */
> -   ADVANCE_BATCH();
> -
> -   BEGIN_BATCH(4);
> -   OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> -   OUT_BATCH(PIPE_CONTROL_DEPTH_STALL);
> -   OUT_BATCH(0); /* address */
> -   OUT_BATCH(0); /* write data */
> -   ADVANCE_BATCH();
> +   brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
> +   brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_CACHE_FLUSH);
> +   brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
>  }
>  
>  /**
> @@ -608,13 +617,8 @@ intel_emit_post_sync_nonzero_flush(struct brw_context *brw)
>     if (!brw->batch.need_workaround_flush)
>        return;
>  
> -   BEGIN_BATCH(4);
> -   OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> -   OUT_BATCH(PIPE_CONTROL_CS_STALL |
> -	     PIPE_CONTROL_STALL_AT_SCOREBOARD);
> -   OUT_BATCH(0); /* address */
> -   OUT_BATCH(0); /* write data */
> -   ADVANCE_BATCH();
> +   brw_emit_pipe_control_flush(brw, PIPE_CONTROL_CS_STALL |
> +                               PIPE_CONTROL_STALL_AT_SCOREBOARD);
>  
>     BEGIN_BATCH(4);
>     OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> @@ -636,46 +640,22 @@ intel_emit_post_sync_nonzero_flush(struct brw_context *brw)
>  void
>  intel_batchbuffer_emit_mi_flush(struct brw_context *brw)
>  {
> -   if (brw->gen >= 6) {
> -      if (brw->batch.ring == BLT_RING) {
> -	 BEGIN_BATCH_BLT(4);
> -	 OUT_BATCH(MI_FLUSH_DW);
> -	 OUT_BATCH(0);
> -	 OUT_BATCH(0);
> -	 OUT_BATCH(0);
> -	 ADVANCE_BATCH();
> -      } else {
> -	 if (brw->gen == 6) {
> -	    /* Hardware workaround: SNB B-Spec says:
> -	     *
> -	     * [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(brw);
> -	 }
> -
> -	 BEGIN_BATCH(4);
> -	 OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2));
> -	 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);
> -	 OUT_BATCH(0); /* write address */
> -	 OUT_BATCH(0); /* write data */
> -	 ADVANCE_BATCH();
> -      }
> -   } else {
> -      BEGIN_BATCH(4);
> -      OUT_BATCH(_3DSTATE_PIPE_CONTROL | (4 - 2) |
> -		PIPE_CONTROL_WRITE_FLUSH |
> -		PIPE_CONTROL_NO_WRITE);
> -      OUT_BATCH(0); /* write address */
> -      OUT_BATCH(0); /* write data */
> -      OUT_BATCH(0); /* write data */
> +   if (unlikely(brw->batch.ring == BLT_RING) && brw->gen >= 6) {
> +      BEGIN_BATCH_BLT(4);
> +      OUT_BATCH(MI_FLUSH_DW);
> +      OUT_BATCH(0);
> +      OUT_BATCH(0);
> +      OUT_BATCH(0);
>        ADVANCE_BATCH();

This shouldn't be marked unlikely.  You should use unlikely for "this
path should be never executed in a performance-sensitive way", not just
"I bet this will be used a bit less frequently than the alternative."
(the compiler does more than just try to hint branch prediction using
this information)

> +   } else {
> +      int flags = PIPE_CONTROL_NO_WRITE | PIPE_CONTROL_WRITE_FLUSH;
> +      if (brw->gen >= 6) {
> +         flags |= PIPE_CONTROL_INSTRUCTION_FLUSH |
> +                  PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +                  PIPE_CONTROL_VF_CACHE_INVALIDATE |
> +                  PIPE_CONTROL_TC_FLUSH |
> +                  PIPE_CONTROL_CS_STALL;
> +      }

Lost the emit_post_sync_nonzero_flush() from the previous code.

> +      brw_emit_pipe_control_flush(brw, flags);
>     }
>  }

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140110/24001885/attachment.pgp>