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.)
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
