fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2025-12-22 20:00:10 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

anv/image: Rework YCbCr image aspects

Browse source 
The Vulkan 1.2.184 spec says:

    "When creating a VkImageView, if sampler Y′CBCR conversion is
    enabled in the sampler, the aspectMask of a subresourceRange used by
    the VkImageView must be VK_IMAGE_ASPECT_COLOR_BIT.

    When creating a VkImageView, if sampler Y′CBCR conversion is not
    enabled in the sampler and the image format is multi-planar, the
    image must have been created with
    VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, and the aspectMask of the
    VkImageView’s subresourceRange must be VK_IMAGE_ASPECT_PLANE_0_BIT,
    VK_IMAGE_ASPECT_PLANE_1_BIT or VK_IMAGE_ASPECT_PLANE_2_BIT."

Previously, for YCbCr images, we were flipping this around.  For single-
plane views where VK_IMAGE_ASPECT_PLANE_N_BIT would be passed in by the
app, we would store VK_IMAGE_ASPECT_COLOR_BIT.  For multi-plane views
where the client says VK_IMAGE_ASPECT_COLOR_BIT, we would store a all of
the planes.  (There was also an extra bit of remapping that would
compact the planes in the non-existent case of a format with a non-
contiguous set of planes.)  The idea behind this was that for things
like rendering or single-plane sampling, storage, or compute, we want it
to look as much like a single-plane image as possible but we wanted the
multi-plane case to be the awkward one.

This commit changes it around so that iview->aspects is always exactly
the subset of image->vk.aspects represented by the view.  This is
identical to how aspects work for depth/stencil so it gains us some
consistency.

This commit also changes anv_image_view::aspect_mask to aspects to force
a full audit of the field.  As can be seen, there are only a few uses of
this field and they're all mostly fine:

 - A bunch of them are used to check for depth/stencil.  That hasn't
   changed.

 - Most of the checks for color already used ANY_COLOR_BIT, only one
   needed fixing.

 - There's a check that both src/depth are color for MSAA resolves.
   However, we don't support MSAA on YCbCr so there's no point in
   checking for ANY_COLOR_BIT.

There is a hidden usage of planes in anv_descriptor_set_write_image_view
that's not as obvious.  However, this function simply looks at
anv_image_view::n_planes and blindly fills out the descriptor
accordingly.  As long as image views with a single plane continue to
claim n_planes == 1, this will be fine.

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12141>
This commit is contained in:
Jason Ekstrand 2021-07-29 16:41:14 -05:00 • committed by Marge Bot
parent 32157f9059
commit 0a93c0364c
4 changed files with 30 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/intel/vulkan/anv_cmd_buffer.c
View file

@ -1332,8 +1332,8 @@ anv_cmd_buffer_get_depth_stencil_view(const struct anv_cmd_buffer *cmd_buffer)
const struct anv_image_view *iview =
cmd_buffer->state.attachments[subpass->depth_stencil_attachment->attachment].image_view;
assert(iview->aspect_mask & (VK_IMAGE_ASPECT_DEPTH_BIT |
VK_IMAGE_ASPECT_STENCIL_BIT));
assert(iview->aspects & (VK_IMAGE_ASPECT_DEPTH_BIT |
VK_IMAGE_ASPECT_STENCIL_BIT));
return iview;
}

42
src/intel/vulkan/anv_image.c
View file

@ -2641,22 +2641,6 @@ anv_image_fill_surface_state(struct anv_device *device,
}
}
static VkImageAspectFlags
remap_aspect_flags(VkImageAspectFlags view_aspects)
{
if (view_aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
if (util_bitcount(view_aspects) == 1)
return VK_IMAGE_ASPECT_COLOR_BIT;
VkImageAspectFlags color_aspects = 0;
for (uint32_t i = 0; i < util_bitcount(view_aspects); i++)
color_aspects |= VK_IMAGE_ASPECT_PLANE_0_BIT << i;
return color_aspects;
}
/* No special remapping needed for depth & stencil aspects. */
return view_aspects;
}
static uint32_t
anv_image_aspect_get_planes(VkImageAspectFlags aspect_mask)
{
@ -2773,24 +2757,16 @@ anv_CreateImageView(VkDevice _device,
break;
}
iview->image = image;
/* First expand aspects to the image's ones (for example
* VK_IMAGE_ASPECT_COLOR_BIT will be converted to
* VK_IMAGE_ASPECT_PLANE_0_BIT | VK_IMAGE_ASPECT_PLANE_1_BIT |
* VK_IMAGE_ASPECT_PLANE_2_BIT for an image of format
* VK_FORMAT_G8_B8_R8_3PLANE_420_UNORM.
*/
VkImageAspectFlags expanded_aspects =
anv_image_expand_aspects(image, range->aspectMask);
iview->image = image;
/* Remap the expanded aspects for the image view. For example if only
* VK_IMAGE_ASPECT_PLANE_1_BIT was given in range->aspectMask, we will
* convert it to VK_IMAGE_ASPECT_COLOR_BIT since from the point of view of
* the image view, it only has a single plane.
*/
iview->aspect_mask = remap_aspect_flags(expanded_aspects);
iview->n_planes = anv_image_aspect_get_planes(iview->aspect_mask);
iview->aspects = anv_image_expand_aspects(image, range->aspectMask);
iview->n_planes = anv_image_aspect_get_planes(iview->aspects);
iview->vk_format = pCreateInfo->format;
/* "If image has an external format, format must be VK_FORMAT_UNDEFINED." */
@ -2809,14 +2785,14 @@ anv_CreateImageView(VkDevice _device,
.depth = anv_minify(image->extent.depth , range->baseMipLevel),
};
/* Now go through the underlying image selected planes (computed in
* expanded_aspects) and map them to planes in the image view.
/* Now go through the underlying image selected planes and map them to
* planes in the image view.
*/
anv_foreach_image_aspect_bit(iaspect_bit, image, expanded_aspects) {
anv_foreach_image_aspect_bit(iaspect_bit, image, iview->aspects) {
const uint32_t iplane =
anv_image_aspect_to_plane(image->aspects, 1UL << iaspect_bit);
const uint32_t vplane =
anv_image_aspect_to_plane(expanded_aspects, 1UL << iaspect_bit);
anv_image_aspect_to_plane(iview->aspects, 1UL << iaspect_bit);
struct anv_format_plane format;
if (image->aspects & (VK_IMAGE_ASPECT_DEPTH_BIT |
VK_IMAGE_ASPECT_STENCIL_BIT)) {
@ -2883,7 +2859,7 @@ anv_CreateImageView(VkDevice _device,
if (view_usage & VK_IMAGE_USAGE_SAMPLED_BIT ||
(view_usage & VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT &&
!(iview->aspect_mask & VK_IMAGE_ASPECT_COLOR_BIT))) {
!(iview->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV))) {
iview->planes[vplane].optimal_sampler_surface_state.state = alloc_surface_state(device);
iview->planes[vplane].general_sampler_surface_state.state = alloc_surface_state(device);

19
src/intel/vulkan/anv_private.h
View file

@ -3781,22 +3781,31 @@ struct anv_format {
};
/**
* Return the aspect's plane relative to all_aspects. For an image, for
* instance, all_aspects would be the set of aspects in the image. For
* an image view, all_aspects would be the subset of aspects represented
* by that particular view.
*
* Return the aspect's _format_ plane, not its _memory_ plane (using the
* vocabulary of VK_EXT_image_drm_format_modifier). As a consequence, \a
* aspect_mask may contain VK_IMAGE_ASPECT_PLANE_*, but must not contain
* VK_IMAGE_ASPECT_MEMORY_PLANE_* .
*/
static inline uint32_t
anv_image_aspect_to_plane(VkImageAspectFlags image_aspects,
VkImageAspectFlags aspect_mask)
anv_image_aspect_to_plane(VkImageAspectFlags all_aspects,
VkImageAspectFlagBits aspect)
{
switch (aspect_mask) {
assert(util_bitcount(aspect) == 1);
if (util_bitcount(all_aspects) == 1)
return 0;
switch (aspect) {
case VK_IMAGE_ASPECT_COLOR_BIT:
case VK_IMAGE_ASPECT_DEPTH_BIT:
case VK_IMAGE_ASPECT_PLANE_0_BIT:
return 0;
case VK_IMAGE_ASPECT_STENCIL_BIT:
if ((image_aspects & VK_IMAGE_ASPECT_DEPTH_BIT) == 0)
if ((all_aspects & VK_IMAGE_ASPECT_DEPTH_BIT) == 0)
return 0;
FALLTHROUGH;
case VK_IMAGE_ASPECT_PLANE_1_BIT:
@ -4373,7 +4382,7 @@ struct anv_image_view {
const struct anv_image *image; /**< VkImageViewCreateInfo::image */
VkImageAspectFlags aspect_mask;
VkImageAspectFlags aspects;
VkFormat vk_format;
VkExtent3D extent; /**< Extent of VkImageViewCreateInfo::baseMipLevel. */

10
src/intel/vulkan/genX_cmd_buffer.c
View file

@ -2723,7 +2723,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
assert(shader->stage == MESA_SHADER_FRAGMENT);
assert(desc->image_view != NULL);
if ((desc->image_view->aspect_mask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) == 0) {
if ((desc->image_view->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) == 0) {
/* For depth and stencil input attachments, we treat it like any
* old texture that a user may have bound.
*/
@ -6342,7 +6342,7 @@ cmd_buffer_end_subpass(struct anv_cmd_buffer *cmd_buffer)
* with depth.
*/
const struct isl_view *ds_view = &iview->planes[0].isl;
if (iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) {
if (iview->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
genX(cmd_buffer_mark_image_written)(cmd_buffer, iview->image,
VK_IMAGE_ASPECT_DEPTH_BIT,
att_state->aux_usage,
@ -6350,7 +6350,7 @@ cmd_buffer_end_subpass(struct anv_cmd_buffer *cmd_buffer)
ds_view->base_array_layer,
fb->layers);
}
if (iview->aspect_mask & VK_IMAGE_ASPECT_STENCIL_BIT) {
if (iview->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) {
/* Even though stencil may be plane 1, it always shares a
* base_level with depth.
*/
@ -6405,8 +6405,8 @@ cmd_buffer_end_subpass(struct anv_cmd_buffer *cmd_buffer)
enum isl_aux_usage dst_aux_usage =
cmd_buffer->state.attachments[dst_att].aux_usage;
assert(src_iview->aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT &&
dst_iview->aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT);
assert(src_iview->aspects == VK_IMAGE_ASPECT_COLOR_BIT &&
dst_iview->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
anv_image_msaa_resolve(cmd_buffer,
src_iview->image, src_aux_usage,