On Tue, Sep 24, 2024 at 10:22 AM Zhang, Yifan <[email protected]> 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 <[email protected]> > 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 <[email protected]> > Reviewed-by: Christian König <[email protected]> > Signed-off-by: Alex Deucher <[email protected]> > > > > -----Original Message----- > From: Alex Deucher <[email protected]> > Sent: Tuesday, September 24, 2024 10:07 PM > To: Zhang, Yifan <[email protected]> > Cc: [email protected]; Yang, Philip <[email protected]>; > Kuehling, Felix <[email protected]> > Subject: Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB > limitation removed > > On Mon, Sep 23, 2024 at 4:28 AM Yifan Zhang <[email protected]> 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 <[email protected]> > > --- > > 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 > >
