[Mesa-dev] [PATCH] ac: change legacy_surf_level::slice_size to dword units

[Marek Olšák]

On Mon, Nov 27, 2017 at 12:54 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 23.11.2017 20:35, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> The next commit will reduce the size even more.
>>
>> v2: typecast to uint64_t manually
>> ---
>>   src/amd/common/ac_surface.c                        |  2 +-
>>   src/amd/common/ac_surface.h                        |  3 ++-
>>   src/amd/vulkan/radv_image.c                        |  8 ++++----
>>   src/gallium/drivers/r600/evergreen_state.c         |  8 ++++----
>>   src/gallium/drivers/r600/r600_state.c              |  8 ++++----
>>   src/gallium/drivers/r600/r600_texture.c            | 14 +++++++-------
>>   src/gallium/drivers/r600/radeon_uvd.c              |  2 +-
>>   src/gallium/drivers/radeon/r600_texture.c          | 10 +++++-----
>>   src/gallium/drivers/radeon/radeon_uvd.c            |  2 +-
>>   src/gallium/drivers/radeonsi/cik_sdma.c            |  4 ++--
>>   src/gallium/drivers/radeonsi/si_dma.c              |  8 ++++----
>>   src/gallium/winsys/radeon/drm/radeon_drm_surface.c |  4 ++--
>>   12 files changed, 37 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/amd/common/ac_surface.c b/src/amd/common/ac_surface.c
>> index f7600a3..2b6c3fb 100644
>> --- a/src/amd/common/ac_surface.c
>> +++ b/src/amd/common/ac_surface.c
>> @@ -297,21 +297,21 @@ static int gfx6_compute_level(ADDR_HANDLE addrlib,
>>         ret = AddrComputeSurfaceInfo(addrlib,
>>                                      AddrSurfInfoIn,
>>                                      AddrSurfInfoOut);
>>         if (ret != ADDR_OK) {
>>                 return ret;
>>         }
>>         surf_level = is_stencil ? &surf->u.legacy.stencil_level[level] :
>> &surf->u.legacy.level[level];
>>         surf_level->offset = align64(surf->surf_size,
>> AddrSurfInfoOut->baseAlign);
>> -       surf_level->slice_size = AddrSurfInfoOut->sliceSize;
>> +       surf_level->slice_size_dw = AddrSurfInfoOut->sliceSize / 4;
>>         surf_level->nblk_x = AddrSurfInfoOut->pitch;
>>         surf_level->nblk_y = AddrSurfInfoOut->height;
>>         switch (AddrSurfInfoOut->tileMode) {
>>         case ADDR_TM_LINEAR_ALIGNED:
>>                 surf_level->mode = RADEON_SURF_MODE_LINEAR_ALIGNED;
>>                 break;
>>         case ADDR_TM_1D_TILED_THIN1:
>>                 surf_level->mode = RADEON_SURF_MODE_1D;
>>                 break;
>> diff --git a/src/amd/common/ac_surface.h b/src/amd/common/ac_surface.h
>> index 1dc95cd..a50aec2 100644
>> --- a/src/amd/common/ac_surface.h
>> +++ b/src/amd/common/ac_surface.h
>> @@ -64,21 +64,22 @@ enum radeon_micro_mode {
>>   /* bits 19 and 20 are reserved for libdrm_radeon, don't use them */
>>   #define RADEON_SURF_FMASK                       (1 << 21)
>>   #define RADEON_SURF_DISABLE_DCC                 (1 << 22)
>>   #define RADEON_SURF_TC_COMPATIBLE_HTILE         (1 << 23)
>>   #define RADEON_SURF_IMPORTED                    (1 << 24)
>>   #define RADEON_SURF_OPTIMIZE_FOR_SPACE          (1 << 25)
>>   #define RADEON_SURF_SHAREABLE                   (1 << 26)
>>     struct legacy_surf_level {
>>       uint64_t                    offset;
>> -    uint64_t                    slice_size;
>> +    /* Declare 32 bits of uint64_t, so that multiplication results in 64
>> bits. */
>> +    uint32_t                    slice_size_dw; /* in dwords; max = 4GB /
>> 4. */
>
>
> The comment is outdated now.
>
>
>>       uint32_t                    dcc_offset; /* relative offset within
>> DCC mip tree */
>>       uint32_t                    dcc_fast_clear_size;
>>       uint16_t                    nblk_x;
>>       uint16_t                    nblk_y;
>>       enum radeon_surf_mode       mode;
>>   };
>>     struct legacy_surf_layout {
>>       unsigned                    bankw:4;  /* max 8 */
>>       unsigned                    bankh:4;  /* max 8 */
>
> [snip]
>>
>> diff --git a/src/gallium/drivers/r600/r600_texture.c
>> b/src/gallium/drivers/r600/r600_texture.c
>> index f7c9b63..cc15e53 100644
>> --- a/src/gallium/drivers/r600/r600_texture.c
>> +++ b/src/gallium/drivers/r600/r600_texture.c
>> @@ -171,29 +171,29 @@ static void r600_copy_from_staging_texture(struct
>> pipe_context *ctx, struct r600
>>   }
>>     static unsigned r600_texture_get_offset(struct r600_common_screen
>> *rscreen,
>>                                         struct r600_texture *rtex,
>> unsigned level,
>>                                         const struct pipe_box *box,
>>                                         unsigned *stride,
>>                                         unsigned *layer_stride)
>>   {
>>         *stride = rtex->surface.u.legacy.level[level].nblk_x *
>>                 rtex->surface.bpe;
>> -       *layer_stride = rtex->surface.u.legacy.level[level].slice_size;
>> +       *layer_stride = rtex->surface.u.legacy.level[level].slice_size_dw
>> * 4;
>
>
> Missing cast, and this was potentially broken anyway... an assert would be
> nice (same for radeonsi).
>
> In general, the casts are systematically missing in r600. Even if there's
> some reason why r600 can never reach 4GB (though I don't see why right now),
> it'd be nice to do the cast for consistency, if code is every moved across
> drivers.

r600 can have at most 2GB of VRAM (Cayman). Let's say it also has 4GB
of GTT. You can't really allocate 4GB of GTT, because there is not
enough contiguous space (or free space even).

Since this is surface.u.legacy (GFX 6-8), radeonsi using this code can
have at most 8GB of VRAM (Hawaii and Polaris10). No other GFX 6-8 chip
has or will have so much memory.

Marek