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

[Bas Nieuwenhuizen]

On Tue, 18 Sep 2018, 17:21 Gustaw Smolarczyk, <wielkiegie at gmail.com> 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.
>
> 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.
>

Totally agree. Was thinking about the descriptors set allocation logic
instead of the pool logic when reviewing this ... Apologies.

>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180918/80ef3c9d/attachment.html>