[Mesa-dev] [PATCH] i965: Improve conditional rendering in fallback paths.

[Chris Wilson]

On Thu, Mar 02, 2017 at 12:06:00AM -0800, Kenneth Graunke wrote:
> We need to fall back in a couple of cases:
> - Sandybridge (it just doesn't do this in hardware)
> - Occlusion queries on Gen7-7.5 with command parser version < 2
> - Transform feedback overflow queries on Gen7, or on Gen7.5 with
>   command parser version < 7
> 
> In these cases, we printed a perf_debug message and fell back to
> _mesa_check_conditional_render(), which stalls until the full
> query result is available.  Additionally, the code to handle this
> was a bit of a mess.
> 
> We can do better by using our normal conditional rendering code,
> and setting a new state, BRW_PREDICATE_STATE_STALL_FOR_QUERY, when
> we would have set BRW_PREDICATE_STATE_USE_BIT.  Only if that state
> is set do we perf_debug and potentially stall.  This means we avoid
> stalls when we have a partial query result (i.e. we know it's > 0,
> but don't have the full value).  The perf_debug should trigger less
> often as well.
> 
> Still, this is primarily intended as a cleanup.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_conditional_render.c | 84 +++++++++++-----------
>  src/mesa/drivers/dri/i965/brw_context.c            |  3 +-
>  src/mesa/drivers/dri/i965/brw_context.h            |  6 +-
>  3 files changed, 48 insertions(+), 45 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_conditional_render.c b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> index 046a42b5f52..c9503c5343d 100644
> --- a/src/mesa/drivers/dri/i965/brw_conditional_render.c
> +++ b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> @@ -52,6 +52,19 @@ set_predicate_for_overflow_query(struct brw_context *brw,
>                                   struct brw_query_object *query,
>                                   int stream_start, int count)
>  {
> +   if (!can_do_mi_math_and_lrr(brw->screen)) {
> +      brw->predicate.state = BRW_PREDICATE_STATE_STALL_FOR_QUERY;
> +      return;
> +   }
> +
> +   brw->predicate.state = BRW_PREDICATE_STATE_USE_BIT;
> +
> +   /* 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);
> +
>     hsw_overflow_result_to_gpr0(brw, query, count);
>     brw_load_register_reg64(brw, HSW_CS_GPR(0), MI_PREDICATE_SRC0);
>     brw_load_register_imm64(brw, MI_PREDICATE_SRC1, 0ull);
> @@ -61,6 +74,19 @@ static void
>  set_predicate_for_occlusion_query(struct brw_context *brw,
>                                    struct brw_query_object *query)
>  {
> +   if (!brw->predicate.supported) {
> +      brw->predicate.state = BRW_PREDICATE_STATE_STALL_FOR_QUERY;
> +      return;
> +   }
> +
> +   brw->predicate.state = BRW_PREDICATE_STATE_USE_BIT;
> +
> +   /* 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);

This flush waits for the previous flushes to retire. You need a CS stall
to wait for this flush to retire before doing the LRM. Unless the earlier
flushes used a post-sync write, there isn't a guarantee that memory is
coherent.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre