Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: fix descriptor pool allocation size
+Dylan as 18.1 Release Manager Hi Dylan, Please be advised that the above patch has been reverted on master in 95bb7d82ca8abf514af2575e3b9f4babfbb034c4 and should not be included in the 18.1.9 release. Thanks, Bas On Tue, Sep 18, 2018 at 8:19 PM Samuel Pitoiset wrote: > > > > 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 > >> napisał(a): > >>> > >>> Reviewed-by: Bas Nieuwenhuizen > >>> On Fri, Sep 14, 2018 at 2:55 PM Samuel Pitoiset > >>> 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 > Signed-off-by: Samuel Pitoiset > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > >>> > >>> ___ > >>> mesa-dev mailing list > >>> mesa-dev@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-sta...@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/mesa-stable > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: fix descriptor pool allocation size
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 napisał(a): Reviewed-by: Bas Nieuwenhuizen On Fri, Sep 14, 2018 at 2:55 PM Samuel Pitoiset 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 Signed-off-by: Samuel Pitoiset --- 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@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-sta...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-stable ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: fix descriptor pool allocation size
On Tue, 2018-09-18 at 17:20 +0200, Gustaw Smolarczyk wrote: > pt., 14 wrz 2018 o 15:00 Bas Nieuwenhuizen > napisał(a): > > > > Reviewed-by: Bas Nieuwenhuizen > > On Fri, Sep 14, 2018 at 2:55 PM Samuel Pitoiset > > 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 > > > Signed-off-by: Samuel Pitoiset > > > --- > > > 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@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > ___ > > mesa-dev mailing list > > mesa-dev@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? 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-sta...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-stable ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev