From f45816181f52187b1fd906bcf689ee77cb500b3a Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:31:39 -0400 Subject: [PATCH 01/14] pan/decode: Print row strides, not line strides I.e. what's actually passed to the hardware, in case of compression or tiling. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/genxml/decode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/panfrost/lib/genxml/decode.c b/src/panfrost/lib/genxml/decode.c index ca1da2024434..03a2d3a93fc7 100644 --- a/src/panfrost/lib/genxml/decode.c +++ b/src/panfrost/lib/genxml/decode.c @@ -663,10 +663,10 @@ pandecode_texture_payload(mali_ptr payload, if (manual_stride && (i & 1)) { /* signed 32-bit snuck in as a 64-bit pointer */ uint64_t stride_set = pointers_and_strides[i]; - int32_t line_stride = stride_set; + int32_t row_stride = stride_set; int32_t surface_stride = stride_set >> 32; - pandecode_log("(mali_ptr) %d /* surface stride */ %d /* line stride */, \n", - surface_stride, line_stride); + pandecode_log("(mali_ptr) %d /* surface stride */ %d /* row stride */, \n", + surface_stride, row_stride); } else { char *a = pointer_as_memory_reference(pointers_and_strides[i]); pandecode_log("%s, \n", a); -- GitLab From a7fdedd247ca6ae66011c7d74894d3c2aaaac340 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:40:44 -0400 Subject: [PATCH 02/14] panvk: Remove unused layout structs PanVK switched to the common Panfrost layout code, but these duplicate structs stuck around. Garbage collect them to prevent confusion. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Boris Brezillon Part-of: --- src/panfrost/vulkan/panvk_private.h | 43 ----------------------------- 1 file changed, 43 deletions(-) diff --git a/src/panfrost/vulkan/panvk_private.h b/src/panfrost/vulkan/panvk_private.h index 350068542b53..105b1f5a43a5 100644 --- a/src/panfrost/vulkan/panvk_private.h +++ b/src/panfrost/vulkan/panvk_private.h @@ -937,49 +937,6 @@ struct panvk_pipeline { VkRect2D scissor; }; -struct panvk_image_level { - VkDeviceSize offset; - VkDeviceSize size; - uint32_t pitch; -}; - -struct panvk_slice_layout { - unsigned width; - unsigned height; - unsigned depth; - unsigned offset; - unsigned line_stride; - unsigned size; - - /* If there is a header preceding each slice, how big is - * that header? Used for AFBC. - */ - unsigned afbc_header_size; - - /* If checksumming is enabled following the slice, what - * is its offset/stride? - */ - struct { - unsigned offset; - unsigned stride; - unsigned size; - } checksum; -}; - -#define PANVK_MAX_MIP_LEVELS 13 - -struct panvk_plane_layout { - struct panvk_slice_layout slices[PANVK_MAX_MIP_LEVELS]; - unsigned offset; - unsigned array_stride; - unsigned size; -}; - -struct panvk_plane_memory { - const struct panfrost_bo *bo; - unsigned offset; -}; - #define PANVK_MAX_PLANES 1 struct panvk_image { -- GitLab From a741bd5db1376b7a354a3472f5b15078cdfced77 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:20:54 -0400 Subject: [PATCH 03/14] panvk: Report row_stride in GetImageSubresourceLayout ...Rather than line_stride. For linear images, these are equivalent. For nonlinear images, rowPitch is implementation-defined. So this isn't strictly a bug fix, but it gets rid of the nonsense nonlinear line_stride. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Boris Brezillon Part-of: --- src/panfrost/vulkan/panvk_image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/panfrost/vulkan/panvk_image.c b/src/panfrost/vulkan/panvk_image.c index c0ef0d434276..7c97b9d3efb9 100644 --- a/src/panfrost/vulkan/panvk_image.c +++ b/src/panfrost/vulkan/panvk_image.c @@ -247,7 +247,7 @@ panvk_GetImageSubresourceLayout(VkDevice _device, (pSubresource->arrayLayer * image->pimage.layout.array_stride); pLayout->size = slice_layout->size; - pLayout->rowPitch = slice_layout->line_stride; + pLayout->rowPitch = slice_layout->row_stride; pLayout->arrayPitch = image->pimage.layout.array_stride; pLayout->depthPitch = slice_layout->surface_stride; } -- GitLab From c40ebd859c5147ff6f2b0ce529bef81899369233 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 12:25:11 -0400 Subject: [PATCH 04/14] panfrost: Add helpers to work with legacy strides Unfortunately, the botched nonlinear "line strides" have become ingrained in the UABI. We need to be work with them. Add safe helpers to convert to/from the legacy strides. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/pan_layout.c | 18 ++++++++++++++++++ src/panfrost/lib/pan_texture.h | 9 +++++++++ 2 files changed, 27 insertions(+) diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index 77842bec5e6b..c53013c194d0 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -164,6 +164,24 @@ panfrost_get_layer_stride(const struct pan_image_layout *layout, return layout->slices[level].surface_stride; } +unsigned +panfrost_get_legacy_stride(const struct pan_image_layout *layout, + unsigned level) +{ + unsigned row_stride = layout->slices[level].row_stride; + unsigned bh = panfrost_block_size(layout->modifier, layout->format).height; + + return row_stride / bh; +} + +unsigned +panfrost_from_legacy_stride(unsigned legacy_stride, + enum pipe_format format, + uint64_t modifier) +{ + return legacy_stride * panfrost_block_size(modifier, format).height; +} + /* Computes the offset into a texture at a particular level/face. Add to * the base address of a texture to get the address to that level/face */ diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index a2febecce3d5..e9497f2c111b 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -229,6 +229,15 @@ bool pan_image_layout_init(struct pan_image_layout *layout, const struct pan_image_explicit_layout *explicit_layout); +unsigned +panfrost_get_legacy_stride(const struct pan_image_layout *layout, + unsigned level); + +unsigned +panfrost_from_legacy_stride(unsigned legacy_stride, + enum pipe_format format, + uint64_t modifier); + struct pan_surface { union { mali_ptr data; -- GitLab From 81a686a714a39bc943c916352ab32642dfbe796c Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:23:37 -0400 Subject: [PATCH 05/14] panfrost: Use row stride for explicit layouts Line strides don't make sense for linear images, so use row strides instead in the API. Then update the layout code accordingly. Note: we need to preserve the old UABI (bug for bug compatibility), so we still use legacy strides externally. But now we use row strides internally, which is better than using both everywhere. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_resource.c | 4 ++-- src/panfrost/lib/pan_layout.c | 12 ++++++------ src/panfrost/lib/pan_texture.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index a6e77b6e1179..53a89f3db8bf 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -86,7 +86,7 @@ panfrost_resource_from_handle(struct pipe_screen *pscreen, PAN_IMAGE_CRC_OOB : PAN_IMAGE_CRC_NONE; struct pan_image_explicit_layout explicit_layout = { .offset = whandle->offset, - .line_stride = whandle->stride, + .row_stride = panfrost_from_legacy_stride(whandle->stride, templat->format, mod) }; rsc->image.layout = (struct pan_image_layout) { @@ -177,7 +177,7 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen, return false; } - handle->stride = rsrc->image.layout.slices[0].line_stride; + handle->stride = panfrost_get_legacy_stride(&rsrc->image.layout, 0); handle->offset = rsrc->image.layout.slices[0].offset; return true; } diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index c53013c194d0..e033dc2bf251 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -246,21 +246,21 @@ pan_image_layout_init(struct pan_image_layout *layout, slice->offset = offset; /* Compute the would-be stride */ - unsigned stride = bytes_per_pixel * effective_width; + unsigned row_stride = bytes_per_pixel * effective_width * block_size.height; if (explicit_layout) { /* Make sure the explicit stride is valid */ - if (explicit_layout->line_stride < stride) + if (explicit_layout->row_stride < row_stride) return false; - stride = explicit_layout->line_stride; + row_stride = explicit_layout->row_stride; } else if (linear) { /* Keep lines alignment on 64 byte for performance */ - stride = ALIGN_POT(stride, 64); + row_stride = ALIGN_POT(row_stride, 64); } - slice->line_stride = stride; - slice->row_stride = stride * block_size.height; + slice->line_stride = row_stride / block_size.height; + slice->row_stride = row_stride; unsigned slice_one_size = slice->line_stride * effective_height; diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index e9497f2c111b..83d057f1bde5 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -222,7 +222,7 @@ struct pan_scoreboard; struct pan_image_explicit_layout { unsigned offset; - unsigned line_stride; + unsigned row_stride; }; bool -- GitLab From 0b788e2ee4a229f3e19420f7c653495e98995af7 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:24:57 -0400 Subject: [PATCH 06/14] panfrost: Rename away from bytes_per_pixel This name is wrong for block-compressed formats. The code worked out anyway, but rename it for clarity. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/pan_layout.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index e033dc2bf251..0d9f56c590f3 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -212,7 +212,7 @@ pan_image_layout_init(struct pan_image_layout *layout, if (explicit_layout && (explicit_layout->offset & 63)) return false; - unsigned bytes_per_pixel = util_format_get_blocksize(layout->format); + unsigned fmt_blocksize = util_format_get_blocksize(layout->format); /* MSAA is implemented as a 3D texture with z corresponding to the * sample #, horrifyingly enough */ @@ -245,8 +245,7 @@ pan_image_layout_init(struct pan_image_layout *layout, slice->offset = offset; - /* Compute the would-be stride */ - unsigned row_stride = bytes_per_pixel * effective_width * block_size.height; + unsigned row_stride = fmt_blocksize * effective_width * block_size.height; if (explicit_layout) { /* Make sure the explicit stride is valid */ -- GitLab From 3a4207dde6b655b0eaa6096f21758e5943b2721b Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:37:40 -0400 Subject: [PATCH 07/14] panfrost: Use row stride to calculate slice size This expresses what's actually happening. Equivalent to the old calculation due to some cancellation. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/pan_layout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index 0d9f56c590f3..32417404bfd1 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -261,7 +261,7 @@ pan_image_layout_init(struct pan_image_layout *layout, slice->line_stride = row_stride / block_size.height; slice->row_stride = row_stride; - unsigned slice_one_size = slice->line_stride * effective_height; + unsigned slice_one_size = row_stride * (effective_height / block_size.height); /* Compute AFBC sizes if necessary */ if (afbc) { -- GitLab From c11df56cff601f7dec9c5e4ec26a8b0e5244b674 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 11:48:47 -0400 Subject: [PATCH 08/14] panfrost: Adapt get_param for row strides Let's preserve the old behaviour -- this is UABI, we need to be bug for bug compatible. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 53a89f3db8bf..c6ffb0ee52bd 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -195,7 +195,7 @@ panfrost_resource_get_param(struct pipe_screen *pscreen, switch (param) { case PIPE_RESOURCE_PARAM_STRIDE: - *value = rsrc->image.layout.slices[level].line_stride; + *value = panfrost_get_legacy_stride(&rsrc->image.layout, level); return true; case PIPE_RESOURCE_PARAM_OFFSET: *value = rsrc->image.layout.slices[level].offset; -- GitLab From c4241a831f3a3b8cff707ea7fb4e2a99c830b697 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 12:10:37 -0400 Subject: [PATCH 09/14] panfrost: Use row_stride even for linear resources In that case, they match up. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_resource.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index c6ffb0ee52bd..6a13e5c94d4f 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -942,7 +942,7 @@ panfrost_ptr_map(struct pipe_context *pctx, /* Staging resources have one LOD: level 0. Query the strides * on this LOD. */ - transfer->base.stride = staging->image.layout.slices[0].line_stride; + transfer->base.stride = staging->image.layout.slices[0].row_stride; transfer->base.layer_stride = panfrost_get_layer_stride(&staging->image.layout, 0); @@ -1088,7 +1088,7 @@ panfrost_ptr_map(struct pipe_context *pctx, if ((usage & dpw) == dpw && rsrc->index_cache) return NULL; - transfer->base.stride = rsrc->image.layout.slices[level].line_stride; + transfer->base.stride = rsrc->image.layout.slices[level].row_stride; transfer->base.layer_stride = panfrost_get_layer_stride(&rsrc->image.layout, level); @@ -1103,7 +1103,7 @@ panfrost_ptr_map(struct pipe_context *pctx, return bo->ptr.cpu + rsrc->image.layout.slices[level].offset + box->z * transfer->base.layer_stride - + box_blocks.y * rsrc->image.layout.slices[level].line_stride + + box_blocks.y * rsrc->image.layout.slices[level].row_stride + box_blocks.x * bytes_per_block; } } @@ -1284,7 +1284,7 @@ panfrost_ptr_unmap(struct pipe_context *pctx, util_copy_rect( bo->ptr.cpu + prsrc->image.layout.slices[0].offset, prsrc->base.format, - prsrc->image.layout.slices[0].line_stride, + prsrc->image.layout.slices[0].row_stride, 0, 0, transfer->box.width, transfer->box.height, -- GitLab From 61100e7011109affa1746593ff72010cfe702913 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 12:04:54 -0400 Subject: [PATCH 10/14] lima,panfrost: Use row stride for tiling routines This makes more sense. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Erico Nunes Part-of: --- src/gallium/drivers/lima/lima_resource.c | 12 ++++++++++-- src/gallium/drivers/panfrost/pan_resource.c | 4 ++-- src/panfrost/shared/pan_tiling.c | 6 ++---- src/panfrost/shared/pan_tiling.h | 4 ++-- src/panfrost/shared/test/test-tiling.cpp | 5 +++-- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/gallium/drivers/lima/lima_resource.c b/src/gallium/drivers/lima/lima_resource.c index edd14cf80e59..e8f5aa6e620c 100644 --- a/src/gallium/drivers/lima/lima_resource.c +++ b/src/gallium/drivers/lima/lima_resource.c @@ -677,6 +677,10 @@ lima_transfer_map(struct pipe_context *pctx, trans->staging = malloc(ptrans->stride * ptrans->box.height * ptrans->box.depth); if (usage & PIPE_MAP_READ) { + unsigned line_stride = res->levels[level].stride; + unsigned row_height = util_format_is_compressed(pres->format) ? 4 : 16; + unsigned row_stride = line_stride * row_height; + unsigned i; for (i = 0; i < ptrans->box.depth; i++) panfrost_load_tiled_image( @@ -685,7 +689,7 @@ lima_transfer_map(struct pipe_context *pctx, ptrans->box.x, ptrans->box.y, ptrans->box.width, ptrans->box.height, ptrans->stride, - res->levels[level].stride, + row_stride, pres->format); } @@ -783,13 +787,17 @@ lima_transfer_unmap_inner(struct lima_context *ctx, /* Update texture descriptor */ ctx->dirty |= LIMA_CONTEXT_DIRTY_TEXTURES; } else { + unsigned line_stride = res->levels[ptrans->level].stride; + unsigned row_height = util_format_is_compressed(pres->format) ? 4 : 16; + unsigned row_stride = line_stride * row_height; + for (i = 0; i < trans->base.box.depth; i++) panfrost_store_tiled_image( bo->map + res->levels[trans->base.level].offset + (i + trans->base.box.z) * res->levels[trans->base.level].layer_stride, trans->staging + i * ptrans->stride * ptrans->box.height, ptrans->box.x, ptrans->box.y, ptrans->box.width, ptrans->box.height, - res->levels[ptrans->level].stride, + row_stride, ptrans->stride, pres->format); } diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 6a13e5c94d4f..583b86d95458 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -876,7 +876,7 @@ panfrost_load_tiled_images(struct panfrost_transfer *transfer, panfrost_load_tiled_image(dst, map, ptrans->box.x, ptrans->box.y, ptrans->box.width, ptrans->box.height, ptrans->stride, - rsrc->image.layout.slices[level].line_stride, + rsrc->image.layout.slices[level].row_stride, rsrc->image.layout.format); } } @@ -902,7 +902,7 @@ panfrost_store_tiled_images(struct panfrost_transfer *transfer, panfrost_store_tiled_image(map, src, ptrans->box.x, ptrans->box.y, ptrans->box.width, ptrans->box.height, - rsrc->image.layout.slices[level].line_stride, + rsrc->image.layout.slices[level].row_stride, ptrans->stride, rsrc->image.layout.format); } } diff --git a/src/panfrost/shared/pan_tiling.c b/src/panfrost/shared/pan_tiling.c index 3adc89db98bf..1e02053d8b36 100644 --- a/src/panfrost/shared/pan_tiling.c +++ b/src/panfrost/shared/pan_tiling.c @@ -175,8 +175,7 @@ panfrost_access_tiled_image_##pixel_t \ { \ uint8_t *dest_start = dst + ((sx >> 4) * PIXELS_PER_TILE * sizeof(pixel_t)); \ for (int y = sy, src_y = 0; src_y < h; ++y, ++src_y) { \ - uint16_t block_y = y & ~0x0f; \ - uint8_t *dest = (uint8_t *) (dest_start + (block_y * dst_stride)); \ + uint8_t *dest = (uint8_t *) (dest_start + ((y >> 4) * dst_stride)); \ pixel_t *source = src + (src_y * src_stride); \ pixel_t *source_end = source + w; \ unsigned expanded_y = bit_duplication[y & 0xF] << shift; \ @@ -201,8 +200,7 @@ TILED_ACCESS_TYPE(pan_uint128_t, 4); #define TILED_UNALIGNED_TYPE(pixel_t, is_store, tile_shift) { \ const unsigned mask = (1 << tile_shift) - 1; \ for (int y = sy, src_y = 0; src_y < h; ++y, ++src_y) { \ - unsigned block_y = y & ~mask; \ - unsigned block_start_s = block_y * dst_stride; \ + unsigned block_start_s = (y >> tile_shift) * dst_stride; \ unsigned source_start = src_y * src_stride; \ unsigned expanded_y = bit_duplication[y & mask]; \ \ diff --git a/src/panfrost/shared/pan_tiling.h b/src/panfrost/shared/pan_tiling.h index 009ba2804346..d63c581edef5 100644 --- a/src/panfrost/shared/pan_tiling.h +++ b/src/panfrost/shared/pan_tiling.h @@ -44,7 +44,7 @@ extern "C" { * @z Region of interest of source in pixels, aligned to block size * @w Region of interest of source in pixels, aligned to block size * @dst_stride Stride in bytes of linear destination - * @src_stride Stride in bytes of tiled source + * @src_stride Number of bytes between adjacent rows of tiles in source. * @format Format of the source and destination image */ void panfrost_load_tiled_image(void *dst, const void *src, @@ -63,7 +63,7 @@ void panfrost_load_tiled_image(void *dst, const void *src, * @y Region of interest of destination in pixels, aligned to block size * @z Region of interest of destination in pixels, aligned to block size * @w Region of interest of destination in pixels, aligned to block size - * @dst_stride Stride in bytes of tiled destination + * @dst_stride Number of bytes between adjacent rows of tiles in destination. * @src_stride Stride in bytes of linear source * @format Format of the source and destination image */ diff --git a/src/panfrost/shared/test/test-tiling.cpp b/src/panfrost/shared/test/test-tiling.cpp index e6b9fe669f51..d5ad9e31c7fd 100644 --- a/src/panfrost/shared/test/test-tiling.cpp +++ b/src/panfrost/shared/test/test-tiling.cpp @@ -61,7 +61,7 @@ tiled_offset(unsigned x, unsigned y, unsigned stride, unsigned tilesize, unsigne unsigned index_in_tile = u_order(x_in_tile, y_in_tile); - unsigned row_offset = tile_y * (stride * tilesize); + unsigned row_offset = tile_y * stride; unsigned col_offset = (tile_x * tilesize * tilesize) * blocksize; unsigned block_offset = index_in_tile * blocksize; @@ -128,10 +128,11 @@ test(unsigned width, unsigned height, unsigned rx, unsigned ry, enum pipe_format format, bool store) { unsigned bpp = util_format_get_blocksize(format); + unsigned tile_height = util_format_is_compressed(format) ? 4 : 16; unsigned tiled_width = ALIGN_POT(width, 16); unsigned tiled_height = ALIGN_POT(height, 16); - unsigned tiled_stride = tiled_width * bpp; + unsigned tiled_stride = tiled_width * tile_height * bpp; unsigned dst_stride = store ? tiled_stride : linear_stride; unsigned src_stride = store ? linear_stride : tiled_stride; -- GitLab From 1842e14a733b442fd6265d1efe7c0282879be048 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 12:12:05 -0400 Subject: [PATCH 11/14] panfrost: Remove line_stride There are no more users. This eliminates a class of issues. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/pan_layout.c | 1 - src/panfrost/lib/pan_texture.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index 32417404bfd1..c6642b156699 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -258,7 +258,6 @@ pan_image_layout_init(struct pan_image_layout *layout, row_stride = ALIGN_POT(row_stride, 64); } - slice->line_stride = row_stride / block_size.height; slice->row_stride = row_stride; unsigned slice_one_size = row_stride * (effective_height / block_size.height); diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index 83d057f1bde5..9c89923a6894 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -49,7 +49,6 @@ extern uint64_t pan_best_modifiers[PAN_MODIFIER_COUNT]; struct pan_image_slice_layout { unsigned offset; - unsigned line_stride; unsigned row_stride; unsigned surface_stride; -- GitLab From 579fd30209fa5ca7fa2af75f0eee90d2d2984afd Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 14:09:01 -0400 Subject: [PATCH 12/14] panfrost: Unify row stride and AFBC row stride Row stride is defined in terms of header blocks for AFBC. Usually, afbc.row_stride is used for AFBC images and row_stride for non-AFBC images; however, the nonsense non-AFBC stride leaked into the UABI. So handle that in the legacy conversion path and use a unified row stride (equal to afbc.row_stride for AFBC images and row_stride otherwise). Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/pan_cs.c | 4 ++-- src/panfrost/lib/pan_layout.c | 34 +++++++++++++++++++++++++++------- src/panfrost/lib/pan_texture.c | 2 +- src/panfrost/lib/pan_texture.h | 13 ++++++++++--- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/panfrost/lib/pan_cs.c b/src/panfrost/lib/pan_cs.c index 4ca908dbcbbf..880bb1878a83 100644 --- a/src/panfrost/lib/pan_cs.c +++ b/src/panfrost/lib/pan_cs.c @@ -206,7 +206,7 @@ pan_prepare_zs(const struct pan_fb_info *fb, #if PAN_ARCH >= 6 const struct pan_image_slice_layout *slice = &zs->image->layout.slices[level]; - ext->zs_afbc_row_stride = slice->afbc.row_stride / + ext->zs_afbc_row_stride = slice->row_stride / AFBC_HEADER_BYTES_PER_TILE; #else ext->zs_block_format = MALI_BLOCK_FORMAT_AFBC; @@ -448,7 +448,7 @@ pan_prepare_rt(const struct pan_fb_info *fb, unsigned idx, const struct pan_image_slice_layout *slice = &rt->image->layout.slices[level]; #if PAN_ARCH >= 6 - cfg->afbc.row_stride = slice->afbc.row_stride / + cfg->afbc.row_stride = slice->row_stride / AFBC_HEADER_BYTES_PER_TILE; cfg->afbc.afbc_wide_block_enable = panfrost_afbc_is_wide(rt->image->layout.modifier); diff --git a/src/panfrost/lib/pan_layout.c b/src/panfrost/lib/pan_layout.c index c6642b156699..ee135f2a5115 100644 --- a/src/panfrost/lib/pan_layout.c +++ b/src/panfrost/lib/pan_layout.c @@ -169,9 +169,17 @@ panfrost_get_legacy_stride(const struct pan_image_layout *layout, unsigned level) { unsigned row_stride = layout->slices[level].row_stride; - unsigned bh = panfrost_block_size(layout->modifier, layout->format).height; + struct pan_block_size block_size = + panfrost_block_size(layout->modifier, layout->format); - return row_stride / bh; + if (drm_is_afbc(layout->modifier)) { + unsigned width = u_minify(layout->width, level); + width = ALIGN_POT(width, block_size.width); + + return width * util_format_get_blocksize(layout->format); + } else { + return row_stride / block_size.height; + } } unsigned @@ -179,7 +187,16 @@ panfrost_from_legacy_stride(unsigned legacy_stride, enum pipe_format format, uint64_t modifier) { - return legacy_stride * panfrost_block_size(modifier, format).height; + struct pan_block_size block_size = + panfrost_block_size(modifier, format); + + if (drm_is_afbc(modifier)) { + unsigned width = legacy_stride / util_format_get_blocksize(format); + + return (width / block_size.width) * AFBC_HEADER_BYTES_PER_TILE; + } else { + return legacy_stride * block_size.height; + } } /* Computes the offset into a texture at a particular level/face. Add to @@ -247,7 +264,7 @@ pan_image_layout_init(struct pan_image_layout *layout, unsigned row_stride = fmt_blocksize * effective_width * block_size.height; - if (explicit_layout) { + if (explicit_layout && !afbc) { /* Make sure the explicit stride is valid */ if (explicit_layout->row_stride < row_stride) return false; @@ -258,8 +275,6 @@ pan_image_layout_init(struct pan_image_layout *layout, row_stride = ALIGN_POT(row_stride, 64); } - slice->row_stride = row_stride; - unsigned slice_one_size = row_stride * (effective_height / block_size.height); /* Compute AFBC sizes if necessary */ @@ -268,10 +283,13 @@ pan_image_layout_init(struct pan_image_layout *layout, panfrost_afbc_header_size(width, height); /* Stride between two rows of AFBC headers */ - slice->afbc.row_stride = + slice->row_stride = (effective_width / block_size.width) * AFBC_HEADER_BYTES_PER_TILE; + if (explicit_layout && explicit_layout->row_stride < slice->row_stride) + return false; + /* AFBC body size */ slice->afbc.body_size = slice_one_size; @@ -289,6 +307,8 @@ pan_image_layout_init(struct pan_image_layout *layout, slice_one_size += slice->afbc.header_size; slice->afbc.surface_stride = slice_one_size; } + } else { + slice->row_stride = row_stride; } unsigned slice_full_size = diff --git a/src/panfrost/lib/pan_texture.c b/src/panfrost/lib/pan_texture.c index 9001389d5dbc..3709ce3b3115 100644 --- a/src/panfrost/lib/pan_texture.c +++ b/src/panfrost/lib/pan_texture.c @@ -242,7 +242,7 @@ panfrost_get_surface_strides(const struct pan_image_layout *layout, if (drm_is_afbc(layout->modifier)) { /* Pre v7 don't have a row stride field. This field is * repurposed as a Y offset which we don't use */ - *row_stride = PAN_ARCH < 7 ? 0 : slice->afbc.row_stride; + *row_stride = PAN_ARCH < 7 ? 0 : slice->row_stride; *surf_stride = slice->afbc.surface_stride; } else { *row_stride = slice->row_stride; diff --git a/src/panfrost/lib/pan_texture.h b/src/panfrost/lib/pan_texture.h index 9c89923a6894..c44777f7534b 100644 --- a/src/panfrost/lib/pan_texture.h +++ b/src/panfrost/lib/pan_texture.h @@ -49,7 +49,17 @@ extern uint64_t pan_best_modifiers[PAN_MODIFIER_COUNT]; struct pan_image_slice_layout { unsigned offset; + + /* For AFBC images, the number of bytes between two rows of AFBC + * headers. + * + * For non-AFBC images, the number of bytes between two rows of texels. + * For linear images, this will equal the logical stride. For + * images that are compressed or interleaved, this will be greater than + * the logical stride. + */ unsigned row_stride; + unsigned surface_stride; struct { @@ -59,9 +69,6 @@ struct pan_image_slice_layout { /* Size of the AFBC body */ unsigned body_size; - /* Stride between two rows of AFBC headers */ - unsigned row_stride; - /* Stride between AFBC headers of two consecutive surfaces. * For 3D textures, this must be set to header size since * AFBC headers are allocated together, for 2D arrays this -- GitLab From 6d0505701d0c5de8a357b736b411f041787adf37 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 14:50:59 -0400 Subject: [PATCH 13/14] panfrost: Unit test stride calculations These have reasonable interpretations now, and the three row strides have been deduplicated. So add stride expectations to our ASTC unit tests. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/tests/test-layout.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/panfrost/lib/tests/test-layout.cpp b/src/panfrost/lib/tests/test-layout.cpp index 7dcc2de1eb10..8fa598cfcf8f 100644 --- a/src/panfrost/lib/tests/test-layout.cpp +++ b/src/panfrost/lib/tests/test-layout.cpp @@ -211,9 +211,12 @@ TEST(Layout, ImplicitLayoutInterleavedASTC5x5) /* The image is 50x50 pixels, with 5x5 blocks. So it is a 10x10 grid of ASTC * blocks. 4x4 tiles of ASTC blocks are u-interleaved, so we have to round up * to a 12x12 grid. So we need space for 144 ASTC blocks. Each ASTC block is - * 16 bytes (128-bits), so we require 2304 bytes. + * 16 bytes (128-bits), so we require 2304 bytes, with a row stride of 12 * + * 16 * 4 = 192 bytes. */ EXPECT_EQ(l.slices[0].offset, 0); + EXPECT_EQ(l.slices[0].row_stride, 768); + EXPECT_EQ(l.slices[0].surface_stride, 2304); EXPECT_EQ(l.slices[0].size, 2304); } @@ -233,10 +236,12 @@ TEST(Layout, ImplicitLayoutLinearASTC5x5) ASSERT_TRUE(pan_image_layout_init(&l, NULL)); /* The image is 50x50 pixels, with 5x5 blocks. So it is a 10x10 grid of ASTC - * blocks. Each ASTC block is 16 bytes, so the row stride is 160 bytes. - * Rows are cache-line aligned to 192 bytes. There are 10 rows, so we have + * blocks. Each ASTC block is 16 bytes, so the row stride is 160 bytes, + * rounded up to the cache line (192 bytes). There are 10 rows, so we have * 1920 bytes total. */ EXPECT_EQ(l.slices[0].offset, 0); + EXPECT_EQ(l.slices[0].row_stride, 192); + EXPECT_EQ(l.slices[0].surface_stride, 1920); EXPECT_EQ(l.slices[0].size, 1920); } -- GitLab From 575068a1656ab4303647ade1491da7d711d36db7 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 27 Apr 2022 14:59:41 -0400 Subject: [PATCH 14/14] panfrost: Unit test "from legacy" helper So we don't regress the UABI. This doesn't get much CI coverage otherwise. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/tests/test-layout.cpp | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/panfrost/lib/tests/test-layout.cpp b/src/panfrost/lib/tests/test-layout.cpp index 8fa598cfcf8f..f726f88c2b89 100644 --- a/src/panfrost/lib/tests/test-layout.cpp +++ b/src/panfrost/lib/tests/test-layout.cpp @@ -162,6 +162,37 @@ TEST(BlockSize, AFBCSuperblock64x4) EXPECT_TRUE(panfrost_afbc_is_wide(modifier)); } +TEST(LegacyStride, FromLegacyLinear) +{ + EXPECT_EQ(panfrost_from_legacy_stride(1920 * 4, PIPE_FORMAT_R8G8B8A8_UINT, DRM_FORMAT_MOD_LINEAR), 1920 * 4); + EXPECT_EQ(panfrost_from_legacy_stride(53, PIPE_FORMAT_R8_SNORM, DRM_FORMAT_MOD_LINEAR), 53); + EXPECT_EQ(panfrost_from_legacy_stride(60, PIPE_FORMAT_ETC2_RGB8, DRM_FORMAT_MOD_LINEAR), 60); +} + +TEST(LegacyStride, FromLegacyInterleaved) +{ + EXPECT_EQ(panfrost_from_legacy_stride(1920 * 4, PIPE_FORMAT_R8G8B8A8_UINT, + DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED), + 1920 * 4 * 16); + + EXPECT_EQ(panfrost_from_legacy_stride(53, PIPE_FORMAT_R8_SNORM, + DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED), 53 * 16); + + EXPECT_EQ(panfrost_from_legacy_stride(60, PIPE_FORMAT_ETC2_RGB8, + DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED), 60 * 4); +} + +TEST(LegacyStride, FromLegacyAFBC) +{ + uint64_t modifier = DRM_FORMAT_MOD_ARM_AFBC( + AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 | + AFBC_FORMAT_MOD_SPARSE | + AFBC_FORMAT_MOD_YTR); + + EXPECT_EQ(panfrost_from_legacy_stride(1920 * 4, PIPE_FORMAT_R8G8B8A8_UINT, modifier), 60 * 16); + EXPECT_EQ(panfrost_from_legacy_stride(64, PIPE_FORMAT_R8_SNORM, modifier), 2 * 16); +} + /* dEQP-GLES3.functional.texture.format.compressed.etc1_2d_pot */ TEST(Layout, ImplicitLayoutInterleavedETC2) { -- GitLab