[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.pitoiset at 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.pitoiset at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev