[PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC

[Christian König]

Am 09.09.20 um 11:39 schrieb Tiezhu Yang:
> On 09/09/2020 12:21 PM, Huacai Chen wrote:
>> Though RADEON_GEM_GTT_WC is initially used for GTT, but this flag is
>> bound to drm_arch_can_wc_memory(), and if arch doesn't support WC, then
>> VRAM should not use WC.
>
> +cc RADEON and AMDGPU DRM DRIVERS maintainer
> Alex Deucher <alexander.deucher at amd.com>
> Christian König <christian.koenig at amd.com>
> amd-gfx at lists.freedesktop.org
>
> Hi all,
>
> In the current code, if CONFIG_CPU_LOONGSON64 is set, 
> drm_arch_can_wc_memory()
> returns false, and then bo->flags clears the flag RADEON_GEM_GTT_WC, 
> so with
> this patch, TTM_PL_FLAG_WC of VRAM is removed on the Loongson platform,
> the writecombine issue for Loongson64 can be fixed [1].

And broken for mostly all other platforms. The patch is complete nonsense.

See the RADEON_GEM_GTT_WC flag means that that system memory can be 
mapped WC instead of cached, but here it is used to map MEMIO uncached 
instead of WC.

> I find this is done by commit 221004c66a58 ("drm: Loongson-3 doesn't 
> fully
> support wc memory"), but I want to know why drm_arch_can_wc_memory() 
> returns
> false for Loongson64, is there some historical reasons?

It looks like Loongson has a platform bug which prevents the PCIe 
extension which allows unsnooped system memory access to work correctly.

Regards,
Christian.

>
> include/drm/drm_cache.h
> static inline bool drm_arch_can_wc_memory(void)
> {
> #if defined(CONFIG_PPC) && !defined(CONFIG_NOT_COHERENT_CACHE)
>     return false;
> #elif defined(CONFIG_MIPS) && defined(CONFIG_CPU_LOONGSON64)
>     return false;
> #elif defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> ...
> }
>
> drivers/gpu/drm/radeon/radeon_object.c
> int radeon_bo_create()
> {
> ...
>     if (!drm_arch_can_wc_memory())
>         bo->flags &= ~RADEON_GEM_GTT_WC;
> ...
> }
>
> [1] 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1285542%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7Ca60b0da3c62448f4b5fe08d854a43420%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637352411519749179&sdata=kiXLj58%2Boma5r5EYRuEsLg%2FwqkXpHtBfxLWVUgC%2B2g0%3D&reserved=0
> gpu/drm: Remove TTM_PL_FLAG_WC of VRAM to fix writecombine issue for 
> Loongson64
> [2] 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D221004c66a58&data=02%7C01%7Cchristian.koenig%40amd.com%7Ca60b0da3c62448f4b5fe08d854a43420%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637352411519749179&sdata=ekNulVe555%2BVnnsqSx5N0wwH%2BepaBGfzBHQnxddbO5Y%3D&reserved=0
>
> Thanks,
> Tiezhu
>
>>
>> Signed-off-by: Huacai Chen <chenhc at lemote.com>
>> ---
>>   drivers/gpu/drm/radeon/radeon_object.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
>> b/drivers/gpu/drm/radeon/radeon_object.c
>> index f3dee01..07b82d9 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.c
>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
>> @@ -117,10 +117,16 @@ void radeon_ttm_placement_from_domain(struct 
>> radeon_bo *rbo, u32 domain)
>>                                TTM_PL_FLAG_VRAM;
>>           }
>>   -        rbo->placements[c].fpfn = 0;
>> -        rbo->placements[c++].flags = TTM_PL_FLAG_WC |
>> -                         TTM_PL_FLAG_UNCACHED |
>> -                         TTM_PL_FLAG_VRAM;
>> +        if (rbo->flags & RADEON_GEM_GTT_WC) {
>> +            rbo->placements[c].fpfn = 0;
>> +            rbo->placements[c++].flags = TTM_PL_FLAG_WC |
>> +                             TTM_PL_FLAG_UNCACHED |
>> +                             TTM_PL_FLAG_VRAM;
>> +        } else {
>> +            rbo->placements[c].fpfn = 0;
>> +            rbo->placements[c++].flags = TTM_PL_FLAG_UNCACHED |
>> +                             TTM_PL_FLAG_VRAM;
>> +        }
>>       }
>>         if (domain & RADEON_GEM_DOMAIN_GTT) {
>