From 2a9d7a3bd01caa1858039d6eb95db4b1b698bcb7 Mon Sep 17 00:00:00 2001 From: Nanley Chery Date: Thu, 26 Sep 2024 06:51:57 -0400 Subject: [PATCH] anv: Support non-0/1 sRGB fast-clear colors on gfx9 We're going to drop a generic restriction on clear color conversions in anv_can_fast_clear_color(). Without preparing for it, the following tests would fail: * piglit.spec.arb_framebuffer_srgb.blit texture srgb msaa disabled clear.gen9_zinkm64 * piglit.spec.arb_framebuffer_srgb.blit renderbuffer srgb msaa disabled clear.gen9_zinkm64 * piglit.spec.arb_framebuffer_srgb.blit texture srgb downsample enabled clear.gen9_zinkm64 * piglit.spec.arb_framebuffer_srgb.blit renderbuffer srgb downsample enabled clear.gen9_zinkm64 * piglit.spec.arb_framebuffer_srgb.blit renderbuffer srgb msaa enabled clear.gen9_zinkm64 * piglit.spec.arb_framebuffer_srgb.blit texture srgb msaa enabled clear.gen9_zinkm64 So, add support for sRGB sampling via BLORP transfer operations and drop the gfx9-specific restriction on sRGB fast-clears. Reviewed-by: Lionel Landwerlin Part-of: --- src/intel/vulkan/anv_blorp.c | 13 ++++++++----- src/intel/vulkan/anv_image.c | 22 ++++++---------------- src/intel/vulkan/anv_image_view.c | 3 ++- src/intel/vulkan/anv_private.h | 15 +++++++++------ src/intel/vulkan/genX_cmd_buffer.c | 25 +++++++++++++++++++++---- 5 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c index 81abfa3eabf..bc43ed60c14 100644 --- a/src/intel/vulkan/anv_blorp.c +++ b/src/intel/vulkan/anv_blorp.c @@ -284,9 +284,9 @@ get_blorp_surf_for_anv_image(const struct anv_cmd_buffer *cmd_buffer, cmd_buffer->queue_family->queueFlags); } + const bool is_dest = usage & VK_IMAGE_USAGE_TRANSFER_DST_BIT; isl_surf_usage_flags_t isl_usage = - get_usage_flag_for_cmd_buffer(cmd_buffer, - usage & VK_IMAGE_USAGE_TRANSFER_DST_BIT, + get_usage_flag_for_cmd_buffer(cmd_buffer, is_dest, anv_image_is_protected(image)); const struct anv_surface *surface = &image->planes[plane].primary_surface; const struct anv_address address = @@ -318,7 +318,8 @@ get_blorp_surf_for_anv_image(const struct anv_cmd_buffer *cmd_buffer, } const struct anv_address clear_color_addr = - anv_image_get_clear_color_addr(device, image, view_fmt, aspect); + anv_image_get_clear_color_addr(device, image, view_fmt, aspect, + !is_dest); blorp_surf->clear_color_addr = anv_to_blorp_address(clear_color_addr); if (aspect & VK_IMAGE_ASPECT_DEPTH_BIT) @@ -1301,7 +1302,8 @@ exec_ccs_op(struct anv_cmd_buffer *cmd_buffer, struct blorp_surf surf; get_blorp_surf_for_anv_image(cmd_buffer, image, aspect, - 0, ANV_IMAGE_LAYOUT_EXPLICIT_AUX, + VK_IMAGE_USAGE_TRANSFER_DST_BIT, + ANV_IMAGE_LAYOUT_EXPLICIT_AUX, image->planes[plane].aux_usage, format, &surf); @@ -1366,7 +1368,8 @@ exec_mcs_op(struct anv_cmd_buffer *cmd_buffer, struct blorp_surf surf; get_blorp_surf_for_anv_image(cmd_buffer, image, aspect, - 0, ANV_IMAGE_LAYOUT_EXPLICIT_AUX, + VK_IMAGE_USAGE_TRANSFER_DST_BIT, + ANV_IMAGE_LAYOUT_EXPLICIT_AUX, image->planes[0].aux_usage, format, &surf); /* Blorp will store the clear color for us if we provide the clear color diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index 388082cf743..93b2001b8bd 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -649,8 +649,13 @@ add_aux_state_tracking_buffer(struct anv_device *device, assert(device->isl_dev.ss.clear_color_state_size == 32); clear_color_state_size = (image->num_view_formats - 1) * 64 + 32 - 8; } else { + /* When sampling or rendering with an sRGB format, HW expects the clear + * color to be in two different color spaces - sRGB in the former and + * linear in the latter. Allocate twice the space to support either + * access. + */ assert(device->isl_dev.ss.clear_value_size == 16); - clear_color_state_size = image->num_view_formats * 16; + clear_color_state_size = image->num_view_formats * 16 * 2; } /* Clear color and fast clear type */ @@ -3582,21 +3587,6 @@ anv_can_fast_clear_color(const struct anv_cmd_buffer *cmd_buffer, return false; } - /* Disable sRGB fast-clears for non-0/1 color values on Gfx9. For texturing - * and draw calls, HW expects the clear color to be in two different color - * spaces after sRGB fast-clears - sRGB in the former and linear in the - * latter. By limiting the allowable values to 0/1, both color space - * requirements are satisfied. - * - * Gfx11+ is fine as the fast clear generate 2 colors at the clear color - * address, raw & converted such that all fixed functions can find the - * value they need. - */ - if (cmd_buffer->device->info->ver == 9 && - isl_format_is_srgb(view_format) && - !isl_color_value_is_zero_one(clear_color, view_format)) - return false; - /* Wa_16021232440: Disable fast clear when height is 16k */ if (intel_needs_workaround(cmd_buffer->device->info, 16021232440) && image->vk.extent.height == 16 * 1024) { diff --git a/src/intel/vulkan/anv_image_view.c b/src/intel/vulkan/anv_image_view.c index 38f58b552da..02dbd40cb83 100644 --- a/src/intel/vulkan/anv_image_view.c +++ b/src/intel/vulkan/anv_image_view.c @@ -108,7 +108,8 @@ anv_image_fill_surface_state(struct anv_device *device, state_inout->aux_address = aux_address; const struct anv_address clear_address = - anv_image_get_clear_color_addr(device, image, view.format, aspect); + anv_image_get_clear_color_addr(device, image, view.format, aspect, + view_usage & ISL_SURF_USAGE_TEXTURE_BIT); state_inout->clear_address = clear_address; if (image->vk.create_flags & VK_IMAGE_CREATE_PROTECTED_BIT) diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index ae66a5f25ef..803c55573eb 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -5585,7 +5585,8 @@ static inline struct anv_address anv_image_get_clear_color_addr(UNUSED const struct anv_device *device, const struct anv_image *image, enum isl_format view_format, - VkImageAspectFlagBits aspect) + VkImageAspectFlagBits aspect, + bool for_sampler) { uint32_t plane = anv_image_aspect_to_plane(image, aspect); const struct anv_image_memory_range *mem_range = @@ -5598,15 +5599,17 @@ anv_image_get_clear_color_addr(UNUSED const struct anv_device *device, if (view_format == ISL_FORMAT_UNSUPPORTED) view_format = image->planes[plane].primary_surface.isl.format; - const unsigned clear_state_size = device->info->ver >= 11 ? 64 : 16; + uint64_t access_offset = device->info->ver == 9 && for_sampler ? 16 : 0; + const unsigned clear_state_size = device->info->ver >= 11 ? 64 : 32; for (int i = 0; i < image->num_view_formats; i++) { if (view_format == image->view_formats[i]) { - return anv_address_add(base_addr, i * clear_state_size); + uint64_t entry_offset = i * clear_state_size + access_offset; + return anv_address_add(base_addr, entry_offset); } } assert(anv_image_view_formats_incomplete(image)); - return base_addr; + return anv_address_add(base_addr, access_offset); } static inline struct anv_address @@ -5618,7 +5621,7 @@ anv_image_get_fast_clear_type_addr(const struct anv_device *device, assert(device->info->ver < 20); struct anv_address addr = anv_image_get_clear_color_addr(device, image, ISL_FORMAT_UNSUPPORTED, - aspect); + aspect, false); /* Refer to add_aux_state_tracking_buffer(). */ unsigned clear_color_state_size; @@ -5627,7 +5630,7 @@ anv_image_get_fast_clear_type_addr(const struct anv_device *device, clear_color_state_size = (image->num_view_formats - 1) * 64 + 32 - 8; } else { assert(device->isl_dev.ss.clear_value_size == 16); - clear_color_state_size = image->num_view_formats * 16; + clear_color_state_size = image->num_view_formats * 16 * 2; } return anv_address_add(addr, clear_color_state_size); diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index b4cc11a0cf2..2dddb3e290f 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -29,6 +29,8 @@ #include "vk_render_pass.h" #include "vk_util.h" +#include "util/format_srgb.h" + #include "genxml/gen_macros.h" #include "genxml/genX_pack.h" @@ -472,7 +474,7 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer, image->planes[depth_plane].primary_surface.isl.format; const struct anv_address clear_color_addr = anv_image_get_clear_color_addr(cmd_buffer->device, image, depth_format, - VK_IMAGE_ASPECT_DEPTH_BIT); + VK_IMAGE_ASPECT_DEPTH_BIT, true); if (!anv_address_is_null(clear_color_addr) && (initial_layout == VK_IMAGE_LAYOUT_UNDEFINED || initial_layout == VK_IMAGE_LAYOUT_PREINITIALIZED)) { @@ -889,7 +891,7 @@ genX(cmd_buffer_load_clear_color)(struct anv_cmd_buffer *cmd_buffer, const struct anv_address entry_addr = anv_image_get_clear_color_addr(cmd_buffer->device, iview->image, iview->planes[0].isl.format, - VK_IMAGE_ASPECT_COLOR_BIT); + VK_IMAGE_ASPECT_COLOR_BIT, false); unsigned copy_size = cmd_buffer->device->isl_dev.ss.clear_value_size; @@ -926,9 +928,20 @@ set_image_clear_color(struct anv_cmd_buffer *cmd_buffer, union isl_color_value clear_color; isl_color_value_unpack(&clear_color, image->view_formats[i], pixel); + UNUSED union isl_color_value sample_color = clear_color; + if (isl_format_is_srgb(image->view_formats[i])) { + sample_color.f32[0] = + util_format_linear_to_srgb_float(clear_color.f32[0]); + sample_color.f32[1] = + util_format_linear_to_srgb_float(clear_color.f32[1]); + sample_color.f32[2] = + util_format_linear_to_srgb_float(clear_color.f32[2]); + } + const struct anv_address addr = anv_image_get_clear_color_addr(cmd_buffer->device, image, - image->view_formats[i], aspect); + image->view_formats[i], aspect, + false); assert(!anv_address_is_null(addr)); #if GFX_VER >= 11 @@ -945,13 +958,17 @@ set_image_clear_color(struct anv_cmd_buffer *cmd_buffer, #else assert(cmd_buffer->device->isl_dev.ss.clear_color_state_size == 0); assert(cmd_buffer->device->isl_dev.ss.clear_value_size == 16); - uint32_t *dw = anv_batch_emitn(&cmd_buffer->batch, 3 + 4, + uint32_t *dw = anv_batch_emitn(&cmd_buffer->batch, 3 + 8, GENX(MI_STORE_DATA_IMM), .StoreQword = true, .Address = addr); dw[3] = clear_color.u32[0]; dw[4] = clear_color.u32[1]; dw[5] = clear_color.u32[2]; dw[6] = clear_color.u32[3]; + dw[7] = sample_color.u32[0]; + dw[8] = sample_color.u32[1]; + dw[9] = sample_color.u32[2]; + dw[10] = sample_color.u32[3]; #endif } }