[Mesa-dev] [Mesa-stable] [PATCH] radv: fix descriptor pool allocation size

[Samuel Pitoiset]

On 9/18/18 8:07 PM, Juan A. Suarez Romero wrote:
> On Tue, 2018-09-18 at 17:20 +0200, Gustaw Smolarczyk wrote:
>> pt., 14 wrz 2018 o 15:00 Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl> napisał(a):
>>>
>>> Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>>> On Fri, Sep 14, 2018 at 2:55 PM Samuel Pitoiset
>>> <samuel.pitoiset at gmail.com> wrote:
>>>>
>>>> The size has to be multiplied by the number of sets.
>>>>
>>>> This gets rid of the OUT_OF_POOL_KHR error and fixes
>>>> the Tangrams demo.
>>>>
>>>> CC: 18.1 18.2 <mesa-stable at lists.freedesktop.org>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/amd/vulkan/radv_descriptor_set.c | 3 ++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_descriptor_set.c
>>>> index c4341f6ac5..49d0811bb0 100644
>>>> --- a/src/amd/vulkan/radv_descriptor_set.c
>>>> +++ b/src/amd/vulkan/radv_descriptor_set.c
>>>> @@ -569,9 +569,10 @@ VkResult radv_CreateDescriptorPool(
>>>>          }
>>>>
>>>>          if (!(pCreateInfo->flags & VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT)) {
>>>> -               uint64_t host_size = pCreateInfo->maxSets * sizeof(struct radv_descriptor_set);
>>>> +               uint64_t host_size = sizeof(struct radv_descriptor_set);
>>>>                  host_size += sizeof(struct radeon_winsys_bo*) * bo_count;
>>>>                  host_size += sizeof(struct radv_descriptor_range) * range_count;
>>>> +               host_size *= pCreateInfo->maxSets;
>>>>                  size += host_size;
>>>>          } else {
>>>>                  size += sizeof(struct radv_descriptor_pool_entry) * pCreateInfo->maxSets;
>>>> --
>>>> 2.19.0
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> I don't think this change is correct.
>>
> 
> This patch was included for the next 18.2.1 release.
> 
> I understand this must be removed from the queue before the release, right? Or
> is there another  patch to apply over or replace this?

You can remove it from the queue and I will revert it on master.
Thanks.

> 
> 
> 
> 	J.A.
> 
>>  From the vkAllocateDescriptorSets documentation [1]:
>>
>>
>> If a call to vkAllocateDescriptorSets would cause the total number of
>> descriptor sets allocated from the pool to exceed the value of
>> VkDescriptorPoolCreateInfo::maxSets used to create
>> pAllocateInfo→descriptorPool, then the allocation may fail due to lack
>> of space in the descriptor pool. Similarly, the allocation may fail
>> due to lack of space if the call to vkAllocateDescriptorSets would
>> cause the number of any given descriptor type to exceed the sum of all
>> the descriptorCount members of each element of
>> VkDescriptorPoolCreateInfo::pPoolSizes with a member equal to that
>> type.
>>
>>
>> What it implies (I think), is that VkDescriptorPoolCreateInfo::maxSets
>> and descriptorCount of each VkDescriptorPoolCreateInfo::pPoolSizes are
>> treated separately. I don't think you should multiply one by another.
>> Each pool should be able to allocate at least maxSets sets and
>> descriptorCount descriptors.
>>
>> Please, correct me if I'm wrong.
>>
>> Regards,
>>
>> Gustaw Smolarczyk
>>
>> [1] https://www.khronos.org/registry/vulkan/specs/1.1-extensions/man/html/vkAllocateDescriptorSets.html
>> _______________________________________________
>> mesa-stable mailing list
>> mesa-stable at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>