* [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later
@ 2015-01-15 6:04 Dongsheng Wang
2015-01-15 6:11 ` Dongsheng.Wang at freescale.com
2015-01-20 0:33 ` Scott Wood
0 siblings, 2 replies; 4+ messages in thread
From: Dongsheng Wang @ 2015-01-15 6:04 UTC (permalink / raw)
To: u-boot
From: Wang Dongsheng <dongsheng.wang@freescale.com>
low power boot means u-boot will put non-boot cpus into a low power
status. Non-boot cpus don't need any more spin wait. e500, e500v2 will
going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state.
e6500rev2 will going to PW20 state.
e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up
by doorbell.
This feature tested on:
POWER UP TEST:
P1022DS(e500v2),96k times.
P4080(e500mc), 110k times.
T1024(e5500), 83k times.
T4240(e6500), 150k times.
CPU HOTPLUG TEST:
P1022DS(e500v2),1.4 million times.
P4080(e500mc), 1.8 million times.
T1024(e5500), 1.3 million times.
T4240(e6500), 1.1 million times.
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
diff --git a/arch/powerpc/cpu/mpc85xx/release.S b/arch/powerpc/cpu/mpc85xx/release.S
index a2c0ad4..a97a1b6 100644
--- a/arch/powerpc/cpu/mpc85xx/release.S
+++ b/arch/powerpc/cpu/mpc85xx/release.S
@@ -297,10 +297,15 @@ __secondary_start_page:
mtspr SPRN_MAS7,r11
tlbwe
+ li r6, 0
+
/*
* __bootpg_addr has the address of __second_half_boot_page
* jump there in AS=1 space with cache enabled
*/
+ .align 6
+ .global jump_half_boot_page
+jump_half_boot_page:
lis r13,toreset(__bootpg_addr)@h
ori r13,r13,toreset(__bootpg_addr)@l
lwz r11,0(r13)
@@ -371,6 +376,9 @@ __second_half_boot_page:
* };
* we pad this struct to 64 bytes so each entry is in its own cacheline
*/
+ cmpwi r6, 1
+ beq 3f
+
li r3,0
li r8,1
mfspr r4,SPRN_PIR
@@ -402,7 +410,132 @@ __second_half_boot_page:
#endif
lwz r4,ENTRY_ADDR_LOWER(r10)
andi. r11,r4,1
- bne 3b
+ beq 6f
+
+ li r6, 0
+ addi r6, r6, 1
+
+ /* External Interrupt exception. */
+ lis r7, toreset(jump_half_boot_page)@h
+ mtspr SPRN_IVPR, r7
+ li r7, toreset(jump_half_boot_page)@l
+
+#ifdef CONFIG_E500MC
+ /* e500MC, e5500, e6500 will use doorbell to send ipi signal */
+ mtspr SPRN_IVOR36, r7
+#endif
+
+ /*
+ * For e500mc later:
+ * EE will open in low power state, IVOR4 make sure we can ACK
+ * trash interrupt and keep we can loop in wait state again until
+ * the desired interrupt coming.
+ *
+ * e500, e500v2:
+ * Kernel will use MPCI to send ipi signal, so we must set IVOR4.
+ */
+ mtspr SPRN_IVOR4, r7
+
+ isync
+
+#ifdef CONFIG_E500MC
+ /* PW20 & AltiVec idle feature only exists for E6500 */
+ mfspr r0, SPRN_PVR
+ rlwinm r11, r0, 16, 16, 31
+ lis r12, 0
+ ori r12, r12, PVR_VER_E6500 at l
+ cmpw r11, r12
+ bne 5f
+
+ /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */
+ rlwinm r11, r0, 0, 16, 31
+ cmpwi r11, 0x20
+ blt 5f
+
+#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
+ /* Core Enter PW20 State */
+ mfspr r7, SPRN_PWRMGTCR0
+ /* Set PW20_WAIT bit, enable pw20 state*/
+ ori r7, r7, PWRMGTCR0_PW20_WAIT
+ li r9, PW20_WAIT_IDLE_BIT
+ /* Set Automatic PW20 Core Idle Count */
+ rlwimi r7, r9, PWRMGTCR0_PW20_ENT_SHIFT, PWRMGTCR0_PW20_ENT
+ mtspr SPRN_PWRMGTCR0, r7
+
+#define AV_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
+ /* Let Altivec Enter Idle State */
+ mfspr r7, SPRN_PWRMGTCR0
+ /* Enable Altivec Idle */
+ oris r7, r7, PWRMGTCR0_AV_IDLE_PD_EN at h
+ li r9, AV_WAIT_IDLE_BIT
+ /* Set Automatic AltiVec Idle Count */
+ rlwimi r7, r9, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT
+ mtspr SPRN_PWRMGTCR0, r7
+
+5:
+ /*
+ * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI
+ * signal.
+ *
+ * When kernel kick one of cpus, all cpus will be wakenup. To make
+ * sure that only the target cpu is effected, other cpus (by checking
+ * spin_table->addr_l) should go back to low power state.
+ *
+ * U-boot has renumber the cpu PIR Why we need to set all of PIR to
+ * the same value?
+ * A: Before kernel kicking cpu, the doorbell message was not configured
+ * for target cpu(cpu_messages->data). If we try to send a
+ * non-configured message to target cpu, it cannot correctly receive
+ * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
+ * let all cpus catch the interrupt.
+ *
+ * Why set PIR to zero?
+ * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us
+ * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
+ * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be
+ * there. So set PIR is zero as a default PIR ID for each CPUs.
+ */
+
+ li r7, 0
+ mtspr SPRN_PIR, r7
+
+ wrteei 1
+ wait
+#else
+ li r5, 0
+ lis r5, HID0_DOZE at h
+ mfspr r7, SPRN_HID0
+ rlwinm r7, r7, 0, ~(HID0_DOZE | HID0_NAP | HID0_SLEEP)
+ or r7, r7, r5
+ isync
+ mtspr SPRN_HID0, r7
+ isync
+
+ mfmsr r7
+ oris r7, r7, MSR_WE at h
+ ori r7, r7, MSR_EE
+ msync
+ mtmsr r7
+ isync
+5: bl 5b
+#endif
+6:
+ wrteei 0
+
+ /*
+ * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT
+ * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT.
+ */
+ mfspr r7, SPRN_EPR
+
+ /* Wait EOI finish */
+7:
+ lwz r7, ENTRY_PIR(r10)
+ andi. r7, r7, 0xffff
+ bne 8f
+ bl 7b
+8:
+
isync
/* get the upper bits of the addr */
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 2ed51b1..4d49317 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -482,6 +482,7 @@
#define SPRN_GIVOR8 0x1bb /* Guest Interrupt Vector Offset Register 8 */
#define SPRN_GIVOR13 0x1bc /* Guest Interrupt Vector Offset Register 13 */
#define SPRN_GIVOR14 0x1bd /* Guest Interrupt Vector Offset Register 14 */
+#define SPRN_EPR 0x2be /* External Proxy Register */
/* e500 definitions */
#define SPRN_L1CFG0 0x203 /* L1 Cache Configuration Register 0 */
@@ -565,6 +566,13 @@
#define SPRN_BBTAR 0x202 /* Branch Buffer Target Address Register */
#define SPRN_PID1 0x279 /* Process ID Register 1 */
#define SPRN_PID2 0x27a /* Process ID Register 2 */
+#define SPRN_PWRMGTCR0 0x3fb /* Power management control register 0 */
+#define PWRMGTCR0_PW20_WAIT (1 << 14) /* PW20 state enable bit */
+#define PWRMGTCR0_PW20_ENT_SHIFT 8
+#define PWRMGTCR0_PW20_ENT 0x3f00
+#define PWRMGTCR0_AV_IDLE_PD_EN (1 << 22) /* Altivec idle enable */
+#define PWRMGTCR0_AV_IDLE_CNT_SHIFT 16
+#define PWRMGTCR0_AV_IDLE_CNT 0x3f0000
#define SPRN_MCSR 0x23c /* Machine Check Syndrome register */
#define SPRN_MCAR 0x23d /* Machine Check Address register */
#define MCSR_MCS 0x80000000 /* Machine Check Summary */
--
2.1.0.27.g96db324
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later
2015-01-15 6:04 [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later Dongsheng Wang
@ 2015-01-15 6:11 ` Dongsheng.Wang at freescale.com
2015-01-20 0:33 ` Scott Wood
1 sibling, 0 replies; 4+ messages in thread
From: Dongsheng.Wang at freescale.com @ 2015-01-15 6:11 UTC (permalink / raw)
To: u-boot
Hi all,
Kernel patch link:
http://patchwork.ozlabs.org/patch/429259/
Regards,
-Dongsheng
> -----Original Message-----
> From: Dongsheng Wang [mailto:dongsheng.wang at freescale.com]
> Sent: Thursday, January 15, 2015 2:05 PM
> To: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472
> Cc: Jin Zhengxiong-R64188; u-boot at lists.denx.de; Wang Dongsheng-B40534
> Subject: [PATCH] powerpc/fsl: support low power boot for e500 and later
>
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
>
> low power boot means u-boot will put non-boot cpus into a low power
> status. Non-boot cpus don't need any more spin wait. e500, e500v2 will
> going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state.
> e6500rev2 will going to PW20 state.
>
> e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up
> by doorbell.
>
> This feature tested on:
> POWER UP TEST:
> P1022DS(e500v2),96k times.
> P4080(e500mc), 110k times.
> T1024(e5500), 83k times.
> T4240(e6500), 150k times.
>
> CPU HOTPLUG TEST:
> P1022DS(e500v2),1.4 million times.
> P4080(e500mc), 1.8 million times.
> T1024(e5500), 1.3 million times.
> T4240(e6500), 1.1 million times.
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
>
> diff --git a/arch/powerpc/cpu/mpc85xx/release.S
> b/arch/powerpc/cpu/mpc85xx/release.S
> index a2c0ad4..a97a1b6 100644
> --- a/arch/powerpc/cpu/mpc85xx/release.S
> +++ b/arch/powerpc/cpu/mpc85xx/release.S
> @@ -297,10 +297,15 @@ __secondary_start_page:
> mtspr SPRN_MAS7,r11
> tlbwe
>
> + li r6, 0
> +
> /*
> * __bootpg_addr has the address of __second_half_boot_page
> * jump there in AS=1 space with cache enabled
> */
> + .align 6
> + .global jump_half_boot_page
> +jump_half_boot_page:
> lis r13,toreset(__bootpg_addr)@h
> ori r13,r13,toreset(__bootpg_addr)@l
> lwz r11,0(r13)
> @@ -371,6 +376,9 @@ __second_half_boot_page:
> * };
> * we pad this struct to 64 bytes so each entry is in its own cacheline
> */
> + cmpwi r6, 1
> + beq 3f
> +
> li r3,0
> li r8,1
> mfspr r4,SPRN_PIR
> @@ -402,7 +410,132 @@ __second_half_boot_page:
> #endif
> lwz r4,ENTRY_ADDR_LOWER(r10)
> andi. r11,r4,1
> - bne 3b
> + beq 6f
> +
> + li r6, 0
> + addi r6, r6, 1
> +
> + /* External Interrupt exception. */
> + lis r7, toreset(jump_half_boot_page)@h
> + mtspr SPRN_IVPR, r7
> + li r7, toreset(jump_half_boot_page)@l
> +
> +#ifdef CONFIG_E500MC
> + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */
> + mtspr SPRN_IVOR36, r7
> +#endif
> +
> + /*
> + * For e500mc later:
> + * EE will open in low power state, IVOR4 make sure we can ACK
> + * trash interrupt and keep we can loop in wait state again until
> + * the desired interrupt coming.
> + *
> + * e500, e500v2:
> + * Kernel will use MPCI to send ipi signal, so we must set IVOR4.
> + */
> + mtspr SPRN_IVOR4, r7
> +
> + isync
> +
> +#ifdef CONFIG_E500MC
> + /* PW20 & AltiVec idle feature only exists for E6500 */
> + mfspr r0, SPRN_PVR
> + rlwinm r11, r0, 16, 16, 31
> + lis r12, 0
> + ori r12, r12, PVR_VER_E6500 at l
> + cmpw r11, r12
> + bne 5f
> +
> + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */
> + rlwinm r11, r0, 0, 16, 31
> + cmpwi r11, 0x20
> + blt 5f
> +
> +#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
> + /* Core Enter PW20 State */
> + mfspr r7, SPRN_PWRMGTCR0
> + /* Set PW20_WAIT bit, enable pw20 state*/
> + ori r7, r7, PWRMGTCR0_PW20_WAIT
> + li r9, PW20_WAIT_IDLE_BIT
> + /* Set Automatic PW20 Core Idle Count */
> + rlwimi r7, r9, PWRMGTCR0_PW20_ENT_SHIFT, PWRMGTCR0_PW20_ENT
> + mtspr SPRN_PWRMGTCR0, r7
> +
> +#define AV_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
> + /* Let Altivec Enter Idle State */
> + mfspr r7, SPRN_PWRMGTCR0
> + /* Enable Altivec Idle */
> + oris r7, r7, PWRMGTCR0_AV_IDLE_PD_EN at h
> + li r9, AV_WAIT_IDLE_BIT
> + /* Set Automatic AltiVec Idle Count */
> + rlwimi r7, r9, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT
> + mtspr SPRN_PWRMGTCR0, r7
> +
> +5:
> + /*
> + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI
> + * signal.
> + *
> + * When kernel kick one of cpus, all cpus will be wakenup. To make
> + * sure that only the target cpu is effected, other cpus (by checking
> + * spin_table->addr_l) should go back to low power state.
> + *
> + * U-boot has renumber the cpu PIR Why we need to set all of PIR to
> + * the same value?
> + * A: Before kernel kicking cpu, the doorbell message was not configured
> + * for target cpu(cpu_messages->data). If we try to send a
> + * non-configured message to target cpu, it cannot correctly receive
> + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
> + * let all cpus catch the interrupt.
> + *
> + * Why set PIR to zero?
> + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us
> + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
> + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be
> + * there. So set PIR is zero as a default PIR ID for each CPUs.
> + */
> +
> + li r7, 0
> + mtspr SPRN_PIR, r7
> +
> + wrteei 1
> + wait
> +#else
> + li r5, 0
> + lis r5, HID0_DOZE at h
> + mfspr r7, SPRN_HID0
> + rlwinm r7, r7, 0, ~(HID0_DOZE | HID0_NAP | HID0_SLEEP)
> + or r7, r7, r5
> + isync
> + mtspr SPRN_HID0, r7
> + isync
> +
> + mfmsr r7
> + oris r7, r7, MSR_WE at h
> + ori r7, r7, MSR_EE
> + msync
> + mtmsr r7
> + isync
> +5: bl 5b
> +#endif
> +6:
> + wrteei 0
> +
> + /*
> + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT
> + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT.
> + */
> + mfspr r7, SPRN_EPR
> +
> + /* Wait EOI finish */
> +7:
> + lwz r7, ENTRY_PIR(r10)
> + andi. r7, r7, 0xffff
> + bne 8f
> + bl 7b
> +8:
> +
> isync
>
> /* get the upper bits of the addr */
> diff --git a/arch/powerpc/include/asm/processor.h
> b/arch/powerpc/include/asm/processor.h
> index 2ed51b1..4d49317 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -482,6 +482,7 @@
> #define SPRN_GIVOR8 0x1bb /* Guest Interrupt Vector Offset Register 8 */
> #define SPRN_GIVOR13 0x1bc /* Guest Interrupt Vector Offset Register 13 */
> #define SPRN_GIVOR14 0x1bd /* Guest Interrupt Vector Offset Register 14 */
> +#define SPRN_EPR 0x2be /* External Proxy Register */
>
> /* e500 definitions */
> #define SPRN_L1CFG0 0x203 /* L1 Cache Configuration Register 0 */
> @@ -565,6 +566,13 @@
> #define SPRN_BBTAR 0x202 /* Branch Buffer Target Address Register */
> #define SPRN_PID1 0x279 /* Process ID Register 1 */
> #define SPRN_PID2 0x27a /* Process ID Register 2 */
> +#define SPRN_PWRMGTCR0 0x3fb /* Power management control register 0 */
> +#define PWRMGTCR0_PW20_WAIT (1 << 14) /* PW20 state enable bit */
> +#define PWRMGTCR0_PW20_ENT_SHIFT 8
> +#define PWRMGTCR0_PW20_ENT 0x3f00
> +#define PWRMGTCR0_AV_IDLE_PD_EN (1 << 22) /* Altivec idle enable */
> +#define PWRMGTCR0_AV_IDLE_CNT_SHIFT 16
> +#define PWRMGTCR0_AV_IDLE_CNT 0x3f0000
> #define SPRN_MCSR 0x23c /* Machine Check Syndrome register */
> #define SPRN_MCAR 0x23d /* Machine Check Address register */
> #define MCSR_MCS 0x80000000 /* Machine Check Summary */
> --
> 2.1.0.27.g96db324
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later
2015-01-15 6:04 [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later Dongsheng Wang
2015-01-15 6:11 ` Dongsheng.Wang at freescale.com
@ 2015-01-20 0:33 ` Scott Wood
[not found] ` <BN1PR03MB1883A87EC254EEF0CD9CBAE9D480@BN1PR03MB188.namprd03.prod.outlook.com>
1 sibling, 1 reply; 4+ messages in thread
From: Scott Wood @ 2015-01-20 0:33 UTC (permalink / raw)
To: u-boot
On Thu, 2015-01-15 at 14:04 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
>
> low power boot means u-boot will put non-boot cpus into a low power
> status. Non-boot cpus don't need any more spin wait. e500, e500v2 will
> going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state.
> e6500rev2 will going to PW20 state.
>
> e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up
> by doorbell.
This will break compatibility with existing kernels (and violate ePAPR
since you haven't changed the release-method string). It must be
optional and (for now) off by default.
What benefits are we getting for this churn and complexity? Do we
really need to optimize for the case where not all CPUs are used?
Where is this new mechanism documented?
> diff --git a/arch/powerpc/cpu/mpc85xx/release.S b/arch/powerpc/cpu/mpc85xx/release.S
> index a2c0ad4..a97a1b6 100644
> --- a/arch/powerpc/cpu/mpc85xx/release.S
> +++ b/arch/powerpc/cpu/mpc85xx/release.S
> @@ -297,10 +297,15 @@ __secondary_start_page:
> mtspr SPRN_MAS7,r11
> tlbwe
>
> + li r6, 0
> +
> /*
> * __bootpg_addr has the address of __second_half_boot_page
> * jump there in AS=1 space with cache enabled
> */
> + .align 6
> + .global jump_half_boot_page
> +jump_half_boot_page:
"half_boot_page" is confusing.
> lis r13,toreset(__bootpg_addr)@h
> ori r13,r13,toreset(__bootpg_addr)@l
> lwz r11,0(r13)
> @@ -371,6 +376,9 @@ __second_half_boot_page:
> * };
> * we pad this struct to 64 bytes so each entry is in its own cacheline
> */
> + cmpwi r6, 1
> + beq 3f
What does a value of 1 mean for r6? What about 0? Could you use
symbolic constants?
Why not just set the IVOR to where you really want to enter, rather than
conditionally branching there based on a value that happens to
correspond to whether you're entering via an interrupt?
> li r3,0
> li r8,1
> mfspr r4,SPRN_PIR
> @@ -402,7 +410,132 @@ __second_half_boot_page:
> #endif
> lwz r4,ENTRY_ADDR_LOWER(r10)
> andi. r11,r4,1
> - bne 3b
> + beq 6f
> +
> + li r6, 0
> + addi r6, r6, 1
Why not just "li r6, 1"?
> + /* External Interrupt exception. */
> + lis r7, toreset(jump_half_boot_page)@h
> + mtspr SPRN_IVPR, r7
> + li r7, toreset(jump_half_boot_page)@l
> +
> +#ifdef CONFIG_E500MC
> + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */
> + mtspr SPRN_IVOR36, r7
> +#endif
> +
> + /*
> + * For e500mc later:
> + * EE will open in low power state, IVOR4 make sure we can ACK
> + * trash interrupt and keep we can loop in wait state again until
> + * the desired interrupt coming.
I don't understand "EE will open". Do you mean "EE will be set"?
Why would we get unexpected interrupts?
> + *
> + * e500, e500v2:
> + * Kernel will use MPCI to send ipi signal, so we must set IVOR4.
> + */
MPIC
> + mtspr SPRN_IVOR4, r7
> +
> + isync
> +
> +#ifdef CONFIG_E500MC
> + /* PW20 & AltiVec idle feature only exists for E6500 */
> + mfspr r0, SPRN_PVR
> + rlwinm r11, r0, 16, 16, 31
> + lis r12, 0
> + ori r12, r12, PVR_VER_E6500 at l
> + cmpw r11, r12
> + bne 5f
> +
> + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */
> + rlwinm r11, r0, 0, 16, 31
> + cmpwi r11, 0x20
> + blt 5f
PW10 isn't enough here?
> +#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
If you must use PW20 please set the timeout long enough that it's
obvious we won't hit it during normal bootup.
> + /*
> + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI
> + * signal.
> + *
> + * When kernel kick one of cpus, all cpus will be wakenup. To make
> + * sure that only the target cpu is effected, other cpus (by checking
> + * spin_table->addr_l) should go back to low power state.
> + *
> + * U-boot has renumber the cpu PIR Why we need to set all of PIR to
> + * the same value?
> + * A: Before kernel kicking cpu, the doorbell message was not configured
> + * for target cpu(cpu_messages->data). If we try to send a
> + * non-configured message to target cpu, it cannot correctly receive
> + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
> + * let all cpus catch the interrupt.
> + *
> + * Why set PIR to zero?
> + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us
> + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
> + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be
> + * there. So set PIR is zero as a default PIR ID for each CPUs.
What does NR_CPUS have to do with the appropriate PIR for each CPU?
> + /*
> + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT
> + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT.
> + */
> + mfspr r7, SPRN_EPR
Where do you limit this to cases when external proxy is enabled? Why
would we care about external proxy at all? The only chips we use IPIs
on don't have external proxy.
> +
> + /* Wait EOI finish */
> +7:
> + lwz r7, ENTRY_PIR(r10)
> + andi. r7, r7, 0xffff
> + bne 8f
> + bl 7b
> +8:
Why are you reading PIR in a loop? What does that have to do with an
EOI finishing?
-Scott
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later
[not found] ` <BN1PR03MB1883A87EC254EEF0CD9CBAE9D480@BN1PR03MB188.namprd03.prod.outlook.com>
@ 2015-01-21 8:23 ` Scott Wood
0 siblings, 0 replies; 4+ messages in thread
From: Scott Wood @ 2015-01-21 8:23 UTC (permalink / raw)
To: u-boot
On Wed, 2015-01-21 at 01:23 -0600, Wang Dongsheng-B40534 wrote:
>
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, January 20, 2015 8:33 AM
> > To: Wang Dongsheng-B40534
> > Cc: Sun York-R58495; Li Yang-Leo-R58472; Jin Zhengxiong-R64188; u-
> > boot at lists.denx.de
> > Subject: Re: [PATCH] powerpc/fsl: support low power boot for e500 and later
> >
> > On Thu, 2015-01-15 at 14:04 +0800, Dongsheng Wang wrote:
> > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >
> > > low power boot means u-boot will put non-boot cpus into a low power
> > > status. Non-boot cpus don't need any more spin wait. e500, e500v2 will
> > > going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state.
> > > e6500rev2 will going to PW20 state.
> > >
> > > e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up
> > > by doorbell.
> >
> > This will break compatibility with existing kernels (and violate ePAPR
> > since you haven't changed the release-method string). It must be
> > optional and (for now) off by default.
> >
>
> Agree.
>
> > What benefits are we getting for this churn and complexity? Do we
> > really need to optimize for the case where not all CPUs are used?
> >
>
> Save the power is our purpose no matter at any stage. If we can save the power
> that we need to do it.
No, we don't "need to do it" if the savings are negligible compared to
the complexity, or if it's for an obscure use case.
Worse, this could end up increasing power savings due to quickly
entering and then leaving low power mode (unlikely with PW10, but if you
end up in PW20 during the normal boot sequence, it could).
> > Where is this new mechanism documented?
> >
>
> I don't understand. Do you mean Hardware datasheet?
No, I mean software documentation for the new bootloader/kernel
interface you're creating. The existing interface is documented in
ePAPR.
> > > diff --git a/arch/powerpc/cpu/mpc85xx/release.S
> > b/arch/powerpc/cpu/mpc85xx/release.S
> > > index a2c0ad4..a97a1b6 100644
> > > --- a/arch/powerpc/cpu/mpc85xx/release.S
> > > +++ b/arch/powerpc/cpu/mpc85xx/release.S
> > > @@ -297,10 +297,15 @@ __secondary_start_page:
> > > mtspr SPRN_MAS7,r11
> > > tlbwe
> > >
> > > + li r6, 0
> > > +
> > > /*
> > > * __bootpg_addr has the address of __second_half_boot_page
> > > * jump there in AS=1 space with cache enabled
> > > */
> > > + .align 6
> > > + .global jump_half_boot_page
> > > +jump_half_boot_page:
> >
> > "half_boot_page" is confusing.
> >
>
> Why there are confused?
I suspect you mean "jump to __second_half_boot_page" but it just doesn't
read that way when you omit the "second" (and it doesn't help that the
meaning of "second half" wasn't immediately obvious from context). It
makes me ask questions like, "Is there a first_half_boot_page?" or "Is
there a full_boot_page?"
> The address of __bootpg_addr is not a physical address.
> Boot page will jump to a virtual address.
What does that have to do with the label name?
> > > lis r13,toreset(__bootpg_addr)@h
> > > ori r13,r13,toreset(__bootpg_addr)@l
> > > lwz r11,0(r13)
> > > @@ -371,6 +376,9 @@ __second_half_boot_page:
> > > * };
> > > * we pad this struct to 64 bytes so each entry is in its own cacheline
> > > */
> > > + cmpwi r6, 1
> > > + beq 3f
> >
> > What does a value of 1 mean for r6? What about 0? Could you use
> > symbolic constants?
> >
>
> Ok, replace the magic number with macro.
>
> 1 means the current process is come from kicked up flow.
> 0 means the current process is first boot up flow.
>
> > Why not just set the IVOR to where you really want to enter, rather than
> > conditionally branching there based on a value that happens to
> > correspond to whether you're entering via an interrupt?
> >
>
> Now the value of IVOR is jump_half_boot_page. In __bootpg_addr I can check the waken
> up cpu that is kernel want to kick. If not the cpu will loop in low power
> state again.
Whatever it is that you want to check for, the first thing you do when
you enter the interrupt is branch to 3f. How does setting IVOR to
jump_half_boot_page rather than 3: help with what you're trying to do?
> > > li r3,0
> > > li r8,1
> > > mfspr r4,SPRN_PIR
> > > @@ -402,7 +410,132 @@ __second_half_boot_page:
> > > #endif
> > > lwz r4,ENTRY_ADDR_LOWER(r10)
> > > andi. r11,r4,1
> > > - bne 3b
> > > + beq 6f
> > > +
> > > + li r6, 0
> > > + addi r6, r6, 1
> >
> > Why not just "li r6, 1"?
> >
>
> Mask the cpu that cpu fall into low power state.
Forget the reason -- just tell me how the resulting state of "li r6,0;
addi r6, r6, 1" will be different from "li r6, 1".
> > > + /* External Interrupt exception. */
> > > + lis r7, toreset(jump_half_boot_page)@h
> > > + mtspr SPRN_IVPR, r7
> > > + li r7, toreset(jump_half_boot_page)@l
> > > +
> > > +#ifdef CONFIG_E500MC
> > > + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */
> > > + mtspr SPRN_IVOR36, r7
> > > +#endif
> > > +
> > > + /*
> > > + * For e500mc later:
> > > + * EE will open in low power state, IVOR4 make sure we can ACK
> > > + * trash interrupt and keep we can loop in wait state again until
> > > + * the desired interrupt coming.
> >
> > I don't understand "EE will open". Do you mean "EE will be set"?
>
> Thanks... Yes, EE will be set.
>
> > Why would we get unexpected interrupts?
> >
>
> When EE be set, cpu can receive external interrupt.
But what else would be sending an interrupt to this CPU at this point?
> If a cpu is not kicked in kernel that be triggered by external interrupt, we
> need to make sure the cpu can be loop in low power again.
No, you can just say that the first interrupt will wake the CPU and it's
up to the OS to program the MPIC properly.
> > > + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */
> > > + rlwinm r11, r0, 0, 16, 31
> > > + cmpwi r11, 0x20
> > > + blt 5f
> >
> > PW10 isn't enough here?
> >
>
> e6500rev2 support PW20, the PW20 lower than PW10, Why not?
See the beginning of the mail.
> > > +#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
> >
> > If you must use PW20 please set the timeout long enough that it's
> > obvious we won't hit it during normal bootup.
> >
>
> Sorry, I don?t understand "obvious we won't hit it"...
What I mean is that we do not want to enter PW20 during normal boot.
The precise time it takes before the OS releases the secondary CPUs can
vary, so the timeout should be large enough that we are highly confident
that the CPUs will be released before the PW20 timer expires.
> > > + /*
> > > + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI
> > > + * signal.
> > > + *
> > > + * When kernel kick one of cpus, all cpus will be wakenup. To make
> > > + * sure that only the target cpu is effected, other cpus (by checking
> > > + * spin_table->addr_l) should go back to low power state.
> > > + *
> > > + * U-boot has renumber the cpu PIR Why we need to set all of PIR to
> > > + * the same value?
> > > + * A: Before kernel kicking cpu, the doorbell message was not configured
> > > + * for target cpu(cpu_messages->data). If we try to send a
> > > + * non-configured message to target cpu, it cannot correctly receive
> > > + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
> > > + * let all cpus catch the interrupt.
> > > + *
> > > + * Why set PIR to zero?
> > > + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us
> > > + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
> > > + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be
> > > + * there. So set PIR is zero as a default PIR ID for each CPUs.
> >
> > What does NR_CPUS have to do with the appropriate PIR for each CPU?
> >
>
> PIR is for each CPU.
I am aware of this.
> The NR_CPUS just a number of kernel control to start the CPU.
It's the maximum number of CPUs that the kernel will support.
> We need to set the same value for each CPU PIR register.
No, we don't.
> Please see my comments for Why we need to do it.
If you think you've answered something and I ask again, it generally
means I'm having a hard time understanding and it would be helpful to
reword rather than just copying and pasting or telling me to read them
again (I think I finally understand what you meant, but it's not clear).
> * U-boot has renumber the cpu PIR Why we need to set all of PIR to
> * the same value?
> * A: Before kernel kicking cpu, the doorbell message was not configured
> * for target cpu(cpu_messages->data). If we try to send a
> * non-configured message to target cpu, it cannot correctly receive
> * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
> * let all cpus catch the interrupt.
So the problem is that you're using a Linux function to send the msgsnd
before it was meant to be called. Don't do that. Either set up the
required data earlier, or use a lower level function that doesn't have
that prerequisite.
> > > + /*
> > > + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT
> > > + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT.
> > > + */
> > > + mfspr r7, SPRN_EPR
> >
> > Where do you limit this to cases when external proxy is enabled? Why
> > would we care about external proxy at all? The only chips we use IPIs
> > on don't have external proxy.
> >
>
> In kernel the MPCI IPI message not be supported on CORENET platform and later.
Yes, it is. We just don't use it.
> After e500v2, proxy mode always enable in MPIC.
No, it's not. It has to be manually enabled by the kernel. U-Boot
cannot know if the kernel has done this (without reading the relevant
MPIC register).
Neither of the above two sentences answer my questions. What stops you
from trying to read EPR on e500v2?
-Scott
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-21 8:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-15 6:04 [U-Boot] [PATCH] powerpc/fsl: support low power boot for e500 and later Dongsheng Wang
2015-01-15 6:11 ` Dongsheng.Wang at freescale.com
2015-01-20 0:33 ` Scott Wood
[not found] ` <BN1PR03MB1883A87EC254EEF0CD9CBAE9D480@BN1PR03MB188.namprd03.prod.outlook.com>
2015-01-21 8:23 ` Scott Wood
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.