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

[Samuel Pitoiset]

On 04/13/2016 10:11 PM, Ilia Mirkin wrote:
> 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.

Okay, I'll fix that.

>
>    -ilia
>