[PATCH] Revert "drm/amdgpu: remove TOPDOWN flags when allocating VRAM in large bar system"

[Arunpravin Paneer Selvam]

Hi Felix,

On 5/21/2023 12:09 AM, Felix Kuehling wrote:
> Am 2023-05-20 um 05:25 schrieb Arunpravin Paneer Selvam:
>> This reverts commit c105518679b6e87232874ffc989ec403bee59664.
>>
>> This patch disables the TOPDOWN flag for APU and few dGPU cards
>> which has the VRAM size equal to the BAR size.
>
> With resizable BARs it's not that rare.
>
>
>>
>> When we enable the TOPDOWN flag, we get the free blocks at
>> the highest available memory region and we don't split the
>> lower order blocks. This change is required to keep off
>> the fragmentation related issues particularly in ASIC
>> which has VRAM space <= 500MiB
>
> If TOPDOWN has these general benefits, then would it make sense to 
> allocate visible memory TOPDOWN as well, on large-BAR GPUs? Without 
> knowing too much about the internals of the allocator, my intuition 
> tells me that using only one allocation strategy has a better chance 
> of minimizing fragmentation than mixing two allocation strategies for 
> no good reason.
We have 2 allocation strategies for visible memory and non-visible 
memory. For visible memory we are going with tree based search as we 
should limit our allocation within a specified range limit. The TOPDOWN 
flag is not applicable for tree based
search allocation. For non-visible memory we pick free blocks from the 
free list and here the TOPDOWN flag is applicable.

For large-BAR GPUs visible memory, we don't go with tree based search 
since the BAR == VRAM size, and if this 
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED flag is set for every allocation 
on large-BAR GPUs, we don't set TOPDOWN flag and
this could create fragmentation issue in case of heavy load. I will 
check the behavior on large BAR GPUs and set the TOPDOWN flag if not set.

Thanks,
Arun.
>
>
> Regards,
>   Felix
>
>
>>
>> Hence, we are reverting this patch.
>>
>> Gitlab issue link - https://gitlab.freedesktop.org/drm/amd/-/issues/2270
>>
>> Signed-off-by: Arunpravin Paneer Selvam 
>> <Arunpravin.PaneerSelvam at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 2bd1a54ee866..ca5fc07faf6c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -139,7 +139,7 @@ void amdgpu_bo_placement_from_domain(struct 
>> amdgpu_bo *abo, u32 domain)
>>             if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
>>               places[c].lpfn = visible_pfn;
>> -        else if (adev->gmc.real_vram_size != 
>> adev->gmc.visible_vram_size)
>> +        else
>>               places[c].flags |= TTM_PL_FLAG_TOPDOWN;
>>             if (flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)