[Mesa-dev] [PATCH] nv50: reinstate dedicated constbuf push path
Samuel Pitoiset
samuel.pitoiset at gmail.com
Thu Jun 9 22:13:51 UTC 2016
The logic sounds quite familiar to nvc0 as you said. :)
I have just started a full piglit run with this patch applied.
On 06/06/2016 01:07 AM, Ilia Mirkin wrote:
> This was disabled due to occasionally incorrect behavior when trying to
> upload data. It later became apparent that nvc0 also had a similar but
> slightly different issue, which was resolved in commit e50c01d5. This
> takes the same logic as nvc0 and applies it to nv50 (which has somewhat
> different interfaces).
>
> Unfortunately I did not note down precisely what was broken with UBOs
> when removing the support from nv50, but I've tested a bunch of local
> traces, and none of them appear to regress. This should hopefully
> improve performance when UBOs are used, but this was not directly
> verified.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> src/gallium/drivers/nouveau/nv50/nv50_context.c | 7 ---
> src/gallium/drivers/nouveau/nv50/nv50_context.h | 3 +-
> .../drivers/nouveau/nv50/nv50_shader_state.c | 1 +
> src/gallium/drivers/nouveau/nv50/nv50_state.c | 5 +-
> src/gallium/drivers/nouveau/nv50/nv50_transfer.c | 63 +++++++++++++++-------
> 5 files changed, 50 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c b/src/gallium/drivers/nouveau/nv50/nv50_context.c
> index 8a148fc..84b5e3f 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
> @@ -318,14 +318,7 @@ nv50_create(struct pipe_screen *pscreen, void *priv, unsigned ctxflags)
> nv50->base.screen = &screen->base;
> nv50->base.copy_data = nv50_m2mf_copy_linear;
> nv50->base.push_data = nv50_sifc_linear_u8;
> - /* FIXME: Make it possible to use this again. The problem is that there is
> - * some clever logic in the card that allows for multiple renders to happen
> - * when there are only constbuf changes. However that relies on the
> - * constbuf updates happening to the right constbuf slots. Currently
> - * implementation just makes it go through a separate slot which doesn't
> - * properly update the right constbuf data.
> nv50->base.push_cb = nv50_cb_push;
> - */
>
> nv50->screen = screen;
> pipe->screen = pscreen;
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h b/src/gallium/drivers/nouveau/nv50/nv50_context.h
> index b7963a4..0582e24 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.h
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h
> @@ -286,8 +286,7 @@ nv50_m2mf_copy_linear(struct nouveau_context *pipe,
> unsigned size);
> void
> nv50_cb_push(struct nouveau_context *nv,
> - struct nouveau_bo *bo, unsigned domain,
> - unsigned base, unsigned size,
> + struct nv04_resource *res,
> unsigned offset, unsigned words, const uint32_t *data);
>
> /* nv50_vbo.c */
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> index f838d15..2326394 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
> @@ -99,6 +99,7 @@ nv50_constbufs_validate(struct nv50_context *nv50)
> BCTX_REFN(nv50->bufctx_3d, 3D_CB(s, i), res, RD);
>
> nv50->cb_dirty = 1; /* Force cache flush for UBO. */
> + res->cb_bindings[s] |= 1 << i;
> } else {
> BEGIN_NV04(push, NV50_3D(SET_PROGRAM_CB), 1);
> PUSH_DATA (push, (i << 8) | p | 0);
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c b/src/gallium/drivers/nouveau/nv50/nv50_state.c
> index 86e74d6..3231aba 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
> @@ -856,9 +856,10 @@ nv50_set_constant_buffer(struct pipe_context *pipe, uint shader, uint index,
> if (nv50->constbuf[s][i].user)
> nv50->constbuf[s][i].u.buf = NULL;
> else
> - if (nv50->constbuf[s][i].u.buf)
> + if (nv50->constbuf[s][i].u.buf) {
> nouveau_bufctx_reset(nv50->bufctx_3d, NV50_BIND_3D_CB(s, i));
> -
> + nv04_resource(nv50->constbuf[s][i].u.buf)->cb_bindings[s] &= ~(1 << i);
> + }
> pipe_resource_reference(&nv50->constbuf[s][i].u.buf, res);
>
> nv50->constbuf[s][i].user = (cb && cb->user_buffer) ? true : false;
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
> index f5c7c57..c751ade 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
> @@ -377,32 +377,24 @@ nv50_miptree_transfer_unmap(struct pipe_context *pctx,
> FREE(tx);
> }
>
> -void
> -nv50_cb_push(struct nouveau_context *nv,
> - struct nouveau_bo *bo, unsigned domain,
> - unsigned base, unsigned size,
> - unsigned offset, unsigned words, const uint32_t *data)
> +static void
> +nv50_cb_bo_push(struct nouveau_context *nv,
> + struct nouveau_bo *bo, unsigned domain,
> + unsigned bufid,
> + unsigned offset, unsigned words,
> + const uint32_t *data)
> {
> struct nouveau_pushbuf *push = nv->pushbuf;
> - struct nouveau_bufctx *bctx = nv50_context(&nv->pipe)->bufctx;
>
> assert(!(offset & 3));
> - size = align(size, 0x100);
> -
> - nouveau_bufctx_refn(bctx, 0, bo, NOUVEAU_BO_WR | domain);
> - nouveau_pushbuf_bufctx(push, bctx);
> - nouveau_pushbuf_validate(push);
>
> while (words) {
> unsigned nr = MIN2(words, NV04_PFIFO_MAX_PACKET_LEN);
>
> - PUSH_SPACE(push, nr + 7);
> - BEGIN_NV04(push, NV50_3D(CB_DEF_ADDRESS_HIGH), 3);
> - PUSH_DATAh(push, bo->offset + base);
> - PUSH_DATA (push, bo->offset + base);
> - PUSH_DATA (push, (NV50_CB_TMP << 16) | (size & 0xffff));
> + PUSH_SPACE(push, nr + 3);
> + PUSH_REFN (push, bo, NOUVEAU_BO_WR | domain);
> BEGIN_NV04(push, NV50_3D(CB_ADDR), 1);
> - PUSH_DATA (push, (offset << 6) | NV50_CB_TMP);
> + PUSH_DATA (push, (offset << 6) | bufid);
> BEGIN_NI04(push, NV50_3D(CB_DATA(0)), nr);
> PUSH_DATAp(push, data, nr);
>
> @@ -410,6 +402,41 @@ nv50_cb_push(struct nouveau_context *nv,
> data += nr;
> offset += nr * 4;
> }
> +}
>
> - nouveau_bufctx_reset(bctx, 0);
> +void
> +nv50_cb_push(struct nouveau_context *nv,
> + struct nv04_resource *res,
> + unsigned offset, unsigned words, const uint32_t *data)
> +{
> + struct nv50_context *nv50 = nv50_context(&nv->pipe);
> + struct nv50_constbuf *cb = NULL;
> + int s, bufid;
> + /* Go through all the constbuf binding points of this buffer and try to
> + * find one which contains the region to be updated.
> + */
> + /* XXX compute? */
> + for (s = 0; s < 3 && !cb; s++) {
> + uint16_t bindings = res->cb_bindings[s];
> + while (bindings) {
> + int i = ffs(bindings) - 1;
> + uint32_t cb_offset = nv50->constbuf[s][i].offset;
> +
> + bindings &= ~(1 << i);
> + if (cb_offset <= offset &&
> + cb_offset + nv50->constbuf[s][i].size >= offset + words * 4) {
> + cb = &nv50->constbuf[s][i];
> + bufid = s * 16 + i;
> + break;
> + }
> + }
> + }
> +
> + if (cb) {
> + nv50_cb_bo_push(nv, res->bo, res->domain,
> + bufid, offset - cb->offset, words, data);
> + } else {
> + nv->push_data(nv, res->bo, res->offset + offset, res->domain,
> + words * 4, data);
> + }
> }
>
More information about the mesa-dev
mailing list