| Message ID | 1404152927-11168-1-git-send-email-a.kesavan@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 30 Jun 2014, Abhilash Kesavan wrote: > Use the MCPM layer to handle core suspend/resume on Exynos5420. > Also, restore the entry address setup code post-resume. > > Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> > --- [...] Could you tell me more about this? > @@ -150,7 +153,13 @@ static void exynos_power_down(void) > BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP); > cpu_use_count[cpu][cluster]--; > if (cpu_use_count[cpu][cluster] == 0) { > - exynos_cpu_power_down(cpunr); > + /* > + * Bypass power down for CPU0 during suspend. This is being > + * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG. > + */ > + if ((cpunr != 0) || > + (__raw_readl(pmu_base_addr + S5P_INFORM1) != S5P_CHECK_SLEEP)) > + exynos_cpu_power_down(cpunr); What happens if CPU0 is the first in the cluster to be idled? Will it remain powered up until all the others have gone idle too? Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Nicolas, On Tue, Jul 1, 2014 at 9:49 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > On Mon, 30 Jun 2014, Abhilash Kesavan wrote: > >> Use the MCPM layer to handle core suspend/resume on Exynos5420. >> Also, restore the entry address setup code post-resume. >> >> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> >> --- > [...] > > Could you tell me more about this? > >> @@ -150,7 +153,13 @@ static void exynos_power_down(void) >> BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP); >> cpu_use_count[cpu][cluster]--; >> if (cpu_use_count[cpu][cluster] == 0) { >> - exynos_cpu_power_down(cpunr); >> + /* >> + * Bypass power down for CPU0 during suspend. This is being >> + * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG. >> + */ >> + if ((cpunr != 0) || >> + (__raw_readl(pmu_base_addr + S5P_INFORM1) != S5P_CHECK_SLEEP)) >> + exynos_cpu_power_down(cpunr); > > What happens if CPU0 is the first in the cluster to be idled? Will it > remain powered up until all the others have gone idle too? This check is only for handling the S2R CPU0 case. In case of idle/switching the S5P_CHECK_SLEEP flag would not be set and hence there would be no change in behavior for them. During suspend, we enable the ARM_USE_STANDBY_WFI0 bit which tells the PMU when the CPU0 is ready to be suspended. This in conjunction with the sleep state core configuration (setting SYS_PWR_REG low) causes the CPU0 to go down. We should not write to the CPU0 power configuration register (exynos_cpu_power_down) along with this during suspend. Regards, Abhilash > > > Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 01, 2014 at 02:14:49PM +0100, Abhilash Kesavan wrote: > Hi Nicolas, > > On Tue, Jul 1, 2014 at 9:49 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > > On Mon, 30 Jun 2014, Abhilash Kesavan wrote: > > > >> Use the MCPM layer to handle core suspend/resume on Exynos5420. > >> Also, restore the entry address setup code post-resume. > >> > >> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> > >> --- > > [...] > > > > Could you tell me more about this? > > > >> @@ -150,7 +153,13 @@ static void exynos_power_down(void) > >> BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP); > >> cpu_use_count[cpu][cluster]--; > >> if (cpu_use_count[cpu][cluster] == 0) { > >> - exynos_cpu_power_down(cpunr); > >> + /* > >> + * Bypass power down for CPU0 during suspend. This is being > >> + * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG. > >> + */ > >> + if ((cpunr != 0) || > >> + (__raw_readl(pmu_base_addr + S5P_INFORM1) != S5P_CHECK_SLEEP)) > >> + exynos_cpu_power_down(cpunr); > > > > What happens if CPU0 is the first in the cluster to be idled? Will it > > remain powered up until all the others have gone idle too? > This check is only for handling the S2R CPU0 case. In case of > idle/switching the S5P_CHECK_SLEEP flag would not be set and hence > there would be no change in behavior for them. > During suspend, we enable the ARM_USE_STANDBY_WFI0 bit which tells the > PMU when the CPU0 is ready to be suspended. This in conjunction with > the sleep state core configuration (setting SYS_PWR_REG low) causes > the CPU0 to go down. We should not write to the CPU0 power > configuration register (exynos_cpu_power_down) along with this during > suspend. I think this should be part of a refactoring that includes the exynos MCPM suspend call parameters. In particular, at the moment there is no code in the back-end to detect if the last man should request core gating or cluster gating (ie last man _always_ request cluster gating, that might not be what we want), there is the residency value that can be also be used to imply a S2R request (eg residency = ~0 ?). The command sent must be implied by the state that is entered, not by peeking at registers that should contain magic values, and that's true not only for exynos but for all ARM platforms out there. What I mean is: we can pass the requested state as a suspend parameter and the power_down state machine will send the required command accordingly. It can be done using the residency value or by passing the power state "index" as suspend parameter, the power down MCPM code will do a look-up and send the respective command. Thoughts appreciated. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 1 Jul 2014, Lorenzo Pieralisi wrote: > On Tue, Jul 01, 2014 at 02:14:49PM +0100, Abhilash Kesavan wrote: > > Hi Nicolas, > > > > On Tue, Jul 1, 2014 at 9:49 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > > > On Mon, 30 Jun 2014, Abhilash Kesavan wrote: > > > > > >> Use the MCPM layer to handle core suspend/resume on Exynos5420. > > >> Also, restore the entry address setup code post-resume. > > >> > > >> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> > > >> --- > > > [...] > > > > > > Could you tell me more about this? > > > > > >> @@ -150,7 +153,13 @@ static void exynos_power_down(void) > > >> BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP); > > >> cpu_use_count[cpu][cluster]--; > > >> if (cpu_use_count[cpu][cluster] == 0) { > > >> - exynos_cpu_power_down(cpunr); > > >> + /* > > >> + * Bypass power down for CPU0 during suspend. This is being > > >> + * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG. > > >> + */ > > >> + if ((cpunr != 0) || > > >> + (__raw_readl(pmu_base_addr + S5P_INFORM1) != S5P_CHECK_SLEEP)) > > >> + exynos_cpu_power_down(cpunr); > > > > > > What happens if CPU0 is the first in the cluster to be idled? Will it > > > remain powered up until all the others have gone idle too? > > This check is only for handling the S2R CPU0 case. In case of > > idle/switching the S5P_CHECK_SLEEP flag would not be set and hence > > there would be no change in behavior for them. > > During suspend, we enable the ARM_USE_STANDBY_WFI0 bit which tells the > > PMU when the CPU0 is ready to be suspended. This in conjunction with > > the sleep state core configuration (setting SYS_PWR_REG low) causes > > the CPU0 to go down. We should not write to the CPU0 power > > configuration register (exynos_cpu_power_down) along with this during > > suspend. > > I think this should be part of a refactoring that includes the exynos MCPM > suspend call parameters. In particular, at the moment there is no code in > the back-end to detect if the last man should request core gating or cluster > gating (ie last man _always_ request cluster gating, that might not be what > we want), there is the residency value that can be also be used to imply a > S2R request (eg residency = ~0 ?). > > The command sent must be implied by the state that is entered, not by > peeking at registers that should contain magic values, and that's true > not only for exynos but for all ARM platforms out there. > > What I mean is: we can pass the requested state as a suspend parameter > and the power_down state machine will send the required command > accordingly. > > It can be done using the residency value or by passing the power state "index" > as suspend parameter, the power down MCPM code will do a look-up and send > the respective command. > > Thoughts appreciated. I agree. Having the MCPM abstraction having to rely on some magic value to be set in a register beforehand for things to work properly is not nice. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c index 037a05e..dd298f3 100644 --- a/arch/arm/mach-exynos/mcpm-exynos.c +++ b/arch/arm/mach-exynos/mcpm-exynos.c @@ -15,6 +15,7 @@ #include <linux/delay.h> #include <linux/io.h> #include <linux/of_address.h> +#include <linux/syscore_ops.h> #include <asm/cputype.h> #include <asm/cp15.h> @@ -30,6 +31,8 @@ #define EXYNOS5420_USE_ARM_CORE_DOWN_STATE BIT(29) #define EXYNOS5420_USE_L2_COMMON_UP_STATE BIT(30) +static void __iomem *ns_sram_base_addr; + /* * The common v7_exit_coherency_flush API could not be used because of the * Erratum 799270 workaround. This macro is the same as the common one (in @@ -150,7 +153,13 @@ static void exynos_power_down(void) BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP); cpu_use_count[cpu][cluster]--; if (cpu_use_count[cpu][cluster] == 0) { - exynos_cpu_power_down(cpunr); + /* + * Bypass power down for CPU0 during suspend. This is being + * taken care by the SYS_PWR_CFG bit in CORE0_SYS_PWR_REG. + */ + if ((cpunr != 0) || + (__raw_readl(pmu_base_addr + S5P_INFORM1) != S5P_CHECK_SLEEP)) + exynos_cpu_power_down(cpunr); if (exynos_cluster_unused(cluster)) { exynos_cluster_power_down(cluster); @@ -319,10 +328,26 @@ static const struct of_device_id exynos_dt_mcpm_match[] = { {}, }; +static void exynos_mcpm_setup_entry_point(void) +{ + /* + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr + * as part of secondary_cpu_start(). Let's redirect it to the + * mcpm_entry_point(). This is done during both secondary boot-up as + * well as system resume. + */ + __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); +} + +static struct syscore_ops exynos_mcpm_syscore_ops = { + .resume = exynos_mcpm_setup_entry_point, +}; + static int __init exynos_mcpm_init(void) { struct device_node *node; - void __iomem *ns_sram_base_addr; unsigned int value, i; int ret; @@ -389,16 +414,9 @@ static int __init exynos_mcpm_init(void) __raw_writel(value, EXYNOS_COMMON_OPTION(i)); } - /* - * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr - * as part of secondary_cpu_start(). Let's redirect it to the - * mcpm_entry_point(). - */ - __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ - __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ - __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); + exynos_mcpm_setup_entry_point(); - iounmap(ns_sram_base_addr); + register_syscore_ops(&exynos_mcpm_syscore_ops); return ret; } diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c index bf8564a..a4f4292 100644 --- a/arch/arm/mach-exynos/pm.c +++ b/arch/arm/mach-exynos/pm.c @@ -24,6 +24,7 @@ #include <asm/cacheflush.h> #include <asm/hardware/cache-l2x0.h> +#include <asm/mcpm.h> #include <asm/smp_scu.h> #include <asm/suspend.h> @@ -191,7 +192,6 @@ int exynos_cluster_power_state(int cluster) pmu_base_addr + S5P_INFORM1)) #define S5P_CHECK_AFTR 0xFCBA0D10 -#define S5P_CHECK_SLEEP 0x00000BAD /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */ static void exynos_set_wakeupmask(long mask) @@ -318,7 +318,10 @@ static void exynos_pm_prepare(void) /* ensure at least INFORM0 has the resume address */ - pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0); + if (soc_is_exynos5420()) + pmu_raw_writel(virt_to_phys(mcpm_entry_point), S5P_INFORM0); + else + pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0); if (soc_is_exynos5420()) { tmp = __raw_readl(pmu_base_addr + EXYNOS5_ARM_L2_OPTION); @@ -490,6 +493,28 @@ static struct syscore_ops exynos_pm_syscore_ops = { .resume = exynos_pm_resume, }; +static int notrace exynos_mcpm_cpu_suspend(unsigned long arg) +{ + /* MCPM works with HW CPU identifiers */ + unsigned int mpidr = read_cpuid_mpidr(); + unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1); + unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0); + + __raw_writel(0x0, sysram_base_addr + EXYNOS5420_CPU_STATE); + + mcpm_set_entry_vector(cpu, cluster, exynos_cpu_resume); + + /* + * Residency value passed to mcpm_cpu_suspend back-end + * has to be given clear semantics. Set to 0 as a + * temporary value. + */ + mcpm_cpu_suspend(0); + + /* return value != 0 means failure */ + return 1; +} + /* * Suspend Ops */ @@ -517,10 +542,17 @@ static int exynos_suspend_enter(suspend_state_t state) flush_cache_all(); s3c_pm_check_store(); - ret = cpu_suspend(0, exynos_cpu_suspend); + /* Use the MCPM layer to suspend 5420 which is a multi-cluster SoC */ + if (soc_is_exynos5420()) + ret = cpu_suspend(0, exynos_mcpm_cpu_suspend); + else + ret = cpu_suspend(0, exynos_cpu_suspend); if (ret) return ret; + if (soc_is_exynos5420()) + mcpm_cpu_powered_up(); + s3c_pm_restore_uarts(); S3C_PMDBG("%s: wakeup stat: %08x\n", __func__, diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h index 3cf0454..e8c75db 100644 --- a/arch/arm/mach-exynos/regs-pmu.h +++ b/arch/arm/mach-exynos/regs-pmu.h @@ -152,6 +152,7 @@ #define S5P_PAD_RET_EBIB_OPTION 0x31A8 #define S5P_CORE_LOCAL_PWR_EN 0x3 +#define S5P_CHECK_SLEEP 0x00000BAD /* Only for EXYNOS4210 */ #define S5P_CMU_CLKSTOP_LCD1_LOWPWR 0x1154
Use the MCPM layer to handle core suspend/resume on Exynos5420. Also, restore the entry address setup code post-resume. Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> --- Hi Lorenzo and Nicolas, I am now using the suspend/powered_up call-backs that were added as part of Chander's CPUIdle series for Exynos5420. exynos_mcpm_cpu_suspend() is based on the code in bL cpuidle driver except for a small addition. This has been tested both on an SMDK5420 and Peach Pit Chromebook on 3.16-rc3. Here are the dependencies (some of these patches did not apply cleanly): 1) Cleanup patches for mach-exynos http://comments.gmane.org/gmane.linux.kernel.samsung-soc/33772 2) PMU cleanup and refactoring for using DT https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg671625.html 3) Exynos5420 PMU/S2R Series http://comments.gmane.org/gmane.linux.kernel.samsung-soc/33898 4) MCPM boot CPU CCI enablement patches www.spinics.net/lists/linux-samsung-soc/msg32923.html 5) Exynos5420 CPUIdle Series which populates MCPM suspend/powered_up call-backs. www.gossamer-threads.com/lists/linux/kernel/1945347 https://patchwork.kernel.org/patch/4357461/ 6) Exynos5420 MCPM cluster power down support http://www.spinics.net/lists/arm-kernel/msg339988.html 7) TPM reset mask patch http://www.spinics.net/lists/arm-kernel/msg341884.html arch/arm/mach-exynos/mcpm-exynos.c | 40 ++++++++++++++++++++++++++---------- arch/arm/mach-exynos/pm.c | 38 +++++++++++++++++++++++++++++++--- arch/arm/mach-exynos/regs-pmu.h | 1 + 3 files changed, 65 insertions(+), 14 deletions(-)