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.
v1->v2: Use "set_c0_config7(BIT(4))" to simplify code and add comment.
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
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)".
1.fix bugs when detecting L2 cache sets value.
2.fix bugs when detecting L2 cache ways value.
Signed-off-by: Zhou Yanjie <[email protected]>
---
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
Enable BTB lookups for short loops to fix bugs when calculate
bogomips and loops_per_jiffy.
Signed-off-by: Zhou Yanjie <[email protected]>
---
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
Hi Zhou,
Le jeu. 1 ao?t 2019 ? 8:16, Zhou Yanjie <[email protected]> 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 <[email protected]>
> ---
> 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
>
>
On 2019年08月02日 09:26, Paul Cercueil wrote:
> Hi Zhou,
>
>
>
> Le jeu. 1 août 2019 à 8:16, Zhou Yanjie <[email protected]> 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 <[email protected]>
>> ---
>> 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
>>
>>
>
>
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.
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 <[email protected]>
---
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
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