[PATCH v3 1/2] drm/amdgpu: Add a max ibs per submission limit.

[Bas Nieuwenhuizen]

On Wed, Apr 12, 2023 at 3:01 PM Ernst Sjöstrand <ernstp at gmail.com> 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 at basnieuwenhuizen.nl>:
>>
>> 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 at basnieuwenhuizen.nl>
>> ---
>>  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
>>