On Fri, Sep 15, 2017 at 3:54 PM, Jason Ekstrand <ja...@jlekstrand.net>
wrote:
>
> On Fri, Sep 15, 2017 at 7:11 AM, Lionel Landwerlin <
> lionel.g.landwer...@intel.com> wrote:
>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>> ---
>> 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].bind
>> ing);
>> - if (pCreateInfo->pBindings[j].pImmutableSamplers)
>> - immutable_sampler_count += pCreateInfo->pBindings[j].desc
>> riptorCount;
>> + 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;
>>
>
> Are you sure about adjusting dynamic_offset_count here?
>
>
>> 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].des
>> criptorCount;
>>
>
> 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.
>
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.
--Jason
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
