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

[Juan A. Suarez Romero]

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?



	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