| Message ID | 1376637789-27330-1-git-send-email-dongsheng.wang@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Aug 16, 2013, at 2:23 AM, Dongsheng Wang wrote: > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > Each core's AltiVec unit may be placed into a power savings mode > by turning off power to the unit. Core hardware will automatically > power down the AltiVec unit after no AltiVec instructions have > executed in N cycles. The AltiVec power-control is triggered by hardware. > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> Why treat this as a idle HW governor vs just some one time setup at boot of the time delay? - k
On Fri, 2013-08-16 at 06:02 -0500, Kumar Gala wrote: > On Aug 16, 2013, at 2:23 AM, Dongsheng Wang wrote: > > > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > > > Each core's AltiVec unit may be placed into a power savings mode > > by turning off power to the unit. Core hardware will automatically > > power down the AltiVec unit after no AltiVec instructions have > > executed in N cycles. The AltiVec power-control is triggered by hardware. > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> > > Why treat this as a idle HW governor vs just some one time setup at boot of the time delay? It is being done as one-time setup, despite the function name. Maybe it should be moved into __setup/restore_cpu_e6500 (BTW, we really should refactor those to reduce duplication) with the timebase bit number hardcoded rather than a time in us. As for the PVR check, the upstream kernel doesn't need to care about rev1, so knowing it's an e6500 is good enough. -Scott
Thanks for your feedback. > -----Original Message----- > From: Wood Scott-B07421 > Sent: Saturday, August 17, 2013 12:51 AM > To: Kumar Gala > Cc: Wang Dongsheng-B40534; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/2] powerpc/85xx: add hardware automatically enter > altivec idle state > > On Fri, 2013-08-16 at 06:02 -0500, Kumar Gala wrote: > > On Aug 16, 2013, at 2:23 AM, Dongsheng Wang wrote: > > > > > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > > > > > Each core's AltiVec unit may be placed into a power savings mode > > > by turning off power to the unit. Core hardware will automatically > > > power down the AltiVec unit after no AltiVec instructions have > > > executed in N cycles. The AltiVec power-control is triggered by > hardware. > > > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> > > > > Why treat this as a idle HW governor vs just some one time setup at > boot of the time delay? > > It is being done as one-time setup, despite the function name. > > Maybe it should be moved into __setup/restore_cpu_e6500 (BTW, we really > should refactor those to reduce duplication) with the timebase bit > number hardcoded rather than a time in us. > The frequency of the different platforms timebase is not the same. If we use it, we need to set different timebase bit on each platform. That is why I did not put the code in __setup/restore_cpu_e6500, I need use tb_ticks_per_usec to get timebase bit. So we only need to set a time for each platform, and not set different timebase bit. > As for the PVR check, the upstream kernel doesn't need to care about > rev1, so knowing it's an e6500 is good enough. > But AltiVec idle & PW20 cannot work on rev1 platform. We didn't have to deal with it? -dongsheng
On Sun, 2013-08-18 at 21:53 -0500, Wang Dongsheng-B40534 wrote: > Thanks for your feedback. > > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Saturday, August 17, 2013 12:51 AM > > To: Kumar Gala > > Cc: Wang Dongsheng-B40534; linuxppc-dev@lists.ozlabs.org > > Subject: Re: [PATCH 1/2] powerpc/85xx: add hardware automatically enter > > altivec idle state > > > > On Fri, 2013-08-16 at 06:02 -0500, Kumar Gala wrote: > > > On Aug 16, 2013, at 2:23 AM, Dongsheng Wang wrote: > > > > > > > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > > > > > > > Each core's AltiVec unit may be placed into a power savings mode > > > > by turning off power to the unit. Core hardware will automatically > > > > power down the AltiVec unit after no AltiVec instructions have > > > > executed in N cycles. The AltiVec power-control is triggered by > > hardware. > > > > > > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> > > > > > > Why treat this as a idle HW governor vs just some one time setup at > > boot of the time delay? > > > > It is being done as one-time setup, despite the function name. > > > > Maybe it should be moved into __setup/restore_cpu_e6500 (BTW, we really > > should refactor those to reduce duplication) with the timebase bit > > number hardcoded rather than a time in us. > > > The frequency of the different platforms timebase is not the same. It's close enough. Remember, this number is a vague initial guess, not something that's been carefully calibrated. Though, it would make it harder to substitute the number with one that's been more carefully calibrated... but wouldn't different chips possibly have different optimal delays anyway? > If we use it, we need to set different timebase bit on each platform. > That is why I did not put the code in __setup/restore_cpu_e6500, I need > use tb_ticks_per_usec to get timebase bit. So we only need to set a time > for each platform, and not set different timebase bit. It just seems wrong to have an ad-hoc mechanism for running core-specific code when we have cputable... If we really need this, maybe we should add a "cpu_setup_late" function pointer. With your patch, when does the power management register get set when hot plugging a cpu? > > As for the PVR check, the upstream kernel doesn't need to care about > > rev1, so knowing it's an e6500 is good enough. > > > But AltiVec idle & PW20 cannot work on rev1 platform. > We didn't have to deal with it? Upstream does not run on rev1. -Scott
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 5d7d9c2..5c7a7ba 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -1053,6 +1053,8 @@ #define PVR_8560 0x80200000 #define PVR_VER_E500V1 0x8020 #define PVR_VER_E500V2 0x8021 +#define PVR_VER_E6500 0x8040 + /* * For the 8xx processors, all of them report the same PVR family for * the PowerPC core. The various versions of these processors must be diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h index b417de3..c047e08 100644 --- a/arch/powerpc/include/asm/reg_booke.h +++ b/arch/powerpc/include/asm/reg_booke.h @@ -170,6 +170,7 @@ #define SPRN_L2CSR1 0x3FA /* L2 Data Cache Control and Status Register 1 */ #define SPRN_DCCR 0x3FA /* Data Cache Cacheability Register */ #define SPRN_ICCR 0x3FB /* Instruction Cache Cacheability Register */ +#define SPRN_PWRMGTCR0 0x3FB /* Power management control register 0 */ #define SPRN_SVR 0x3FF /* System Version Register */ /* @@ -216,6 +217,9 @@ #define CCR1_DPC 0x00000100 /* Disable L1 I-Cache/D-Cache parity checking */ #define CCR1_TCS 0x00000080 /* Timer Clock Select */ +/* Bit definitions for PWRMGTCR0. */ +#define PWRMGTCR0_ALTIVEC_IDLE (1 << 22) /* Altivec idle enable */ + /* Bit definitions for the MCSR. */ #define MCSR_MCS 0x80000000 /* Machine Check Summary */ #define MCSR_IB 0x40000000 /* Instruction PLB Error */ diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c index d0861a0..dbbbc24 100644 --- a/arch/powerpc/platforms/85xx/common.c +++ b/arch/powerpc/platforms/85xx/common.c @@ -7,10 +7,22 @@ */ #include <linux/of_platform.h> +#include <asm/time.h> + #include <sysdev/cpm2_pic.h> #include "mpc85xx.h" +#define MAX_BIT 64 + +#define ALTIVEC_COUNT_OFFSET 16 +#define ALTIVEC_IDLE_COUNT_MASK 0x003f0000 + +/* + * FIXME - We don't know the AltiVec application scenarios. + */ +#define ALTIVEC_IDLE_TIME 1000 /* 1ms */ + static struct of_device_id __initdata mpc85xx_common_ids[] = { { .type = "soc", }, { .compatible = "soc", }, @@ -80,3 +92,63 @@ void __init mpc85xx_cpm2_pic_init(void) irq_set_chained_handler(irq, cpm2_cascade); } #endif + +static bool has_pw20_altivec_idle(void) +{ + u32 pvr; + + pvr = mfspr(SPRN_PVR); + + /* PW20 & AltiVec idle feature only exists for E6500 */ + if (PVR_VER(pvr) != PVR_VER_E6500) + return false; + + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */ + if (PVR_REV(pvr) < 0x20) + return false; + + return true; +} + +static unsigned int get_idle_ticks_bit(unsigned int us) +{ + unsigned int cycle; + + /* + * The time control by TB turn over bit, so we need + * to be divided by 2. + */ + cycle = (us / 2) * tb_ticks_per_usec; + + return ilog2(cycle) + 1; +} + +static void setup_altivec_idle(void *unused) +{ + u32 altivec_idle, bit; + + if (!has_pw20_altivec_idle()) + return; + + /* Enable Altivec Idle */ + altivec_idle = mfspr(SPRN_PWRMGTCR0); + altivec_idle |= PWRMGTCR0_ALTIVEC_IDLE; + + /* Set Automatic AltiVec Idle Count */ + /* clear count */ + altivec_idle &= ~ALTIVEC_IDLE_COUNT_MASK; + + /* set count */ + bit = get_idle_ticks_bit(ALTIVEC_IDLE_TIME); + altivec_idle |= ((MAX_BIT - bit) << ALTIVEC_COUNT_OFFSET); + + mtspr(SPRN_PWRMGTCR0, altivec_idle); +} + +static int __init setup_idle_hw_governor(void) +{ + on_each_cpu(setup_altivec_idle, NULL, 1); + + return 0; +} +late_initcall(setup_idle_hw_governor);