iris: Avoid uploading SURFACE_STATE descriptors for UBOs if possible

If we can entirely push uniform data, we don't need a SURFACE_STATE
descriptor for pulling data.  Since constant uploads are a very common
operation, and being able to push all data is also very common, we would
like to avoid the overhead in this case.

This patch defers uploading new descriptors.  Instead of handling that
at iris_set_constant_buffer, we do it at iris_update_compiled_shaders,
where we can see the currently bound shader variants.  If any need pull
descriptors, and descriptors are missing, we update them and flag that
the binding table also needs to be refreshed.

Improves performance in GFXBench5 gl_driver2 on an i7-6770HQ by
31.9774% +/- 1.12947% (n=15).

Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>

diff --git a/src/gallium/drivers/iris/iris_draw.c b/src/gallium/drivers/iris/iris_draw.c
index caf5c00..fc2c14e 100644 (file)
--- a/src/gallium/drivers/iris/iris_draw.c
+++ b/src/gallium/drivers/iris/iris_draw.c
@@ -332,8 +332,7 @@ iris_launch_grid(struct pipe_context *ctx, const struct pipe_grid_info *grid)
 
    iris_batch_maybe_flush(batch, 1500);
 
-   if (ice->state.dirty & IRIS_DIRTY_UNCOMPILED_CS)
-      iris_update_compiled_compute_shader(ice);
+   iris_update_compiled_compute_shader(ice);
 
    iris_update_grid_size_resource(ice, grid);
 

diff --git a/src/gallium/drivers/iris/iris_program.c b/src/gallium/drivers/iris/iris_program.c
index 50ea72a..1b80af5 100644 (file)
--- a/src/gallium/drivers/iris/iris_program.c
+++ b/src/gallium/drivers/iris/iris_program.c
@@ -1640,6 +1640,35 @@ update_last_vue_map(struct iris_context *ice,
    ice->shaders.last_vue_map = &vue_prog_data->vue_map;
 }
 
+static void
+iris_update_pull_constant_descriptors(struct iris_context *ice,
+                                      gl_shader_stage stage)
+{
+   struct iris_compiled_shader *shader = ice->shaders.prog[stage];
+
+   if (!shader || !shader->prog_data->has_ubo_pull)
+      return;
+
+   struct iris_shader_state *shs = &ice->state.shaders[stage];
+   bool any_new_descriptors =
+      shader->num_system_values > 0 && shs->sysvals_need_upload;
+
+   unsigned bound_cbufs = shs->bound_cbufs;
+
+   while (bound_cbufs) {
+      const int i = u_bit_scan(&bound_cbufs);
+      struct pipe_shader_buffer *cbuf = &shs->constbuf[i];
+      struct iris_state_ref *surf_state = &shs->constbuf_surf_state[i];
+      if (!surf_state->res && cbuf->buffer) {
+         iris_upload_ubo_ssbo_surf_state(ice, cbuf, surf_state, false);
+         any_new_descriptors = true;
+      }
+   }
+
+   if (any_new_descriptors)
+      ice->state.dirty |= IRIS_DIRTY_BINDINGS_VS << stage;
+}
+
 /**
  * Get the prog_data for a given stage, or NULL if the stage is disabled.
  */
@@ -1754,6 +1783,11 @@ iris_update_compiled_shaders(struct iris_context *ice)
          }
       }
    }
+
+   for (int i = MESA_SHADER_VERTEX; i <= MESA_SHADER_FRAGMENT; i++) {
+      if (ice->state.dirty & (IRIS_DIRTY_CONSTANTS_VS << i))
+         iris_update_pull_constant_descriptors(ice, i);
+   }
 }
 
 static struct iris_compiled_shader *
@@ -1808,8 +1842,8 @@ iris_compile_cs(struct iris_context *ice,
    return shader;
 }
 
-void
-iris_update_compiled_compute_shader(struct iris_context *ice)
+static void
+iris_update_compiled_cs(struct iris_context *ice)
 {
    struct iris_shader_state *shs = &ice->state.shaders[MESA_SHADER_COMPUTE];
    struct iris_uncompiled_shader *ish =
@@ -1840,6 +1874,16 @@ iris_update_compiled_compute_shader(struct iris_context *ice)
 }
 
 void
+iris_update_compiled_compute_shader(struct iris_context *ice)
+{
+   if (ice->state.dirty & IRIS_DIRTY_UNCOMPILED_CS)
+      iris_update_compiled_cs(ice);
+
+   if (ice->state.dirty & IRIS_DIRTY_CONSTANTS_CS)
+      iris_update_pull_constant_descriptors(ice, MESA_SHADER_COMPUTE);
+}
+
+void
 iris_fill_cs_push_const_buffer(struct brw_cs_prog_data *cs_prog_data,
                                uint32_t *dst)
 {

diff --git a/src/gallium/drivers/iris/iris_state.c b/src/gallium/drivers/iris/iris_state.c
index fad3a7f..ce68e99 100644 (file)
--- a/src/gallium/drivers/iris/iris_state.c
+++ b/src/gallium/drivers/iris/iris_state.c
@@ -2730,6 +2730,9 @@ iris_set_constant_buffer(struct pipe_context *ctx,
    struct iris_shader_state *shs = &ice->state.shaders[stage];
    struct pipe_shader_buffer *cbuf = &shs->constbuf[index];
 
+   /* TODO: Only do this if the buffer changes? */
+   pipe_resource_reference(&shs->constbuf_surf_state[index].res, NULL);
+
    if (input && input->buffer_size && (input->buffer || input->user_buffer)) {
       shs->bound_cbufs |= 1u << index;
 
@@ -2760,21 +2763,12 @@ iris_set_constant_buffer(struct pipe_context *ctx,
       struct iris_resource *res = (void *) cbuf->buffer;
       res->bind_history |= PIPE_BIND_CONSTANT_BUFFER;
       res->bind_stages |= 1 << stage;
-
-      iris_upload_ubo_ssbo_surf_state(ice, cbuf,
-                                      &shs->constbuf_surf_state[index],
-                                      false);
    } else {
       shs->bound_cbufs &= ~(1u << index);
       pipe_resource_reference(&cbuf->buffer, NULL);
-      pipe_resource_reference(&shs->constbuf_surf_state[index].res, NULL);
    }
 
    ice->state.dirty |= IRIS_DIRTY_CONSTANTS_VS << stage;
-   // XXX: maybe not necessary all the time...?
-   // XXX: we need 3DS_BTP to commit these changes, and if we fell back to
-   // XXX: pull model we may need actual new bindings...
-   ice->state.dirty |= IRIS_DIRTY_BINDINGS_VS << stage;
 }
 
 static void
@@ -4245,7 +4239,7 @@ use_ubo_ssbo(struct iris_batch *batch,
              struct iris_state_ref *surf_state,
              bool writable)
 {
-   if (!buf->buffer)
+   if (!buf->buffer || !surf_state->res)
       return use_null_surface(batch, ice);
 
    iris_use_pinned_bo(batch, iris_resource_bo(buf->buffer), writable);
@@ -6063,9 +6057,8 @@ iris_rebind_buffer(struct iris_context *ice,
             struct iris_state_ref *surf_state = &shs->constbuf_surf_state[i];
 
             if (res->bo == iris_resource_bo(cbuf->buffer)) {
-               iris_upload_ubo_ssbo_surf_state(ice, cbuf, surf_state, false);
-               ice->state.dirty |=
-                  (IRIS_DIRTY_CONSTANTS_VS | IRIS_DIRTY_BINDINGS_VS) << s;
+               pipe_resource_reference(&surf_state->res, NULL);
+               ice->state.dirty |= IRIS_DIRTY_CONSTANTS_VS << s;
             }
          }
       }