[PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed

[Alex Deucher]

On Tue, Sep 24, 2024 at 10:22 AM Zhang, Yifan <Yifan1.Zhang at amd.com> wrote:
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> 2GB limitation in VRAM allocation is removed in below patch. My patch is a follow up refine for this. The remaing_size calculation was to address the 2GB limitation in contiguous VRAM allocation, and no longer needed after the limitation is removed.
>

Thanks.  Would be good to add a reference to this commit in the commit
message so it's clear what you are talking about and also provide a
bit more of a description about why this can be cleaned up (like you
did above).  One other comment below as well.


> commit b2dba064c9bdd18c7dd39066d25453af28451dbf
> Author: Philip Yang <Philip.Yang at amd.com>
> Date:   Fri Apr 19 16:27:00 2024 -0400
>
>     drm/amdgpu: Handle sg size limit for contiguous allocation
>
>     Define macro AMDGPU_MAX_SG_SEGMENT_SIZE 2GB, because struct scatterlist
>     length is unsigned int, and some users of it cast to a signed int, so
>     every segment of sg table is limited to size 2GB maximum.
>
>     For contiguous VRAM allocation, don't limit the max buddy block size in
>     order to get contiguous VRAM memory. To workaround the sg table segment
>     size limit, allocate multiple segments if contiguous size is bigger than
>     AMDGPU_MAX_SG_SEGMENT_SIZE.
>
>     Signed-off-by: Philip Yang <Philip.Yang at amd.com>
>     Reviewed-by: Christian König <christian.koenig at amd.com>
>     Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>
>
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Tuesday, September 24, 2024 10:07 PM
> To: Zhang, Yifan <Yifan1.Zhang at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; Yang, Philip <Philip.Yang at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>
> Subject: Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed
>
> On Mon, Sep 23, 2024 at 4:28 AM Yifan Zhang <yifan1.zhang at amd.com> wrote:
> >
> > Make vram alloc loop simpler after 2GB limitation removed.
>
> Can you provide more context?  What 2GB limitation are you referring to?
>
> Alex
>
> >
> > Signed-off-by: Yifan Zhang <yifan1.zhang at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 15 +++++----------
> >  1 file changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > index 7d26a962f811..3d129fd61fa7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > @@ -455,7 +455,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> >         struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> >         u64 vis_usage = 0, max_bytes, min_block_size;
> >         struct amdgpu_vram_mgr_resource *vres;
> > -       u64 size, remaining_size, lpfn, fpfn;
> > +       u64 size, lpfn, fpfn;
> >         unsigned int adjust_dcc_size = 0;
> >         struct drm_buddy *mm = &mgr->mm;
> >         struct drm_buddy_block *block; @@ -516,25 +516,23 @@ static
> > int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> >             adev->gmc.gmc_funcs->get_dcc_alignment)
> >                 adjust_dcc_size = amdgpu_gmc_get_dcc_alignment(adev);
> >
> > -       remaining_size = (u64)vres->base.size;
> > +       size = (u64)vres->base.size;
> >         if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS && adjust_dcc_size) {
> >                 unsigned int dcc_size;
> >
> >                 dcc_size = roundup_pow_of_two(vres->base.size + adjust_dcc_size);
> > -               remaining_size = (u64)dcc_size;
> > +               size = (u64)dcc_size;
> >
> >                 vres->flags |= DRM_BUDDY_TRIM_DISABLE;
> >         }
> >
> >         mutex_lock(&mgr->lock);
> > -       while (remaining_size) {
> > +       while (true) {

I think you can also drop this while loop now too.

Alex

> >                 if (tbo->page_alignment)
> >                         min_block_size = (u64)tbo->page_alignment << PAGE_SHIFT;
> >                 else
> >                         min_block_size = mgr->default_page_size;
> >
> > -               size = remaining_size;
> > -
> >                 if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS && adjust_dcc_size)
> >                         min_block_size = size;
> >                 else if ((size >= (u64)pages_per_block << PAGE_SHIFT)
> > && @@ -562,10 +560,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> >                 if (unlikely(r))
> >                         goto error_free_blocks;
> >
> > -               if (size > remaining_size)
> > -                       remaining_size = 0;
> > -               else
> > -                       remaining_size -= size;
> > +               break;
> >         }
> >         mutex_unlock(&mgr->lock);
> >
> > --
> > 2.43.0
> >