From 3beaaa9ae8c8a913fb87b4ff10eb8dae8ddda1e8 Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Tue, 4 Apr 2023 20:45:36 +0300 Subject: [PATCH] anv: drop lowered storage images code Signed-off-by: Lionel Landwerlin Reviewed-by: Ivan Briano Part-of: --- src/intel/vulkan/anv_descriptor_set.c | 4 - src/intel/vulkan/anv_image.c | 251 ++++++----------------- src/intel/vulkan/anv_nir_apply_pipeline_layout.c | 10 +- src/intel/vulkan/anv_pipeline.c | 8 +- src/intel/vulkan/anv_private.h | 19 +- src/intel/vulkan/genX_cmd_buffer.c | 24 +-- 6 files changed, 78 insertions(+), 238 deletions(-) diff --git a/src/intel/vulkan/anv_descriptor_set.c b/src/intel/vulkan/anv_descriptor_set.c index aee525c..c12dd0c 100644 --- a/src/intel/vulkan/anv_descriptor_set.c +++ b/src/intel/vulkan/anv_descriptor_set.c @@ -1530,8 +1530,6 @@ anv_descriptor_set_write_image_view(struct anv_device *device, struct anv_storage_image_descriptor desc_data = { .vanilla = anv_surface_state_to_handle( image_view->planes[0].storage_surface_state.state), - .lowered = anv_surface_state_to_handle( - image_view->planes[0].lowered_storage_surface_state.state), }; memcpy(desc_map, &desc_data, sizeof(desc_data)); } @@ -1582,8 +1580,6 @@ anv_descriptor_set_write_buffer_view(struct anv_device *device, struct anv_storage_image_descriptor desc_data = { .vanilla = anv_surface_state_to_handle( buffer_view->storage_surface_state), - .lowered = anv_surface_state_to_handle( - buffer_view->lowered_storage_surface_state), }; memcpy(desc_map, &desc_data, sizeof(desc_data)); } diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index d556c69..887eba8 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -429,22 +429,6 @@ anv_get_isl_format_with_usage(const struct intel_device_info *devinfo, anv_get_format_aspect(devinfo, vk_format, vk_aspect, vk_tiling); - if ((vk_usage == VK_IMAGE_USAGE_STORAGE_BIT) && - isl_is_storage_image_format(devinfo, format.isl_format)) { - enum isl_format lowered_format = - isl_lower_storage_image_format(devinfo, format.isl_format); - - /* If we lower the format, we should ensure either they both match in - * bits per channel or that there is no swizzle, because we can't use - * the swizzle for a different bit pattern. - */ - assert(isl_formats_have_same_bits_per_channel(lowered_format, - format.isl_format) || - isl_swizzle_is_identity(format.swizzle)); - - format.isl_format = lowered_format; - } - return format.isl_format; } @@ -2418,123 +2402,75 @@ anv_image_fill_surface_state(struct anv_device *device, const struct anv_address address = anv_image_address(image, &surface->memory_range); - if (view_usage == ISL_SURF_USAGE_STORAGE_BIT && - (flags & ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED) && - !isl_has_matching_typed_storage_image_format(device->info, - view.format)) { - /* In this case, we are a writeable storage buffer which needs to be - * lowered to linear. All tiling and offset calculations will be done in - * the shader. - */ - assert(aux_usage == ISL_AUX_USAGE_NONE); - isl_buffer_fill_state(&device->isl_dev, state_inout->state.map, - .address = anv_address_physical(address), - .size_B = surface->isl.size_B, - .format = ISL_FORMAT_RAW, - .swizzle = ISL_SWIZZLE_IDENTITY, - .stride_B = 1, - .mocs = anv_mocs(device, address.bo, view_usage)); - state_inout->address = address, - state_inout->aux_address = ANV_NULL_ADDRESS; - state_inout->clear_address = ANV_NULL_ADDRESS; - } else { - if (view_usage == ISL_SURF_USAGE_STORAGE_BIT && - (flags & ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED)) { - /* Typed surface reads support a very limited subset of the shader - * image formats. Translate it into the closest format the hardware - * supports. - */ - enum isl_format lower_format = - isl_lower_storage_image_format(device->info, view.format); - if (aux_usage != ISL_AUX_USAGE_NONE) { - assert(device->info->verx10 >= 125); - assert(aux_usage == ISL_AUX_USAGE_CCS_E); - assert(isl_formats_are_ccs_e_compatible(device->info, - view.format, - lower_format)); - } - - /* If we lower the format, we should ensure either they both match in - * bits per channel or that there is no swizzle, because we can't use - * the swizzle for a different bit pattern. - */ - assert(isl_formats_have_same_bits_per_channel(lower_format, - view.format) || - isl_swizzle_is_identity_for_format(view.format, view.swizzle)); - - view.format = lower_format; - } - - const struct isl_surf *isl_surf = &surface->isl; + const struct isl_surf *isl_surf = &surface->isl; - struct isl_surf tmp_surf; - uint64_t offset_B = 0; - uint32_t tile_x_sa = 0, tile_y_sa = 0; - if (isl_format_is_compressed(surface->isl.format) && - !isl_format_is_compressed(view.format)) { - /* We're creating an uncompressed view of a compressed surface. This - * is allowed but only for a single level/layer. - */ - assert(surface->isl.samples == 1); - assert(view.levels == 1); - assert(surface->isl.dim == ISL_SURF_DIM_3D || view.array_len == 1); - - ASSERTED bool ok = - isl_surf_get_uncompressed_surf(&device->isl_dev, isl_surf, &view, - &tmp_surf, &view, - &offset_B, &tile_x_sa, &tile_y_sa); - assert(ok); - isl_surf = &tmp_surf; - } + struct isl_surf tmp_surf; + uint64_t offset_B = 0; + uint32_t tile_x_sa = 0, tile_y_sa = 0; + if (isl_format_is_compressed(surface->isl.format) && + !isl_format_is_compressed(view.format)) { + /* We're creating an uncompressed view of a compressed surface. This is + * allowed but only for a single level/layer. + */ + assert(surface->isl.samples == 1); + assert(view.levels == 1); + assert(surface->isl.dim == ISL_SURF_DIM_3D || view.array_len == 1); + + ASSERTED bool ok = + isl_surf_get_uncompressed_surf(&device->isl_dev, isl_surf, &view, + &tmp_surf, &view, + &offset_B, &tile_x_sa, &tile_y_sa); + assert(ok); + isl_surf = &tmp_surf; + } - state_inout->address = anv_address_add(address, offset_B); + state_inout->address = anv_address_add(address, offset_B); - struct anv_address aux_address = ANV_NULL_ADDRESS; - if (aux_usage != ISL_AUX_USAGE_NONE) - aux_address = anv_image_address(image, &aux_surface->memory_range); - state_inout->aux_address = aux_address; + struct anv_address aux_address = ANV_NULL_ADDRESS; + if (aux_usage != ISL_AUX_USAGE_NONE) + aux_address = anv_image_address(image, &aux_surface->memory_range); + state_inout->aux_address = aux_address; - struct anv_address clear_address = ANV_NULL_ADDRESS; - if (device->info->ver >= 10 && isl_aux_usage_has_fast_clears(aux_usage)) { - clear_address = anv_image_get_clear_color_addr(device, image, aspect); - } - state_inout->clear_address = clear_address; - - isl_surf_fill_state(&device->isl_dev, state_inout->state.map, - .surf = isl_surf, - .view = &view, - .address = anv_address_physical(state_inout->address), - .clear_color = *clear_color, - .aux_surf = &aux_surface->isl, - .aux_usage = aux_usage, - .aux_address = anv_address_physical(aux_address), - .clear_address = anv_address_physical(clear_address), - .use_clear_address = !anv_address_is_null(clear_address), - .mocs = anv_mocs(device, state_inout->address.bo, - view_usage), - .x_offset_sa = tile_x_sa, - .y_offset_sa = tile_y_sa, - .robust_image_access = - device->vk.enabled_features.robustImageAccess || - device->vk.enabled_features.robustImageAccess2); - - /* With the exception of gfx8, the bottom 12 bits of the MCS base address - * are used to store other information. This should be ok, however, - * because the surface buffer addresses are always 4K page aligned. - */ - if (!anv_address_is_null(aux_address)) { - uint32_t *aux_addr_dw = state_inout->state.map + - device->isl_dev.ss.aux_addr_offset; - assert((aux_address.offset & 0xfff) == 0); - state_inout->aux_address.offset |= *aux_addr_dw & 0xfff; - } + struct anv_address clear_address = ANV_NULL_ADDRESS; + if (device->info->ver >= 10 && isl_aux_usage_has_fast_clears(aux_usage)) { + clear_address = anv_image_get_clear_color_addr(device, image, aspect); + } + state_inout->clear_address = clear_address; + + isl_surf_fill_state(&device->isl_dev, state_inout->state.map, + .surf = isl_surf, + .view = &view, + .address = anv_address_physical(state_inout->address), + .clear_color = *clear_color, + .aux_surf = &aux_surface->isl, + .aux_usage = aux_usage, + .aux_address = anv_address_physical(aux_address), + .clear_address = anv_address_physical(clear_address), + .use_clear_address = !anv_address_is_null(clear_address), + .mocs = anv_mocs(device, state_inout->address.bo, + view_usage), + .x_offset_sa = tile_x_sa, + .y_offset_sa = tile_y_sa, + .robust_image_access = + device->vk.enabled_features.robustImageAccess || + device->vk.enabled_features.robustImageAccess2); + + /* With the exception of gfx8, the bottom 12 bits of the MCS base address + * are used to store other information. This should be ok, however, because + * the surface buffer addresses are always 4K page aligned. + */ + if (!anv_address_is_null(aux_address)) { + uint32_t *aux_addr_dw = state_inout->state.map + + device->isl_dev.ss.aux_addr_offset; + assert((aux_address.offset & 0xfff) == 0); + state_inout->aux_address.offset |= *aux_addr_dw & 0xfff; + } - if (device->info->ver >= 10 && clear_address.bo) { - uint32_t *clear_addr_dw = state_inout->state.map + - device->isl_dev.ss.clear_color_state_offset; - assert((clear_address.offset & 0x3f) == 0); - state_inout->clear_address.offset |= *clear_addr_dw & 0x3f; - } + if (device->info->ver >= 10 && clear_address.bo) { + uint32_t *clear_addr_dw = state_inout->state.map + + device->isl_dev.ss.clear_color_state_offset; + assert((clear_address.offset & 0x3f) == 0); + state_inout->clear_address.offset |= *clear_addr_dw & 0x3f; } } @@ -2686,33 +2622,6 @@ anv_CreateImageView(VkDevice _device, general_aux_usage, NULL, 0, &iview->planes[vplane].storage_surface_state); - - iview->planes[vplane].lowered_storage_surface_state.state = - alloc_bindless_surface_state(device); - if (isl_is_storage_image_format(device->info, format.isl_format)) { - anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit, - &storage_view, - ISL_SURF_USAGE_STORAGE_BIT, - general_aux_usage, NULL, - ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED, - &iview->planes[vplane].lowered_storage_surface_state); - } else { - /* In this case, we support the format but, because there's no - * SPIR-V format specifier corresponding to it, we only support it - * if the hardware can do it natively. This is possible for some - * reads but for most writes. Instead of hanging if someone gets - * it wrong, we give them a NULL descriptor. - */ - isl_null_fill_state(&device->isl_dev, - iview->planes[vplane].lowered_storage_surface_state.state.map, - .size = { - .w = image->vk.extent.width, - .h = image->vk.extent.height, - .d = image->vk.extent.depth, - }); - - iview->planes[vplane].lowered_surface_state_is_null = true; - } } } @@ -2746,11 +2655,6 @@ anv_DestroyImageView(VkDevice _device, VkImageView _iview, anv_state_pool_free(&device->bindless_surface_state_pool, iview->planes[plane].storage_surface_state.state); } - - if (iview->planes[plane].lowered_storage_surface_state.state.alloc_size) { - anv_state_pool_free(&device->bindless_surface_state_pool, - iview->planes[plane].lowered_storage_surface_state.state); - } } vk_image_view_destroy(&device->vk, pAllocator, &iview->vk); @@ -2796,36 +2700,13 @@ anv_CreateBufferView(VkDevice _device, if (buffer->vk.usage & VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT) { view->storage_surface_state = alloc_bindless_surface_state(device); - view->lowered_storage_surface_state = alloc_bindless_surface_state(device); anv_fill_buffer_surface_state(device, view->storage_surface_state, format.isl_format, format.swizzle, ISL_SURF_USAGE_STORAGE_BIT, view->address, view->range, format_bs); - - enum isl_format lowered_format = - isl_has_matching_typed_storage_image_format(device->info, - format.isl_format) ? - isl_lower_storage_image_format(device->info, format.isl_format) : - ISL_FORMAT_RAW; - - /* If we lower the format, we should ensure either they both match in - * bits per channel or that there is no swizzle because we can't use - * the swizzle for a different bit pattern. - */ - assert(isl_formats_have_same_bits_per_channel(lowered_format, - format.isl_format) || - isl_swizzle_is_identity(format.swizzle)); - - anv_fill_buffer_surface_state(device, view->lowered_storage_surface_state, - lowered_format, format.swizzle, - ISL_SURF_USAGE_STORAGE_BIT, - view->address, view->range, - (lowered_format == ISL_FORMAT_RAW ? 1 : - isl_format_get_layout(lowered_format)->bpb / 8)); } else { view->storage_surface_state = (struct anv_state){ 0 }; - view->lowered_storage_surface_state = (struct anv_state){ 0 }; } *pView = anv_buffer_view_to_handle(view); @@ -2851,9 +2732,5 @@ anv_DestroyBufferView(VkDevice _device, VkBufferView bufferView, anv_state_pool_free(&device->bindless_surface_state_pool, view->storage_surface_state); - if (view->lowered_storage_surface_state.alloc_size > 0) - anv_state_pool_free(&device->bindless_surface_state_pool, - view->lowered_storage_surface_state); - vk_object_free(&device->vk, pAllocator, view); } diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c index b56f2cc..9194f98 100644 --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c @@ -983,11 +983,8 @@ lower_image_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin, b->cursor = nir_before_instr(&intrin->instr); if (binding_offset > MAX_BINDING_TABLE_SIZE) { - const unsigned desc_comp = - image_binding_needs_lowered_surface(var) ? 1 : 0; - nir_ssa_def *desc = - build_load_var_deref_descriptor_mem(b, deref, 0, 2, 32, state); - nir_ssa_def *handle = nir_channel(b, desc, desc_comp); + nir_ssa_def *handle = + build_load_var_deref_descriptor_mem(b, deref, 0, 1, 32, state); nir_rewrite_image_intrinsic(intrin, handle, true); } else { unsigned array_size = @@ -1542,9 +1539,6 @@ anv_nir_apply_pipeline_layout(nir_shader *shader, for (unsigned i = 0; i < array_size; i++) { assert(pipe_binding[i].set == set); assert(pipe_binding[i].index == bind_layout->descriptor_index + i); - - pipe_binding[i].lowered_storage_surface = - image_binding_needs_lowered_surface(var); } } diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c index 12dcd57..0744d5e 100644 --- a/src/intel/vulkan/anv_pipeline.c +++ b/src/intel/vulkan/anv_pipeline.c @@ -913,11 +913,15 @@ anv_pipeline_lower_nir(struct anv_pipeline *pipeline, NIR_PASS(_, nir, brw_nir_lower_storage_image, &(struct brw_nir_lower_storage_image_opts) { + /* Anv only supports Gfx9+ which has better defined typed read + * behavior. It allows us to only have to care about lowering + * loads/atomics. + */ .devinfo = compiler->devinfo, .lower_loads = true, - .lower_stores = true, + .lower_stores = false, .lower_atomics = true, - .lower_get_size = true, + .lower_get_size = false, }); NIR_PASS(_, nir, nir_lower_explicit_io, nir_var_mem_global, diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index ce2f006..ac36d0d 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1622,7 +1622,8 @@ struct anv_storage_image_descriptor { * SURFACE_STATE table index is in the top 20 bits. */ uint32_t vanilla; - uint32_t lowered; + + uint32_t pad; }; /** Struct representing a address/range descriptor @@ -1833,7 +1834,6 @@ struct anv_buffer_view { struct anv_state surface_state; struct anv_state storage_surface_state; - struct anv_state lowered_storage_surface_state; }; struct anv_push_descriptor_set { @@ -1996,9 +1996,6 @@ struct anv_pipeline_binding { */ uint8_t dynamic_offset_index; }; - - /** For a storage image, whether it requires a lowered surface */ - uint8_t lowered_storage_surface; }; struct anv_push_range { @@ -4130,22 +4127,14 @@ struct anv_image_view { struct anv_surface_state general_sampler_surface_state; /** - * RENDER_SURFACE_STATE when using image as a storage image. Separate - * states for vanilla (with the original format) and one which has been - * lowered to a format suitable for reading. This may be a raw surface - * in extreme cases or simply a surface with a different format where we - * expect some conversion to be done in the shader. + * RENDER_SURFACE_STATE when using image as a storage image. */ struct anv_surface_state storage_surface_state; - struct anv_surface_state lowered_storage_surface_state; - - bool lowered_surface_state_is_null; } planes[3]; }; enum anv_image_view_state_flags { - ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED = (1 << 0), - ANV_IMAGE_VIEW_STATE_TEXTURE_OPTIMAL = (1 << 1), + ANV_IMAGE_VIEW_STATE_TEXTURE_OPTIMAL = (1 << 0), }; void anv_image_fill_surface_state(struct anv_device *device, diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index 91d705a..89b7545 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -2180,27 +2180,9 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: { if (desc->image_view) { struct anv_surface_state sstate = - binding->lowered_storage_surface - ? desc->image_view->planes[binding->plane].lowered_storage_surface_state - : desc->image_view->planes[binding->plane].storage_surface_state; - const bool lowered_surface_state_is_null = - desc->image_view->planes[binding->plane].lowered_surface_state_is_null; + desc->image_view->planes[binding->plane].storage_surface_state; surface_state = anv_bindless_state_for_binding_table(sstate.state); assert(surface_state.alloc_size); - if (binding->lowered_storage_surface && lowered_surface_state_is_null) { - mesa_loge("Bound a image to a descriptor where the " - "descriptor does not have NonReadable " - "set and the image does not have a " - "corresponding SPIR-V format enum."); - vk_debug_report(&cmd_buffer->device->physical->instance->vk, - VK_DEBUG_REPORT_ERROR_BIT_EXT, - &desc->image_view->vk.base, - __LINE__, 0, "anv", - "Bound a image to a descriptor where the " - "descriptor does not have NonReadable " - "set and the image does not have a " - "corresponding SPIR-V format enum."); - } } else { surface_state = anv_bindless_state_for_binding_table( cmd_buffer->device->null_surface_state); @@ -2274,9 +2256,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: if (desc->buffer_view) { surface_state = anv_bindless_state_for_binding_table( - binding->lowered_storage_surface - ? desc->buffer_view->lowered_storage_surface_state - : desc->buffer_view->storage_surface_state); + desc->buffer_view->storage_surface_state); assert(surface_state.alloc_size); } else { surface_state = anv_bindless_state_for_binding_table( -- 2.7.4