[Mesa-dev] [PATCH] i965: Move control flush into pipelined conditional render

[Kenneth Graunke]

On Friday, August 21, 2015 04:14:25 PM Chris Wilson wrote:
> The nv_conditional_render piglits were sporadically failing. Moving
> the control flush from the write and placing it just before the read
> was sufficient to make the piglits pass a 1000/1000 times. The bspec
> says that the flush enable bit "waits until all previous writes of
> immediate data from post sync circles are complete before executing the
> next commend" - the operative word being previous!
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Neil Roberts <neil at linux.intel.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_conditional_render.c |  6 ++++++
>  src/mesa/drivers/dri/i965/brw_queryobj.c           | 19 +++++--------------
>  2 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_conditional_render.c b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> index 6d37c3b..122a4ec 100644
> --- a/src/mesa/drivers/dri/i965/brw_conditional_render.c
> +++ b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> @@ -56,6 +56,12 @@ set_predicate_for_result(struct brw_context *brw,
>  
>     assert(query->bo != NULL);
>  
> +   /* Needed to ensure the memory is coherent for the MI_LOAD_REGISTER_MEM
> +    * command when loading the values into the predicate source registers for
> +    * conditional rendering.
> +    */
> +   brw_emit_pipe_control_flush(brw, PIPE_CONTROL_FLUSH_ENABLE);
> +
>     brw_load_register_mem64(brw,
>                             MI_PREDICATE_SRC0,
>                             query->bo,
> diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c b/src/mesa/drivers/dri/i965/brw_queryobj.c
> index d6b012c..a8e5aba 100644
> --- a/src/mesa/drivers/dri/i965/brw_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/brw_queryobj.c
> @@ -66,20 +66,11 @@ brw_write_timestamp(struct brw_context *brw, drm_intel_bo *query_bo, int idx)
>  void
>  brw_write_depth_count(struct brw_context *brw, drm_intel_bo *query_bo, int idx)
>  {
> -   uint32_t flags;
> -
> -   flags = (PIPE_CONTROL_WRITE_DEPTH_COUNT |
> -            PIPE_CONTROL_DEPTH_STALL);
> -
> -   /* Needed to ensure the memory is coherent for the MI_LOAD_REGISTER_MEM
> -    * command when loading the values into the predicate source registers for
> -    * conditional rendering.
> -    */
> -   if (brw->predicate.supported)
> -      flags |= PIPE_CONTROL_FLUSH_ENABLE;
> -
> -   brw_emit_pipe_control_write(brw, flags, query_bo,
> -                               idx * sizeof(uint64_t), 0, 0);
> +   brw_emit_pipe_control_write(brw,
> +                               PIPE_CONTROL_WRITE_DEPTH_COUNT |
> +                               PIPE_CONTROL_DEPTH_STALL,
> +                               query_bo, idx * sizeof(uint64_t),
> +                               0, 0);
>  }
>  
>  /**
> 

Seems reasonable to me - thanks Chris.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- 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/20150821/70204b83/attachment.sig>