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

Ilia Mirkin imirkin at alum.mit.edu
Sun Feb 21 17:49:13 UTC 2016


On Wed, Feb 17, 2016 at 4:27 PM, Samuel Pitoiset
<samuel.pitoiset at 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.pitoiset at 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.)

  -ilia


More information about the mesa-dev mailing list