[Mesa-dev] [PATCH 2/2] radv/winsys: Fix radv_amdgpu_cs_grow min_size argument. (v2)

[Gustaw Smolarczyk]

2016-10-10 22:04 GMT+02:00 Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>:
> Hi Gustaw,
>
> The patch is
> reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>

Thanks, please push it since I don't have commit access.

>
> What needs to be done too, is checking if the resulting IB becomes too
> large in the SI case, and handling that gracefully. I don't care if
> that happens with this patch, or if someone writes a follow up patch
> though

By handling gracefully do you mean setting cs->failed? Or should we
try to work around the limitation instead?

Also, I am not sure what is too large for SI. From a quick look at
cs_submit_sysmem I see that if there was a cs with cdw > 0xffff8 we
would get assertion failure since cnt would be 0 (1Mi dwords seems to
be the limitation here). Should we try to divide such a cs or is that
not really possible without INDIRECT_BUFFER packet? I imagine that
just submitting a few cs's one after another might have other
consequences that are not really simple to reason about on this level
and we would need to divide them on a higher level.

I am very new to the radeon driver development so I might be asking
stupid questions. If so, I am sorry for taking your time.

Regards,
Gustaw

>
> Yours sincerely,
> Bas Nieuwenhuizen
>
>
> On Oct 10, 2016 1:22 PM, "Gustaw Smolarczyk" <wielkiegie at gmail.com> wrote:
>>
>> Ping.
>>
>> 2016-10-06 19:50 GMT+02:00 Gustaw Smolarczyk <wielkiegie at gmail.com>:
>> > It's supposed to be how much at least we want to grow the cs, not the
>> > minimum size of the cs after growth.
>> >
>> > v2: Unbreak use_ib_bos.
>> >     Don't mask the ib_size when !use_ib_bos, since it's not needed.
>> > ---
>> >  src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 11 +++++++----
>> >  1 file changed, 7 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
>> > index dedc778..c07c092 100644
>> > --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
>> > +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
>> > @@ -178,10 +178,6 @@ radv_amdgpu_cs_create(struct radeon_winsys *ws,
>> >  static void radv_amdgpu_cs_grow(struct radeon_winsys_cs *_cs, size_t min_size)
>> >  {
>> >         struct radv_amdgpu_cs *cs = radv_amdgpu_cs(_cs);
>> > -       uint64_t ib_size = MAX2(min_size * 4 + 16, cs->base.max_dw * 4 * 2);
>> > -
>> > -       /* max that fits in the chain size field. */
>> > -       ib_size = MIN2(ib_size, 0xfffff);
>> >
>> >         if (cs->failed) {
>> >                 cs->base.cdw = 0;
>> > @@ -189,6 +185,8 @@ static void radv_amdgpu_cs_grow(struct radeon_winsys_cs *_cs, size_t min_size)
>> >         }
>> >
>> >         if (!cs->ws->use_ib_bos) {
>> > +               uint64_t ib_size = MAX2((cs->base.cdw + min_size) * 4 + 16,
>> > +                                       cs->base.max_dw * 4 * 2);
>> >                 uint32_t *new_buf = realloc(cs->base.buf, ib_size);
>> >                 if (new_buf) {
>> >                         cs->base.buf = new_buf;
>> > @@ -200,6 +198,11 @@ static void radv_amdgpu_cs_grow(struct radeon_winsys_cs *_cs, size_t min_size)
>> >                 return;
>> >         }
>> >
>> > +       uint64_t ib_size = MAX2(min_size * 4 + 16, cs->base.max_dw * 4 * 2);
>> > +
>> > +       /* max that fits in the chain size field. */
>> > +       ib_size = MIN2(ib_size, 0xfffff);
>> > +
>> >         while (!cs->base.cdw || (cs->base.cdw & 7) != 4)
>> >                 cs->base.buf[cs->base.cdw++] = 0xffff1000;
>> >
>> > --
>> > 2.10.0
>> >
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev