From a620f33b7c5ac5d3c526f0f48f16668c4b5cebfb Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Fri, 5 Sep 2025 17:01:22 +0200 Subject: [PATCH] panvk: Add planar Z24S8 support It allows us to use AFBC even if separateDepthStencilLayouts=true. Unfortunately, AFBC(S8) is only supported on v9+, so we keep using interleaved Z24S8 on earlier gens. Signed-off-by: Boris Brezillon Reviewed-by: Eric R. Smith Reviewed-by: Christoph Pillmayer Part-of: --- src/panfrost/vulkan/panvk_host_copy.c | 7 +- src/panfrost/vulkan/panvk_image.c | 103 ++++++++++++++-------- src/panfrost/vulkan/panvk_image.h | 34 +++++++ src/panfrost/vulkan/panvk_meta.h | 25 ++++-- src/panfrost/vulkan/panvk_vX_cmd_draw.c | 28 +++--- src/panfrost/vulkan/panvk_vX_image_view.c | 52 ++++++----- 6 files changed, 156 insertions(+), 93 deletions(-) diff --git a/src/panfrost/vulkan/panvk_host_copy.c b/src/panfrost/vulkan/panvk_host_copy.c index b95aa407324..d7ead8221af 100644 --- a/src/panfrost/vulkan/panvk_host_copy.c +++ b/src/panfrost/vulkan/panvk_host_copy.c @@ -92,9 +92,10 @@ panvk_copy_image_to_from_memory(struct image_params img, /* D24S8 is a special case because the aspects are interleaved in a single * plane */ - VkFormat vkfmt = img.img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT ? - img.img->vk.format : - vk_format_get_aspect_format(img.img->vk.format, img.subres.aspectMask); + VkFormat vkfmt = panvk_image_is_interleaved_depth_stencil(img.img) + ? img.img->vk.format + : vk_format_get_aspect_format(img.img->vk.format, + img.subres.aspectMask); enum pipe_format pfmt = vk_format_to_pipe_format(vkfmt); const struct util_format_description *fmt = util_format_description(pfmt); diff --git a/src/panfrost/vulkan/panvk_image.c b/src/panfrost/vulkan/panvk_image.c index c2132b9d6c2..db628852eaa 100644 --- a/src/panfrost/vulkan/panvk_image.c +++ b/src/panfrost/vulkan/panvk_image.c @@ -131,6 +131,17 @@ get_iusage(struct panvk_image *image, const VkImageCreateInfo *create_info) static unsigned get_plane_count(struct panvk_image *image) { + bool combined_ds = vk_format_aspects(image->vk.format) == + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); + + /* Only depth+stencil images can be made multiplanar behind the scene. */ + if (!combined_ds) + return vk_format_get_plane_count(image->vk.format); + + struct panvk_physical_device *phys_dev = + to_panvk_physical_device(image->vk.base.device->physical); + unsigned arch = pan_arch(phys_dev->kmod.props.gpu_id); + /* Z32_S8X24 is not supported on v9+, and we don't want to use it * on v7- anyway, because it's less efficient than the multiplanar * alternative. @@ -138,7 +149,49 @@ get_plane_count(struct panvk_image *image) if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT) return 2; - return vk_format_get_plane_count(image->vk.format); + assert(image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT); + + /* We can do AFBC(S8) on Valhall and we're thus better off using planar + * Z24+S8 so we can use AFBC when separateDepthStencilLayouts=true. + */ + return arch >= 9 ? 2 : 1; +} + +static enum pipe_format +select_depth_plane_pfmt(struct panvk_image *image) +{ + switch (image->vk.format) { + case VK_FORMAT_D24_UNORM_S8_UINT: + return PIPE_FORMAT_Z24_UNORM_PACKED; + case VK_FORMAT_D32_SFLOAT_S8_UINT: + return PIPE_FORMAT_Z32_FLOAT; + default: + UNREACHABLE("Invalid depth+stencil format"); + } +} + +static enum pipe_format +select_stencil_plane_pfmt(struct panvk_image *image) +{ + switch (image->vk.format) { + case VK_FORMAT_D24_UNORM_S8_UINT: + case VK_FORMAT_D32_SFLOAT_S8_UINT: + return PIPE_FORMAT_S8_UINT; + default: + UNREACHABLE("Invalid depth+stencil format"); + } +} + +static enum pipe_format +select_plane_pfmt(struct panvk_image *image, unsigned plane) +{ + if (panvk_image_is_planar_depth_stencil(image)) { + return plane > 0 ? select_stencil_plane_pfmt(image) + : select_depth_plane_pfmt(image); + } + + VkFormat plane_format = vk_format_get_plane_format(image->vk.format, plane); + return vk_format_to_pipe_format(plane_format); } static bool @@ -184,14 +237,11 @@ panvk_image_can_use_mod(struct panvk_image *image, return false; /* We can't have separate depth/stencil layout transitions with - * interleaved Z24S8, so make sure we always disallow AFBC on Z24S8 until - * we've extended the image logic to support planar Z24+S8. Note that - * AFBC(S8) is not supported on Bifrost, so we want to keep support for - * interleaved Z24S8 to at least have AFBC(Z24S8) when - * separateDepthStencilLayouts=false. + * interleaved ZS, so make sure we disallow AFBC on ZS unless + * it's using a planar layout. */ if (image->vk.base.device->enabled_features.separateDepthStencilLayouts && - image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT) + panvk_image_is_interleaved_depth_stencil(image)) return false; } @@ -220,26 +270,14 @@ panvk_image_can_use_mod(struct panvk_image *image, /* Defer the rest of the checks to the mod handler. */ struct pan_image_props iprops = { .modifier = mod, - .format = vk_format_to_pipe_format(image->vk.format), .dim = panvk_image_type_to_mali_tex_dim(image->vk.image_type), .array_size = image->vk.array_layers, .nr_samples = image->vk.samples, .nr_slices = image->vk.mip_levels, }; - const unsigned plane_count = - image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT - ? 2 - : vk_format_get_plane_count(image->vk.format); - for (uint8_t plane = 0; plane < plane_count; plane++) { - VkFormat format; - - if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT) - format = plane == 0 ? VK_FORMAT_D32_SFLOAT : VK_FORMAT_S8_UINT; - else - format = vk_format_get_plane_format(image->vk.format, plane); - - iprops.format = vk_format_to_pipe_format(format); + for (uint8_t plane = 0; plane < image->plane_count; plane++) { + iprops.format = select_plane_pfmt(image, plane); iprops.extent_px = (struct pan_image_extent){ .width = vk_format_get_plane_width(image->vk.format, plane, image->vk.extent.width), @@ -332,7 +370,7 @@ static bool is_disjoint(const struct panvk_image *image) { assert((image->plane_count > 1 && - image->vk.format != VK_FORMAT_D32_SFLOAT_S8_UINT) || + !vk_format_is_depth_or_stencil(image->vk.format)) || (image->vk.create_flags & VK_IMAGE_CREATE_ALIAS_BIT) || !(image->vk.create_flags & VK_IMAGE_CREATE_DISJOINT_BIT)); return image->vk.create_flags & VK_IMAGE_CREATE_DISJOINT_BIT; @@ -364,27 +402,13 @@ panvk_image_init_layouts(struct panvk_image *image, pCreateInfo->pNext, IMAGE_DRM_FORMAT_MODIFIER_EXPLICIT_CREATE_INFO_EXT); - image->plane_count = vk_format_get_plane_count(pCreateInfo->format); - - /* Z32_S8X24 is not supported on v9+, and we don't want to use it - * on v7- anyway, because it's less efficient than the multiplanar - * alternative. - */ - if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT) - image->plane_count = 2; - const struct pan_mod_handler *mod_handler = pan_mod_get_handler(arch, image->vk.drm_format_mod); struct pan_image_layout_constraints plane_layout = { .offset_B = 0, }; for (uint8_t plane = 0; plane < image->plane_count; plane++) { - VkFormat format; - - if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT) - format = plane == 0 ? VK_FORMAT_D32_SFLOAT : VK_FORMAT_S8_UINT; - else - format = vk_format_get_plane_format(image->vk.format, plane); + enum pipe_format pfmt = select_plane_pfmt(image, plane); if (explicit_info) { plane_layout = (struct pan_image_layout_constraints){ @@ -396,7 +420,7 @@ panvk_image_init_layouts(struct panvk_image *image, image->planes[plane].image = (struct pan_image){ .props = { .modifier = image->vk.drm_format_mod, - .format = vk_format_to_pipe_format(format), + .format = pfmt, .dim = panvk_image_type_to_mali_tex_dim(image->vk.image_type), .extent_px = { .width = vk_format_get_plane_width(image->vk.format, plane, @@ -621,7 +645,8 @@ get_image_subresource_layout(const struct panvk_image *image, * path because we need to interleave the depth/stencil components. For * the stencil aspect, the copied data only needs 1 byte/px instead of 4. */ - if (image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT) { + if (image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT && + image->plane_count == 1) { switch (subres->aspectMask) { case VK_IMAGE_ASPECT_DEPTH_BIT: memcpy_size->size = slice_layout->size_B; diff --git a/src/panfrost/vulkan/panvk_image.h b/src/panfrost/vulkan/panvk_image.h index 39f284539ef..06bc60fff48 100644 --- a/src/panfrost/vulkan/panvk_image.h +++ b/src/panfrost/vulkan/panvk_image.h @@ -6,6 +6,9 @@ #ifndef PANVK_IMAGE_H #define PANVK_IMAGE_H +#include "util/format/u_format.h" + +#include "vk_format.h" #include "vk_image.h" #include "pan_image.h" @@ -64,6 +67,37 @@ panvk_plane_index(const struct panvk_image *image, } } +static inline bool +panvk_image_is_interleaved_depth_stencil(const struct panvk_image *image){ + return image->plane_count == 1 && + vk_format_aspects(image->vk.format) == + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); +} + +static inline bool +panvk_image_is_planar_depth_stencil(const struct panvk_image *image){ + return image->plane_count > 1 && + vk_format_aspects(image->vk.format) == + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); +} + +static inline enum pipe_format +panvk_image_depth_only_pfmt(const struct panvk_image *image) +{ + assert(vk_format_has_depth(image->vk.format)); + + return util_format_get_depth_only(image->planes[0].image.props.format); +} + +static inline enum pipe_format +panvk_image_stencil_only_pfmt(const struct panvk_image *image) +{ + assert(vk_format_has_stencil(image->vk.format)); + + return util_format_stencil_only( + image->planes[image->plane_count - 1].image.props.format); +} + VkResult panvk_image_init(struct panvk_image *image, const VkImageCreateInfo *pCreateInfo); diff --git a/src/panfrost/vulkan/panvk_meta.h b/src/panfrost/vulkan/panvk_meta.h index 04371162a53..155a1013ae4 100644 --- a/src/panfrost/vulkan/panvk_meta.h +++ b/src/panfrost/vulkan/panvk_meta.h @@ -97,11 +97,21 @@ panvk_meta_copy_get_image_properties(struct panvk_image *img, props.stencil.component_mask = BITFIELD_MASK(1); break; case VK_FORMAT_D24_UNORM_S8_UINT: - props.depth.view_format = - use_unorm ? VK_FORMAT_R8G8B8A8_UNORM : VK_FORMAT_R8G8B8A8_UINT; - props.depth.component_mask = BITFIELD_MASK(3); - props.stencil.view_format = props.depth.view_format; - props.stencil.component_mask = BITFIELD_BIT(3); + if (panvk_image_is_planar_depth_stencil(img)) { + props.depth.view_format = + use_unorm ? VK_FORMAT_R8G8B8_UNORM : VK_FORMAT_R8G8B8_UINT; + props.depth.component_mask = BITFIELD_MASK(3); + props.stencil.view_format = + use_unorm ? VK_FORMAT_R8_UNORM : VK_FORMAT_R8_UINT; + props.stencil.component_mask = BITFIELD_BIT(0); + } else { + props.depth.view_format = use_unorm + ? VK_FORMAT_R8G8B8A8_UNORM + : VK_FORMAT_R8G8B8A8_UINT; + props.depth.component_mask = BITFIELD_MASK(3); + props.stencil.view_format = props.depth.view_format; + props.stencil.component_mask = BITFIELD_BIT(3); + } break; case VK_FORMAT_X8_D24_UNORM_PACK32: props.depth.view_format = @@ -109,8 +119,9 @@ panvk_meta_copy_get_image_properties(struct panvk_image *img, props.depth.component_mask = BITFIELD_MASK(3); break; case VK_FORMAT_D32_SFLOAT_S8_UINT: - props.depth.view_format = use_unorm ? VK_FORMAT_R8G8B8A8_UNORM - : VK_FORMAT_R8G8B8A8_UINT; + assert(panvk_image_is_planar_depth_stencil(img)); + props.depth.view_format = + use_unorm ? VK_FORMAT_R8G8B8A8_UNORM : VK_FORMAT_R8G8B8A8_UINT; props.depth.component_mask = BITFIELD_MASK(4); props.stencil.view_format = use_unorm ? VK_FORMAT_R8_UNORM : VK_FORMAT_R8_UINT; diff --git a/src/panfrost/vulkan/panvk_vX_cmd_draw.c b/src/panfrost/vulkan/panvk_vX_cmd_draw.c index 7100dce4a6f..d66a41a16e8 100644 --- a/src/panfrost/vulkan/panvk_vX_cmd_draw.c +++ b/src/panfrost/vulkan/panvk_vX_cmd_draw.c @@ -129,11 +129,9 @@ render_state_set_z_attachment(struct panvk_cmd_buffer *cmdbuf, state->render.zs_pview = iview->pview; fbinfo->zs.view.zs = &state->render.zs_pview; - /* D32_S8 is a multiplanar format, so we need to adjust the format of the - * depth-only view to match the one of the depth plane. - */ - if (iview->pview.format == PIPE_FORMAT_Z32_FLOAT_S8X24_UINT) - state->render.zs_pview.format = PIPE_FORMAT_Z32_FLOAT; + /* Fixup view format when the image is multiplanar. */ + if (panvk_image_is_planar_depth_stencil(img)) + state->render.zs_pview.format = panvk_image_depth_only_pfmt(img); state->render.zs_pview.planes[0] = (struct pan_image_plane_ref){ .image = &img->planes[0].image, @@ -149,13 +147,12 @@ render_state_set_z_attachment(struct panvk_cmd_buffer *cmdbuf, * If we touch the depth component, we need to make sure the stencil * component is preserved, hence the preload, and the view format adjusment. */ - if (img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT) { + if (panvk_image_is_interleaved_depth_stencil(img)) { fbinfo->zs.preload.s = true; cmdbuf->state.gfx.render.zs_pview.format = - PIPE_FORMAT_Z24_UNORM_S8_UINT; + img->planes[0].image.props.format; } else { - state->render.zs_pview.format = - vk_format_to_pipe_format(vk_format_depth_only(img->vk.format)); + state->render.zs_pview.format = panvk_image_depth_only_pfmt(img); } if (att->loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR) @@ -195,19 +192,15 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf, state->render.s_pview = iview->pview; fbinfo->zs.view.s = &state->render.s_pview; - /* D32_S8 is a multiplanar format, so we need to adjust the format of the - * stencil-only view to match the one of the stencil plane. - */ - state->render.s_pview.format = img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT - ? PIPE_FORMAT_Z24_UNORM_S8_UINT - : PIPE_FORMAT_S8_UINT; - if (img->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT) { + if (panvk_image_is_planar_depth_stencil(img)) { + state->render.s_pview.format = panvk_image_stencil_only_pfmt(img); state->render.s_pview.planes[0] = (struct pan_image_plane_ref){0}; state->render.s_pview.planes[1] = (struct pan_image_plane_ref){ .image = &img->planes[1].image, .plane_idx = 0, }; } else { + state->render.s_pview.format = panvk_image_stencil_only_pfmt(img); state->render.s_pview.planes[0] = (struct pan_image_plane_ref){ .image = &img->planes[0].image, .plane_idx = 0, @@ -224,7 +217,7 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf, * and the format is D24S8, we can combine them in a single view * addressing both components. */ - if (img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT && + if (state->render.s_pview.format == PIPE_FORMAT_X24S8_UINT && state->render.z_attachment.iview && state->render.z_attachment.iview->vk.image == iview->vk.image) { state->render.zs_pview.format = PIPE_FORMAT_Z24_UNORM_S8_UINT; @@ -236,6 +229,7 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf, * is not supported on the stencil-only slot on Bifrost. */ } else if (img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT && + state->render.s_pview.format == PIPE_FORMAT_X24S8_UINT && fbinfo->zs.view.zs == NULL) { fbinfo->zs.view.zs = &state->render.s_pview; state->render.s_pview.format = PIPE_FORMAT_Z24_UNORM_S8_UINT; diff --git a/src/panfrost/vulkan/panvk_vX_image_view.c b/src/panfrost/vulkan/panvk_vX_image_view.c index b8d7d7bbecc..66f695583cc 100644 --- a/src/panfrost/vulkan/panvk_vX_image_view.c +++ b/src/panfrost/vulkan/panvk_vX_image_view.c @@ -83,12 +83,15 @@ prepare_tex_descs(struct panvk_image_view *view) struct panvk_image *image = container_of(view->vk.image, struct panvk_image, vk); struct panvk_device *dev = to_panvk_device(view->vk.base.device); + bool img_combined_ds = + vk_format_aspects(image->vk.format) == + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); + bool view_combined_ds = view->vk.aspects == + (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); bool can_preload_other_aspect = (view->vk.usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) && - (image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT || - (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT && - view->vk.aspects == - (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))); + (img_combined_ds && + (view_combined_ds || panvk_image_is_interleaved_depth_stencil(image))); if (util_format_is_depth_or_stencil(view->pview.format)) { /* Vulkan wants R001, where the depth/stencil is stored in the red @@ -113,8 +116,12 @@ prepare_tex_descs(struct panvk_image_view *view) /* If the view contains both stencil and depth, we need to keep only the * depth. We'll create another texture with only the stencil. */ - if (pview.format == PIPE_FORMAT_Z32_FLOAT_S8X24_UINT) - pview.format = PIPE_FORMAT_Z32_FLOAT; + if (view->vk.aspects & VK_IMAGE_ASPECT_DEPTH_BIT) { + /* View and image formats must match. */ + assert(view->vk.format == vk_format_depth_only(image->vk.format) || + view->vk.format == image->vk.format); + pview.format = panvk_image_depth_only_pfmt(image); + } uint32_t plane_count = vk_format_get_plane_count(view->vk.format); uint32_t tex_payload_size = GENX(pan_texture_estimate_payload_size)(&pview); @@ -196,23 +203,13 @@ prepare_tex_descs(struct panvk_image_view *view) if (!can_preload_other_aspect) return VK_SUCCESS; - switch (pview.format) { - case PIPE_FORMAT_Z24X8_UNORM: - case PIPE_FORMAT_Z24_UNORM_S8_UINT: - pview.format = PIPE_FORMAT_X24S8_UINT; - break; - case PIPE_FORMAT_X24S8_UINT: - pview.format = PIPE_FORMAT_Z24X8_UNORM; - break; - case PIPE_FORMAT_Z32_FLOAT: - pview.format = PIPE_FORMAT_S8_UINT; - break; - case PIPE_FORMAT_S8_UINT: - pview.format = PIPE_FORMAT_Z32_FLOAT; - break; - default: - assert(!"Invalid format"); - } + /* If the depth was present in the aspects mask, we've handled it already, so + * move on to the stencil. If it wasn't present, it's the stencil texture we + * create first, and we need t handle the depth here. + */ + pview.format = (view->vk.aspects & VK_IMAGE_ASPECT_DEPTH_BIT) + ? panvk_image_stencil_only_pfmt(image) + : panvk_image_depth_only_pfmt(image); ptr.cpu += tex_payload_size; ptr.gpu += tex_payload_size; @@ -346,7 +343,7 @@ panvk_per_arch(CreateImageView)(VkDevice _device, /* Depth/stencil are viewed as color for copies. */ if (view->vk.aspects == VK_IMAGE_ASPECT_COLOR_BIT && - image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT && + panvk_image_is_planar_depth_stencil(image) && vk_format_get_blocksize(view->vk.view_format) == 1) { view->pview.planes[0] = (struct pan_image_plane_ref) { .image = &image->planes[1].image, @@ -358,9 +355,10 @@ panvk_per_arch(CreateImageView)(VkDevice _device, * depth and stencil but the view only contains one of these components, so * we can ignore the component we don't use. */ - if (view->vk.view_format == VK_FORMAT_S8_UINT && - image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT) - view->pview.format = PIPE_FORMAT_X24S8_UINT; + if (view->vk.aspects == VK_IMAGE_ASPECT_STENCIL_BIT) + view->pview.format = panvk_image_stencil_only_pfmt(image); + else if (view->vk.aspects == VK_IMAGE_ASPECT_DEPTH_BIT) + view->pview.format = panvk_image_depth_only_pfmt(image); /* Attachments need a texture for the FB preload logic. */ VkImageUsageFlags tex_usage_mask =