<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Tue, 18 Sep 2018, 17:21 Gustaw Smolarczyk, <<a href="mailto:wielkiegie@gmail.com">wielkiegie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">pt., 14 wrz 2018 o 15:00 Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank" rel="noreferrer">bas@basnieuwenhuizen.nl</a>> napisał(a):<br>
><br>
> Reviewed-by: Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank" rel="noreferrer">bas@basnieuwenhuizen.nl</a>><br>
> On Fri, Sep 14, 2018 at 2:55 PM Samuel Pitoiset<br>
> <<a href="mailto:samuel.pitoiset@gmail.com" target="_blank" rel="noreferrer">samuel.pitoiset@gmail.com</a>> wrote:<br>
> ><br>
> > The size has to be multiplied by the number of sets.<br>
> ><br>
> > This gets rid of the OUT_OF_POOL_KHR error and fixes<br>
> > the Tangrams demo.<br>
> ><br>
> > CC: 18.1 18.2 <<a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank" rel="noreferrer">mesa-stable@lists.freedesktop.org</a>><br>
> > Signed-off-by: Samuel Pitoiset <<a href="mailto:samuel.pitoiset@gmail.com" target="_blank" rel="noreferrer">samuel.pitoiset@gmail.com</a>><br>
> > ---<br>
> > src/amd/vulkan/radv_descriptor_set.c | 3 ++-<br>
> > 1 file changed, 2 insertions(+), 1 deletion(-)<br>
> ><br>
> > diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_descriptor_set.c<br>
> > index c4341f6ac5..49d0811bb0 100644<br>
> > --- a/src/amd/vulkan/radv_descriptor_set.c<br>
> > +++ b/src/amd/vulkan/radv_descriptor_set.c<br>
> > @@ -569,9 +569,10 @@ VkResult radv_CreateDescriptorPool(<br>
> > }<br>
> ><br>
> > if (!(pCreateInfo->flags & VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT)) {<br>
> > - uint64_t host_size = pCreateInfo->maxSets * sizeof(struct radv_descriptor_set);<br>
> > + uint64_t host_size = sizeof(struct radv_descriptor_set);<br>
> > host_size += sizeof(struct radeon_winsys_bo*) * bo_count;<br>
> > host_size += sizeof(struct radv_descriptor_range) * range_count;<br>
> > + host_size *= pCreateInfo->maxSets;<br>
> > size += host_size;<br>
> > } else {<br>
> > size += sizeof(struct radv_descriptor_pool_entry) * pCreateInfo->maxSets;<br>
> > --<br>
> > 2.19.0<br>
> ><br>
> > _______________________________________________<br>
> > mesa-dev mailing list<br>
> > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank" rel="noreferrer">mesa-dev@lists.freedesktop.org</a><br>
> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank" rel="noreferrer">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
<br>
I don't think this change is correct.<br>
<br>
>From the vkAllocateDescriptorSets documentation [1]:<br>
<br>
<br>
If a call to vkAllocateDescriptorSets would cause the total number of<br>
descriptor sets allocated from the pool to exceed the value of<br>
VkDescriptorPoolCreateInfo::maxSets used to create<br>
pAllocateInfo→descriptorPool, then the allocation may fail due to lack<br>
of space in the descriptor pool. Similarly, the allocation may fail<br>
due to lack of space if the call to vkAllocateDescriptorSets would<br>
cause the number of any given descriptor type to exceed the sum of all<br>
the descriptorCount members of each element of<br>
VkDescriptorPoolCreateInfo::pPoolSizes with a member equal to that<br>
type.<br>
<br>
<br>
What it implies (I think), is that VkDescriptorPoolCreateInfo::maxSets<br>
and descriptorCount of each VkDescriptorPoolCreateInfo::pPoolSizes are<br>
treated separately. I don't think you should multiply one by another.<br>
Each pool should be able to allocate at least maxSets sets and<br>
descriptorCount descriptors.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Totally agree. Was thinking about the descriptors set allocation logic instead of the pool logic when reviewing this ... Apologies.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Please, correct me if I'm wrong.<br>
<br>
Regards,<br>
<br>
Gustaw Smolarczyk<br>
<br>
[1] <a href="https://www.khronos.org/registry/vulkan/specs/1.1-extensions/man/html/vkAllocateDescriptorSets.html" rel="noreferrer noreferrer" target="_blank">https://www.khronos.org/registry/vulkan/specs/1.1-extensions/man/html/vkAllocateDescriptorSets.html</a><br>
</blockquote></div></div></div>