[PATCH v2] drm/amdgpu: Fix VRAM BO swap issue

[Christian König]

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 at 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.