[PATCH] drm/amdgpu: add coreboot workaround for KV/KB

[Christian König]

Am 17.01.20 um 18:07 schrieb Felix Kuehling:
> On 2020-01-17 3:17 a.m., Christian König wrote:
>> Am 17.01.20 um 03:01 schrieb Felix Kuehling:
>>> On 2020-01-16 8:09, Christian König wrote:
>>>> Coreboot seems to have a problem correctly setting up access to the 
>>>> stolen VRAM
>>>> on KV/KB. Use the direct access only when necessary.
>>>
>>> I'm not sure what you mean by "necessary".
>>
>> Necessary for better performance.
>
> Right. So your patch description says that sometimes better 
> performance is not necessary.

Well what I want to say is that it is not necessary for better 
performance. If FB is small enough we can use the BAR as well.

> The criteria is based on the size of the BAR, which doesn't really 
> have anything to do with performance.

Why? Most of the performance gain comes from not shuffling around VRAM 
buffers for CPU access any more.

Additional to that there is a reduction in latency when accessing the 
VRAM, but that usually doesn't matter for performance.

>
>
>>
>>>
>>>>
>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 3 ++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> index 19d5b133e1d7..9da9596a3638 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>> @@ -381,7 +381,8 @@ static int gmc_v7_0_mc_init(struct 
>>>> amdgpu_device *adev)
>>>>       adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
>>>>     #ifdef CONFIG_X86_64
>>>> -    if (adev->flags & AMD_IS_APU) {
>>>> +    if (adev->flags & AMD_IS_APU &&
>>>> +        adev->gmc.real_vram_size > adev->gmc.aper_size) {
>>>
>>> CPU access to the whole VRAM isn't really necessary. I thought the 
>>> main motivation for accessing FB directly on APUs was better 
>>> performance. You're disabling that optimization on all APUs where 
>>> the FB is smaller than the aperture size.
>>
>> Correct, yes. For some reason coreboot seems to explicitly setup the 
>> memory used for the FB as write-protected.
>>
>> The GPU can still read/write it normally cause it ignores that 
>> protection, but the CPU can't change the FB memory any more with that 
>> setup.
>
> Can we test whether writing to FB works and make the decision based on 
> that?

Thought about that as well. But it is complicated to implement and we 
would need to test the whole FB to be sure and that could take a while.

Christian.

>
> Thanks,
>   Felix
>
>>
>> No idea why they do this, most likely just an over conservative 
>> protection of a reserved area of memory.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>>   Felix
>>>
>>>
>>>>           adev->gmc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 
>>>> 22;
>>>>           adev->gmc.aper_size = adev->gmc.real_vram_size;
>>>>       }
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cfelix.kuehling%40amd.com%7Cd59490f0e7f547d3a3fc08d79b25bed4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637148458759658617&sdata=To9fsEHvQ4dokZKSwTZQ7V8LIREA%2Bj58ouvaUVtHfpI%3D&reserved=0 
>>>
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx