From ee917d2b78a2e19f88076bf1da2c99e95cdf1966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Pi=C3=B1eiro?= Date: Tue, 7 Apr 2020 00:33:14 +0200 Subject: [PATCH] v3dv/descriptor_set: combine texture and sampler indices OpenGL doesn't have the concept of individual texture and sampler, so texture and sampler indexes have the same value. v3d compiler uses this assumption, so for example, the texture info at the v3d key include values that you need to use the texture format and the sampler to fill (like the return_size). One option would be to adapt the v3d compiler to handle both, but then we would need to adapt to the lowerings it uses, like nir_lower_tex, that also take the same assumption. We deal with this on the Vulkan driver, by reassigning the texture and sampler index to a combined one. We add a hash table to map the combined texture idx and sampler idx to this combined idx, and a simple array to the opposite map. On the driver we work with the separate indices to fill up the data, while the v3d compiler works with the combined one. As mentioned, this is needed to properly fill up the texture return size, so as we are here, we fix that. This gets tests like the following working: dEQP-VK.glsl.texture_gather.basic.2d.depth32f.base_level.level_2 Part-of: --- src/broadcom/vulkan/v3dv_cmd_buffer.c | 58 ++++++++++++++++++--------- src/broadcom/vulkan/v3dv_device.c | 1 + src/broadcom/vulkan/v3dv_formats.c | 4 +- src/broadcom/vulkan/v3dv_pipeline.c | 44 ++++++++++++++++++++ src/broadcom/vulkan/v3dv_private.h | 45 ++++++++++++++++++++- src/broadcom/vulkan/v3dv_uniforms.c | 14 +++++-- 6 files changed, 141 insertions(+), 25 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index aac8a37c912..69c791e6cab 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -1951,35 +1951,55 @@ static bool cmd_buffer_populate_v3d_key(struct v3d_key *key, struct v3dv_cmd_buffer *cmd_buffer) { - struct v3dv_descriptor_map *texture_map = &cmd_buffer->state.pipeline->texture_map; - struct v3dv_descriptor_state *descriptor_state = - &cmd_buffer->state.descriptor_state; + if (cmd_buffer->state.pipeline->combined_index_map != NULL) { + struct v3dv_descriptor_map *texture_map = &cmd_buffer->state.pipeline->texture_map; + struct v3dv_descriptor_map *sampler_map = &cmd_buffer->state.pipeline->sampler_map; + struct v3dv_descriptor_state *descriptor_state = + &cmd_buffer->state.descriptor_state; - for (uint32_t i = 0; i < texture_map->num_desc; i++) { - struct v3dv_image_view *image_view = - v3dv_descriptor_map_get_image_view(descriptor_state, - texture_map, + hash_table_foreach(cmd_buffer->state.pipeline->combined_index_map, entry) { + uint32_t combined_idx = (uint32_t)(uintptr_t) (entry->data); + uint32_t combined_idx_key = + cmd_buffer->state.pipeline->combined_index_to_key_map[combined_idx]; + uint32_t texture_idx; + uint32_t sampler_idx; + + v3dv_pipeline_combined_index_key_unpack(combined_idx_key, + &texture_idx, &sampler_idx); + + struct v3dv_image_view *image_view = + v3dv_descriptor_map_get_image_view(descriptor_state, + texture_map, + cmd_buffer->state.pipeline->layout, + texture_idx); + + if (image_view == NULL) + return false; + + const struct v3dv_sampler *sampler = + v3dv_descriptor_map_get_sampler(descriptor_state, + sampler_map, cmd_buffer->state.pipeline->layout, - i); + sampler_idx); + if (sampler == NULL) + return false; - if (image_view == NULL) - return false; + key->tex[combined_idx].return_size = + v3dv_get_tex_return_size(image_view->format, + sampler ? sampler->compare_enable : false); - key->tex[i].return_size = - v3dv_get_tex_return_size(image_view->format, - 0); /* FIXME: how to get the sampler compare mode? */ - - if (key->tex[i].return_size == 16) { - key->tex[i].return_channels = 2; - } else { - key->tex[i].return_channels = 4; - } + if (key->tex[combined_idx].return_size == 16) { + key->tex[combined_idx].return_channels = 2; + } else { + key->tex[combined_idx].return_channels = 4; + } /* Note: we don't need to do anything for the swizzle, as that is * handled with the swizzle info at the Texture State, and the * default values for key->tex[].swizzle were already filled up on * the pipeline populate. */ + } } return true; diff --git a/src/broadcom/vulkan/v3dv_device.c b/src/broadcom/vulkan/v3dv_device.c index 6a25e231b60..afb332f45e1 100644 --- a/src/broadcom/vulkan/v3dv_device.c +++ b/src/broadcom/vulkan/v3dv_device.c @@ -2036,6 +2036,7 @@ v3dv_CreateSampler(VkDevice _device, } } + sampler->compare_enable = pCreateInfo->compareEnable; pack_sampler_state(sampler, pCreateInfo); *pSampler = v3dv_sampler_to_handle(sampler); diff --git a/src/broadcom/vulkan/v3dv_formats.c b/src/broadcom/vulkan/v3dv_formats.c index db75598ebe6..aa5431111fb 100644 --- a/src/broadcom/vulkan/v3dv_formats.c +++ b/src/broadcom/vulkan/v3dv_formats.c @@ -310,9 +310,9 @@ v3dv_get_format_swizzle(VkFormat f) uint8_t v3dv_get_tex_return_size(const struct v3dv_format *vf, - enum pipe_tex_compare compare) + bool compare_enable) { - if (compare == PIPE_TEX_COMPARE_R_TO_TEXTURE) + if (compare_enable) return 16; return vf->return_size; diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 7ff1d47fb62..21f92356453 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -131,6 +131,9 @@ v3dv_DestroyPipeline(VkDevice _device, pipeline->default_attribute_values = NULL; } + if (pipeline->combined_index_map) + _mesa_hash_table_destroy(pipeline->combined_index_map, NULL); + vk_free2(&device->alloc, pAllocator, pipeline); } @@ -504,6 +507,41 @@ lower_vulkan_resource_index(nir_builder *b, nir_instr_remove(&instr->instr); } +static struct hash_table * +pipeline_ensure_combined_index_map(struct v3dv_pipeline *pipeline) +{ + if (pipeline->combined_index_map == NULL) { + pipeline->combined_index_map = + _mesa_hash_table_create(NULL, _mesa_hash_u32, _mesa_key_u32_equal); + pipeline->next_combined_index = 0; + } + + assert(pipeline->combined_index_map); + + return pipeline->combined_index_map; +} + +static uint32_t +get_combined_index(struct v3dv_pipeline *pipeline, + uint32_t texture_index, + uint32_t sampler_index) +{ + struct hash_table *ht = pipeline_ensure_combined_index_map(pipeline); + uint32_t key = v3dv_pipeline_combined_index_key_create(texture_index, sampler_index); + struct hash_entry *entry = _mesa_hash_table_search(ht, &key); + + if (entry) + return (uint32_t)(uintptr_t) (entry->data); + + uint32_t new_index = pipeline->next_combined_index; + + _mesa_hash_table_insert(ht, &key, (void *)(uintptr_t) (new_index)); + pipeline->combined_index_to_key_map[new_index] = key; + pipeline->next_combined_index++; + + return new_index; +} + static void lower_tex_src_to_offset(nir_builder *b, nir_tex_instr *instr, unsigned src_idx, struct v3dv_pipeline *pipeline, @@ -600,6 +638,12 @@ lower_sampler(nir_builder *b, nir_tex_instr *instr, if (texture_idx < 0 && sampler_idx < 0) return false; + int combined_index = + get_combined_index(pipeline, instr->texture_index, instr->sampler_index); + + instr->texture_index = combined_index; + instr->sampler_index = combined_index; + return true; } diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index fc93fb04978..cbbebc298a7 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -1018,6 +1018,8 @@ struct v3dv_descriptor_map { }; struct v3dv_sampler { + bool compare_enable; + /* FIXME: here we store the packet SAMPLER_STATE, that is referenced as part * of the tmu configuration, and the content is set per sampler. A possible * perf improvement, to avoid bo fragmentation, would be to save the state @@ -1028,6 +1030,32 @@ struct v3dv_sampler { struct v3dv_bo *state; }; +/* + * Following two methods are using on the combined to/from texture/sampler + * indices maps at v3dv_pipeline. + */ +static inline uint32_t +v3dv_pipeline_combined_index_key_create(uint32_t texture_index, + uint32_t sampler_index) +{ + return texture_index << 24 | sampler_index; +} + +static inline void +v3dv_pipeline_combined_index_key_unpack(uint32_t combined_index_key, + uint32_t *texture_index, + uint32_t *sampler_index) +{ + uint32_t texture = combined_index_key >> 24; + uint32_t sampler = combined_index_key & 0xffffff; + + if (texture_index) + *texture_index = texture; + + if (sampler_index) + *sampler_index = sampler; +} + struct v3dv_pipeline { struct v3dv_device *device; @@ -1084,6 +1112,21 @@ struct v3dv_pipeline { struct v3dv_descriptor_map sampler_map; struct v3dv_descriptor_map texture_map; + /* + * Vulkan has separate texture and sampler objects. Previous sampler and + * texture map uses a sampler and texture index respectively, that can be + * different. But OpenGL combine both (or in other words, they are the + * same). The v3d compiler and all the nir lowerings that they use were + * written under that assumption. In order to not update all those, we + * combine the indexes, and we use the following maps to get one or the + * other. In general the driver side uses the tex/sampler indexes to gather + * resources, and the compiler side uses the combined index (so the v3d key + * texture info will be indexed using the combined index). + */ + struct hash_table *combined_index_map; + uint32_t combined_index_to_key_map[32]; + uint32_t next_combined_index; + /* FIXME: this bo is another candidate to data to be uploaded using a * resource manager, instead of a individual bo */ @@ -1204,7 +1247,7 @@ void v3dv_loge_v(const char *format, va_list va); const struct v3dv_format *v3dv_get_format(VkFormat); const uint8_t *v3dv_get_format_swizzle(VkFormat f); void v3dv_get_internal_type_bpp_for_output_format(uint32_t format, uint32_t *type, uint32_t *bpp); -uint8_t v3dv_get_tex_return_size(const struct v3dv_format *vf, enum pipe_tex_compare compare); +uint8_t v3dv_get_tex_return_size(const struct v3dv_format *vf, bool compare_enable); uint32_t v3d_utile_width(int cpp); diff --git a/src/broadcom/vulkan/v3dv_uniforms.c b/src/broadcom/vulkan/v3dv_uniforms.c index 48ba4e7ec17..49fac70437f 100644 --- a/src/broadcom/vulkan/v3dv_uniforms.c +++ b/src/broadcom/vulkan/v3dv_uniforms.c @@ -89,14 +89,18 @@ write_tmu_p0(struct v3dv_cmd_buffer *cmd_buffer, uint32_t data) { int unit = v3d_unit_data_get_unit(data); - + uint32_t texture_idx; struct v3dv_job *job = cmd_buffer->state.job; struct v3dv_descriptor_state *descriptor_state = &cmd_buffer->state.descriptor_state; + v3dv_pipeline_combined_index_key_unpack(pipeline->combined_index_to_key_map[unit], + &texture_idx, + NULL); + struct v3dv_image_view *image_view = v3dv_descriptor_map_get_image_view(descriptor_state, &pipeline->texture_map, - pipeline->layout, unit); + pipeline->layout, texture_idx); assert(image_view); @@ -116,13 +120,17 @@ write_tmu_p1(struct v3dv_cmd_buffer *cmd_buffer, uint32_t data) { uint32_t unit = v3d_unit_data_get_unit(data); + uint32_t sampler_idx; struct v3dv_job *job = cmd_buffer->state.job; struct v3dv_descriptor_state *descriptor_state = &cmd_buffer->state.descriptor_state; + v3dv_pipeline_combined_index_key_unpack(pipeline->combined_index_to_key_map[unit], + NULL, &sampler_idx); + const struct v3dv_sampler *sampler = v3dv_descriptor_map_get_sampler(descriptor_state, &pipeline->sampler_map, - pipeline->layout, unit); + pipeline->layout, sampler_idx); assert(sampler);