[Mesa-dev] [PATCH 3/4] nvc0: allow a non-user buffer to be bound at position 0

[Karol Herbst]

On Fri, Jul 26, 2019 at 2:59 PM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> Thanks! I had to make a small update to the asserts:
>
> assert(nvc0->constbuf[5][0].user || !nvc0->constbuf[5][0].u.buf);
>
> u.buf is not valid to check when .user is set. (in fact it aliases
> with the "data" pointer)
>
> Let me know if you want me to resend.
>

no, that's fine..

> On Fri, Jul 26, 2019 at 5:51 AM Karol Herbst <kherbst at redhat.com> wrote:
> >
> > Reviewed-by: Karol Herbst <kherbst at redhat.com>
> >
> > On Fri, Jul 26, 2019 at 5:31 AM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > >
> > > Previously the code only handled it for positions 1 and up (as would be
> > > for UBO's in GL). It's not a lot of trouble to handle this, and vl or
> > > vdpau want this.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111213
> > > Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> > > Cc: mesa-stable at lists.freedesktop.org
> > > ---
> > >  .../drivers/nouveau/nvc0/nve4_compute.c       | 45 +++++++++++--------
> > >  1 file changed, 27 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
> > > index c5e4dec20bd..a1c40d1e6b9 100644
> > > --- a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
> > > +++ b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
> > > @@ -393,23 +393,24 @@ nve4_compute_validate_constbufs(struct nvc0_context *nvc0)
> > >              uint64_t address
> > >                 = nvc0->screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s);
> > >
> > > -            assert(i > 0); /* we really only want uniform buffer objects */
> > > -
> > > -            BEGIN_NVC0(push, NVE4_CP(UPLOAD_DST_ADDRESS_HIGH), 2);
> > > -            PUSH_DATAh(push, address + NVC0_CB_AUX_UBO_INFO(i - 1));
> > > -            PUSH_DATA (push, address + NVC0_CB_AUX_UBO_INFO(i - 1));
> > > -            BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
> > > -            PUSH_DATA (push, 4 * 4);
> > > -            PUSH_DATA (push, 0x1);
> > > -            BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + 4);
> > > -            PUSH_DATA (push, NVE4_COMPUTE_UPLOAD_EXEC_LINEAR | (0x20 << 1));
> > > -
> > > -            PUSH_DATA (push, res->address + nvc0->constbuf[s][i].offset);
> > > -            PUSH_DATAh(push, res->address + nvc0->constbuf[s][i].offset);
> > > -            PUSH_DATA (push, nvc0->constbuf[5][i].size);
> > > -            PUSH_DATA (push, 0);
> > > -            BCTX_REFN(nvc0->bufctx_cp, CP_CB(i), res, RD);
> > > +            /* constbufs above 0 will are fetched via ubo info in the shader */
> > > +            if (i > 0) {
> > > +               BEGIN_NVC0(push, NVE4_CP(UPLOAD_DST_ADDRESS_HIGH), 2);
> > > +               PUSH_DATAh(push, address + NVC0_CB_AUX_UBO_INFO(i - 1));
> > > +               PUSH_DATA (push, address + NVC0_CB_AUX_UBO_INFO(i - 1));
> > > +               BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
> > > +               PUSH_DATA (push, 4 * 4);
> > > +               PUSH_DATA (push, 0x1);
> > > +               BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + 4);
> > > +               PUSH_DATA (push, NVE4_COMPUTE_UPLOAD_EXEC_LINEAR | (0x20 << 1));
> > > +
> > > +               PUSH_DATA (push, res->address + nvc0->constbuf[s][i].offset);
> > > +               PUSH_DATAh(push, res->address + nvc0->constbuf[s][i].offset);
> > > +               PUSH_DATA (push, nvc0->constbuf[s][i].size);
> > > +               PUSH_DATA (push, 0);
> > > +            }
> > >
> > > +            BCTX_REFN(nvc0->bufctx_cp, CP_CB(i), res, RD);
> > >              res->cb_bindings[s] |= 1 << i;
> > >           }
> > >        }
> > > @@ -554,9 +555,9 @@ nve4_compute_derive_cache_split(struct nvc0_context *nvc0, uint32_t shared_size)
> > >  static void
> > >  nve4_compute_setup_buf_cb(struct nvc0_context *nvc0, bool gp100, void *desc)
> > >  {
> > > -   // only user constant buffers 1-6 can be put in the descriptor, the rest are
> > > +   // only user constant buffers 0-6 can be put in the descriptor, the rest are
> > >     // loaded through global memory
> > > -   for (int i = 1; i <= 6; i++) {
> > > +   for (int i = 0; i <= 6; i++) {
> > >        if (nvc0->constbuf[5][i].user || !nvc0->constbuf[5][i].u.buf)
> > >           continue;
> > >
> > > @@ -609,6 +610,10 @@ nve4_compute_setup_launch_desc(struct nvc0_context *nvc0,
> > >     if (nvc0->constbuf[5][0].user || cp->parm_size) {
> > >        nve4_cp_launch_desc_set_cb(desc, 0, screen->uniform_bo,
> > >                                   NVC0_CB_USR_INFO(5), 1 << 16);
> > > +
> > > +      // Later logic will attempt to bind a real buffer at position 0. That
> > > +      // should not happen if we've bound a user buffer.
> > > +      assert(!nvc0->constbuf[5][0].u.buf);
> > >     }
> > >     nve4_cp_launch_desc_set_cb(desc, 7, screen->uniform_bo,
> > >                                NVC0_CB_AUX_INFO(5), 1 << 11);
> > > @@ -649,6 +654,10 @@ gp100_compute_setup_launch_desc(struct nvc0_context *nvc0,
> > >     if (nvc0->constbuf[5][0].user || cp->parm_size) {
> > >        gp100_cp_launch_desc_set_cb(desc, 0, screen->uniform_bo,
> > >                                    NVC0_CB_USR_INFO(5), 1 << 16);
> > > +
> > > +      // Later logic will attempt to bind a real buffer at position 0. That
> > > +      // should not happen if we've bound a user buffer.
> > > +      assert(!nvc0->constbuf[5][0].u.buf);
> > >     }
> > >     gp100_cp_launch_desc_set_cb(desc, 7, screen->uniform_bo,
> > >                                 NVC0_CB_AUX_INFO(5), 1 << 11);
> > > --
> > > 2.21.0
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev