Re: [PATCH] drm/amdgpu: Fix the vram base start address

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 01.11.23 um 20:13 schrieb Arunpravin Paneer Selvam:
> Hi Christian,
>
> On 10/30/2023 9:34 PM, Christian König wrote:
>>
>>
>> Am 30.10.23 um 13:22 schrieb Arunpravin Paneer Selvam:
>>> If the size returned by drm buddy allocator is higher than
>>> the required size, we take the higher size to calculate
>>> the buffer start address. This is required if we couldn't
>>> trim the buffer to the requested size. This will fix the
>>> display corruption issue on APU's which has limited VRAM
>>> size.
>>>
>>> gitlab issue link: https://gitlab.freedesktop.org/drm/amd/-/issues/2859
>>> JIRA ticket link: https://ontrack-internal.amd.com/browse/SWDEV-425461
>>>
>>> Fixes: 0a1844bf0b53 ("drm/buddy: Improve contiguous memory allocation")
>>> Signed-off-by: Arunpravin Paneer Selvam 
>>> <Arunpravin.PaneerSelvam@amd.com>
>>
>> Acked-by: Christian König <christian.koenig@amd.com>
>>
>> IIRC that hack with the start address is actually not needed any 
>> more, but we need to double check this.
> okay, can we just remove this hack and keep the vres->base.start value 
> as the start address of the first block from the
> allocated list.

Please double check if we don't have any more cases where we compare the 
start address against the visible VRAM limit.

I think we now fixed all those cases and replaced them with calls to 
check if all segments are visible, but I'm not 100% sure.

Regards,
Christian.

>
> Thanks,
> Arun
>>
>> Christian.
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 15 +++++++++++++--
>>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> index 18f58efc9dc7..08916538a615 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> @@ -77,7 +77,16 @@ static inline bool 
>>> amdgpu_is_vram_mgr_blocks_contiguous(struct list_head *head)
>>>       return true;
>>>   }
>>>   +static inline u64 amdgpu_vram_mgr_blocks_size(struct list_head 
>>> *head)
>>> +{
>>> +    struct drm_buddy_block *block;
>>> +    u64 size = 0;
>>>   +    list_for_each_entry(block, head, link)
>>> +        size += amdgpu_vram_mgr_block_size(block);
>>> +
>>> +    return size;
>>> +}
>>>     /**
>>>    * DOC: mem_info_vram_total
>>> @@ -516,6 +525,8 @@ static int amdgpu_vram_mgr_new(struct 
>>> ttm_resource_manager *man,
>>>       mutex_unlock(&mgr->lock);
>>>         vres->base.start = 0;
>>> +    size = max_t(u64, amdgpu_vram_mgr_blocks_size(&vres->blocks),
>>> +             vres->base.size);
>>>       list_for_each_entry(block, &vres->blocks, link) {
>>>           unsigned long start;
>>>   @@ -523,8 +534,8 @@ static int amdgpu_vram_mgr_new(struct 
>>> ttm_resource_manager *man,
>>>               amdgpu_vram_mgr_block_size(block);
>>>           start >>= PAGE_SHIFT;
>>>   -        if (start > PFN_UP(vres->base.size))
>>> -            start -= PFN_UP(vres->base.size);
>>> +        if (start > PFN_UP(size))
>>> +            start -= PFN_UP(size);
>>>           else
>>>               start = 0;
>>>           vres->base.start = max(vres->base.start, start);
>>
>