anv: drop lowered storage images code

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22302>

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));
    }

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 &&
+   const struct isl_surf *isl_surf = &surface->isl;
-       (flags & ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED) &&
+
-       !isl_has_matching_typed_storage_image_format(device->info,
+   struct isl_surf tmp_surf;
-                                                    view.format)) {
+   uint64_t offset_B = 0;
-      /* In this case, we are a writeable storage buffer which needs to be
+   uint32_t tile_x_sa = 0, tile_y_sa = 0;
-       * lowered to linear. All tiling and offset calculations will be done in
+   if (isl_format_is_compressed(surface->isl.format) &&
-       * the shader.
+       !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(aux_usage == ISL_AUX_USAGE_NONE);
+      assert(surface->isl.samples == 1);
-      isl_buffer_fill_state(&device->isl_dev, state_inout->state.map,
+      assert(view.levels == 1);
-                            .address = anv_address_physical(address),
+      assert(surface->isl.dim == ISL_SURF_DIM_3D || view.array_len == 1);
-                            .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
+      ASSERTED bool ok =
-          * bits per channel or that there is no swizzle, because we can't use
+         isl_surf_get_uncompressed_surf(&device->isl_dev, isl_surf, &view,
-          * the swizzle for a different bit pattern.
+                                        &tmp_surf, &view,
-          */
+                                        &offset_B, &tile_x_sa, &tile_y_sa);
-         assert(isl_formats_have_same_bits_per_channel(lower_format,
+      assert(ok);
-                                                       view.format) ||
+      isl_surf = &tmp_surf;
-                isl_swizzle_is_identity_for_format(view.format, view.swizzle));
+   }
 
-         view.format = lower_format;
+   state_inout->address = anv_address_add(address, offset_B);
-      }
 
-      const struct isl_surf *isl_surf = &surface->isl;
+   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 isl_surf tmp_surf;
+   struct anv_address clear_address = ANV_NULL_ADDRESS;
-      uint64_t offset_B = 0;
+   if (device->info->ver >= 10 && isl_aux_usage_has_fast_clears(aux_usage)) {
-      uint32_t tile_x_sa = 0, tile_y_sa = 0;
+      clear_address = anv_image_get_clear_color_addr(device, image, aspect);
-      if (isl_format_is_compressed(surface->isl.format) &&
+   }
-          !isl_format_is_compressed(view.format)) {
+   state_inout->clear_address = clear_address;
-         /* 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_fill_state(&device->isl_dev, state_inout->state.map,
-            isl_surf_get_uncompressed_surf(&device->isl_dev, isl_surf, &view,
+                       .surf = isl_surf,
-                                           &tmp_surf, &view,
+                       .view = &view,
-                                           &offset_B, &tile_x_sa, &tile_y_sa);
+                       .address = anv_address_physical(state_inout->address),
-         assert(ok);
+                       .clear_color = *clear_color,
-         isl_surf = &tmp_surf;
+                       .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);
 
-      state_inout->address = anv_address_add(address, offset_B);
+   /* 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 aux_address = ANV_NULL_ADDRESS;
+   if (device->info->ver >= 10 && clear_address.bo) {
-      if (aux_usage != ISL_AUX_USAGE_NONE)
+      uint32_t *clear_addr_dw = state_inout->state.map +
-         aux_address = anv_image_address(image, &aux_surface->memory_range);
+         device->isl_dev.ss.clear_color_state_offset;
-      state_inout->aux_address = aux_address;
+      assert((clear_address.offset & 0x3f) == 0);
-
+      state_inout->clear_address.offset |= *clear_addr_dw & 0x3f;
-      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;
-      }
    }
 }
 
@@ -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);
 }

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 =
+      nir_ssa_def *handle =
-         image_binding_needs_lowered_surface(var) ? 1 : 0;
+         build_load_var_deref_descriptor_mem(b, deref, 0, 1, 32, state);
-      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_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);
       }
    }
 

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,

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
+       * RENDER_SURFACE_STATE when using image as a storage image.
-       * 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.
        */
       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 << 0),
-   ANV_IMAGE_VIEW_STATE_TEXTURE_OPTIMAL      = (1 << 1),
 };
 
 void anv_image_fill_surface_state(struct anv_device *device,

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].storage_surface_state;
-                  ? 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;
                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->storage_surface_state);
-                  ? desc->buffer_view->lowered_storage_surface_state
-                  : desc->buffer_view->storage_surface_state);
                assert(surface_state.alloc_size);
             } else {
                surface_state = anv_bindless_state_for_binding_table(