[Mesa-dev] [RFC 03/24] nvc0: bind images on compute shaders for Kepler

[Ilia Mirkin]

On Wed, Apr 13, 2016 at 11:26 AM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> On 04/13/2016 03:38 AM, Ilia Mirkin wrote:
>>
>> On Tue, Apr 12, 2016 at 7:56 PM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>>
>>> Old surfaces validation code will be removed once images are completely
>>> done for Fermi/Kepler, that explains why I only disable it for now.
>>>
>>> set_surface_info() which sticks surfaces information into the driver
>>> constant buffer needs to be updated because now we are dealing with
>>> pipe_image_view instead of pipe_surface.
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>>   src/gallium/drivers/nouveau/nvc0/nvc0_context.h |  5 ++-
>>>   src/gallium/drivers/nouveau/nvc0/nvc0_program.c |  4 +-
>>>   src/gallium/drivers/nouveau/nvc0/nvc0_tex.c     | 54
>>> +++++++++++++------------
>>>   src/gallium/drivers/nouveau/nvc0/nve4_compute.c | 37 ++++++++++++++++-
>>>   4 files changed, 72 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
>>> index 17733f5..107b737 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
>>> @@ -130,6 +130,9 @@
>>>   /* 32 user buffers, at 4 32-bits integers each */
>>>   #define NVC0_CB_AUX_BUF_INFO(i)     0x200 + (i) * 4 * 4
>>>   #define NVC0_CB_AUX_BUF_SIZE        (NVC0_MAX_BUFFERS * 4 * 4)
>>> +/* 8 surfaces, at 16 32-bits integers each */
>>> +#define NVC0_CB_AUX_SU_INFO(i)      0x400 + (i) * 16 * 4
>>> +#define nVC0_CB_AUX_SU_SIZE         (NVC0_MAX_IMAGES * 16 * 4)
>>>   /* 4 32-bits floats for the vertex runout, put at the end */
>>>   #define NVC0_CB_AUX_RUNOUT_INFO     NVC0_CB_USR_SIZE + NVC0_CB_AUX_SIZE
>>>
>>> @@ -324,7 +327,7 @@ void nvc0_validate_textures(struct nvc0_context *);
>>>   void nvc0_validate_samplers(struct nvc0_context *);
>>>   void nve4_set_tex_handles(struct nvc0_context *);
>>>   void nvc0_validate_surfaces(struct nvc0_context *);
>>> -void nve4_set_surface_info(struct nouveau_pushbuf *, struct pipe_surface
>>> *,
>>> +void nve4_set_surface_info(struct nouveau_pushbuf *, struct
>>> pipe_image_view *,
>>>                              struct nvc0_screen *);
>>>
>>>   struct pipe_sampler_view *
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>>> index d3024f9..ced8130 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>>> @@ -551,11 +551,13 @@ nvc0_program_translate(struct nvc0_program *prog,
>>> uint16_t chipset,
>>>            info->io.texBindBase = NVC0_CB_AUX_TEX_INFO(0);
>>>            info->prop.cp.gridInfoBase = NVC0_CB_AUX_GRID_INFO;
>>>            info->io.uboInfoBase = NVC0_CB_AUX_UBO_INFO(0);
>>> +         info->io.suInfoBase = NVC0_CB_AUX_SU_INFO(0);
>>> +      } else {
>>> +         info->io.suInfoBase = 0; /* TODO */
>>>         }
>>>         info->io.msInfoCBSlot = 0;
>>>         info->io.msInfoBase = NVC0_CB_AUX_MS_INFO;
>>>         info->io.bufInfoBase = NVC0_CB_AUX_BUF_INFO(0);
>>> -      info->io.suInfoBase = 0; /* TODO */
>>>      } else {
>>>         if (chipset >= NVISA_GK104_CHIPSET) {
>>>            info->io.texBindBase = NVC0_CB_AUX_TEX_INFO(0);
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>>> index 647c2a6..585b1e5 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>>> @@ -745,21 +745,21 @@ static const uint16_t
>>> nve4_suldp_lib_offset[PIPE_FORMAT_COUNT];
>>>
>>>   void
>>>   nve4_set_surface_info(struct nouveau_pushbuf *push,
>>> -                      struct pipe_surface *psf,
>>> +                      struct pipe_image_view *view,
>>>                         struct nvc0_screen *screen)
>>>   {
>>> -   struct nv50_surface *sf = nv50_surface(psf);
>>>      struct nv04_resource *res;
>>>      uint64_t address;
>>>      uint32_t *const info = push->cur;
>>> +   int width, height, depth;
>>>      uint8_t log2cpp;
>>>
>>> -   if (psf && !nve4_su_format_map[psf->format])
>>> +   if (view && !nve4_su_format_map[view->format])
>>>         NOUVEAU_ERR("unsupported surface format, try
>>> is_format_supported() !\n");
>>>
>>>      push->cur += 16;
>>>
>>> -   if (!psf || !nve4_su_format_map[psf->format]) {
>>> +   if (!view || !nve4_su_format_map[view->format]) {
>>>         memset(info, 0, 16 * sizeof(*info));
>>>
>>>         info[0] = 0xbadf0000;
>>> @@ -768,13 +768,17 @@ nve4_set_surface_info(struct nouveau_pushbuf *push,
>>>            screen->lib_code->start;
>>>         return;
>>>      }
>>> -   res = nv04_resource(sf->base.texture);
>>> +   res = nv04_resource(view->resource);
>>>
>>> -   address = res->address + sf->offset;
>>> +   address = res->address;
>>>
>>> -   info[8] = sf->width;
>>> -   info[9] = sf->height;
>>> -   info[10] = sf->depth;
>>> +   width = u_minify(view->resource->width0, view->u.tex.level);
>>
>>
>> You never check that it's a texture... if it's a buffer, this is
>> undefined. AFAIK you have some later patch which rectifies this...
>> perhaps fold this in?
>
>
> This doesn't really matter because this code won't be hit until
> ARB_shader_image_load_store is enabled.

So? The code is wrong. We should try to avoid patches that have wrong
code in them, esp if there's a later patch in the series that fixes
it.

  -ilia