linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC
@ 2020-09-09  4:21 Huacai Chen
  2020-09-09  4:22 ` [PATCH 2/2] drm/amdgpu: Don't use WC for VRAM if !AMDGPU_GEM_CREATE_CPU_GTT_USWC Huacai Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Huacai Chen @ 2020-09-09  4:21 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Thomas Gleixner, Jason Cooper, Marc Zyngier
  Cc: linux-mips, Fuxin Zhang, Huacai Chen, Jiaxun Yang, Huacai Chen

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.

Signed-off-by: Huacai Chen <chenhc@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) {
-- 
2.7.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] drm/amdgpu: Don't use WC for VRAM if !AMDGPU_GEM_CREATE_CPU_GTT_USWC
  2020-09-09  4:21 [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Huacai Chen
@ 2020-09-09  4:22 ` Huacai Chen
  2020-09-09  8:05 ` [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Sergei Shtylyov
  2020-09-09  9:39 ` Tiezhu Yang
  2 siblings, 0 replies; 5+ messages in thread
From: Huacai Chen @ 2020-09-09  4:22 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Thomas Gleixner, Jason Cooper, Marc Zyngier
  Cc: linux-mips, Fuxin Zhang, Huacai Chen, Jiaxun Yang, Huacai Chen

Though AMDGPU_GEM_CREATE_CPU_GTT_USWC 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.

Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 5ac7b55..04299f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -136,8 +136,10 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
 
 		places[c].fpfn = 0;
 		places[c].lpfn = 0;
-		places[c].flags = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
-			TTM_PL_FLAG_VRAM;
+		places[c].flags = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_VRAM;
+
+		if (flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
+			places[c].flags |= TTM_PL_FLAG_WC;
 
 		if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
 			places[c].lpfn = visible_pfn;
-- 
2.7.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC
  2020-09-09  4:21 [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Huacai Chen
  2020-09-09  4:22 ` [PATCH 2/2] drm/amdgpu: Don't use WC for VRAM if !AMDGPU_GEM_CREATE_CPU_GTT_USWC Huacai Chen
@ 2020-09-09  8:05 ` Sergei Shtylyov
  2020-09-09  9:39 ` Tiezhu Yang
  2 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2020-09-09  8:05 UTC (permalink / raw)
  To: Huacai Chen, Thomas Bogendoerfer, Thomas Gleixner, Jason Cooper,
	Marc Zyngier
  Cc: linux-mips, Fuxin Zhang, Huacai Chen, Jiaxun Yang

Hello!

On 09.09.2020 7:21, 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.
> 
> Signed-off-by: Huacai Chen <chenhc@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;

    Shouldn't this statement be placed outside *if* as before?

> +			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) {

MBR, Sergei

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC
  2020-09-09  4:21 [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Huacai Chen
  2020-09-09  4:22 ` [PATCH 2/2] drm/amdgpu: Don't use WC for VRAM if !AMDGPU_GEM_CREATE_CPU_GTT_USWC Huacai Chen
  2020-09-09  8:05 ` [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Sergei Shtylyov
@ 2020-09-09  9:39 ` Tiezhu Yang
  2020-09-09 11:00   ` Christian König
  2 siblings, 1 reply; 5+ messages in thread
From: Tiezhu Yang @ 2020-09-09  9:39 UTC (permalink / raw)
  To: Huacai Chen, Thomas Bogendoerfer, Thomas Gleixner, Jason Cooper,
	Marc Zyngier
  Cc: linux-mips, Fuxin Zhang, Huacai Chen, Jiaxun Yang, Alex Deucher,
	christian.koenig, michel.daenzer, amd-gfx

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@amd.com>
Christian König <christian.koenig@amd.com>
amd-gfx@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].

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?

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://lore.kernel.org/patchwork/patch/1285542/
gpu/drm: Remove TTM_PL_FLAG_WC of VRAM to fix writecombine issue for 
Loongson64
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=221004c66a58

Thanks,
Tiezhu

>
> Signed-off-by: Huacai Chen <chenhc@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) {


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC
  2020-09-09  9:39 ` Tiezhu Yang
@ 2020-09-09 11:00   ` Christian König
  0 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2020-09-09 11:00 UTC (permalink / raw)
  To: Tiezhu Yang, Huacai Chen, Thomas Bogendoerfer, Thomas Gleixner,
	Jason Cooper, Marc Zyngier
  Cc: linux-mips, Fuxin Zhang, Huacai Chen, Jiaxun Yang, Alex Deucher,
	michel.daenzer, amd-gfx

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@amd.com>
> Christian König <christian.koenig@amd.com>
> amd-gfx@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&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Ca60b0da3c62448f4b5fe08d854a43420%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637352411519749179&amp;sdata=kiXLj58%2Boma5r5EYRuEsLg%2FwqkXpHtBfxLWVUgC%2B2g0%3D&amp;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&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Ca60b0da3c62448f4b5fe08d854a43420%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637352411519749179&amp;sdata=ekNulVe555%2BVnnsqSx5N0wwH%2BepaBGfzBHQnxddbO5Y%3D&amp;reserved=0
>
> Thanks,
> Tiezhu
>
>>
>> Signed-off-by: Huacai Chen <chenhc@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) {
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-09 11:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09  4:21 [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Huacai Chen
2020-09-09  4:22 ` [PATCH 2/2] drm/amdgpu: Don't use WC for VRAM if !AMDGPU_GEM_CREATE_CPU_GTT_USWC Huacai Chen
2020-09-09  8:05 ` [PATCH 1/2] drm/radeon: Don't use WC for VRAM if !RADEON_GEM_GTT_WC Sergei Shtylyov
2020-09-09  9:39 ` Tiezhu Yang
2020-09-09 11:00   ` Christian König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).