<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 15, 2017 at 3:54 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> <div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Fri, Sep 15, 2017 at 7:11 AM, Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Signed-off-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>> --- src/intel/vulkan/anv_descriptor_set.c | 107 +++++++++++++++++------ src/intel/vulkan/anv_nir_apply_pipeline_layout.c | 44 +++++----- src/intel/vulkan/anv_private.h | 24 ++++- src/intel/vulkan/genX_cmd_buffer.c | 14 ++- src/intel/vulkan/genX_state.c | 81 +++++++++-------- 5 files changed, 178 insertions(+), 92 deletions(-) diff --git a/src/intel/vulkan/anv_descriptor_set.c b/src/intel/vulkan/anv_descriptor_set.c index 91387c065e4..4e29310f5fa 100644 --- a/src/intel/vulkan/anv_descriptor_set.c +++ b/src/intel/vulkan/anv_descriptor_set.c @@ -35,6 +35,21 @@ * Descriptor set layouts. */ +static uint32_t +layout_binding_get_descriptor_count(const VkDescriptorSetLayoutBinding *binding) +{ + if (binding->pImmutableSamplers == NULL) + return binding->descriptorCount; + + uint32_t immutable_sampler_count = 0; + for (uint32_t i = 0; i < binding->descriptorCount; i++) { + ANV_FROM_HANDLE(anv_sampler, sampler, binding->pImmutableSamplers[i]); + immutable_sampler_count += sampler->nb_planes; + } + + return immutable_sampler_count; +} + VkResult anv_CreateDescriptorSetLayout( VkDevice _device, const VkDescriptorSetLayoutCreateInfo* pCreateInfo, @@ -49,13 +64,13 @@ VkResult anv_CreateDescriptorSetLayout( uint32_t immutable_sampler_count = 0; for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) { max_binding = MAX2(max_binding, pCreateInfo->pBindings[j].binding); - if (pCreateInfo->pBindings[j].pImmutableSamplers) - immutable_sampler_count += pCreateInfo->pBindings[j].descriptorCount; + immutable_sampler_count += + layout_binding_get_descriptor_count(&pCreateInfo->pBindings[j]); } struct anv_descriptor_set_layout *set_layout; struct anv_descriptor_set_binding_layout *bindings; - struct anv_sampler **samplers; + struct anv_descriptor_set_immutable_sampler *samplers; ANV_MULTIALLOC(ma); anv_multialloc_add(&ma, &set_layout, 1); @@ -74,6 +89,7 @@ VkResult anv_CreateDescriptorSetLayout( memset(&set_layout->binding[b], -1, sizeof(set_layout->binding[b])); set_layout->binding[b].array_size = 0; + set_layout->binding[b].descriptor_size = 0; set_layout->binding[b].immutable_samplers = NULL; } @@ -108,17 +124,20 @@ VkResult anv_CreateDescriptorSetLayout( set_layout->binding[b].type = binding->descriptorType; #endif set_layout->binding[b].array_size = binding->descriptorCount; + set_layout->binding[b].descriptor_size = + layout_binding_get_descriptor_count(binding); set_layout->binding[b].descriptor_index = set_layout->size; - set_layout->size += binding->descriptorCount; + set_layout->size += set_layout->binding[b].descriptor_size; switch (binding->descriptorType) { case VK_DESCRIPTOR_TYPE_SAMPLER: - case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: + case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: { anv_foreach_stage(s, binding->stageFlags) { set_layout->binding[b].stage[s].sampler_index = sampler_count[s]; - sampler_count[s] += binding->descriptorCount; + sampler_count[s] += set_layout->binding[b].descriptor_size; } break; + } default: break; } @@ -140,7 +159,7 @@ VkResult anv_CreateDescriptorSetLayout( case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: anv_foreach_stage(s, binding->stageFlags) { set_layout->binding[b].stage[s].surface_index = surface_count[s]; - surface_count[s] += binding->descriptorCount; + surface_count[s] += set_layout->binding[b].descriptor_size; } break; default: @@ -151,7 +170,7 @@ VkResult anv_CreateDescriptorSetLayout( case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: set_layout->binding[b].dynamic_offset_index = dynamic_offset_count; - dynamic_offset_count += binding->descriptorCount; + dynamic_offset_count += set_layout->binding[b].descriptor_size; </blockquote><div> </div></div></div><div>Are you sure about adjusting dynamic_offset_count here? </div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> break; default: break; @@ -162,7 +181,7 @@ VkResult anv_CreateDescriptorSetLayout( case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: anv_foreach_stage(s, binding->stageFlags) { set_layout->binding[b].stage[s].image_index = image_count[s]; - image_count[s] += binding->descriptorCount; + image_count[s] += set_layout->binding[b].descriptor_size; } break; default: @@ -171,11 +190,18 @@ VkResult anv_CreateDescriptorSetLayout( if (binding->pImmutableSamplers) { set_layout->binding[b].immutable_samplers = samplers; - samplers += binding->descriptorCount; - for (uint32_t i = 0; i < binding->descriptorCount; i++) - set_layout->binding[b].immutable_samplers[i] = - anv_sampler_from_handle(binding->pImmutableSamplers[i]); + uint32_t sampler_offset = 0; + for (uint32_t i = 0; i < binding->descriptorCount; i++) { + ANV_FROM_HANDLE(anv_sampler, sampler, binding->pImmutableSamplers[i]); + + set_layout->binding[b].immutable_samplers[i].sampler = sampler; + set_layout->binding[b].immutable_samplers[i].descriptor_index_offset = + sampler_offset; + sampler_offset += sampler->nb_planes; + } + + samplers += sampler_offset; } else { set_layout->binding[b].immutable_samplers = NULL; } @@ -250,7 +276,7 @@ VkResult anv_CreatePipelineLayout( if (set_layout->binding[b].dynamic_offset_index < 0) continue; - dynamic_offset_count += set_layout->binding[b].array_size; + dynamic_offset_count += set_layout->binding[b].descriptor_size; for (gl_shader_stage s = 0; s < MESA_SHADER_STAGES; s++) { if (set_layout->binding[b].stage[s].surface_index >= 0) layout->stage[s].has_dynamic_offsets = true; @@ -318,11 +344,24 @@ VkResult anv_CreateDescriptorPool( uint32_t buffer_count = 0; for (uint32_t i = 0; i < pCreateInfo->poolSizeCount; i++) { switch (pCreateInfo->pPoolSizes[i].type) { + case VK_DESCRIPTOR_TYPE_SAMPLER: + case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: + /* We have to account that some descriptor layouts might have + * multiplanar images (atm up to 3 through the conversion + * VK_KHR_sampler_ycbcr_conversion extension). Therefor take the + * maximum amount of space that can be occupied by a single of those + * entries. + */ + descriptor_count += 3 * pCreateInfo->pPoolSizes[i].descriptorCount; </blockquote><div> </div></div></div><div>I'm not quite sure how I expected you to solve the multi-descriptor problem but it wasn't like this. :) An alternate solution would be to keep each image_view and sampler a single descriptor in the descriptor set and expand it out to multiple descriptors as-needed in anv_nir_apply_pipeline_layout by adding a "plane" entry to anv_pipeline_binding. I'm honestly not sure which one would work out better in the end. </div></div></div></div> </blockquote></div></div><div class="gmail_extra"> </div><div class="gmail_extra">If you don't mind too badly, I'd like it if you at least took a crack at my alternate solution. There's some things that this method requires us to add to descriptor set layouts that make me a bit uncomfortable. At some point, I intend to do something of an overhaul of descriptor sets and we will probably end up going sort-of in the direction here but the current code I think would work better if we maintained one descriptor per image.</div><div class="gmail_extra"> </div><div class="gmail_extra">--Jason </div></div>