* [PATCH v2] drm/amdgpu: Fix VRAM BO swap issue
@ 2022-09-22 15:26 Arunpravin Paneer Selvam
2022-09-22 17:14 ` Christian König
0 siblings, 1 reply; 3+ messages in thread
From: Arunpravin Paneer Selvam @ 2022-09-22 15:26 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, Jun.Ma2, christian.koenig, Arunpravin Paneer Selvam
DRM buddy manager allocates the contiguous memory requests in
a single block or multiple blocks. So for the ttm move operation
(incase of low vram memory) we should consider all the blocks to
compute the total memory size which compared with the struct
ttm_resource num_pages in order to verify that the blocks are
contiguous for the eviction process.
v2: Added a Fixes tag
Fixes: c9cad937c0c5 ("drm/amdgpu: add drm buddy support to amdgpu")
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b1c455329023..b1223c8e30c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -426,6 +426,7 @@ static bool amdgpu_mem_visible(struct amdgpu_device *adev,
{
uint64_t mem_size = (u64)mem->num_pages << PAGE_SHIFT;
struct amdgpu_res_cursor cursor;
+ u64 start, size, total_size = 0;
if (mem->mem_type == TTM_PL_SYSTEM ||
mem->mem_type == TTM_PL_TT)
@@ -435,8 +436,23 @@ static bool amdgpu_mem_visible(struct amdgpu_device *adev,
amdgpu_res_first(mem, 0, mem_size, &cursor);
- /* ttm_resource_ioremap only supports contiguous memory */
- if (cursor.size != mem_size)
+ do {
+ start = cursor.start;
+ size = cursor.size;
+
+ total_size += size;
+
+ amdgpu_res_next(&cursor, cursor.size);
+
+ if (!cursor.remaining)
+ break;
+
+ /* ttm_resource_ioremap only supports contiguous memory */
+ if (start + size != cursor.start)
+ return false;
+ } while (1);
+
+ if (total_size != mem_size)
return false;
return cursor.start + cursor.size <= adev->gmc.visible_vram_size;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/amdgpu: Fix VRAM BO swap issue
2022-09-22 15:26 [PATCH v2] drm/amdgpu: Fix VRAM BO swap issue Arunpravin Paneer Selvam
@ 2022-09-22 17:14 ` Christian König
2022-09-22 17:43 ` Christian König
0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2022-09-22 17:14 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher, Jun.Ma2
Am 22.09.22 um 17:26 schrieb Arunpravin Paneer Selvam:
> DRM buddy manager allocates the contiguous memory requests in
> a single block or multiple blocks. So for the ttm move operation
> (incase of low vram memory) we should consider all the blocks to
> compute the total memory size which compared with the struct
> ttm_resource num_pages in order to verify that the blocks are
> contiguous for the eviction process.
>
> v2: Added a Fixes tag
>
> Fixes: c9cad937c0c5 ("drm/amdgpu: add drm buddy support to amdgpu")
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index b1c455329023..b1223c8e30c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -426,6 +426,7 @@ static bool amdgpu_mem_visible(struct amdgpu_device *adev,
> {
> uint64_t mem_size = (u64)mem->num_pages << PAGE_SHIFT;
> struct amdgpu_res_cursor cursor;
> + u64 start, size, total_size = 0;
>
> if (mem->mem_type == TTM_PL_SYSTEM ||
> mem->mem_type == TTM_PL_TT)
> @@ -435,8 +436,23 @@ static bool amdgpu_mem_visible(struct amdgpu_device *adev,
>
> amdgpu_res_first(mem, 0, mem_size, &cursor);
>
> - /* ttm_resource_ioremap only supports contiguous memory */
> - if (cursor.size != mem_size)
> + do {
> + start = cursor.start;
> + size = cursor.size;
> +
> + total_size += size;
> +
> + amdgpu_res_next(&cursor, cursor.size);
> +
> + if (!cursor.remaining)
> + break;
> +
> + /* ttm_resource_ioremap only supports contiguous memory */
> + if (start + size != cursor.start)
> + return false;
> + } while (1);
> +
> + if (total_size != mem_size)
> return false;
I would completely drop this extra check.
> return cursor.start + cursor.size <= adev->gmc.visible_vram_size;
Instead of this you should be able to do all of this in one go.
Something like this here should work:
amdgpu_res_first(...
end = cursor.start + cursor.size;
do (
amdgpu_res_next(....
if (end != cursor.start)
return false;
end = cursor.start + cursor.size;
} while (cursor.remaining);
return end <= visible_vram_size;
Saves a bit of extra calculations and variables.
Regards,
Christian.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/amdgpu: Fix VRAM BO swap issue
2022-09-22 17:14 ` Christian König
@ 2022-09-22 17:43 ` Christian König
0 siblings, 0 replies; 3+ messages in thread
From: Christian König @ 2022-09-22 17:43 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher, Jun.Ma2
Am 22.09.22 um 19:14 schrieb Christian König:
>
>
> Am 22.09.22 um 17:26 schrieb Arunpravin Paneer Selvam:
>> DRM buddy manager allocates the contiguous memory requests in
>> a single block or multiple blocks. So for the ttm move operation
>> (incase of low vram memory) we should consider all the blocks to
>> compute the total memory size which compared with the struct
>> ttm_resource num_pages in order to verify that the blocks are
>> contiguous for the eviction process.
>>
>> v2: Added a Fixes tag
>>
>> Fixes: c9cad937c0c5 ("drm/amdgpu: add drm buddy support to amdgpu")
>> Signed-off-by: Arunpravin Paneer Selvam
>> <Arunpravin.PaneerSelvam@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index b1c455329023..b1223c8e30c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -426,6 +426,7 @@ static bool amdgpu_mem_visible(struct
>> amdgpu_device *adev,
>> {
>> uint64_t mem_size = (u64)mem->num_pages << PAGE_SHIFT;
>> struct amdgpu_res_cursor cursor;
>> + u64 start, size, total_size = 0;
>> if (mem->mem_type == TTM_PL_SYSTEM ||
>> mem->mem_type == TTM_PL_TT)
>> @@ -435,8 +436,23 @@ static bool amdgpu_mem_visible(struct
>> amdgpu_device *adev,
>> amdgpu_res_first(mem, 0, mem_size, &cursor);
>> - /* ttm_resource_ioremap only supports contiguous memory */
>> - if (cursor.size != mem_size)
>> + do {
>> + start = cursor.start;
>> + size = cursor.size;
>> +
>> + total_size += size;
>> +
>> + amdgpu_res_next(&cursor, cursor.size);
>> +
>> + if (!cursor.remaining)
>> + break;
>> +
>> + /* ttm_resource_ioremap only supports contiguous memory */
>> + if (start + size != cursor.start)
>> + return false;
>> + } while (1);
>> +
>> + if (total_size != mem_size)
>> return false;
>
> I would completely drop this extra check.
>
>> return cursor.start + cursor.size <= adev->gmc.visible_vram_size;
>
> Instead of this you should be able to do all of this in one go.
>
> Something like this here should work:
>
> amdgpu_res_first(...
> end = cursor.start + cursor.size;
> do (
> amdgpu_res_next(....
> if (end != cursor.start)
> return false;
> end = cursor.start + cursor.size;
> } while (cursor.remaining);
My fault, this should have been
while (cursor.remaining) {
...
}
Regards,
Christian.
>
> return end <= visible_vram_size;
>
> Saves a bit of extra calculations and variables.
>
> Regards,
> Christian.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-22 17:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 15:26 [PATCH v2] drm/amdgpu: Fix VRAM BO swap issue Arunpravin Paneer Selvam
2022-09-22 17:14 ` Christian König
2022-09-22 17:43 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.