[Mesa-dev] [PATCH] i965: Fix ARB_indirect_parameters logic.

[Plamena Manolova]

This patch modifies the ARB_indirect_parameters logic in
brw_draw_prims, so that our implementation isn't affected if
another application attempts to use predicates. Previously we
were using a predicate with a DELTAS_EQUAL comparison operation
and relying on the MI_PREDICATE_DATA register being 0. This
assumtion is incorrect when another program is using predicates
which store data in MI_PREDICATE_DATA. This patch introduces a
different approach which uses 2 predicates (one using SRC_EQUAL
and the other DELTAS_EQUAL) that makes no assumption about the
states of any of the predicate registers.

Fixes: piglit.spec.arb_indirect_parameters.tf-count-arrays
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103085

Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
CC: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_draw.c | 75 +++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
index 80d4891f6f..81465c79fb 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -866,7 +866,6 @@ brw_draw_prims(struct gl_context *ctx,
    struct brw_context *brw = brw_context(ctx);
    const struct gl_vertex_array **arrays = ctx->Array._DrawArrays;
    int predicate_state = brw->predicate.state;
-   int combine_op = MI_PREDICATE_COMBINEOP_SET;
    struct brw_transform_feedback_object *xfb_obj =
       (struct brw_transform_feedback_object *) gl_xfb_obj;
 
@@ -910,49 +909,63 @@ brw_draw_prims(struct gl_context *ctx,
     * to it.
     */
 
-    if (brw->draw.draw_params_count_bo &&
-        predicate_state == BRW_PREDICATE_STATE_USE_BIT) {
-      /* We need to empty the MI_PREDICATE_DATA register since it might
-       * already be set.
-       */
-
-      BEGIN_BATCH(4);
-      OUT_BATCH(MI_PREDICATE_DATA);
-      OUT_BATCH(0u);
-      OUT_BATCH(MI_PREDICATE_DATA + 4);
-      OUT_BATCH(0u);
-      ADVANCE_BATCH();
-
-      /* We need to combine the results of both predicates.*/
-      combine_op = MI_PREDICATE_COMBINEOP_AND;
-   }
-
    for (i = 0; i < nr_prims; i++) {
       /* Implementation of ARB_indirect_parameters via predicates */
       if (brw->draw.draw_params_count_bo) {
-         struct brw_bo *draw_id_bo = NULL;
-         uint32_t draw_id_offset;
-
-         intel_upload_data(brw, &prims[i].draw_id, 4, 4, &draw_id_bo,
-                           &draw_id_offset);
-
          brw_emit_pipe_control_flush(brw, PIPE_CONTROL_FLUSH_ENABLE);
 
+         /*
+          * Upload the current draw count from the draw parameters buffer to
+          * MI_PREDICATE_SRC0
+          */
          brw_load_register_mem(brw, MI_PREDICATE_SRC0,
                                brw->draw.draw_params_count_bo,
                                brw->draw.draw_params_count_offset);
-         brw_load_register_mem(brw, MI_PREDICATE_SRC1, draw_id_bo,
-                               draw_id_offset);
-
+         /*
+          * Upload the id of the current primitive to MI_PREDICATE_SRC1.
+          */
+         brw_load_register_imm64(brw, MI_PREDICATE_SRC1, prims[i].draw_id);
+
+         /*
+          * This calculates MI_PREDICATE_SRC0 - MI_PREDICATE_SRC1 and stores it
+          * in MI_PREDICATE_DATA without modifying the Predicate State Bit since
+          * we don't need the comparision result (it's
+          * MI_PREDICATE_SRC0 == MI_PREDICATE_SRC0).
+          */
          BEGIN_BATCH(1);
          OUT_BATCH(GEN7_MI_PREDICATE |
-                   MI_PREDICATE_LOADOP_LOADINV | combine_op |
-                   MI_PREDICATE_COMPAREOP_DELTAS_EQUAL);
+                   MI_PREDICATE_LOADOP_KEEP | MI_PREDICATE_COMBINEOP_SET |
+                   MI_PREDICATE_COMPAREOP_SRCS_EQUAL);
          ADVANCE_BATCH();
 
-         brw->predicate.state = BRW_PREDICATE_STATE_USE_BIT;
 
-         brw_bo_unreference(draw_id_bo);
+         /* We set both MI_PREDICATE_SRC0 and MI_PREDICATE_SRC1 to 0 */
+         brw_load_register_imm64(brw, MI_PREDICATE_SRC0, 0);
+         brw_load_register_imm64(brw, MI_PREDICATE_SRC1, 0);
+
+         BEGIN_BATCH(1);
+         if (brw->predicate.state != BRW_PREDICATE_STATE_USE_BIT) {
+            /*
+             * This calculates MI_PREDICATE_SRC0 - MI_PREDICATE_SRC1 which
+             * is 0, compares it to the value stored in MI_PREDICATE_DATA
+             * (MI_PREDICATE_DATA == 0) and sets the Predicate State Bit.
+             */
+            OUT_BATCH(GEN7_MI_PREDICATE |
+                      MI_PREDICATE_LOADOP_LOADINV | MI_PREDICATE_COMBINEOP_SET |
+                      MI_PREDICATE_COMPAREOP_DELTAS_EQUAL);
+         } else {
+            /*
+             * We should be able to use conditional rendering with indirect
+             * parameters, so we use MI_PREDICATE_COMBINEOP_AND to combine the
+             * result with the one stored in the Predicate State Bit.
+             */
+            OUT_BATCH(GEN7_MI_PREDICATE |
+                      MI_PREDICATE_LOADOP_LOADINV | MI_PREDICATE_COMBINEOP_AND |
+                      MI_PREDICATE_COMPAREOP_DELTAS_EQUAL);
+         }
+         ADVANCE_BATCH();
+
+         brw->predicate.state = BRW_PREDICATE_STATE_USE_BIT;
       }
 
       brw_draw_single_prim(ctx, arrays, &prims[i], i, xfb_obj, stream,
-- 
2.11.0