Re: [Mesa-dev] [PATCH v3 02/14] nvc0: bind constant buffers for compute on Fermi

Samuel Pitoiset Sun, 21 Feb 2016 14:21:12 -0800


On 02/21/2016 06:49 PM, Ilia Mirkin wrote:

On Wed, Feb 17, 2016 at 4:27 PM, Samuel Pitoiset
<samuel.pitoi...@gmail.com> wrote:

Loosely based on 3D.

Changs from v3:
  - invalidate COMPUTE CBs after validating 3D CBs because they are
    aliased

Changes from v2:
  - get rid of the 's' param to nvc0_cb_bo_push() because it doesn't
    matter to upload constbufs for compute using the 3d chan

Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c    | 72 +++++++++++++++++++---
  src/gallium/drivers/nouveau/nvc0/nvc0_context.c    | 11 +++-
  src/gallium/drivers/nouveau/nvc0/nvc0_screen.h     |  2 +-
  src/gallium/drivers/nouveau/nvc0/nvc0_state.c      |  4 +-
  .../drivers/nouveau/nvc0/nvc0_state_validate.c     |  5 ++
  5 files changed, 81 insertions(+), 13 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index 5c7dc0e..0fe6353 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -138,11 +138,71 @@ nvc0_compute_validate_program(struct nvc0_context *nvc0)
     return false;
  }

+static void
+nvc0_compute_validate_constbufs(struct nvc0_context *nvc0)
+{
+   struct nouveau_pushbuf *push = nvc0->base.pushbuf;
+   const int s = 5;
+
+   while (nvc0->constbuf_dirty[s]) {
+      int i = ffs(nvc0->constbuf_dirty[s]) - 1;
+      nvc0->constbuf_dirty[s] &= ~(1 << i);
+
+      if (nvc0->constbuf[s][i].user) {
+         struct nouveau_bo *bo = nvc0->screen->uniform_bo;
+         const unsigned base = s << 16;
+         const unsigned size = nvc0->constbuf[s][0].size;
+         assert(i == 0); /* we really only want OpenGL uniforms here */
+         assert(nvc0->constbuf[s][0].u.data);
+
+         if (nvc0->state.uniform_buffer_bound[s] < size) {
+            nvc0->state.uniform_buffer_bound[s] = align(size, 0x100);
+
+            BEGIN_NVC0(push, NVC0_COMPUTE(CB_SIZE), 3);
+            PUSH_DATA (push, nvc0->state.uniform_buffer_bound[s]);
+            PUSH_DATAh(push, bo->offset + base);
+            PUSH_DATA (push, bo->offset + base);
+            BEGIN_NVC0(push, NVC0_COMPUTE(CB_BIND), 1);
+            PUSH_DATA (push, (0 << 8) | 1);
+         }
+         nvc0_cb_bo_push(&nvc0->base, bo, NV_VRAM_DOMAIN(&nvc0->screen->base),
+                         base, nvc0->state.uniform_buffer_bound[s],
+                         0, (size + 3) / 4,
+                         nvc0->constbuf[s][0].u.data);
+      } else {
+         struct nv04_resource *res =
+            nv04_resource(nvc0->constbuf[s][i].u.buf);
+         if (res) {
+            BEGIN_NVC0(push, NVC0_COMPUTE(CB_SIZE), 3);
+            PUSH_DATA (push, nvc0->constbuf[s][i].size);
+            PUSH_DATAh(push, res->address + nvc0->constbuf[s][i].offset);
+            PUSH_DATA (push, res->address + nvc0->constbuf[s][i].offset);
+            BEGIN_NVC0(push, NVC0_COMPUTE(CB_BIND), 1);
+            PUSH_DATA (push, (i << 8) | 1);
+
+            BCTX_REFN(nvc0->bufctx_cp, CP_CB(i), res, RD);
+
+            res->cb_bindings[s] |= 1 << i;
+         } else {
+            BEGIN_NVC0(push, NVC0_COMPUTE(CB_BIND), 1);
+            PUSH_DATA (push, (i << 8) | 0);
+         }
+         if (i == 0)
+            nvc0->state.uniform_buffer_bound[s] = 0;
+      }
+   }
+
+   BEGIN_NVC0(push, NVC0_COMPUTE(FLUSH), 1);
+   PUSH_DATA (push, NVC0_COMPUTE_FLUSH_CB);
+}
+
  static bool
  nvc0_compute_state_validate(struct nvc0_context *nvc0)
  {
     if (!nvc0_compute_validate_program(nvc0))
        return false;
+   if (nvc0->dirty_cp & NVC0_NEW_CP_CONSTBUF)
+      nvc0_compute_validate_constbufs(nvc0);

     /* TODO: textures, samplers, surfaces, global memory buffers */

@@ -188,7 +248,7 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct 
pipe_grid_info *info)
     struct nvc0_context *nvc0 = nvc0_context(pipe);
     struct nouveau_pushbuf *push = nvc0->base.pushbuf;
     struct nvc0_program *cp = nvc0->compprog;
-   unsigned s, i;
+   unsigned s;
     int ret;

     ret = !nvc0_compute_state_validate(nvc0);
@@ -242,14 +302,10 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct 
pipe_grid_info *info)
     BEGIN_NVC0(push, SUBC_COMPUTE(0x0360), 1);
     PUSH_DATA (push, 0x1);

-   /* rebind all the 3D constant buffers
-    * (looks like binding a CB on COMPUTE clobbers 3D state) */
+   /* Invalidate all 3D constbufs because they are aliased with COMPUTE. */
     nvc0->dirty |= NVC0_NEW_CONSTBUF;
     for (s = 0; s < 5; s++) {
-      for (i = 0; i < NVC0_MAX_PIPE_CONSTBUFS; i++)
-         if (nvc0->constbuf[s][i].u.buf)
-            nvc0->constbuf_dirty[s] |= 1 << i;
+      nvc0->constbuf_dirty[s] |= nvc0->constbuf_valid[s];
+      nvc0->state.uniform_buffer_bound[s] = 0;
     }
-   memset(nvc0->state.uniform_buffer_bound, 0,
-          sizeof(nvc0->state.uniform_buffer_bound));
  }
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
index 547b8f5..4fed7b2 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
@@ -241,15 +241,20 @@ nvc0_invalidate_resource_storage(struct nouveau_context 
*ctx,
        }
        }

-      for (s = 0; s < 5; ++s) {
+      for (s = 0; s < 6; ++s) {
        for (i = 0; i < NVC0_MAX_PIPE_CONSTBUFS; ++i) {
           if (!(nvc0->constbuf_valid[s] & (1 << i)))
              continue;
           if (!nvc0->constbuf[s][i].user &&
               nvc0->constbuf[s][i].u.buf == res) {
-            nvc0->dirty |= NVC0_NEW_CONSTBUF;
              nvc0->constbuf_dirty[s] |= 1 << i;
-            nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_CB(s, i));
+            if (unlikely(s == 5)) {
+               nvc0->dirty_cp |= NVC0_NEW_CP_CONSTBUF;
+               nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_CB(i));
+            } else {
+               nvc0->dirty |= NVC0_NEW_CONSTBUF;
+               nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_CB(s, i));
+            }
              if (!--ref)
                 return ref;
           }
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
index e86fe43..8487abc 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
@@ -51,7 +51,7 @@ struct nvc0_graph_state {
     uint8_t c14_bound; /* whether immediate array constbuf is bound */
     uint8_t clip_enable;
     uint32_t clip_mode;
-   uint32_t uniform_buffer_bound[5];
+   uint32_t uniform_buffer_bound[6];
     struct nvc0_transform_feedback_state *tfb;
     bool seamless_cube_map;
  };
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
index 448211f..157d628 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
@@ -839,7 +839,9 @@ nvc0_set_constant_buffer(struct pipe_context *pipe, uint 
shader, uint index,
     const unsigned i = index;

     if (unlikely(shader == PIPE_SHADER_COMPUTE)) {
-      assert(!cb || !cb->user_buffer);
+      if (nvc0->constbuf[s][i].user)
+         nvc0->constbuf[s][i].u.buf = NULL;
+      else
        if (nvc0->constbuf[s][i].u.buf)
           nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_CB(i));

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
index 5ac3676..2a210e9 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
@@ -467,6 +467,11 @@ nvc0_constbufs_validate(struct nvc0_context *nvc0)
           }
        }
     }
+
+   /* Invalidate all COMPUTE constbufs because they are aliased with 3D. */
+   nvc0->dirty_cp |= NVC0_NEW_CP_CONSTBUF;
+   nvc0->constbuf_dirty[5] |= nvc0->constbuf_valid[5];
+   nvc0->state.uniform_buffer_bound[5] = 0;


I know that I gave you a R-b and you already pushed this, but.... it
occurs to me that this should actually be more like

nvc0->constbuf_dirty[5] |= nvc0->constbuf_valid[5];
nvc0->constbuf_dirty[5] |= nvc0->constbuf_valid[s];

Not only do you want the valid constbufs to be bound, but you also
want the "old" ones to be properly *unbound*, since they will not be
in the bufctx, and who knows what the hw will do even if the shader in
question doesn't read from them. (And a similar change to the compute
side of things.)


Mmmh yeah probably.

Anyway, we should also only dirty the CBs which are really overwritten,this will avoid some re-binding.

I'll think more about this and send a patch.


   -ilia

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 02/14] nvc0: bind constant buffers for compute on Fermi

Reply via email to