diff --git a/src/intel/vulkan/anv_descriptor_set.c b/src/intel/vulkan/anv_descriptor_set.c index 578ecfb9b93..401fabcdae3 100644 --- a/src/intel/vulkan/anv_descriptor_set.c +++ b/src/intel/vulkan/anv_descriptor_set.c @@ -1360,10 +1360,10 @@ anv_descriptor_set_write_image_view(struct anv_device *device, assert(!(bind_layout->data & ANV_DESCRIPTOR_IMAGE_PARAM)); assert(image_view->n_planes == 1); struct anv_storage_image_descriptor desc_data = { - .read_write = anv_surface_state_to_handle( + .vanilla = anv_surface_state_to_handle( image_view->planes[0].storage_surface_state.state), - .write_only = anv_surface_state_to_handle( - image_view->planes[0].writeonly_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)); } @@ -1372,7 +1372,7 @@ anv_descriptor_set_write_image_view(struct anv_device *device, /* Storage images can only ever have one plane */ assert(image_view->n_planes == 1); const struct brw_image_param *image_param = - &image_view->planes[0].storage_image_param; + &image_view->planes[0].lowered_storage_image_param; anv_descriptor_set_write_image_param(desc_map, image_param); } @@ -1437,17 +1437,17 @@ anv_descriptor_set_write_buffer_view(struct anv_device *device, if (bind_layout->data & ANV_DESCRIPTOR_STORAGE_IMAGE) { assert(!(bind_layout->data & ANV_DESCRIPTOR_IMAGE_PARAM)); struct anv_storage_image_descriptor desc_data = { - .read_write = anv_surface_state_to_handle( + .vanilla = anv_surface_state_to_handle( buffer_view->storage_surface_state), - .write_only = anv_surface_state_to_handle( - buffer_view->writeonly_storage_surface_state), + .lowered = anv_surface_state_to_handle( + buffer_view->lowered_storage_surface_state), }; memcpy(desc_map, &desc_data, sizeof(desc_data)); } if (bind_layout->data & ANV_DESCRIPTOR_IMAGE_PARAM) { anv_descriptor_set_write_image_param(desc_map, - &buffer_view->storage_image_param); + &buffer_view->lowered_storage_image_param); } } diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index bcc2e8de2cd..99050a1ce34 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -2387,7 +2387,7 @@ anv_image_fill_surface_state(struct anv_device *device, anv_image_address(image, &surface->memory_range); if (view_usage == ISL_SURF_USAGE_STORAGE_BIT && - !(flags & ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY) && + (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 @@ -2407,7 +2407,7 @@ anv_image_fill_surface_state(struct anv_device *device, state_inout->clear_address = ANV_NULL_ADDRESS; } else { if (view_usage == ISL_SURF_USAGE_STORAGE_BIT && - !(flags & ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY)) { + (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. @@ -2628,37 +2628,38 @@ anv_CreateImageView(VkDevice _device, /* NOTE: This one needs to go last since it may stomp isl_view.format */ if (iview->vk.usage & VK_IMAGE_USAGE_STORAGE_BIT) { + iview->planes[vplane].storage_surface_state.state = alloc_surface_state(device); + anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit, + &iview->planes[vplane].isl, + ISL_SURF_USAGE_STORAGE_BIT, + ISL_AUX_USAGE_NONE, NULL, + 0, + &iview->planes[vplane].storage_surface_state, + NULL); + if (isl_is_storage_image_format(format.isl_format)) { - iview->planes[vplane].storage_surface_state.state = + iview->planes[vplane].lowered_storage_surface_state.state = alloc_surface_state(device); anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit, &iview->planes[vplane].isl, ISL_SURF_USAGE_STORAGE_BIT, ISL_AUX_USAGE_NONE, NULL, - 0, - &iview->planes[vplane].storage_surface_state, - &iview->planes[vplane].storage_image_param); + ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED, + &iview->planes[vplane].lowered_storage_surface_state, + &iview->planes[vplane].lowered_storage_image_param); } else { /* In this case, we support the format but, because there's no - * SPIR-V format specifier corresponding to it, we only support - * NonReadable (writeonly in GLSL) access. Instead of hanging in - * these invalid cases, we give them a NULL descriptor. + * 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. */ assert(isl_format_supports_typed_writes(&device->info, format.isl_format)); iview->planes[vplane].storage_surface_state.state = device->null_surface_state; } - - iview->planes[vplane].writeonly_storage_surface_state.state = alloc_surface_state(device); - anv_image_fill_surface_state(device, image, 1ULL << iaspect_bit, - &iview->planes[vplane].isl, - ISL_SURF_USAGE_STORAGE_BIT, - ISL_AUX_USAGE_NONE, NULL, - ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY, - &iview->planes[vplane].writeonly_storage_surface_state, - NULL); } } @@ -2697,9 +2698,9 @@ anv_DestroyImageView(VkDevice _device, VkImageView _iview, iview->planes[plane].storage_surface_state.state); } - if (iview->planes[plane].writeonly_storage_surface_state.state.offset) { + if (iview->planes[plane].lowered_storage_surface_state.state.offset) { anv_state_pool_free(&device->surface_state_pool, - iview->planes[plane].writeonly_storage_surface_state.state); + iview->planes[plane].lowered_storage_surface_state.state); } } @@ -2746,32 +2747,31 @@ anv_CreateBufferView(VkDevice _device, if (buffer->usage & VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT) { view->storage_surface_state = alloc_surface_state(device); - view->writeonly_storage_surface_state = alloc_surface_state(device); + view->lowered_storage_surface_state = alloc_surface_state(device); - enum isl_format storage_format = + anv_fill_buffer_surface_state(device, view->storage_surface_state, + view->format, ISL_SURF_USAGE_STORAGE_BIT, + view->address, view->range, + isl_format_get_layout(view->format)->bpb / 8); + + enum isl_format lowered_format = isl_has_matching_typed_storage_image_format(&device->info, view->format) ? isl_lower_storage_image_format(&device->info, view->format) : ISL_FORMAT_RAW; - anv_fill_buffer_surface_state(device, view->storage_surface_state, - storage_format, ISL_SURF_USAGE_STORAGE_BIT, + anv_fill_buffer_surface_state(device, view->lowered_storage_surface_state, + lowered_format, ISL_SURF_USAGE_STORAGE_BIT, view->address, view->range, - (storage_format == ISL_FORMAT_RAW ? 1 : - isl_format_get_layout(storage_format)->bpb / 8)); - - /* Write-only accesses should use the original format. */ - anv_fill_buffer_surface_state(device, view->writeonly_storage_surface_state, - view->format, ISL_SURF_USAGE_STORAGE_BIT, - view->address, view->range, - isl_format_get_layout(view->format)->bpb / 8); + (lowered_format == ISL_FORMAT_RAW ? 1 : + isl_format_get_layout(lowered_format)->bpb / 8)); isl_buffer_fill_image_param(&device->isl_dev, - &view->storage_image_param, + &view->lowered_storage_image_param, view->format, view->range); } else { view->storage_surface_state = (struct anv_state){ 0 }; - view->writeonly_storage_surface_state = (struct anv_state){ 0 }; + view->lowered_storage_surface_state = (struct anv_state){ 0 }; } *pView = anv_buffer_view_to_handle(view); @@ -2797,9 +2797,9 @@ anv_DestroyBufferView(VkDevice _device, VkBufferView bufferView, anv_state_pool_free(&device->surface_state_pool, view->storage_surface_state); - if (view->writeonly_storage_surface_state.alloc_size > 0) + if (view->lowered_storage_surface_state.alloc_size > 0) anv_state_pool_free(&device->surface_state_pool, - view->writeonly_storage_surface_state); + 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 0f508490110..d39eae12839 100644 --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c @@ -1003,6 +1003,13 @@ lower_get_ssbo_size(nir_builder *b, nir_intrinsic_instr *intrin, return true; } +static bool +image_binding_needs_lowered_surface(nir_variable *var) +{ + return !(var->data.access & ACCESS_NON_READABLE) && + var->data.image.format != PIPE_FORMAT_NONE; +} + static bool lower_image_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin, struct apply_pipeline_layout_state *state) @@ -1031,11 +1038,11 @@ lower_image_intrinsic(nir_builder *b, nir_intrinsic_instr *intrin, nir_ssa_def_rewrite_uses(&intrin->dest.ssa, desc); } else if (binding_offset > MAX_BINDING_TABLE_SIZE) { - const bool write_only = - (var->data.access & ACCESS_NON_READABLE) != 0; + 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, write_only ? 1 : 0); + nir_ssa_def *handle = nir_channel(b, desc, desc_comp); nir_rewrite_image_intrinsic(intrin, handle, true); } else { unsigned array_size = @@ -1609,9 +1616,8 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice, dim == GLSL_SAMPLER_DIM_SUBPASS_MS) pipe_binding[i].input_attachment_index = var->data.index + i; - /* NOTE: This is a uint8_t so we really do need to != 0 here */ - pipe_binding[i].write_only = - (var->data.access & ACCESS_NON_READABLE) != 0; + pipe_binding[i].lowered_storage_surface = + image_binding_needs_lowered_surface(var); } } diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 159ce4cc1c0..ef804260f68 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1825,8 +1825,8 @@ struct anv_storage_image_descriptor { * These are expected to already be shifted such that the 20-bit * SURFACE_STATE table index is in the top 20 bits. */ - uint32_t read_write; - uint32_t write_only; + uint32_t vanilla; + uint32_t lowered; }; /** Struct representing a address/range descriptor @@ -2027,9 +2027,9 @@ struct anv_buffer_view { struct anv_state surface_state; struct anv_state storage_surface_state; - struct anv_state writeonly_storage_surface_state; + struct anv_state lowered_storage_surface_state; - struct brw_image_param storage_image_param; + struct brw_image_param lowered_storage_image_param; }; struct anv_push_descriptor_set { @@ -2226,8 +2226,8 @@ struct anv_pipeline_binding { uint8_t dynamic_offset_index; }; - /** For a storage image, whether it is write-only */ - uint8_t write_only; + /** For a storage image, whether it requires a lowered surface */ + uint8_t lowered_storage_surface; /** Pad to 64 bits so that there are no holes and we can safely memcmp * assuming POD zero-initialization. @@ -4345,18 +4345,20 @@ struct anv_image_view { /** * RENDER_SURFACE_STATE when using image as a storage image. Separate - * states for write-only and readable, using the real format for - * write-only and the lowered format for readable. + * 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 writeonly_storage_surface_state; + struct anv_surface_state lowered_storage_surface_state; - struct brw_image_param storage_image_param; + struct brw_image_param lowered_storage_image_param; } planes[3]; }; enum anv_image_view_state_flags { - ANV_IMAGE_VIEW_STATE_STORAGE_WRITE_ONLY = (1 << 0), + ANV_IMAGE_VIEW_STATE_STORAGE_LOWERED = (1 << 0), ANV_IMAGE_VIEW_STATE_TEXTURE_OPTIMAL = (1 << 1), }; diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index 38250983b48..b122628a882 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -2762,8 +2762,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->write_only) - ? desc->image_view->planes[binding->plane].writeonly_storage_surface_state + 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; surface_state = sstate.state; assert(surface_state.alloc_size); @@ -2846,8 +2847,8 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: if (desc->buffer_view) { - surface_state = (binding->write_only) - ? desc->buffer_view->writeonly_storage_surface_state + surface_state = binding->lowered_storage_surface + ? desc->buffer_view->lowered_storage_surface_state : desc->buffer_view->storage_surface_state; assert(surface_state.alloc_size); if (need_client_mem_relocs) {