iris: Rework iris_update_draw_parameters to be more efficient
authorKenneth Graunke <kenneth@whitecape.org>
Thu, 19 Sep 2019 03:32:36 +0000 (20:32 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Thu, 19 Sep 2019 05:50:52 +0000 (22:50 -0700)
This improves a couple of things:

1. We now only update anything if the shader actually cares.

   Previously, is_indexed_draw was causing us to flag dirty vertex
   buffers, elements, and SGVs every time the shader switched between
   indexed and non-indexed draws.  This is a very common situation,
   but we only need that information if the shader uses gl_BaseVertex.

   We were also flagging things when switching between indirect/direct
   draws as well, and now we only bother if it matters.

2. We upload new draw parameters only when necessary.

   When we detect that the draw parameters have changed, we upload a
   new copy, and use that.  Previously we were uploading it every time
   the vertex buffers were dirty (for possibly unrelated reasons) and
   the shader needed that info.  Tying these together also makes the
   code a bit easier to follow.

In Civilization VI's benchmark, this code was flagging dirty state
many times per frame (49 average, 16 median, 614 maximum).  Now it
occurs exactly once for the entire run.

src/gallium/drivers/iris/iris_context.h
src/gallium/drivers/iris/iris_draw.c
src/gallium/drivers/iris/iris_state.c

index ed814ee6824954f6c3deac3b1d04655739416192..970c5688e95ec42cd97c18d78c8e7b1f56a04624 100644 (file)
@@ -550,6 +550,13 @@ struct iris_context {
          int baseinstance;
       } params;
 
+      /**
+       * Are the above values the ones stored in the draw_params buffer?
+       * If so, we can compare them against new values to see if anything
+       * changed.  If not, we need to assume they changed.
+       */
+      bool params_valid;
+
       /**
        * Resource and offset that stores draw_parameters from the indirect
        * buffer or to the buffer that stures the previous values for non
@@ -577,8 +584,6 @@ struct iris_context {
        * drawid and is_indexed_draw. They will go in their own vertex element.
        */
       struct iris_state_ref derived_draw_params;
-
-      bool is_indirect;
    } draw;
 
    struct {
index 005da3f7a6d7143d5bf4ff460450fcd001872f00..9cdf1a2dd0e1099651a961017a24aed30e8add1b 100644 (file)
@@ -113,35 +113,56 @@ static void
 iris_update_draw_parameters(struct iris_context *ice,
                             const struct pipe_draw_info *info)
 {
-   if (info->indirect) {
-      pipe_resource_reference(&ice->draw.draw_params.res,
-                              info->indirect->buffer);
-      ice->draw.draw_params.offset = info->indirect->offset +
-                                     (info->index_size ? 12 : 8);
-      ice->draw.params.firstvertex = 0;
-      ice->draw.params.baseinstance = 0;
-      ice->state.dirty |= IRIS_DIRTY_VERTEX_BUFFERS |
-                          IRIS_DIRTY_VERTEX_ELEMENTS |
-                          IRIS_DIRTY_VF_SGVS;
-   } else if (ice->draw.is_indirect ||
-              ice->draw.params.firstvertex !=
-              (info->index_size ? info->index_bias : info->start) ||
-              (ice->draw.params.baseinstance != info->start_instance)) {
-      pipe_resource_reference(&ice->draw.draw_params.res, NULL);
-      ice->draw.draw_params.offset = 0;
-      ice->draw.params.firstvertex =
-         info->index_size ? info->index_bias : info->start;
-      ice->draw.params.baseinstance = info->start_instance;
-      ice->state.dirty |= IRIS_DIRTY_VERTEX_BUFFERS |
-                          IRIS_DIRTY_VERTEX_ELEMENTS |
-                          IRIS_DIRTY_VF_SGVS;
+   bool changed = false;
+
+   if (ice->state.vs_uses_draw_params) {
+      struct iris_state_ref *draw_params = &ice->draw.draw_params;
+
+      if (info->indirect) {
+         pipe_resource_reference(&draw_params->res, info->indirect->buffer);
+         draw_params->offset =
+            info->indirect->offset + (info->index_size ? 12 : 8);
+
+         changed = true;
+         ice->draw.params_valid = false;
+      } else {
+         int firstvertex = info->index_size ? info->index_bias : info->start;
+
+         if (!ice->draw.params_valid ||
+             ice->draw.params.firstvertex != firstvertex ||
+             ice->draw.params.baseinstance != info->start_instance) {
+
+            changed = true;
+            ice->draw.params.firstvertex = firstvertex;
+            ice->draw.params.baseinstance = info->start_instance;
+            ice->draw.params_valid = true;
+
+            u_upload_data(ice->ctx.stream_uploader, 0,
+                          sizeof(ice->draw.params), 4, &ice->draw.params,
+                          &draw_params->offset, &draw_params->res);
+         }
+      }
+   }
+
+   if (ice->state.vs_uses_derived_draw_params) {
+      struct iris_state_ref *derived_params = &ice->draw.derived_draw_params;
+      int is_indexed_draw = info->index_size ? -1 : 0;
+
+      if (ice->draw.derived_params.drawid != info->drawid ||
+          ice->draw.derived_params.is_indexed_draw != is_indexed_draw) {
+
+         changed = true;
+         ice->draw.derived_params.drawid = info->drawid;
+         ice->draw.derived_params.is_indexed_draw = is_indexed_draw;
+
+         u_upload_data(ice->ctx.stream_uploader, 0,
+                       sizeof(ice->draw.derived_params), 4,
+                       &ice->draw.derived_params,
+                       &derived_params->offset, &derived_params->res);
+      }
    }
-   ice->draw.is_indirect = info->indirect;
 
-   if (ice->draw.derived_params.drawid != info->drawid ||
-       ice->draw.derived_params.is_indexed_draw != (info->index_size ? ~0 : 0)) {
-      ice->draw.derived_params.drawid = info->drawid;
-      ice->draw.derived_params.is_indexed_draw = info->index_size ? ~0 : 0;
+   if (changed) {
       ice->state.dirty |= IRIS_DIRTY_VERTEX_BUFFERS |
                           IRIS_DIRTY_VERTEX_ELEMENTS |
                           IRIS_DIRTY_VF_SGVS;
index 609115bce60a46c4c974c5a36c4ceebeb64d2082..370dc52df38e1ccd2aa69ed4fbecc36bff2fb846 100644 (file)
@@ -5293,11 +5293,6 @@ iris_upload_dirty_render_state(struct iris_context *ice,
       int dynamic_bound = ice->state.bound_vertex_buffers;
 
       if (ice->state.vs_uses_draw_params) {
-         if (ice->draw.draw_params.offset == 0) {
-            u_upload_data(ice->ctx.stream_uploader, 0, sizeof(ice->draw.params),
-                          4, &ice->draw.params, &ice->draw.draw_params.offset,
-                          &ice->draw.draw_params.res);
-         }
          assert(ice->draw.draw_params.res);
 
          struct iris_vertex_buffer_state *state =
@@ -5320,12 +5315,6 @@ iris_upload_dirty_render_state(struct iris_context *ice,
       }
 
       if (ice->state.vs_uses_derived_draw_params) {
-         u_upload_data(ice->ctx.stream_uploader, 0,
-                       sizeof(ice->draw.derived_params), 4,
-                       &ice->draw.derived_params,
-                       &ice->draw.derived_draw_params.offset,
-                       &ice->draw.derived_draw_params.res);
-
          struct iris_vertex_buffer_state *state =
             &(ice->state.genx->vertex_buffers[count]);
          pipe_resource_reference(&state->resource,
@@ -6983,6 +6972,7 @@ genX(init_state)(struct iris_context *ice)
    ice->state.num_viewports = 1;
    ice->state.prim_mode = PIPE_PRIM_MAX;
    ice->state.genx = calloc(1, sizeof(struct iris_genx_state));
+   ice->draw.derived_params.drawid = -1;
 
    /* Make a 1x1x1 null surface for unbound textures */
    void *null_surf_map =