On Wed, Apr 12, 2023 at 3:01 PM Ernst Sjöstrand <ernstp@xxxxxxxxx> wrote: > > Sorry if I read this completely wrong, but the other patch says "radv currently has > a limit of 192 which seems to work for most gfx submissions, but > is way too high for e.g. compute or sdma.", while you return 192 for AMDGPU_RING_TYPE_COMPUTE > here still? Yeah this totally changes the limits (to simplify), relying on the code in amdgpu_ring_init to make the ring a bit larger. If I go the WARN_ON route I have to lower them or go to the rings and make them larger there. > > Regards > //Ernst > > Den ons 12 apr. 2023 kl 14:20 skrev Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>: >> >> And ensure each ring supports that many submissions. This makes >> sure that we don't get surprises after the submission has been >> scheduled where the ring allocation actually gets rejected. >> >> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2498 >> Signed-off-by: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 25 ++++++++++++++++++++++++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 + >> 3 files changed, 29 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 7af3041ccd0e..8362738974c8 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -110,6 +110,9 @@ static int amdgpu_cs_p1_ib(struct amdgpu_cs_parser *p, >> if (r < 0) >> return r; >> >> + if (num_ibs[r] >= amdgpu_ring_max_ibs(chunk_ib->ip_type)) >> + return -EINVAL; >> + >> ++(num_ibs[r]); >> p->gang_leader_idx = r; >> return 0; >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c >> index dc474b809604..abd70d2f26f6 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c >> @@ -49,6 +49,25 @@ >> * them until the pointers are equal again. >> */ >> >> +/** >> + * amdgpu_ring_max_ibs - Return max IBs that fit in a single submission. >> + * >> + * @type: ring type for which to return the limit. >> + */ >> +unsigned int amdgpu_ring_max_ibs(enum amdgpu_ring_type type) >> +{ >> + switch (type) { >> + case AMDGPU_RING_TYPE_GFX: >> + case AMDGPU_RING_TYPE_COMPUTE: >> + /* gfx/compute are often used more extensively and radv >> + * has historically assumed the limit was 192. >> + */ >> + return 192; >> + default: >> + return 50; >> + } >> +} >> + >> /** >> * amdgpu_ring_alloc - allocate space on the ring buffer >> * >> @@ -182,6 +201,7 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, >> int sched_hw_submission = amdgpu_sched_hw_submission; >> u32 *num_sched; >> u32 hw_ip; >> + unsigned int max_ibs_dw; >> >> /* Set the hw submission limit higher for KIQ because >> * it's used for a number of gfx/compute tasks by both >> @@ -290,6 +310,11 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, >> return r; >> } >> >> + max_ibs_dw = ring->funcs->emit_frame_size + >> + amdgpu_ring_max_ibs(ring->funcs->type) * ring->funcs->emit_ib_size; >> + max_ibs_dw = (max_ibs_dw + ring->funcs->align_mask) & ~ring->funcs->align_mask; >> + max_dw = max(max_dw, max_ibs_dw); >> + >> ring->ring_size = roundup_pow_of_two(max_dw * 4 * sched_hw_submission); >> >> ring->buf_mask = (ring->ring_size / 4) - 1; >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> index 3989e755a5b4..e6e672727529 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> @@ -317,6 +317,7 @@ struct amdgpu_ring { >> #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o)) >> #define amdgpu_ring_preempt_ib(r) (r)->funcs->preempt_ib(r) >> >> +unsigned int amdgpu_ring_max_ibs(enum amdgpu_ring_type type); >> int amdgpu_ring_alloc(struct amdgpu_ring *ring, unsigned ndw); >> void amdgpu_ring_ib_begin(struct amdgpu_ring *ring); >> void amdgpu_ring_ib_end(struct amdgpu_ring *ring); >> -- >> 2.40.0 >>
