MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Zhou Yanjie]

1.fix bugs when detecting L2 cache sets value.
2.fix bugs when detecting L2 cache ways value.
3.fix bugs when calculate bogoMips and loops_per_jiffy.

[PATCH] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Zhou Yanjie]

1.fix bugs when detecting L2 cache sets value.
2.fix bugs when detecting L2 cache ways value.
3.fix bugs when calculate bogoMips and loops_per_jiffy.

Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
---
 arch/mips/kernel/cpu-probe.c |  7 ++++++-
 arch/mips/mm/sc-mips.c       | 18 +++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index eb527a1..a914435 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1960,11 +1960,16 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
 	c->options &= ~MIPS_CPU_COUNTER;
 	BUG_ON(!__builtin_constant_p(cpu_has_counter) || cpu_has_counter);
 	switch (c->processor_id & PRID_IMP_MASK) {
-	case PRID_IMP_XBURST:
+	case PRID_IMP_XBURST: {
+		unsigned int config7;
 		c->cputype = CPU_XBURST;
 		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
 		__cpu_name[cpu] = "Ingenic XBurst";
+		config7 = read_c0_config7();
+		config7 |= (1 << 4);
+		write_c0_config7(config7);
 		break;
+	}
 	default:
 		panic("Unknown Ingenic Processor ID!");
 		break;
diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
index 9385ddb..ed953d4 100644
--- a/arch/mips/mm/sc-mips.c
+++ b/arch/mips/mm/sc-mips.c
@@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
 	else
 		return 0;
 
+	/*
+	 * According to config2 it would be 512-sets, but that is contradicted
+	 * by all documentation.
+	 */
+	if (current_cpu_type() == CPU_XBURST &&
+				mips_machtype == MACH_INGENIC_X1000)
+		c->scache.sets = 256;
+
 	tmp = (config2 >> 0) & 0x0f;
 	if (tmp <= 7)
 		c->scache.ways = tmp + 1;
@@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
 	 * According to config2 it would be 5-ways, but that is contradicted
 	 * by all documentation.
 	 */
-	if (current_cpu_type() == CPU_XBURST &&
-				mips_machtype == MACH_INGENIC_JZ4770)
-		c->scache.ways = 4;
+	if (current_cpu_type() == CPU_XBURST) {
+		switch (mips_machtype) {
+		case MACH_INGENIC_JZ4770:
+		case MACH_INGENIC_X1000:
+			c->scache.ways = 4;
+		}
+	}
 
 	c->scache.waysize = c->scache.sets * c->scache.linesz;
 	c->scache.waybit = __ffs(c->scache.waysize);
-- 
2.7.4

Re: [PATCH] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Paul Cercueil]

Hi Zhou,



Le mar. 30 juil. 2019 à 10:55, Zhou Yanjie <zhouyanjie@zoho.com> a 
écrit :
> 1.fix bugs when detecting L2 cache sets value.
> 2.fix bugs when detecting L2 cache ways value.
> 3.fix bugs when calculate bogoMips and loops_per_jiffy.
> 
> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
> ---
>  arch/mips/kernel/cpu-probe.c |  7 ++++++-
>  arch/mips/mm/sc-mips.c       | 18 +++++++++++++++---
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/mips/kernel/cpu-probe.c 
> b/arch/mips/kernel/cpu-probe.c
> index eb527a1..a914435 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -1960,11 +1960,16 @@ static inline void cpu_probe_ingenic(struct 
> cpuinfo_mips *c, unsigned int cpu)
>  	c->options &= ~MIPS_CPU_COUNTER;
>  	BUG_ON(!__builtin_constant_p(cpu_has_counter) || cpu_has_counter);
>  	switch (c->processor_id & PRID_IMP_MASK) {
> -	case PRID_IMP_XBURST:
> +	case PRID_IMP_XBURST: {
> +		unsigned int config7;
>  		c->cputype = CPU_XBURST;
>  		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>  		__cpu_name[cpu] = "Ingenic XBurst";
> +		config7 = read_c0_config7();
> +		config7 |= (1 << 4);
> +		write_c0_config7(config7);

If you add __BUILD_SET_C0(config7) in arch/mips/include/asm/mipsregs.h
(search for this macro) then you can call directly 
set_c0_config7(BIT(4)).

It's preferred to use the BIT(x) macro instead of the (1 << x) 
construct.

Finally, what does that bit do? I can't find it any documentation about
it. Please add a comment describing what it does.


>  		break;
> +	}
>  	default:
>  		panic("Unknown Ingenic Processor ID!");
>  		break;
> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
> index 9385ddb..ed953d4 100644
> --- a/arch/mips/mm/sc-mips.c
> +++ b/arch/mips/mm/sc-mips.c
> @@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
>  	else
>  		return 0;
> 
> +	/*
> +	 * According to config2 it would be 512-sets, but that is 
> contradicted
> +	 * by all documentation.
> +	 */
> +	if (current_cpu_type() == CPU_XBURST &&
> +				mips_machtype == MACH_INGENIC_X1000)
> +		c->scache.sets = 256;
> +
>  	tmp = (config2 >> 0) & 0x0f;
>  	if (tmp <= 7)
>  		c->scache.ways = tmp + 1;
> @@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
>  	 * According to config2 it would be 5-ways, but that is contradicted
>  	 * by all documentation.
>  	 */
> -	if (current_cpu_type() == CPU_XBURST &&
> -				mips_machtype == MACH_INGENIC_JZ4770)
> -		c->scache.ways = 4;
> +	if (current_cpu_type() == CPU_XBURST) {
> +		switch (mips_machtype) {
> +		case MACH_INGENIC_JZ4770:
> +		case MACH_INGENIC_X1000:
> +			c->scache.ways = 4;
> +		}
> +	}
> 
>  	c->scache.waysize = c->scache.sets * c->scache.linesz;
>  	c->scache.waybit = __ffs(c->scache.waysize);
> --
> 2.7.4
> 
> 

Re: [PATCH] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Zhou Yanjie]

Hi Paul,
On 2019年07月31日 02:02, Paul Cercueil wrote:
> Hi Zhou,
>
>
>
> Le mar. 30 juil. 2019 à 10:55, Zhou Yanjie <zhouyanjie@zoho.com> a 
> écrit :
>> 1.fix bugs when detecting L2 cache sets value.
>> 2.fix bugs when detecting L2 cache ways value.
>> 3.fix bugs when calculate bogoMips and loops_per_jiffy.
>>
>> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
>> ---
>>  arch/mips/kernel/cpu-probe.c |  7 ++++++-
>>  arch/mips/mm/sc-mips.c       | 18 +++++++++++++++---
>>  2 files changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index eb527a1..a914435 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -1960,11 +1960,16 @@ static inline void cpu_probe_ingenic(struct 
>> cpuinfo_mips *c, unsigned int cpu)
>>      c->options &= ~MIPS_CPU_COUNTER;
>>      BUG_ON(!__builtin_constant_p(cpu_has_counter) || cpu_has_counter);
>>      switch (c->processor_id & PRID_IMP_MASK) {
>> -    case PRID_IMP_XBURST:
>> +    case PRID_IMP_XBURST: {
>> +        unsigned int config7;
>>          c->cputype = CPU_XBURST;
>>          c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>>          __cpu_name[cpu] = "Ingenic XBurst";
>> +        config7 = read_c0_config7();
>> +        config7 |= (1 << 4);
>> +        write_c0_config7(config7);
>
> If you add __BUILD_SET_C0(config7) in arch/mips/include/asm/mipsregs.h
> (search for this macro) then you can call directly 
> set_c0_config7(BIT(4)).
>
> It's preferred to use the BIT(x) macro instead of the (1 << x) construct.
Thanks for your suggestion, I have used set_c0_config7(BIT(4)) instead
of the (1 << x) construct in v2.
> Finally, what does that bit do? I can't find it any documentation about
> it. Please add a comment describing what it does.
According to Ingenic's explanation, in the XBurst architecture this bit is
used to control a low-power mode, which is intended to allow the CPU
to further reduce power consumption, but due to some bugs, it is easy
to cause errors in the calculation of bogomips and loops_per_jiffy, So
Ingenic's suggestion is to set this bit to turn it off.
>
>>          break;
>> +    }
>>      default:
>>          panic("Unknown Ingenic Processor ID!");
>>          break;
>> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
>> index 9385ddb..ed953d4 100644
>> --- a/arch/mips/mm/sc-mips.c
>> +++ b/arch/mips/mm/sc-mips.c
>> @@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
>>      else
>>          return 0;
>>
>> +    /*
>> +     * According to config2 it would be 512-sets, but that is 
>> contradicted
>> +     * by all documentation.
>> +     */
>> +    if (current_cpu_type() == CPU_XBURST &&
>> +                mips_machtype == MACH_INGENIC_X1000)
>> +        c->scache.sets = 256;
>> +
>>      tmp = (config2 >> 0) & 0x0f;
>>      if (tmp <= 7)
>>          c->scache.ways = tmp + 1;
>> @@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
>>       * According to config2 it would be 5-ways, but that is 
>> contradicted
>>       * by all documentation.
>>       */
>> -    if (current_cpu_type() == CPU_XBURST &&
>> -                mips_machtype == MACH_INGENIC_JZ4770)
>> -        c->scache.ways = 4;
>> +    if (current_cpu_type() == CPU_XBURST) {
>> +        switch (mips_machtype) {
>> +        case MACH_INGENIC_JZ4770:
>> +        case MACH_INGENIC_X1000:
>> +            c->scache.ways = 4;
>> +        }
>> +    }
>>
>>      c->scache.waysize = c->scache.sets * c->scache.linesz;
>>      c->scache.waybit = __ffs(c->scache.waysize);
>> -- 
>> 2.7.4
>>
>>
>
>

MIPS: Ingenic: Fix bugs when detecting X1000's parameters v2.

[Zhou Yanjie]

v1->v2: Use "set_c0_config7(BIT(4))" to simplify code and add comment.

[PATCH v2] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Zhou Yanjie]

1.fix bugs when detecting L2 cache sets value.
2.fix bugs when detecting L2 cache ways value.
3.fix bugs when calculate bogoMips and loops_per_jiffy.

Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
---
 arch/mips/include/asm/mipsregs.h |  1 +
 arch/mips/kernel/cpu-probe.c     |  7 +++++++
 arch/mips/mm/sc-mips.c           | 18 +++++++++++++++---
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 1e6966e..01e0fcb 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -2813,6 +2813,7 @@ __BUILD_SET_C0(status)
 __BUILD_SET_C0(cause)
 __BUILD_SET_C0(config)
 __BUILD_SET_C0(config5)
+__BUILD_SET_C0(config7)
 __BUILD_SET_C0(intcontrol)
 __BUILD_SET_C0(intctl)
 __BUILD_SET_C0(srsmap)
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index eb527a1..547c9a0 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
 		c->cputype = CPU_XBURST;
 		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
 		__cpu_name[cpu] = "Ingenic XBurst";
+		/*
+		 * config7 bit 4 is used to control a low-power mode in
+		 * XBurst architecture. This mode may cause errors in the
+		 * calculation of bogomips and loops_per_jiffy, set config7
+		 * bit 4 to disable this feature to prevent that.
+		 */
+		set_c0_config7(BIT(4));
 		break;
 	default:
 		panic("Unknown Ingenic Processor ID!");
diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
index 9385ddb..ed953d4 100644
--- a/arch/mips/mm/sc-mips.c
+++ b/arch/mips/mm/sc-mips.c
@@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
 	else
 		return 0;
 
+	/*
+	 * According to config2 it would be 512-sets, but that is contradicted
+	 * by all documentation.
+	 */
+	if (current_cpu_type() == CPU_XBURST &&
+				mips_machtype == MACH_INGENIC_X1000)
+		c->scache.sets = 256;
+
 	tmp = (config2 >> 0) & 0x0f;
 	if (tmp <= 7)
 		c->scache.ways = tmp + 1;
@@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
 	 * According to config2 it would be 5-ways, but that is contradicted
 	 * by all documentation.
 	 */
-	if (current_cpu_type() == CPU_XBURST &&
-				mips_machtype == MACH_INGENIC_JZ4770)
-		c->scache.ways = 4;
+	if (current_cpu_type() == CPU_XBURST) {
+		switch (mips_machtype) {
+		case MACH_INGENIC_JZ4770:
+		case MACH_INGENIC_X1000:
+			c->scache.ways = 4;
+		}
+	}
 
 	c->scache.waysize = c->scache.sets * c->scache.linesz;
 	c->scache.waybit = __ffs(c->scache.waysize);
-- 
2.7.4

Re: [PATCH v2] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Paul Burton]

Hi Zhou,

On Wed, Jul 31, 2019 at 12:39:03PM +0800, Zhou Yanjie wrote:
> 1.fix bugs when detecting L2 cache sets value.
> 2.fix bugs when detecting L2 cache ways value.
> 3.fix bugs when calculate bogoMips and loops_per_jiffy.

This should be split into 2 patches - one that fixes the L2 cache
detection problems, and possibly one that fixes the bogomips/lpj issue.

> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
> ---
>  arch/mips/include/asm/mipsregs.h |  1 +
>  arch/mips/kernel/cpu-probe.c     |  7 +++++++
>  arch/mips/mm/sc-mips.c           | 18 +++++++++++++++---
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
> index 1e6966e..01e0fcb 100644
> --- a/arch/mips/include/asm/mipsregs.h
> +++ b/arch/mips/include/asm/mipsregs.h
> @@ -2813,6 +2813,7 @@ __BUILD_SET_C0(status)
>  __BUILD_SET_C0(cause)
>  __BUILD_SET_C0(config)
>  __BUILD_SET_C0(config5)
> +__BUILD_SET_C0(config7)
>  __BUILD_SET_C0(intcontrol)
>  __BUILD_SET_C0(intctl)
>  __BUILD_SET_C0(srsmap)
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index eb527a1..547c9a0 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
>  		c->cputype = CPU_XBURST;
>  		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>  		__cpu_name[cpu] = "Ingenic XBurst";
> +		/*
> +		 * config7 bit 4 is used to control a low-power mode in
> +		 * XBurst architecture. This mode may cause errors in the
> +		 * calculation of bogomips and loops_per_jiffy, set config7
> +		 * bit 4 to disable this feature to prevent that.
> +		 */
> +		set_c0_config7(BIT(4));

I happen to know what this bit does - see this old patch for an
explanation:

  https://github.com/paulburton/linux/commit/0d72377bd615d00e99733adc0d37e6a2373fcde7

In short it disables a loop optimization in the CPU that is supposed to
special case loops & prevent them from relying upon the BTB.
Unfortunately that loop optimization negatively affects short loops,
such as in __delay(), and Ingenic's vendor kernels have generally set
this bit to disable it.

Note though that bogomips is bogus, so changing the bogomips value is
really not good justification for the patch at all (which is why I've so
far not bothered upstreaming the patch linked above). The best
justification I can think of is that Ingenic set the bit in their
downstream kernels, which presumably means it's beneficial overall (or
just that someone cares too much about bogomips).

In any case, one thing I don't know for sure is which CPU versions are
affected. I don't believe this affected older devices like the JZ4740,
and my copy of the XBurst1 CPU Core Programming Manual documents the bit
as reserved. Given that you're seeing the X1000 is affected, and I know
the JZ4780 is affected, that covers at least 2 different PRIDs so we
can't just check for that.

Hopefully writing to the bit is just a no-op on older systems if it is
actually reserved, but it'd be great if we could test that.

At the very least we should define the bit in asm/mipsregs.h & properly
document what it does - using BIT(4) here may be a little nicer than
(1<<4), but it's still just a magic number. I don't mind if you want to
fix your patch to do that, or one of us can resurrect mine which has
that information already.

>  		break;
>  	default:
>  		panic("Unknown Ingenic Processor ID!");
> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
> index 9385ddb..ed953d4 100644
> --- a/arch/mips/mm/sc-mips.c
> +++ b/arch/mips/mm/sc-mips.c
> @@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
>  	else
>  		return 0;
>  
> +	/*
> +	 * According to config2 it would be 512-sets, but that is contradicted
> +	 * by all documentation.
> +	 */
> +	if (current_cpu_type() == CPU_XBURST &&
> +				mips_machtype == MACH_INGENIC_X1000)
> +		c->scache.sets = 256;
> +
>  	tmp = (config2 >> 0) & 0x0f;
>  	if (tmp <= 7)
>  		c->scache.ways = tmp + 1;
> @@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
>  	 * According to config2 it would be 5-ways, but that is contradicted
>  	 * by all documentation.
>  	 */
> -	if (current_cpu_type() == CPU_XBURST &&
> -				mips_machtype == MACH_INGENIC_JZ4770)
> -		c->scache.ways = 4;
> +	if (current_cpu_type() == CPU_XBURST) {
> +		switch (mips_machtype) {
> +		case MACH_INGENIC_JZ4770:
> +		case MACH_INGENIC_X1000:
> +			c->scache.ways = 4;
> +		}
> +	}
>  
>  	c->scache.waysize = c->scache.sets * c->scache.linesz;
>  	c->scache.waybit = __ffs(c->scache.waysize);

Given that we need to fix up both sets & ways, I think this would be
cleaner if both were done in the switch statement here after we've read
the values from Config2. ie. something like:

	if (current_cpu_type() == CPU_XBURST) {
		switch (mips_machtype) {
		case MACH_INGENIC_JZ4770:
			c->scache.ways = 4;
			break;

		case MACH_INGENIC_X1000:
			c->scache.sets = 256;
			c->scache.ways = 4;
			break;
		}
	}

With appropriate comments added for each machine/SoC.

Thanks,
    Paul

Re: [PATCH v2] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Zhou Yanjie]

Hi Paul,

On 2019年08月01日 04:34, Paul Burton wrote:
> Hi Zhou,
>
> On Wed, Jul 31, 2019 at 12:39:03PM +0800, Zhou Yanjie wrote:
>> 1.fix bugs when detecting L2 cache sets value.
>> 2.fix bugs when detecting L2 cache ways value.
>> 3.fix bugs when calculate bogoMips and loops_per_jiffy.
> This should be split into 2 patches - one that fixes the L2 cache
> detection problems, and possibly one that fixes the bogomips/lpj issue.
Thanks for your advice. I have split it in v3.
>
>> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
>> ---
>>   arch/mips/include/asm/mipsregs.h |  1 +
>>   arch/mips/kernel/cpu-probe.c     |  7 +++++++
>>   arch/mips/mm/sc-mips.c           | 18 +++++++++++++++---
>>   3 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
>> index 1e6966e..01e0fcb 100644
>> --- a/arch/mips/include/asm/mipsregs.h
>> +++ b/arch/mips/include/asm/mipsregs.h
>> @@ -2813,6 +2813,7 @@ __BUILD_SET_C0(status)
>>   __BUILD_SET_C0(cause)
>>   __BUILD_SET_C0(config)
>>   __BUILD_SET_C0(config5)
>> +__BUILD_SET_C0(config7)
>>   __BUILD_SET_C0(intcontrol)
>>   __BUILD_SET_C0(intctl)
>>   __BUILD_SET_C0(srsmap)
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index eb527a1..547c9a0 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
>>   		c->cputype = CPU_XBURST;
>>   		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>>   		__cpu_name[cpu] = "Ingenic XBurst";
>> +		/*
>> +		 * config7 bit 4 is used to control a low-power mode in
>> +		 * XBurst architecture. This mode may cause errors in the
>> +		 * calculation of bogomips and loops_per_jiffy, set config7
>> +		 * bit 4 to disable this feature to prevent that.
>> +		 */
>> +		set_c0_config7(BIT(4));
> I happen to know what this bit does - see this old patch for an
> explanation:
>
>    https://github.com/paulburton/linux/commit/0d72377bd615d00e99733adc0d37e6a2373fcde7
>
> In short it disables a loop optimization in the CPU that is supposed to
> special case loops & prevent them from relying upon the BTB.
> Unfortunately that loop optimization negatively affects short loops,
> such as in __delay(), and Ingenic's vendor kernels have generally set
> this bit to disable it.
>
> Note though that bogomips is bogus, so changing the bogomips value is
> really not good justification for the patch at all (which is why I've so
> far not bothered upstreaming the patch linked above). The best
> justification I can think of is that Ingenic set the bit in their
> downstream kernels, which presumably means it's beneficial overall (or
> just that someone cares too much about bogomips).
>
> In any case, one thing I don't know for sure is which CPU versions are
> affected. I don't believe this affected older devices like the JZ4740,
> and my copy of the XBurst1 CPU Core Programming Manual documents the bit
> as reserved. Given that you're seeing the X1000 is affected, and I know
> the JZ4780 is affected, that covers at least 2 different PRIDs so we
> can't just check for that.
>
> Hopefully writing to the bit is just a no-op on older systems if it is
> actually reserved, but it'd be great if we could test that.
>
> At the very least we should define the bit in asm/mipsregs.h & properly
> document what it does - using BIT(4) here may be a little nicer than
> (1<<4), but it's still just a magic number. I don't mind if you want to
> fix your patch to do that, or one of us can resurrect mine which has
> that information already.
I have verified the Ingenic's CPU department. According to their reply,
all XBurst1 cores support this feature. Therefore, it is safe to turn off it
in XBurst1.

However, the hardware that I can use normally is JZ4760 and later, so
the JZ4730/JZ4740/JZ4750 has not been tested. So for safe we can also
turn off this feature only in JZ4760 and later if necessary. Wait for your
comments and if necessary I will change it in v4.
>>   		break;
>>   	default:
>>   		panic("Unknown Ingenic Processor ID!");
>> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
>> index 9385ddb..ed953d4 100644
>> --- a/arch/mips/mm/sc-mips.c
>> +++ b/arch/mips/mm/sc-mips.c
>> @@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
>>   	else
>>   		return 0;
>>   
>> +	/*
>> +	 * According to config2 it would be 512-sets, but that is contradicted
>> +	 * by all documentation.
>> +	 */
>> +	if (current_cpu_type() == CPU_XBURST &&
>> +				mips_machtype == MACH_INGENIC_X1000)
>> +		c->scache.sets = 256;
>> +
>>   	tmp = (config2 >> 0) & 0x0f;
>>   	if (tmp <= 7)
>>   		c->scache.ways = tmp + 1;
>> @@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
>>   	 * According to config2 it would be 5-ways, but that is contradicted
>>   	 * by all documentation.
>>   	 */
>> -	if (current_cpu_type() == CPU_XBURST &&
>> -				mips_machtype == MACH_INGENIC_JZ4770)
>> -		c->scache.ways = 4;
>> +	if (current_cpu_type() == CPU_XBURST) {
>> +		switch (mips_machtype) {
>> +		case MACH_INGENIC_JZ4770:
>> +		case MACH_INGENIC_X1000:
>> +			c->scache.ways = 4;
>> +		}
>> +	}
>>   
>>   	c->scache.waysize = c->scache.sets * c->scache.linesz;
>>   	c->scache.waybit = __ffs(c->scache.waysize);
> Given that we need to fix up both sets & ways, I think this would be
> cleaner if both were done in the switch statement here after we've read
> the values from Config2. ie. something like:
>
> 	if (current_cpu_type() == CPU_XBURST) {
> 		switch (mips_machtype) {
> 		case MACH_INGENIC_JZ4770:
> 			c->scache.ways = 4;
> 			break;
>
> 		case MACH_INGENIC_X1000:
> 			c->scache.sets = 256;
> 			c->scache.ways = 4;
> 			break;
> 		}
> 	}
>
> With appropriate comments added for each machine/SoC.

Thanks for you advice. I have change it in v3.
>
> Thanks,
>      Paul

MIPS: Ingenic: Fix bugs when detecting X1000's parameters v3

[Zhou Yanjie]

v1->v2: Use "set_c0_config7(BIT(4))" to simplify code and add comment.
v2->v3: Split patch and use "MIPS_CONF7_BTB_LOOP_EN" to instead "BIT(4)".

[PATCH 1/2 v3] MIPS: Ingenic: Fix bugs when detecting X1000's L2 cache.

[Zhou Yanjie]

1.fix bugs when detecting L2 cache sets value.
2.fix bugs when detecting L2 cache ways value.

Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
---
 arch/mips/mm/sc-mips.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
index 9385ddb..dbdbfe5 100644
--- a/arch/mips/mm/sc-mips.c
+++ b/arch/mips/mm/sc-mips.c
@@ -221,13 +221,26 @@ static inline int __init mips_sc_probe(void)
 	else
 		return 0;
 
-	/*
-	 * According to config2 it would be 5-ways, but that is contradicted
-	 * by all documentation.
-	 */
-	if (current_cpu_type() == CPU_XBURST &&
-				mips_machtype == MACH_INGENIC_JZ4770)
-		c->scache.ways = 4;
+	if (current_cpu_type() == CPU_XBURST) {
+		switch (mips_machtype) {
+		/*
+		 * According to config2 it would be 5-ways, but that is
+		 * contradicted by all documentation.
+		 */
+		case MACH_INGENIC_JZ4770:
+			c->scache.ways = 4;
+			break;
+
+		/*
+		 * According to config2 it would be 5-ways and 512-sets,
+		 * but that is contradicted by all documentation.
+		 */
+		case MACH_INGENIC_X1000:
+			c->scache.sets = 256;
+			c->scache.ways = 4;
+			break;
+		}
+	}
 
 	c->scache.waysize = c->scache.sets * c->scache.linesz;
 	c->scache.waybit = __ffs(c->scache.waysize);
-- 
2.7.4

[PATCH 2/2 v3] MIPS: Ingenic: Fix bugs when calculate bogomips/lpj.

[Zhou Yanjie]

Enable BTB lookups for short loops to fix bugs when calculate
bogomips and loops_per_jiffy.

Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
---
 arch/mips/include/asm/mipsregs.h | 4 ++++
 arch/mips/kernel/cpu-probe.c     | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 1e6966e..bdbdc19 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -689,6 +689,9 @@
 #define MIPS_CONF7_IAR		(_ULCAST_(1) << 10)
 #define MIPS_CONF7_AR		(_ULCAST_(1) << 16)
 
+/* Ingenic Config7 bits */
+#define MIPS_CONF7_BTB_LOOP_EN	(_ULCAST_(1) << 4)
+
 /* Config7 Bits specific to MIPS Technologies. */
 
 /* Performance counters implemented Per TC */
@@ -2813,6 +2816,7 @@ __BUILD_SET_C0(status)
 __BUILD_SET_C0(cause)
 __BUILD_SET_C0(config)
 __BUILD_SET_C0(config5)
+__BUILD_SET_C0(config7)
 __BUILD_SET_C0(intcontrol)
 __BUILD_SET_C0(intctl)
 __BUILD_SET_C0(srsmap)
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index eb527a1..2bdd3e1 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
 		c->cputype = CPU_XBURST;
 		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
 		__cpu_name[cpu] = "Ingenic XBurst";
+		/*
+		 * The XBurst core by default attempts to avoid branch target
+		 * buffer lookups by detecting & special casing loops. This
+		 * feature will cause BogoMIPS and lpj calculate in error.
+		 * Set cp0 config7 bit 4 to disable this feature.
+		 */
+		set_c0_config7(MIPS_CONF7_BTB_LOOP_EN);
 		break;
 	default:
 		panic("Unknown Ingenic Processor ID!");
-- 
2.7.4

Re: [PATCH 2/2 v3] MIPS: Ingenic: Fix bugs when calculate bogomips/lpj.

[Paul Cercueil]

Hi Zhou,



Le jeu. 1 août 2019 à 8:16, Zhou Yanjie <zhouyanjie@zoho.com> a 
écrit :
> Enable BTB lookups for short loops to fix bugs when calculate
> bogomips and loops_per_jiffy.

The commit description and the code comment below seem to say two
different things. Are we enabling the BTB lookup optimization, or not?

Also, maybe change the commit title to something more meaningful, e.g.
"MIPS: ingenic: Disable broken BTB lookup optimization" or similar.


> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
> ---
>  arch/mips/include/asm/mipsregs.h | 4 ++++
>  arch/mips/kernel/cpu-probe.c     | 7 +++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/arch/mips/include/asm/mipsregs.h 
> b/arch/mips/include/asm/mipsregs.h
> index 1e6966e..bdbdc19 100644
> --- a/arch/mips/include/asm/mipsregs.h
> +++ b/arch/mips/include/asm/mipsregs.h
> @@ -689,6 +689,9 @@
>  #define MIPS_CONF7_IAR		(_ULCAST_(1) << 10)
>  #define MIPS_CONF7_AR		(_ULCAST_(1) << 16)
> 
> +/* Ingenic Config7 bits */
> +#define MIPS_CONF7_BTB_LOOP_EN	(_ULCAST_(1) << 4)
> +
>  /* Config7 Bits specific to MIPS Technologies. */
> 
>  /* Performance counters implemented Per TC */
> @@ -2813,6 +2816,7 @@ __BUILD_SET_C0(status)
>  __BUILD_SET_C0(cause)
>  __BUILD_SET_C0(config)
>  __BUILD_SET_C0(config5)
> +__BUILD_SET_C0(config7)
>  __BUILD_SET_C0(intcontrol)
>  __BUILD_SET_C0(intctl)
>  __BUILD_SET_C0(srsmap)
> diff --git a/arch/mips/kernel/cpu-probe.c 
> b/arch/mips/kernel/cpu-probe.c
> index eb527a1..2bdd3e1 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct 
> cpuinfo_mips *c, unsigned int cpu)
>  		c->cputype = CPU_XBURST;
>  		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>  		__cpu_name[cpu] = "Ingenic XBurst";
> +		/*
> +		 * The XBurst core by default attempts to avoid branch target
> +		 * buffer lookups by detecting & special casing loops. This
> +		 * feature will cause BogoMIPS and lpj calculate in error.
> +		 * Set cp0 config7 bit 4 to disable this feature.
> +		 */
> +		set_c0_config7(MIPS_CONF7_BTB_LOOP_EN);

Shouldn't it be MIPS_CONF7_BTB_LOOP_DIS then?
Since the feature is disabled when the bit is set.


>  		break;
>  	default:
>  		panic("Unknown Ingenic Processor ID!");
> --
> 2.7.4
> 
> 

Re: [PATCH 2/2 v3] MIPS: Ingenic: Fix bugs when calculate bogomips/lpj.

[Zhou Yanjie]

On 2019年08月02日 09:26, Paul Cercueil wrote:
> Hi Zhou,
>
>
>
> Le jeu. 1 août 2019 à 8:16, Zhou Yanjie <zhouyanjie@zoho.com> a écrit :
>> Enable BTB lookups for short loops to fix bugs when calculate
>> bogomips and loops_per_jiffy.
>
> The commit description and the code comment below seem to say two
> different things. Are we enabling the BTB lookup optimization, or not?
>
By set config7 bit 4 we are disable the BTB lookup optimization and enable
BTB lookup for short loop.
> Also, maybe change the commit title to something more meaningful, e.g.
> "MIPS: ingenic: Disable broken BTB lookup optimization" or similar.
>
OK, I'll change it in v4.
>
>> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
>> ---
>>  arch/mips/include/asm/mipsregs.h | 4 ++++
>>  arch/mips/kernel/cpu-probe.c     | 7 +++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/arch/mips/include/asm/mipsregs.h 
>> b/arch/mips/include/asm/mipsregs.h
>> index 1e6966e..bdbdc19 100644
>> --- a/arch/mips/include/asm/mipsregs.h
>> +++ b/arch/mips/include/asm/mipsregs.h
>> @@ -689,6 +689,9 @@
>>  #define MIPS_CONF7_IAR        (_ULCAST_(1) << 10)
>>  #define MIPS_CONF7_AR        (_ULCAST_(1) << 16)
>>
>> +/* Ingenic Config7 bits */
>> +#define MIPS_CONF7_BTB_LOOP_EN    (_ULCAST_(1) << 4)
>> +
>>  /* Config7 Bits specific to MIPS Technologies. */
>>
>>  /* Performance counters implemented Per TC */
>> @@ -2813,6 +2816,7 @@ __BUILD_SET_C0(status)
>>  __BUILD_SET_C0(cause)
>>  __BUILD_SET_C0(config)
>>  __BUILD_SET_C0(config5)
>> +__BUILD_SET_C0(config7)
>>  __BUILD_SET_C0(intcontrol)
>>  __BUILD_SET_C0(intctl)
>>  __BUILD_SET_C0(srsmap)
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index eb527a1..2bdd3e1 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct 
>> cpuinfo_mips *c, unsigned int cpu)
>>          c->cputype = CPU_XBURST;
>>          c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>>          __cpu_name[cpu] = "Ingenic XBurst";
>> +        /*
>> +         * The XBurst core by default attempts to avoid branch target
>> +         * buffer lookups by detecting & special casing loops. This
>> +         * feature will cause BogoMIPS and lpj calculate in error.
>> +         * Set cp0 config7 bit 4 to disable this feature.
>> +         */
>> +        set_c0_config7(MIPS_CONF7_BTB_LOOP_EN);
>
> Shouldn't it be MIPS_CONF7_BTB_LOOP_DIS then?
> Since the feature is disabled when the bit is set.
>
According to Ingenic's explanation and Paul's old patch in

https://github.com/paulburton/linux/commit/0d72377bd615d00e99733adc0d37e6a2373fcde7

In order to further reduce power consumption, the XBurst core by default 
attempts
to avoid branch target buffer lookups by detecting & special casing 
loops, this is
what you mentioned as "BTB lookup optimization". Enabling this feature 
will avoid
branch target buffer lookups for loops. And now we are disable this 
feature, so it
should be "MIPS_CONF7_BTB_LOOP_EN".
>
>>          break;
>>      default:
>>          panic("Unknown Ingenic Processor ID!");
>> -- 
>> 2.7.4
>>
>>
>
>

MIPS: Ingenic: Fix bugs when detecting X1000's parameters v4

[Zhou Yanjie]

v1->v2: Use "set_c0_config7(BIT(4))" to simplify code and add comment.
v2->v3: Split patch and use "MIPS_CONF7_BTB_LOOP_EN" to instead "BIT(4)".
v3->v4: Change the commit title.

[PATCH 1/2 v4] MIPS: Ingenic: Fix bugs when detecting X1000's L2 cache.

[Zhou Yanjie]

1.fix bugs when detecting L2 cache sets value.
2.fix bugs when detecting L2 cache ways value.

Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
---
 arch/mips/mm/sc-mips.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
index 9385ddb..dbdbfe5 100644
--- a/arch/mips/mm/sc-mips.c
+++ b/arch/mips/mm/sc-mips.c
@@ -221,13 +221,26 @@ static inline int __init mips_sc_probe(void)
 	else
 		return 0;
 
-	/*
-	 * According to config2 it would be 5-ways, but that is contradicted
-	 * by all documentation.
-	 */
-	if (current_cpu_type() == CPU_XBURST &&
-				mips_machtype == MACH_INGENIC_JZ4770)
-		c->scache.ways = 4;
+	if (current_cpu_type() == CPU_XBURST) {
+		switch (mips_machtype) {
+		/*
+		 * According to config2 it would be 5-ways, but that is
+		 * contradicted by all documentation.
+		 */
+		case MACH_INGENIC_JZ4770:
+			c->scache.ways = 4;
+			break;
+
+		/*
+		 * According to config2 it would be 5-ways and 512-sets,
+		 * but that is contradicted by all documentation.
+		 */
+		case MACH_INGENIC_X1000:
+			c->scache.sets = 256;
+			c->scache.ways = 4;
+			break;
+		}
+	}
 
 	c->scache.waysize = c->scache.sets * c->scache.linesz;
 	c->scache.waybit = __ffs(c->scache.waysize);
-- 
2.7.4

[PATCH 2/2 v4] MIPS: Ingenic: Disable broken BTB lookup optimization.

[Zhou Yanjie]

In order to further reduce power consumption, the XBurst core
by default attempts to avoid branch target buffer lookups by
detecting & special casing loops. This feature will cause
BogoMIPS and lpj calculate in error. Set cp0 config7 bit 4 to
disable this feature.

Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
---
 arch/mips/include/asm/mipsregs.h | 4 ++++
 arch/mips/kernel/cpu-probe.c     | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 1e6966e..bdbdc19 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -689,6 +689,9 @@
 #define MIPS_CONF7_IAR		(_ULCAST_(1) << 10)
 #define MIPS_CONF7_AR		(_ULCAST_(1) << 16)
 
+/* Ingenic Config7 bits */
+#define MIPS_CONF7_BTB_LOOP_EN	(_ULCAST_(1) << 4)
+
 /* Config7 Bits specific to MIPS Technologies. */
 
 /* Performance counters implemented Per TC */
@@ -2813,6 +2816,7 @@ __BUILD_SET_C0(status)
 __BUILD_SET_C0(cause)
 __BUILD_SET_C0(config)
 __BUILD_SET_C0(config5)
+__BUILD_SET_C0(config7)
 __BUILD_SET_C0(intcontrol)
 __BUILD_SET_C0(intctl)
 __BUILD_SET_C0(srsmap)
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index eb527a1..2bdd3e1 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
 		c->cputype = CPU_XBURST;
 		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
 		__cpu_name[cpu] = "Ingenic XBurst";
+		/*
+		 * The XBurst core by default attempts to avoid branch target
+		 * buffer lookups by detecting & special casing loops. This
+		 * feature will cause BogoMIPS and lpj calculate in error.
+		 * Set cp0 config7 bit 4 to disable this feature.
+		 */
+		set_c0_config7(MIPS_CONF7_BTB_LOOP_EN);
 		break;
 	default:
 		panic("Unknown Ingenic Processor ID!");
-- 
2.7.4

Re: [PATCH 2/2 v3] MIPS: Ingenic: Fix bugs when calculate bogomips/lpj.

[Paul Burton]

Hi Paul,

On Thu, Aug 01, 2019 at 09:26:09PM -0400, Paul Cercueil wrote:
> > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> > index eb527a1..2bdd3e1 100644
> > --- a/arch/mips/kernel/cpu-probe.c
> > +++ b/arch/mips/kernel/cpu-probe.c
> > @@ -1964,6 +1964,13 @@ static inline void cpu_probe_ingenic(struct
> > cpuinfo_mips *c, unsigned int cpu)
> >  		c->cputype = CPU_XBURST;
> >  		c->writecombine = _CACHE_UNCACHED_ACCELERATED;
> >  		__cpu_name[cpu] = "Ingenic XBurst";
> > +		/*
> > +		 * The XBurst core by default attempts to avoid branch target
> > +		 * buffer lookups by detecting & special casing loops. This
> > +		 * feature will cause BogoMIPS and lpj calculate in error.
> > +		 * Set cp0 config7 bit 4 to disable this feature.
> > +		 */
> > +		set_c0_config7(MIPS_CONF7_BTB_LOOP_EN);
> 
> Shouldn't it be MIPS_CONF7_BTB_LOOP_DIS then?
> Since the feature is disabled when the bit is set.

The name comes from the fact that we're enabling loops to use the BTB,
in contrast to the default state where this bit is zero & the bad
optimization kicks in causing loops not to use the BTB.

Thanks,
    Paul

Re: [PATCH 1/2 v4] MIPS: Ingenic: Fix bugs when detecting X1000's L2 cache.

[Paul Burton]

Hello,

Zhou Yanjie wrote:
> 1.fix bugs when detecting L2 cache sets value.
> 2.fix bugs when detecting L2 cache ways value.

Series applied to mips-next.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]