From 43202320ee8cc0ca5da828e86d87cbddcfb29889 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 26 Apr 2022 11:34:45 -0400 Subject: [PATCH] panfrost: Always use 64-bit SD with strides Midgard has multiple Surface Descriptor formats selectable in the texture descriptor. Previously, we have used both the "64-bit surface descriptor" and the "64-bit surface descriptor with 32-bit line stride and 32-bit layer stride". A delicate routine tried to guess what stride the hardware will use if we don't specify it explicitly, and omit the stride if it matches. Unfortunately, that routine is broken in at least two ways: * Textures with ASTC must always specify an explicit stride. Failing to do so (like we were doing) is invalid. * It applies even for interleaved textures. The comment above the function saying otherwise is incorrect. (TODO: double check this) Bifrost onwards always specify the strides explicitly. Let's just do that and unify the gens. What is lost from doing this? A ludicrously trivial amount of memory and texture descriptor cache space. 8 bytes per layer*level per texture, in fact. Compared to the size of the textures being addressed, the memory usage is trivial. The texture descriptor cache size maybe matters more. But given Arm's hardware people went this direction for Bifrost and stuck to it, I doubt it matters much. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/lib/pan_texture.c | 70 ++++++---------------------------- 1 file changed, 11 insertions(+), 59 deletions(-) diff --git a/src/panfrost/lib/pan_texture.c b/src/panfrost/lib/pan_texture.c index 5eeb2430e1c..ce65ff52ebf 100644 --- a/src/panfrost/lib/pan_texture.c +++ b/src/panfrost/lib/pan_texture.c @@ -407,7 +407,6 @@ panfrost_emit_plane(const struct pan_image_layout *layout, static void panfrost_emit_texture_payload(const struct pan_image_view *iview, enum pipe_format format, - bool manual_stride, void *payload) { const struct pan_image_layout *layout = &iview->image->layout; @@ -457,63 +456,21 @@ panfrost_emit_texture_payload(const struct pan_image_view *iview, iter.level, iter.layer, iter.face, iter.sample); - if (!manual_stride) { -#if PAN_ARCH <= 5 - pan_pack(payload, SURFACE, cfg) { - cfg.pointer = pointer; - } - payload += pan_size(SURFACE); -#else - unreachable("must use explicit stride on Bifrost"); -#endif - } else { #if PAN_ARCH >= 9 - panfrost_emit_plane(layout, format, pointer, iter.level, payload); - payload += pan_size(PLANE); + panfrost_emit_plane(layout, format, pointer, iter.level, payload); + payload += pan_size(PLANE); #else - pan_pack(payload, SURFACE_WITH_STRIDE, cfg) { - cfg.pointer = pointer; - panfrost_get_surface_strides(layout, iter.level, - &cfg.row_stride, - &cfg.surface_stride); - } - payload += pan_size(SURFACE_WITH_STRIDE); -#endif + pan_pack(payload, SURFACE_WITH_STRIDE, cfg) { + cfg.pointer = pointer; + panfrost_get_surface_strides(layout, iter.level, + &cfg.row_stride, + &cfg.surface_stride); } + payload += pan_size(SURFACE_WITH_STRIDE); +#endif } } -/* Check if we need to set a custom stride by computing the "expected" - * stride and comparing it to what the user actually wants. Only applies - * to linear textures, since tiled/compressed textures have strict - * alignment requirements for their strides as it is */ - -static bool -panfrost_needs_explicit_stride(const struct pan_image_view *iview) -{ - /* Stride is explicit on Bifrost */ - if (PAN_ARCH >= 6) - return true; - - if (iview->image->layout.modifier != DRM_FORMAT_MOD_LINEAR) - return false; - - unsigned bytes_per_block = util_format_get_blocksize(iview->format); - unsigned block_w = util_format_get_blockwidth(iview->format); - - for (unsigned l = iview->first_level; l <= iview->last_level; ++l) { - unsigned actual = iview->image->layout.slices[l].line_stride; - unsigned expected = - DIV_ROUND_UP(u_minify(iview->image->layout.width, l), block_w) * - bytes_per_block; - - if (actual != expected) - return true; - } - - return false; -} - #if PAN_ARCH <= 7 /* Map modifiers to mali_texture_layout for packing in a texture descriptor */ @@ -567,12 +524,7 @@ GENX(panfrost_new_texture)(const struct panfrost_device *dev, swizzle = panfrost_translate_swizzle_4(iview->swizzle); } - bool manual_stride = - panfrost_needs_explicit_stride(iview); - - panfrost_emit_texture_payload(iview, format, - manual_stride, - payload->cpu); + panfrost_emit_texture_payload(iview, format, payload->cpu); unsigned array_size = iview->last_layer - iview->first_layer + 1; @@ -625,7 +577,7 @@ GENX(panfrost_new_texture)(const struct panfrost_device *dev, cfg.minimum_lod = FIXED_16(0, false); cfg.maximum_lod = FIXED_16(cfg.levels - 1, false); #else - cfg.manual_stride = manual_stride; + cfg.manual_stride = true; #endif } }