* [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