Hi,
This patch series adds support for AFTR idle mode on boards with
secure firmware enabled and allows EXYNOS cpuidle driver usage on
Exynos4x12 SoCs.
It has been tested on Trats2 board (using Exynos4412 SoC with secure
firmware enabled) on which AFTR mode reduces power consumption by ~12%
when EXYNOS cpuidle driver is enabled (in both cases the default
exynos_defconfig config is used and CPU1-3 are offlined).
v3:
- rebased on top of next-20140708 +
http://www.mail-archive.com/[email protected]/msg32809.html
(with rejects fixed)
http://www.mail-archive.com/[email protected]/msg32808.html
http://www.mail-archive.com/[email protected]/msg32991.html
(with rejects fixed in patch #2)
- addressed review comments from Tomasz Figa and Daniel Lezcano
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz (4):
ARM: EXYNOS: PM: replace EXYNOS_BOOT_VECTOR_* macros by static inlines
ARM: EXYNOS: add AFTR mode support to firmware do_idle method
ARM: EXYNOS: cpuidle: add secure firmware support to AFTR mode code
ARM: EXYNOS: cpuidle: allow driver usage on Exynos4x12 SoCs
arch/arm/include/asm/firmware.h | 2 +-
arch/arm/mach-exynos/common.h | 7 ++++++-
arch/arm/mach-exynos/exynos.c | 3 ++-
arch/arm/mach-exynos/firmware.c | 36 ++++++++++++++++++++++++++----------
arch/arm/mach-exynos/pm.c | 39 +++++++++++++++++++++++++++++----------
drivers/cpuidle/cpuidle-exynos.c | 6 +++---
6 files changed, 67 insertions(+), 26 deletions(-)
--
1.8.2.3
Replace EXYNOS_BOOT_VECTOR_ADDR and EXYNOS_BOOT_VECTOR_FLAG macros
by exynos_boot_vector_addr() and exynos_boot_vector_flag() static
inlines.
This patch shouldn't cause any functionality changes.
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by: Kyungmin Park <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
v3:
- added Acked-by from Daniel
arch/arm/mach-exynos/pm.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index ed20318..c722454 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -167,12 +167,23 @@ int exynos_cluster_power_state(int cluster)
S5P_CORE_LOCAL_PWR_EN);
}
-#define EXYNOS_BOOT_VECTOR_ADDR (samsung_rev() == EXYNOS4210_REV_1_1 ? \
- S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
- (sysram_base_addr + 0x24) : S5P_INFORM0))
-#define EXYNOS_BOOT_VECTOR_FLAG (samsung_rev() == EXYNOS4210_REV_1_1 ? \
- S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
- (sysram_base_addr + 0x20) : S5P_INFORM1))
+static inline void __iomem *exynos_boot_vector_addr(void)
+{
+ if (samsung_rev() == EXYNOS4210_REV_1_1)
+ return S5P_INFORM7;
+ else if (samsung_rev() == EXYNOS4210_REV_1_0)
+ return sysram_base_addr + 0x24;
+ return S5P_INFORM0;
+}
+
+static inline void __iomem *exynos_boot_vector_flag(void)
+{
+ if (samsung_rev() == EXYNOS4210_REV_1_1)
+ return S5P_INFORM6;
+ else if (samsung_rev() == EXYNOS4210_REV_1_0)
+ return sysram_base_addr + 0x20;
+ return S5P_INFORM1;
+}
#define S5P_CHECK_AFTR 0xFCBA0D10
#define S5P_CHECK_SLEEP 0x00000BAD
@@ -185,8 +196,9 @@ static void exynos_set_wakeupmask(long mask)
static void exynos_cpu_set_boot_vector(long flags)
{
- __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR);
- __raw_writel(flags, EXYNOS_BOOT_VECTOR_FLAG);
+ __raw_writel(virt_to_phys(exynos_cpu_resume),
+ exynos_boot_vector_addr());
+ __raw_writel(flags, exynos_boot_vector_flag());
}
void exynos_enter_aftr(void)
--
1.8.2.3
On some platforms (i.e. EXYNOS ones) more than one idle mode is
available and we need to distinguish them in firmware do_idle method.
Add mode parameter to do_idle firmware method and AFTR mode support
to EXYNOS do_idle implementation.
This change is a preparation for adding secure firmware support to
EXYNOS cpuidle driver.
This patch shouldn't cause any functionality changes (please note
that do_idle firmware method is unused currently).
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by: Kyungmin Park <[email protected]>
---
v3:
- convert 'mode' parameter from int to unsigned long
- rename FW_DO_IDLE_NORMAL to FW_DO_IDLE_SLEEP
- move FW_DO_IDLE_* enums to common.h
arch/arm/include/asm/firmware.h | 2 +-
arch/arm/mach-exynos/common.h | 5 +++++
arch/arm/mach-exynos/firmware.c | 10 ++++++++--
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
index 5904f59..89aefe1 100644
--- a/arch/arm/include/asm/firmware.h
+++ b/arch/arm/include/asm/firmware.h
@@ -28,7 +28,7 @@ struct firmware_ops {
/*
* Enters CPU idle mode
*/
- int (*do_idle)(void);
+ int (*do_idle)(unsigned long mode);
/*
* Sets boot address of specified physical CPU
*/
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 152b464..297ac39 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -115,6 +115,11 @@ extern void __iomem *sysram_ns_base_addr;
extern void __iomem *sysram_base_addr;
void exynos_sysram_init(void);
+enum {
+ FW_DO_IDLE_SLEEP,
+ FW_DO_IDLE_AFTR,
+};
+
void exynos_firmware_init(void);
extern u32 exynos_get_eint_wake_mask(void);
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index e50b5ec..a52dd975 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -27,9 +27,15 @@
#define EXYNOS_BOOT_ADDR 0x8
#define EXYNOS_BOOT_FLAG 0xc
-static int exynos_do_idle(void)
+static int exynos_do_idle(unsigned long mode)
{
- exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
+ switch (mode) {
+ case FW_DO_IDLE_AFTR:
+ exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
+ break;
+ case FW_DO_IDLE_SLEEP:
+ exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
+ }
return 0;
}
--
1.8.2.3
Register cpuidle platform device on Exynos4x12 SoCs allowing EXYNOS
cpuidle driver usage on these SoCs.
AFTR mode reduces power consumption on Trats2 board (Exynos4412 SoC
with secure firmware enabled) by ~12% when EXYNOS cpuidle driver is
enabled (in both cases the default exynos_defconfig config is used
and CPU1-3 are offlined).
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by: Kyungmin Park <[email protected]>
---
v3:
- new patch
arch/arm/mach-exynos/exynos.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 4361abd..23003d9 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -173,7 +173,8 @@ static struct platform_device exynos_cpuidle = {
void __init exynos_cpuidle_init(void)
{
- if (soc_is_exynos4210() || soc_is_exynos5250())
+ if (soc_is_exynos4210() || soc_is_exynos4212() ||
+ soc_is_exynos4412() || soc_is_exynos5250())
platform_device_register(&exynos_cpuidle);
}
--
1.8.2.3
* Move cp15 registers saving to exynos_save_cp15() helper and add
additional helper usage to do_idle firmware method.
* Use sysram_ns_base_addr + 0x24/0x20 addresses instead of the default
ones used by exynos_cpu_set_boot_vector() on boards with secure
firmware enabled.
* Use do_idle firmware method instead of cpu_do_idle() on boards with
secure firmware enabled.
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by: Kyungmin Park <[email protected]>
---
v3:
- make exynos_enter_aftr() return a value
- add cp15 registers handling to do_idle firmware method
- set sysram_ns_base_addr + 0x24/0x20 in do_idle firmware method
- move calling of do_idle firmware method from cpuidle-exynos.c
to pm.c
arch/arm/mach-exynos/common.h | 2 +-
arch/arm/mach-exynos/firmware.c | 26 ++++++++++++++++++--------
arch/arm/mach-exynos/pm.c | 11 +++++++++--
drivers/cpuidle/cpuidle-exynos.c | 6 +++---
4 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index a6a200f..0829808 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -170,7 +170,7 @@ extern int exynos_cpu_power_state(int cpu);
extern void exynos_cluster_power_down(int cluster);
extern void exynos_cluster_power_up(int cluster);
extern int exynos_cluster_power_state(int cluster);
-extern void exynos_enter_aftr(void);
+extern int exynos_enter_aftr(void);
extern void s5p_init_cpu(void __iomem *cpuid_addr);
extern unsigned int samsung_rev(void);
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 53fbf5c..163f5b9 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -24,13 +24,30 @@
#include "smc.h"
#define EXYNOS_SLEEP_MAGIC 0x00000bad
+#define EXYNOS_AFTR_MAGIC 0xfcba0d10
#define EXYNOS_BOOT_ADDR 0x8
#define EXYNOS_BOOT_FLAG 0xc
+/* For Cortex-A9 Diagnostic and Power control register */
+static unsigned int cp15_power;
+static unsigned int cp15_diag;
+
+static void exynos_save_cp15(void)
+{
+ /* Save Power control and Diagnostic registers */
+ asm ("mrc p15, 0, %0, c15, c0, 0\n"
+ "mrc p15, 0, %1, c15, c0, 1\n"
+ : "=r" (cp15_power), "=r" (cp15_diag) : : "cc");
+}
+
static int exynos_do_idle(unsigned long mode)
{
switch (mode) {
case FW_DO_IDLE_AFTR:
+ exynos_save_cp15();
+ __raw_writel(virt_to_phys(exynos_cpu_resume),
+ sysram_ns_base_addr + 0x24);
+ __raw_writel(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
break;
case FW_DO_IDLE_SLEEP:
@@ -76,10 +93,6 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
return 0;
}
-/* For Cortex-A9 Diagnostic and Power control register */
-static unsigned int cp15_power;
-static unsigned int cp15_diag;
-
static int exynos_cpu_suspend(unsigned long arg)
{
flush_cache_all();
@@ -94,10 +107,7 @@ static int exynos_cpu_suspend(unsigned long arg)
static int exynos_suspend(void)
{
- /* Save Power control and Diagnostic registers */
- asm ("mrc p15, 0, %0, c15, c0, 0\n"
- "mrc p15, 0, %1, c15, c0, 1\n"
- : "=r" (cp15_power), "=r" (cp15_diag) : : "cc");
+ exynos_save_cp15();
writel(EXYNOS_SLEEP_MAGIC, sysram_ns_base_addr + EXYNOS_BOOT_FLAG);
writel(virt_to_phys(cpu_resume),
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c722454..af0d4bf 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -201,12 +201,19 @@ static void exynos_cpu_set_boot_vector(long flags)
__raw_writel(flags, exynos_boot_vector_flag());
}
-void exynos_enter_aftr(void)
+int exynos_enter_aftr(void)
{
+ int ret;
+
exynos_set_wakeupmask(0x0000ff3e);
- exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
/* Set value of power down register for aftr mode */
exynos_sys_powerdown_conf(SYS_AFTR);
+
+ ret = call_firmware_op(do_idle, FW_DO_IDLE_AFTR);
+ if (ret == -ENOSYS)
+ exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
+
+ return ret;
}
/* For Cortex-A9 Diagnostic and Power control register */
diff --git a/drivers/cpuidle/cpuidle-exynos.c b/drivers/cpuidle/cpuidle-exynos.c
index 7c01512..c5b36d3 100644
--- a/drivers/cpuidle/cpuidle-exynos.c
+++ b/drivers/cpuidle/cpuidle-exynos.c
@@ -18,12 +18,12 @@
#include <asm/suspend.h>
#include <asm/cpuidle.h>
-static void (*exynos_enter_aftr)(void);
+static int (*exynos_enter_aftr)(void);
static int idle_finisher(unsigned long flags)
{
- exynos_enter_aftr();
- cpu_do_idle();
+ if (exynos_enter_aftr() == -ENOSYS)
+ cpu_do_idle();
return 1;
}
--
1.8.2.3
On 09.07.2014 19:17, Bartlomiej Zolnierkiewicz wrote:
> * Move cp15 registers saving to exynos_save_cp15() helper and add
> additional helper usage to do_idle firmware method.
>
> * Use sysram_ns_base_addr + 0x24/0x20 addresses instead of the default
> ones used by exynos_cpu_set_boot_vector() on boards with secure
> firmware enabled.
>
> * Use do_idle firmware method instead of cpu_do_idle() on boards with
> secure firmware enabled.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Acked-by: Kyungmin Park <[email protected]>
> ---
> v3:
> - make exynos_enter_aftr() return a value
> - add cp15 registers handling to do_idle firmware method
> - set sysram_ns_base_addr + 0x24/0x20 in do_idle firmware method
> - move calling of do_idle firmware method from cpuidle-exynos.c
> to pm.c
>
> arch/arm/mach-exynos/common.h | 2 +-
> arch/arm/mach-exynos/firmware.c | 26 ++++++++++++++++++--------
> arch/arm/mach-exynos/pm.c | 11 +++++++++--
> drivers/cpuidle/cpuidle-exynos.c | 6 +++---
> 4 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index a6a200f..0829808 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -170,7 +170,7 @@ extern int exynos_cpu_power_state(int cpu);
> extern void exynos_cluster_power_down(int cluster);
> extern void exynos_cluster_power_up(int cluster);
> extern int exynos_cluster_power_state(int cluster);
> -extern void exynos_enter_aftr(void);
> +extern int exynos_enter_aftr(void);
>
> extern void s5p_init_cpu(void __iomem *cpuid_addr);
> extern unsigned int samsung_rev(void);
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index 53fbf5c..163f5b9 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -24,13 +24,30 @@
> #include "smc.h"
>
> #define EXYNOS_SLEEP_MAGIC 0x00000bad
> +#define EXYNOS_AFTR_MAGIC 0xfcba0d10
> #define EXYNOS_BOOT_ADDR 0x8
> #define EXYNOS_BOOT_FLAG 0xc
>
> +/* For Cortex-A9 Diagnostic and Power control register */
> +static unsigned int cp15_power;
> +static unsigned int cp15_diag;
> +
> +static void exynos_save_cp15(void)
> +{
> + /* Save Power control and Diagnostic registers */
> + asm ("mrc p15, 0, %0, c15, c0, 0\n"
> + "mrc p15, 0, %1, c15, c0, 1\n"
> + : "=r" (cp15_power), "=r" (cp15_diag) : : "cc");
Hi,
On Exynos3250 I encounter "Oops - undefined instruction" on this asm
while entering AFTR:
[ 2.277946] CPUidle CPU1: going off
[ 2.278110] CPUidle CPU0: going AFTR
[ 2.279478] Internal error: Oops - undefined instruction: 0 [#1]
PREEMPT SMP ARM
Are you sure it should be called on each SoC?
Full dmesg attached.
Best regards,
Krzysztof
> +}
> +
> static int exynos_do_idle(unsigned long mode)
> {
> switch (mode) {
> case FW_DO_IDLE_AFTR:
> + exynos_save_cp15();
> + __raw_writel(virt_to_phys(exynos_cpu_resume),
> + sysram_ns_base_addr + 0x24);
> + __raw_writel(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
> exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
> break;
> case FW_DO_IDLE_SLEEP:
> @@ -76,10 +93,6 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> return 0;
> }
>
> -/* For Cortex-A9 Diagnostic and Power control register */
> -static unsigned int cp15_power;
> -static unsigned int cp15_diag;
> -
> static int exynos_cpu_suspend(unsigned long arg)
> {
> flush_cache_all();
> @@ -94,10 +107,7 @@ static int exynos_cpu_suspend(unsigned long arg)
>
> static int exynos_suspend(void)
> {
> - /* Save Power control and Diagnostic registers */
> - asm ("mrc p15, 0, %0, c15, c0, 0\n"
> - "mrc p15, 0, %1, c15, c0, 1\n"
> - : "=r" (cp15_power), "=r" (cp15_diag) : : "cc");
> + exynos_save_cp15();
>
> writel(EXYNOS_SLEEP_MAGIC, sysram_ns_base_addr + EXYNOS_BOOT_FLAG);
> writel(virt_to_phys(cpu_resume),
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c722454..af0d4bf 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -201,12 +201,19 @@ static void exynos_cpu_set_boot_vector(long flags)
> __raw_writel(flags, exynos_boot_vector_flag());
> }
>
> -void exynos_enter_aftr(void)
> +int exynos_enter_aftr(void)
> {
> + int ret;
> +
> exynos_set_wakeupmask(0x0000ff3e);
> - exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
> /* Set value of power down register for aftr mode */
> exynos_sys_powerdown_conf(SYS_AFTR);
> +
> + ret = call_firmware_op(do_idle, FW_DO_IDLE_AFTR);
> + if (ret == -ENOSYS)
> + exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
> +
> + return ret;
> }
>
> /* For Cortex-A9 Diagnostic and Power control register */
> diff --git a/drivers/cpuidle/cpuidle-exynos.c b/drivers/cpuidle/cpuidle-exynos.c
> index 7c01512..c5b36d3 100644
> --- a/drivers/cpuidle/cpuidle-exynos.c
> +++ b/drivers/cpuidle/cpuidle-exynos.c
> @@ -18,12 +18,12 @@
> #include <asm/suspend.h>
> #include <asm/cpuidle.h>
>
> -static void (*exynos_enter_aftr)(void);
> +static int (*exynos_enter_aftr)(void);
>
> static int idle_finisher(unsigned long flags)
> {
> - exynos_enter_aftr();
> - cpu_do_idle();
> + if (exynos_enter_aftr() == -ENOSYS)
> + cpu_do_idle();
>
> return 1;
> }
>
Hi,
On Thursday, July 10, 2014 10:27:10 AM Krzysztof Kozlowski wrote:
> On 09.07.2014 19:17, Bartlomiej Zolnierkiewicz wrote:
> > * Move cp15 registers saving to exynos_save_cp15() helper and add
> > additional helper usage to do_idle firmware method.
> >
> > * Use sysram_ns_base_addr + 0x24/0x20 addresses instead of the default
> > ones used by exynos_cpu_set_boot_vector() on boards with secure
> > firmware enabled.
> >
> > * Use do_idle firmware method instead of cpu_do_idle() on boards with
> > secure firmware enabled.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > Acked-by: Kyungmin Park <[email protected]>
> > ---
> > v3:
> > - make exynos_enter_aftr() return a value
> > - add cp15 registers handling to do_idle firmware method
> > - set sysram_ns_base_addr + 0x24/0x20 in do_idle firmware method
> > - move calling of do_idle firmware method from cpuidle-exynos.c
> > to pm.c
> >
> > arch/arm/mach-exynos/common.h | 2 +-
> > arch/arm/mach-exynos/firmware.c | 26 ++++++++++++++++++--------
> > arch/arm/mach-exynos/pm.c | 11 +++++++++--
> > drivers/cpuidle/cpuidle-exynos.c | 6 +++---
> > 4 files changed, 31 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> > index a6a200f..0829808 100644
> > --- a/arch/arm/mach-exynos/common.h
> > +++ b/arch/arm/mach-exynos/common.h
> > @@ -170,7 +170,7 @@ extern int exynos_cpu_power_state(int cpu);
> > extern void exynos_cluster_power_down(int cluster);
> > extern void exynos_cluster_power_up(int cluster);
> > extern int exynos_cluster_power_state(int cluster);
> > -extern void exynos_enter_aftr(void);
> > +extern int exynos_enter_aftr(void);
> >
> > extern void s5p_init_cpu(void __iomem *cpuid_addr);
> > extern unsigned int samsung_rev(void);
> > diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> > index 53fbf5c..163f5b9 100644
> > --- a/arch/arm/mach-exynos/firmware.c
> > +++ b/arch/arm/mach-exynos/firmware.c
> > @@ -24,13 +24,30 @@
> > #include "smc.h"
> >
> > #define EXYNOS_SLEEP_MAGIC 0x00000bad
> > +#define EXYNOS_AFTR_MAGIC 0xfcba0d10
> > #define EXYNOS_BOOT_ADDR 0x8
> > #define EXYNOS_BOOT_FLAG 0xc
> >
> > +/* For Cortex-A9 Diagnostic and Power control register */
> > +static unsigned int cp15_power;
> > +static unsigned int cp15_diag;
> > +
> > +static void exynos_save_cp15(void)
> > +{
> > + /* Save Power control and Diagnostic registers */
> > + asm ("mrc p15, 0, %0, c15, c0, 0\n"
> > + "mrc p15, 0, %1, c15, c0, 1\n"
> > + : "=r" (cp15_power), "=r" (cp15_diag) : : "cc");
>
> Hi,
>
> On Exynos3250 I encounter "Oops - undefined instruction" on this asm
> while entering AFTR:
> [ 2.277946] CPUidle CPU1: going off
> [ 2.278110] CPUidle CPU0: going AFTR
> [ 2.279478] Internal error: Oops - undefined instruction: 0 [#1]
> PREEMPT SMP ARM
>
>
> Are you sure it should be called on each SoC?
This should not be called on Exynos3250 (Cortex A7) but this is not
an issue currently with upstream since:
- Exynos3250 cpuidle support is disabled in Tomasz's earlier patch
- Exynos3250 need other patches (i.e. PMU support) to make cpuidle
work in upstream kernel anyway
- there is no support for any Exynos3250 board in upstream yet
Please also note that corresponding code for cp15 registers restore
needs also to be disabled for Cortex A7 and it is not present in
Tomasz's firmware patches yet. He is going to fix his patches to
check for Cortex A9 before restoring registers. Once it is done
I'll rebase my patches and also add Cortex A9 checking to cp15
registers saving code.
When it comes to our internal tree (that you posted log from)
please fix it with adding Cortex A9 check:
if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) to
cp15 registers saving/restoring code.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Hi Bartlomiej,
On Wed, Jul 9, 2014 at 6:17 PM, Bartlomiej Zolnierkiewicz
<[email protected]> wrote:
> This patch series adds support for AFTR idle mode on boards with
> secure firmware enabled and allows EXYNOS cpuidle driver usage on
> Exynos4x12 SoCs.
>
> It has been tested on Trats2 board (using Exynos4412 SoC with secure
> firmware enabled) on which AFTR mode reduces power consumption by ~12%
> when EXYNOS cpuidle driver is enabled (in both cases the default
> exynos_defconfig config is used and CPU1-3 are offlined).
Thanks for this, I have tested it on ODROID-U2.
I don't have any equipment to measure the power usage, but after
offlining CPUs 1,2,3 via sysfs "online" file, a printk I added
confirms that the system is going to enter aftr mode, and system
stability seems as good as ever.
Is there any automatic way that the cpus should be offlined, or is the
intention that it must be done "by hand" like this?
Thanks,
Daniel
On Monday, July 14, 2014 04:41:25 PM Daniel Drake wrote:
> Hi Bartlomiej,
Hi,
> On Wed, Jul 9, 2014 at 6:17 PM, Bartlomiej Zolnierkiewicz
> <[email protected]> wrote:
> > This patch series adds support for AFTR idle mode on boards with
> > secure firmware enabled and allows EXYNOS cpuidle driver usage on
> > Exynos4x12 SoCs.
> >
> > It has been tested on Trats2 board (using Exynos4412 SoC with secure
> > firmware enabled) on which AFTR mode reduces power consumption by ~12%
> > when EXYNOS cpuidle driver is enabled (in both cases the default
> > exynos_defconfig config is used and CPU1-3 are offlined).
>
> Thanks for this, I have tested it on ODROID-U2.
>
> I don't have any equipment to measure the power usage, but after
> offlining CPUs 1,2,3 via sysfs "online" file, a printk I added
> confirms that the system is going to enter aftr mode, and system
> stability seems as good as ever.
Thank you for testing!
> Is there any automatic way that the cpus should be offlined, or is the
> intention that it must be done "by hand" like this?
AFAIK Android kernel disables unused CPUs by itself. There is also
ongoing work on Exynos coupled cpuidle driver for upstream kernels
which will handle offlining of CPUs for AFTR mode transparently to
the kernel (currently only Exynos4210 SoC is supported by the driver
posted by Daniel Lezcano, we are working on Exynos3250 SoC support
and once it is done we might try to also do Exynos4x12 SoCs version).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On 07/09/2014 07:17 PM, Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> This patch series adds support for AFTR idle mode on boards with
> secure firmware enabled and allows EXYNOS cpuidle driver usage on
> Exynos4x12 SoCs.
>
> It has been tested on Trats2 board (using Exynos4412 SoC with secure
> firmware enabled) on which AFTR mode reduces power consumption by ~12%
> when EXYNOS cpuidle driver is enabled (in both cases the default
> exynos_defconfig config is used and CPU1-3 are offlined).
Hi Bartlomiej,
what is the status of this patchset ? Is it supposed to land for 3.17 ?
Thanks
-- Daniel
> v3:
> - rebased on top of next-20140708 +
> http://www.mail-archive.com/[email protected]/msg32809.html
> (with rejects fixed)
> http://www.mail-archive.com/[email protected]/msg32808.html
> http://www.mail-archive.com/[email protected]/msg32991.html
> (with rejects fixed in patch #2)
> - addressed review comments from Tomasz Figa and Daniel Lezcano
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
>
> Bartlomiej Zolnierkiewicz (4):
> ARM: EXYNOS: PM: replace EXYNOS_BOOT_VECTOR_* macros by static inlines
> ARM: EXYNOS: add AFTR mode support to firmware do_idle method
> ARM: EXYNOS: cpuidle: add secure firmware support to AFTR mode code
> ARM: EXYNOS: cpuidle: allow driver usage on Exynos4x12 SoCs
>
> arch/arm/include/asm/firmware.h | 2 +-
> arch/arm/mach-exynos/common.h | 7 ++++++-
> arch/arm/mach-exynos/exynos.c | 3 ++-
> arch/arm/mach-exynos/firmware.c | 36 ++++++++++++++++++++++++++----------
> arch/arm/mach-exynos/pm.c | 39 +++++++++++++++++++++++++++++----------
> drivers/cpuidle/cpuidle-exynos.c | 6 +++---
> 6 files changed, 67 insertions(+), 26 deletions(-)
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Hi Bartlomiej,
I tried to apply this patchset on top of next-20140708 but it does not
apply.
Do you have a more recent version ?
Thanks in advance
-- Daniel
On 07/09/2014 07:17 PM, Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> This patch series adds support for AFTR idle mode on boards with
> secure firmware enabled and allows EXYNOS cpuidle driver usage on
> Exynos4x12 SoCs.
>
> It has been tested on Trats2 board (using Exynos4412 SoC with secure
> firmware enabled) on which AFTR mode reduces power consumption by ~12%
> when EXYNOS cpuidle driver is enabled (in both cases the default
> exynos_defconfig config is used and CPU1-3 are offlined).
>
> v3:
> - rebased on top of next-20140708 +
> http://www.mail-archive.com/[email protected]/msg32809.html
> (with rejects fixed)
> http://www.mail-archive.com/[email protected]/msg32808.html
> http://www.mail-archive.com/[email protected]/msg32991.html
> (with rejects fixed in patch #2)
> - addressed review comments from Tomasz Figa and Daniel Lezcano
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
>
> Bartlomiej Zolnierkiewicz (4):
> ARM: EXYNOS: PM: replace EXYNOS_BOOT_VECTOR_* macros by static inlines
> ARM: EXYNOS: add AFTR mode support to firmware do_idle method
> ARM: EXYNOS: cpuidle: add secure firmware support to AFTR mode code
> ARM: EXYNOS: cpuidle: allow driver usage on Exynos4x12 SoCs
>
> arch/arm/include/asm/firmware.h | 2 +-
> arch/arm/mach-exynos/common.h | 7 ++++++-
> arch/arm/mach-exynos/exynos.c | 3 ++-
> arch/arm/mach-exynos/firmware.c | 36 ++++++++++++++++++++++++++----------
> arch/arm/mach-exynos/pm.c | 39 +++++++++++++++++++++++++++++----------
> drivers/cpuidle/cpuidle-exynos.c | 6 +++---
> 6 files changed, 67 insertions(+), 26 deletions(-)
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog