Re: [PATCH] MIPS: Introduce cmdline argument writecombine=

[Jiaxun Yang <jiaxun.yang@flygoat.com>]

在 2020/8/6 下午3:09, Tiezhu Yang 写道:
> Loongson processors have a writecombine issue that maybe failed to
> write back framebuffer used with ATI Radeon or AMD GPU at times,
> after commit 8a08e50cee66 ("drm: Permit video-buffers writecombine
> mapping for MIPS"), there exists some errors such as blurred screen
> and lockup, and so on.
>
> With this patch, disable writecombine by default for Loongson64 to
> work well with ATI Radeon or AMD GPU, and it has no influence on the
> other platforms due to writecombine is enabled by default.
>
> Additionally, if it is necessary, writecombine=on can be set manually
> in the cmdline to enhance the performance for Loongson LS7A integrated
> graphics in the future.
>
> [   60.958721] radeon 0000:03:00.0: ring 0 stalled for more than 10079msec
> [   60.965315] radeon 0000:03:00.0: GPU lockup (current fence id 0x0000000000000112 last fence id 0x000000000000011d on ring 0)
> [   60.976525] radeon 0000:03:00.0: ring 3 stalled for more than 10086msec
> [   60.983156] radeon 0000:03:00.0: GPU lockup (current fence id 0x0000000000000374 last fence id 0x00000000000003a8 on ring 3)
Hi Tiezhu,

Thanks for your patch.
Personally I didn't have any issue with writecombine on my test systems, 
but there
are some complains about unstable graphic card from users. So generally 
a cmdline
writecombine switch is necessary.

>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>   arch/mips/include/asm/pgtable.h |  4 ++++
>   arch/mips/kernel/cpu-probe.c    | 19 +++++++++++++++++++
>   2 files changed, 23 insertions(+)
>
> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
> index dd7a0f5..34869f7 100644
> --- a/arch/mips/include/asm/pgtable.h
> +++ b/arch/mips/include/asm/pgtable.h
> @@ -473,6 +473,10 @@ static inline pgprot_t pgprot_noncached(pgprot_t _prot)
>   static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
>   {
>   	unsigned long prot = pgprot_val(_prot);
> +	extern bool mips_writecombine;
> +
> +	if (!mips_writecombine)
> +		return pgprot_noncached(_prot);

You can simply override c->writecombine to _CACHE_UNCACHED in 
cpu-probe.c with
out this kind of hijack.

>   
>   	/* cpu_data[0].writecombine is already shifted by _CACHE_SHIFT */
>   	prot = (prot & ~_CACHE_MASK) | cpu_data[0].writecombine;
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index e2955f1..98777ca 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -459,6 +459,25 @@ static int __init ftlb_disable(char *s)
>   
>   __setup("noftlb", ftlb_disable);
>   
> +#ifdef CONFIG_MACH_LOONGSON64
> +bool mips_writecombine; /* initialise to false by default */
> +#else
> +bool mips_writecombine = true;
> +#endif
> +EXPORT_SYMBOL(mips_writecombine);
There is no need to export this symbol, see comment before.
> +
> +static int __init writecombine_setup(char *str)
> +{
> +	if (strcmp(str, "on") == 0)
> +		mips_writecombine = true;
> +	else if (strcmp(str, "off") == 0)
> +		mips_writecombine = false;
> +
> +	return 1;
> +}
> +
> +__setup("writecombine=", writecombine_setup);

Use early_param here seems more reasonable, it will be probed earlier.

> +
>   /*
>    * Check if the CPU has per tc perf counters
>    */
Thanks

- Jiaxun