Re: [Mesa-dev] [PATCH 02/12] nvc0: allow to push constant buffers for compute on Fermi

[Samuel Pitoiset]

On 02/06/2016 11:52 PM, Ilia Mirkin wrote:
On Sat, Feb 6, 2016 at 5:38 PM, Samuel Pitoiset
<samuel.pitoi...@gmail.com> wrote:
Constant buffers must be bound for compute like for 3D. This is done
by adding a new 'shader' parameter to nvc0_cb_bo_push() which allows
to use the compute channel for compute shaders, and the 3D channel
for other shader types.

Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
  src/gallium/drivers/nouveau/nvc0/nvc0_context.h        |  2 +-
  src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c |  2 +-
  src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c       | 15 +++++++++++----
  3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
index 2e726e6..2ab70e8 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
@@ -301,7 +301,7 @@ nve4_p2mf_push_linear(struct nouveau_context *nv,
                        unsigned size, const void *data);
  void
  nvc0_cb_bo_push(struct nouveau_context *,
-                struct nouveau_bo *bo, unsigned domain,
+                struct nouveau_bo *bo, unsigned domain, unsigned shader,
                  unsigned base, unsigned size,
                  unsigned offset, unsigned words, const uint32_t *data);

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
index 2bb9b44..97fcfbc 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
@@ -441,7 +441,7 @@ nvc0_constbufs_validate(struct nvc0_context *nvc0)
                 PUSH_DATA (push, (0 << 4) | 1);
              }
              nvc0_cb_bo_push(&nvc0->base, bo, 
NV_VRAM_DOMAIN(&nvc0->screen->base),
-                         base, nvc0->state.uniform_buffer_bound[s],
+                         base, nvc0->state.uniform_buffer_bound[s], s,
                           0, (size + 3) / 4,
                           nvc0->constbuf[s][0].u.data);
           } else {
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
index 279c7e9..5cea822 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
@@ -4,6 +4,7 @@
  #include "nvc0/nvc0_context.h"

  #include "nv50/nv50_defs.xml.h"
+#include "nvc0/nvc0_compute.xml.h"

  struct nvc0_transfer {
     struct pipe_transfer base;
@@ -532,7 +533,7 @@ nvc0_cb_push(struct nouveau_context *nv,

     if (cb) {
        nvc0_cb_bo_push(nv, res->bo, res->domain,
-                      res->offset + cb->offset, cb->size,
+                      res->offset + cb->offset, cb->size, s,

I think you want s - 1 here. Also if a CB is bound to both a graphics
and compute pipeline, it'll get uploaded via the graphics pipeline.
Which leads me to my below comment:

Yeah, it's s-1.


                        offset - cb->offset, words, data);
     } else {
        nv->push_data(nv, res->bo, res->offset + offset, res->domain,
@@ -543,7 +544,7 @@ nvc0_cb_push(struct nouveau_context *nv,
  void
  nvc0_cb_bo_push(struct nouveau_context *nv,
                  struct nouveau_bo *bo, unsigned domain,
-                unsigned base, unsigned size,
+                unsigned base, unsigned size, unsigned shader,
                  unsigned offset, unsigned words, const uint32_t *data)
  {
     struct nouveau_pushbuf *push = nv->pushbuf;
@@ -557,7 +558,10 @@ nvc0_cb_bo_push(struct nouveau_context *nv,
     assert(offset < size);
     assert(offset + words * 4 <= size);

-   BEGIN_NVC0(push, NVC0_3D(CB_SIZE), 3);
+   if (unlikely(shader == 5))
+      BEGIN_NVC0(push, NVC0_COMPUTE(CB_SIZE), 3);
+   else
+      BEGIN_NVC0(push, NVC0_3D(CB_SIZE), 3);

Did you observe this to *actually* matter (i.e. using the compute cb
upload thing)? I find that hard to believe... Also aren't these gone
on kepler compute? That'll be fun...

I'll try to upload constbufs used for compute with the 3D chan and we 
will see.. Anyway, uploading CB for compute can clobber CB for 3D on 
Fermi... that's sad.

Yes, this stuff has gone on Kepler. :-)


     PUSH_DATA (push, size);
     PUSH_DATAh(push, bo->offset + base);
     PUSH_DATA (push, bo->offset + base);
@@ -567,7 +571,10 @@ nvc0_cb_bo_push(struct nouveau_context *nv,

        PUSH_SPACE(push, nr + 2);
        PUSH_REFN (push, bo, NOUVEAU_BO_WR | domain);
-      BEGIN_1IC0(push, NVC0_3D(CB_POS), nr + 1);
+      if (unlikely(shader == 5))
+         BEGIN_1IC0(push, NVC0_COMPUTE(CB_POS), nr + 1);
+      else
+         BEGIN_1IC0(push, NVC0_3D(CB_POS), nr + 1);
        PUSH_DATA (push, offset);
        PUSH_DATAp(push, data, nr);

--
2.6.4

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