On Mon, Oct 10, 2016 at 9:19 PM, Gustaw Smolarczyk <[email protected]> wrote: > 2016-10-10 22:04 GMT+02:00 Bas Nieuwenhuizen <[email protected]>: >> Hi Gustaw, >> >> The patch is >> reviewed-by: Bas Nieuwenhuizen <[email protected]> > > 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. That limit is correct (though don't forget to multiply by 4 for words vs bytes). We indeed want to set failed and also set cdw to 0, so the driver does not do out of bounds accesses. (basically the same path as if new_buf is NULL) Dividing the CS is going to be hard, as we need to re-emit all state at division, and radv isn't set up to handle that currently. Also IIRC the 1 million dwords is more than a factor 10 larger than the largest I've seen a GL app use per frame, so it probably isn't that useful either. > > 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" <[email protected]> wrote: >>> >>> Ping. >>> >>> 2016-10-06 19:50 GMT+02:00 Gustaw Smolarczyk <[email protected]>: >>> > 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 >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
