From ecc90e1bb3f5954a891ec4ebe929770b337c93d8 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Fri, 16 May 2025 15:02:18 -0700 Subject: [PATCH] intel/isl: don't clamp num_elements to (1 << 27) The BSpec page for Structure_RENDER_SURFACE_STATE says: "For typed buffer and structured buffer surfaces, the number of entries in the buffer ranges from 1 to 2^27. For raw buffer surfaces, the number of entries in the buffer is the number of bytes which can range from 1 to 2^30. After subtracting one from the number of entries, software must place the fields of the resulting 27-bit value into the Height, Width, and Depth fields as indicated, right-justified in each field. Unused upper bits must be set to zero." According to the vkd3d-proton developers, this is what is happening with the applications: "There's also the problematic case of games using typed descriptors but passing non-typed buffer descriptors, which is an extremely common app bug that works on all D3D12 drivers that we need to work around by creating typed views." Previously, we had an assert() to check for "num_elements > (1 << 27)", but that assert was preventing us from running games such as Marvel's Spider-Man Remastered and Assassin's Creed: Valhalla in Debug mode. So not only I removed the assert, but I also made the code clamp num_elements to the maximum of (1 << 27) based on my incorrect interpretation of the paragraph quoted above from BSpec. What I did not realize was that num_elements is being used just to calculate Structure_RENDER_SURFACE_STATE Height, Width and Depth, and our register bit fields on SKL and newer are big enough to fit any number of num_elements up to 2^32, not only 2^27. Clamping num_elements results in an incorrect value for S.Depth, which generates visual corruption in some games. On Marvel's Spider-Man Remastered, without this patch the texture of the asphalt in some streets (like the very first one you jump to when the game starts) gets rendered incorrectly. Testcase: vkd3d-proton/d3d12/test_large_texel_buffer_view Link: https://github.com/HansKristian-Work/vkd3d-proton/issues/2071 Link: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12827 Fixes: f3c7e14f091a ("isl: don't assert(num_elements > (1ull << 27))") Reviewed-by: Lionel Landwerlin Signed-off-by: Paulo Zanoni Part-of: --- src/intel/isl/isl_surface_state.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/intel/isl/isl_surface_state.c b/src/intel/isl/isl_surface_state.c index b492a67edb0..1ad7339d452 100644 --- a/src/intel/isl/isl_surface_state.c +++ b/src/intel/isl/isl_surface_state.c @@ -995,11 +995,13 @@ isl_genX(buffer_fill_state_s)(const struct isl_device *dev, void *state, * VUID-VkDescriptorGetInfoEXT-type-09428 * VUID-VkBufferViewCreateInfo-range-00930 * VUID-VkBufferViewCreateInfo-range-04059 + * + * Regardless, the bit fields we program in our registers on SKL and + * newer are enough to fit 32bit num_elements. */ if (num_elements > (1 << 27)) { mesa_logw("%s: num_elements is too big: %u (buffer size: %"PRIu64")\n", __func__, num_elements, buffer_size); - num_elements = 1 << 27; } }