* [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed @ 2024-09-23 8:19 Yifan Zhang 2024-09-24 14:06 ` Alex Deucher 2024-09-27 7:13 ` Christian König 0 siblings, 2 replies; 7+ messages in thread From: Yifan Zhang @ 2024-09-23 8:19 UTC (permalink / raw) To: amd-gfx; +Cc: Philip.Yang, Felix.Kuehling, Yifan Zhang Make vram alloc loop simpler after 2GB limitation removed. Signed-off-by: Yifan Zhang <yifan1.zhang@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) { 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed 2024-09-23 8:19 [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed Yifan Zhang @ 2024-09-24 14:06 ` Alex Deucher 2024-09-24 14:22 ` Zhang, Yifan 2024-09-27 7:13 ` Christian König 1 sibling, 1 reply; 7+ messages in thread From: Alex Deucher @ 2024-09-24 14:06 UTC (permalink / raw) To: Yifan Zhang; +Cc: amd-gfx, Philip.Yang, Felix.Kuehling On Mon, Sep 23, 2024 at 4:28 AM Yifan Zhang <yifan1.zhang@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@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) { > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed 2024-09-24 14:06 ` Alex Deucher @ 2024-09-24 14:22 ` Zhang, Yifan 2024-09-25 17:16 ` Alex Deucher 0 siblings, 1 reply; 7+ messages in thread From: Zhang, Yifan @ 2024-09-24 14:22 UTC (permalink / raw) To: Alex Deucher; +Cc: amd-gfx@lists.freedesktop.org, Yang, Philip, Kuehling, Felix [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. commit b2dba064c9bdd18c7dd39066d25453af28451dbf Author: Philip Yang <Philip.Yang@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@amd.com> Reviewed-by: Christian König <christian.koenig@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> -----Original Message----- From: Alex Deucher <alexdeucher@gmail.com> Sent: Tuesday, September 24, 2024 10:07 PM To: Zhang, Yifan <Yifan1.Zhang@amd.com> Cc: amd-gfx@lists.freedesktop.org; Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix <Felix.Kuehling@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@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@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) { > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed 2024-09-24 14:22 ` Zhang, Yifan @ 2024-09-25 17:16 ` Alex Deucher 2024-09-26 14:45 ` Zhang, Yifan 0 siblings, 1 reply; 7+ messages in thread From: Alex Deucher @ 2024-09-25 17:16 UTC (permalink / raw) To: Zhang, Yifan; +Cc: amd-gfx@lists.freedesktop.org, Yang, Philip, Kuehling, Felix On Tue, Sep 24, 2024 at 10:22 AM Zhang, Yifan <Yifan1.Zhang@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@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@amd.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > > > > -----Original Message----- > From: Alex Deucher <alexdeucher@gmail.com> > Sent: Tuesday, September 24, 2024 10:07 PM > To: Zhang, Yifan <Yifan1.Zhang@amd.com> > Cc: amd-gfx@lists.freedesktop.org; Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix <Felix.Kuehling@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@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@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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed 2024-09-25 17:16 ` Alex Deucher @ 2024-09-26 14:45 ` Zhang, Yifan 2024-09-26 15:21 ` Alex Deucher 0 siblings, 1 reply; 7+ messages in thread From: Zhang, Yifan @ 2024-09-26 14:45 UTC (permalink / raw) To: Alex Deucher; +Cc: amd-gfx@lists.freedesktop.org, Yang, Philip, Kuehling, Felix [AMD Official Use Only - AMD Internal Distribution Only] -----Original Message----- From: Alex Deucher <alexdeucher@gmail.com> Sent: Thursday, September 26, 2024 1:17 AM To: Zhang, Yifan <Yifan1.Zhang@amd.com> Cc: amd-gfx@lists.freedesktop.org; Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com> Subject: Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed On Tue, Sep 24, 2024 at 10:22 AM Zhang, Yifan <Yifan1.Zhang@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. Sure, I will send patch V2 to change commit message. > commit b2dba064c9bdd18c7dd39066d25453af28451dbf > Author: Philip Yang <Philip.Yang@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@amd.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > > > > -----Original Message----- > From: Alex Deucher <alexdeucher@gmail.com> > Sent: Tuesday, September 24, 2024 10:07 PM > To: Zhang, Yifan <Yifan1.Zhang@amd.com> > Cc: amd-gfx@lists.freedesktop.org; Yang, Philip <Philip.Yang@amd.com>; > Kuehling, Felix <Felix.Kuehling@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@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@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 There is still a "continue" path left in the loop, I think the logic may be broken here if loop dropped. r = drm_buddy_alloc_blocks(mm, fpfn, lpfn, size, min_block_size, &vres->blocks, vres->flags); if (unlikely(r == -ENOSPC) && pages_per_block == ~0ul && !(place->flags & TTM_PL_FLAG_CONTIGUOUS)) { vres->flags &= ~DRM_BUDDY_CONTIGUOUS_ALLOCATION; pages_per_block = max_t(u32, 2UL << (20UL - PAGE_SHIFT), tbo->page_alignment); continue; /* continue in the loop */ } > > 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed 2024-09-26 14:45 ` Zhang, Yifan @ 2024-09-26 15:21 ` Alex Deucher 0 siblings, 0 replies; 7+ messages in thread From: Alex Deucher @ 2024-09-26 15:21 UTC (permalink / raw) To: Zhang, Yifan; +Cc: amd-gfx@lists.freedesktop.org, Yang, Philip, Kuehling, Felix On Thu, Sep 26, 2024 at 10:45 AM Zhang, Yifan <Yifan1.Zhang@amd.com> wrote: > > [AMD Official Use Only - AMD Internal Distribution Only] > > -----Original Message----- > From: Alex Deucher <alexdeucher@gmail.com> > Sent: Thursday, September 26, 2024 1:17 AM > To: Zhang, Yifan <Yifan1.Zhang@amd.com> > Cc: amd-gfx@lists.freedesktop.org; Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com> > Subject: Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed > > On Tue, Sep 24, 2024 at 10:22 AM Zhang, Yifan <Yifan1.Zhang@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. > > Sure, I will send patch V2 to change commit message. > > > commit b2dba064c9bdd18c7dd39066d25453af28451dbf > > Author: Philip Yang <Philip.Yang@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@amd.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > > > > > > > > -----Original Message----- > > From: Alex Deucher <alexdeucher@gmail.com> > > Sent: Tuesday, September 24, 2024 10:07 PM > > To: Zhang, Yifan <Yifan1.Zhang@amd.com> > > Cc: amd-gfx@lists.freedesktop.org; Yang, Philip <Philip.Yang@amd.com>; > > Kuehling, Felix <Felix.Kuehling@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@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@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 > > There is still a "continue" path left in the loop, I think the logic may be broken here if loop dropped. Ah, yes, sorry I missed that continue. Alex > > r = drm_buddy_alloc_blocks(mm, fpfn, > lpfn, > size, > min_block_size, > &vres->blocks, > vres->flags); > > if (unlikely(r == -ENOSPC) && pages_per_block == ~0ul && > !(place->flags & TTM_PL_FLAG_CONTIGUOUS)) { > vres->flags &= ~DRM_BUDDY_CONTIGUOUS_ALLOCATION; > pages_per_block = max_t(u32, 2UL << (20UL - PAGE_SHIFT), > tbo->page_alignment); > > continue; /* continue in the loop */ > } > > > > > 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 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed 2024-09-23 8:19 [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed Yifan Zhang 2024-09-24 14:06 ` Alex Deucher @ 2024-09-27 7:13 ` Christian König 1 sibling, 0 replies; 7+ messages in thread From: Christian König @ 2024-09-27 7:13 UTC (permalink / raw) To: Yifan Zhang, amd-gfx, Paneer Selvam, Arunpravin Cc: Philip.Yang, Felix.Kuehling Arun please take a look at that. Looks valid to me in general. Thanks, Christian. Am 23.09.24 um 10:19 schrieb Yifan Zhang: > Make vram alloc loop simpler after 2GB limitation removed. > > Signed-off-by: Yifan Zhang <yifan1.zhang@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) { > 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); > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-09-27 7:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-23 8:19 [PATCH] drm/amdgpu: simplify vram alloc logic since 2GB limitation removed Yifan Zhang 2024-09-24 14:06 ` Alex Deucher 2024-09-24 14:22 ` Zhang, Yifan 2024-09-25 17:16 ` Alex Deucher 2024-09-26 14:45 ` Zhang, Yifan 2024-09-26 15:21 ` Alex Deucher 2024-09-27 7:13 ` Christian König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox