radv: Increase maxDescriptorSet{Uniform, Storage}BuffersDynamic limits

Submitted by Alex Smith on March 2, 2018, 3:28 p.m.

Details

Message ID 20180302152845.20557-1-asmith@feralinteractive.com
State New
Headers show
Series "radv: Increase maxDescriptorSet{Uniform, Storage}BuffersDynamic limits" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Alex Smith March 2, 2018, 3:28 p.m.
These were set to MAX_DYNAMIC_BUFFERS / 2, which is too restrictive
since an app may have it's total usage of both uniform and storage
within MAX_DYNAMIC_BUFFERS, but exceed the limit for one of the types.

Recently the validation layers have started raising errors for when
these limits are exceeded, so these are firing for something that
actually works just fine.

Set the limit for both to MAX_DYNAMIC_BUFFERS. Not ideal because it
now allows the total across both to exceed the real limit, but we have
no way to express that limit properly.

Cc: <mesa-stable@lists.freedesktop.org>
Signed-off-by: Alex Smith <asmith@feralinteractive.com>
---
 src/amd/vulkan/radv_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 36d7a406bf..1e81ddb891 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -717,9 +717,9 @@  void radv_GetPhysicalDeviceProperties(
 		.maxPerStageResources                     = max_descriptor_set_size,
 		.maxDescriptorSetSamplers                 = max_descriptor_set_size,
 		.maxDescriptorSetUniformBuffers           = max_descriptor_set_size,
-		.maxDescriptorSetUniformBuffersDynamic    = MAX_DYNAMIC_BUFFERS / 2,
+		.maxDescriptorSetUniformBuffersDynamic    = MAX_DYNAMIC_BUFFERS,
 		.maxDescriptorSetStorageBuffers           = max_descriptor_set_size,
-		.maxDescriptorSetStorageBuffersDynamic    = MAX_DYNAMIC_BUFFERS / 2,
+		.maxDescriptorSetStorageBuffersDynamic    = MAX_DYNAMIC_BUFFERS,
 		.maxDescriptorSetSampledImages            = max_descriptor_set_size,
 		.maxDescriptorSetStorageImages            = max_descriptor_set_size,
 		.maxDescriptorSetInputAttachments         = max_descriptor_set_size,

Comments

Hi Alex,

How many do you need of either type?

- Bas

On Fri, Mar 2, 2018 at 4:28 PM, Alex Smith <asmith@feralinteractive.com> wrote:
> These were set to MAX_DYNAMIC_BUFFERS / 2, which is too restrictive
> since an app may have it's total usage of both uniform and storage
> within MAX_DYNAMIC_BUFFERS, but exceed the limit for one of the types.
>
> Recently the validation layers have started raising errors for when
> these limits are exceeded, so these are firing for something that
> actually works just fine.
>
> Set the limit for both to MAX_DYNAMIC_BUFFERS. Not ideal because it
> now allows the total across both to exceed the real limit, but we have
> no way to express that limit properly.
>
> Cc: <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Alex Smith <asmith@feralinteractive.com>
> ---
>  src/amd/vulkan/radv_device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 36d7a406bf..1e81ddb891 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -717,9 +717,9 @@ void radv_GetPhysicalDeviceProperties(
>                 .maxPerStageResources                     = max_descriptor_set_size,
>                 .maxDescriptorSetSamplers                 = max_descriptor_set_size,
>                 .maxDescriptorSetUniformBuffers           = max_descriptor_set_size,
> -               .maxDescriptorSetUniformBuffersDynamic    = MAX_DYNAMIC_BUFFERS / 2,
> +               .maxDescriptorSetUniformBuffersDynamic    = MAX_DYNAMIC_BUFFERS,
>                 .maxDescriptorSetStorageBuffers           = max_descriptor_set_size,
> -               .maxDescriptorSetStorageBuffersDynamic    = MAX_DYNAMIC_BUFFERS / 2,
> +               .maxDescriptorSetStorageBuffersDynamic    = MAX_DYNAMIC_BUFFERS,
>                 .maxDescriptorSetSampledImages            = max_descriptor_set_size,
>                 .maxDescriptorSetStorageImages            = max_descriptor_set_size,
>                 .maxDescriptorSetInputAttachments         = max_descriptor_set_size,
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
I just checked what Rise of the Tomb Raider is using. Maximum it hits for
uniform buffers is 15, and 6 for storage buffers. The highest combined
total is 15.

Alex

On 2 March 2018 at 20:11, Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> wrote:

> Hi Alex,
>
> How many do you need of either type?
>
> - Bas
>
> On Fri, Mar 2, 2018 at 4:28 PM, Alex Smith <asmith@feralinteractive.com>
> wrote:
> > These were set to MAX_DYNAMIC_BUFFERS / 2, which is too restrictive
> > since an app may have it's total usage of both uniform and storage
> > within MAX_DYNAMIC_BUFFERS, but exceed the limit for one of the types.
> >
> > Recently the validation layers have started raising errors for when
> > these limits are exceeded, so these are firing for something that
> > actually works just fine.
> >
> > Set the limit for both to MAX_DYNAMIC_BUFFERS. Not ideal because it
> > now allows the total across both to exceed the real limit, but we have
> > no way to express that limit properly.
> >
> > Cc: <mesa-stable@lists.freedesktop.org>
> > Signed-off-by: Alex Smith <asmith@feralinteractive.com>
> > ---
> >  src/amd/vulkan/radv_device.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> > index 36d7a406bf..1e81ddb891 100644
> > --- a/src/amd/vulkan/radv_device.c
> > +++ b/src/amd/vulkan/radv_device.c
> > @@ -717,9 +717,9 @@ void radv_GetPhysicalDeviceProperties(
> >                 .maxPerStageResources                     =
> max_descriptor_set_size,
> >                 .maxDescriptorSetSamplers                 =
> max_descriptor_set_size,
> >                 .maxDescriptorSetUniformBuffers           =
> max_descriptor_set_size,
> > -               .maxDescriptorSetUniformBuffersDynamic    =
> MAX_DYNAMIC_BUFFERS / 2,
> > +               .maxDescriptorSetUniformBuffersDynamic    =
> MAX_DYNAMIC_BUFFERS,
> >                 .maxDescriptorSetStorageBuffers           =
> max_descriptor_set_size,
> > -               .maxDescriptorSetStorageBuffersDynamic    =
> MAX_DYNAMIC_BUFFERS / 2,
> > +               .maxDescriptorSetStorageBuffersDynamic    =
> MAX_DYNAMIC_BUFFERS,
> >                 .maxDescriptorSetSampledImages            =
> max_descriptor_set_size,
> >                 .maxDescriptorSetStorageImages            =
> max_descriptor_set_size,
> >                 .maxDescriptorSetInputAttachments         =
> max_descriptor_set_size,
> > --
> > 2.14.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Ping.

Maybe it'd be better to just increase MAX_DYNAMIC_BUFFERS? I can't see any
side effects of that other than increasing the size of radv_cmd_buffer?

Alex

On 5 March 2018 at 09:59, Alex Smith <asmith@feralinteractive.com> wrote:

> I just checked what Rise of the Tomb Raider is using. Maximum it hits for
> uniform buffers is 15, and 6 for storage buffers. The highest combined
> total is 15.
>
> Alex
>
> On 2 March 2018 at 20:11, Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> wrote:
>
>> Hi Alex,
>>
>> How many do you need of either type?
>>
>> - Bas
>>
>> On Fri, Mar 2, 2018 at 4:28 PM, Alex Smith <asmith@feralinteractive.com>
>> wrote:
>> > These were set to MAX_DYNAMIC_BUFFERS / 2, which is too restrictive
>> > since an app may have it's total usage of both uniform and storage
>> > within MAX_DYNAMIC_BUFFERS, but exceed the limit for one of the types.
>> >
>> > Recently the validation layers have started raising errors for when
>> > these limits are exceeded, so these are firing for something that
>> > actually works just fine.
>> >
>> > Set the limit for both to MAX_DYNAMIC_BUFFERS. Not ideal because it
>> > now allows the total across both to exceed the real limit, but we have
>> > no way to express that limit properly.
>> >
>> > Cc: <mesa-stable@lists.freedesktop.org>
>> > Signed-off-by: Alex Smith <asmith@feralinteractive.com>
>> > ---
>> >  src/amd/vulkan/radv_device.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
>> > index 36d7a406bf..1e81ddb891 100644
>> > --- a/src/amd/vulkan/radv_device.c
>> > +++ b/src/amd/vulkan/radv_device.c
>> > @@ -717,9 +717,9 @@ void radv_GetPhysicalDeviceProperties(
>> >                 .maxPerStageResources                     =
>> max_descriptor_set_size,
>> >                 .maxDescriptorSetSamplers                 =
>> max_descriptor_set_size,
>> >                 .maxDescriptorSetUniformBuffers           =
>> max_descriptor_set_size,
>> > -               .maxDescriptorSetUniformBuffersDynamic    =
>> MAX_DYNAMIC_BUFFERS / 2,
>> > +               .maxDescriptorSetUniformBuffersDynamic    =
>> MAX_DYNAMIC_BUFFERS,
>> >                 .maxDescriptorSetStorageBuffers           =
>> max_descriptor_set_size,
>> > -               .maxDescriptorSetStorageBuffersDynamic    =
>> MAX_DYNAMIC_BUFFERS / 2,
>> > +               .maxDescriptorSetStorageBuffersDynamic    =
>> MAX_DYNAMIC_BUFFERS,
>> >                 .maxDescriptorSetSampledImages            =
>> max_descriptor_set_size,
>> >                 .maxDescriptorSetStorageImages            =
>> max_descriptor_set_size,
>> >                 .maxDescriptorSetInputAttachments         =
>> max_descriptor_set_size,
>> > --
>> > 2.14.3
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
Sorry, for the delay, I just sent a patch that makes the limits 16 + 8.

Going forward it might be an idea to just make this a dynamically
sized array and seriously increase the limits.

On Fri, Mar 9, 2018 at 5:15 PM, Alex Smith <asmith@feralinteractive.com> wrote:
> Ping.
>
> Maybe it'd be better to just increase MAX_DYNAMIC_BUFFERS? I can't see any
> side effects of that other than increasing the size of radv_cmd_buffer?
>
> Alex
>
> On 5 March 2018 at 09:59, Alex Smith <asmith@feralinteractive.com> wrote:
>>
>> I just checked what Rise of the Tomb Raider is using. Maximum it hits for
>> uniform buffers is 15, and 6 for storage buffers. The highest combined total
>> is 15.
>>
>> Alex
>>
>> On 2 March 2018 at 20:11, Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> wrote:
>>>
>>> Hi Alex,
>>>
>>> How many do you need of either type?
>>>
>>> - Bas
>>>
>>> On Fri, Mar 2, 2018 at 4:28 PM, Alex Smith <asmith@feralinteractive.com>
>>> wrote:
>>> > These were set to MAX_DYNAMIC_BUFFERS / 2, which is too restrictive
>>> > since an app may have it's total usage of both uniform and storage
>>> > within MAX_DYNAMIC_BUFFERS, but exceed the limit for one of the types.
>>> >
>>> > Recently the validation layers have started raising errors for when
>>> > these limits are exceeded, so these are firing for something that
>>> > actually works just fine.
>>> >
>>> > Set the limit for both to MAX_DYNAMIC_BUFFERS. Not ideal because it
>>> > now allows the total across both to exceed the real limit, but we have
>>> > no way to express that limit properly.
>>> >
>>> > Cc: <mesa-stable@lists.freedesktop.org>
>>> > Signed-off-by: Alex Smith <asmith@feralinteractive.com>
>>> > ---
>>> >  src/amd/vulkan/radv_device.c | 4 ++--
>>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/src/amd/vulkan/radv_device.c
>>> > b/src/amd/vulkan/radv_device.c
>>> > index 36d7a406bf..1e81ddb891 100644
>>> > --- a/src/amd/vulkan/radv_device.c
>>> > +++ b/src/amd/vulkan/radv_device.c
>>> > @@ -717,9 +717,9 @@ void radv_GetPhysicalDeviceProperties(
>>> >                 .maxPerStageResources                     =
>>> > max_descriptor_set_size,
>>> >                 .maxDescriptorSetSamplers                 =
>>> > max_descriptor_set_size,
>>> >                 .maxDescriptorSetUniformBuffers           =
>>> > max_descriptor_set_size,
>>> > -               .maxDescriptorSetUniformBuffersDynamic    =
>>> > MAX_DYNAMIC_BUFFERS / 2,
>>> > +               .maxDescriptorSetUniformBuffersDynamic    =
>>> > MAX_DYNAMIC_BUFFERS,
>>> >                 .maxDescriptorSetStorageBuffers           =
>>> > max_descriptor_set_size,
>>> > -               .maxDescriptorSetStorageBuffersDynamic    =
>>> > MAX_DYNAMIC_BUFFERS / 2,
>>> > +               .maxDescriptorSetStorageBuffersDynamic    =
>>> > MAX_DYNAMIC_BUFFERS,
>>> >                 .maxDescriptorSetSampledImages            =
>>> > max_descriptor_set_size,
>>> >                 .maxDescriptorSetStorageImages            =
>>> > max_descriptor_set_size,
>>> >                 .maxDescriptorSetInputAttachments         =
>>> > max_descriptor_set_size,
>>> > --
>>> > 2.14.3
>>> >
>>> > _______________________________________________
>>> > mesa-dev mailing list
>>> > mesa-dev@lists.freedesktop.org
>>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>