| Message ID | 1564661791-47731-3-git-send-email-zhouyanjie@zoho.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2,v3] MIPS: Ingenic: Fix bugs when detecting X1000's L2 cache. | expand |
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 > >
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 >> >> > >
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
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!");
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(+)