From 275a18322d029cdb4b1dda53580b0c52f089d6fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Pi=C3=B1eiro?= Date: Wed, 19 Jan 2022 14:24:59 +0100 Subject: [PATCH] v3dv: check correct format when load/storing on a depth/stencil buffer When we create a image view with D24S8 format we made a reformatting to RGBA8UI if the aspect selected is just STENCIL. But when configuring the stores we select the aspects based on the attachment format. Quoting from cmd_buffer_render_pass_emit_stores: /* From the Vulkan spec, VkImageSubresourceRange: * * "When an image view of a depth/stencil image is used as a * depth/stencil framebuffer attachment, the aspectMask is ignored * and both depth and stencil image subresources are used." * * So we ignore the aspects from the subresource range of the image * view for the depth/stencil attachment, but we still need to restrict * the to aspects compatible with the render pass and the image. */ const VkImageAspectFlags aspects = vk_format_aspects(ds_attachment->desc.format); So we could ending trying to store on a Z+Stencil buffer, using a RGBA8UI format. So far this only affected some tests when using the simulator (assert). Those were working on the real hw, but probably would fail on other situations, so lets use the original image format on that case. v2 (Iago) * Improve comment grammar * Do the same on load too (not just store) v3 (Iago) * Re-word comments. Reviewed-by: Iago Toral Quiroga Part-of: --- src/broadcom/vulkan/v3dvx_cmd_buffer.c | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/broadcom/vulkan/v3dvx_cmd_buffer.c b/src/broadcom/vulkan/v3dvx_cmd_buffer.c index 39deb4e5455..12e7385fdf3 100644 --- a/src/broadcom/vulkan/v3dvx_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dvx_cmd_buffer.c @@ -117,6 +117,20 @@ cmd_buffer_render_pass_emit_load(struct v3dv_cmd_buffer *cmd_buffer, load.address = v3dv_cl_address(image->mem->bo, layer_offset); load.input_image_format = iview->format->rt_type; + + /* If we create an image view with only the stencil format, we + * re-interpret the format as RGBA8_UINT, as it is want we want in + * general (see CreateImageView). + * + * However, when we are loading/storing tiles from the ZSTENCIL tile + * buffer, we need to use the underlying DS format. + */ + if (buffer == ZSTENCIL && + iview->format->rt_type == V3D_OUTPUT_IMAGE_FORMAT_RGBA8UI) { + assert(image->format->rt_type == V3D_OUTPUT_IMAGE_FORMAT_D24S8); + load.input_image_format = image->format->rt_type; + } + load.r_b_swap = iview->swap_rb; load.channel_reverse = iview->channel_reverse; load.memory_format = slice->tiling; @@ -305,6 +319,20 @@ cmd_buffer_render_pass_emit_store(struct v3dv_cmd_buffer *cmd_buffer, store.clear_buffer_being_stored = clear; store.output_image_format = iview->format->rt_type; + + /* If we create an image view with only the stencil format, we + * re-interpret the format as RGBA8_UINT, as it is want we want in + * general (see CreateImageView). + * + * However, when we are loading/storing tiles from the ZSTENCIL tile + * buffer, we need to use the underlying DS format. + */ + if (buffer == ZSTENCIL && + iview->format->rt_type == V3D_OUTPUT_IMAGE_FORMAT_RGBA8UI) { + assert(image->format->rt_type == V3D_OUTPUT_IMAGE_FORMAT_D24S8); + store.output_image_format = image->format->rt_type; + } + store.r_b_swap = iview->swap_rb; store.channel_reverse = iview->channel_reverse; store.memory_format = slice->tiling;