* [PATCH 1/2] ARM: imx: no unmask/mask GINT for WAIT_CLOCKED @ 2017-12-30 13:53 Peng Fan 2017-12-30 13:53 ` [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active Peng Fan 0 siblings, 1 reply; 6+ messages in thread From: Peng Fan @ 2017-12-30 13:53 UTC (permalink / raw) To: linux-arm-kernel WAIT_CLOCKED is for RUN mode, there is no need to unmask/mask IRQ32 in GPC. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- V1: This is to upstream patch: http://git.freescale.com/git/cgit.cgi/imx/linux-imx.git/commit/?h=imx_4.9.11_1.0.0_ga&id=0d980646ee068b92db71fd5e4e4efcbc33749cbd arch/arm/mach-imx/pm-imx6.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c index 153a0afc7645..56bfd9b5229e 100644 --- a/arch/arm/mach-imx/pm-imx6.c +++ b/arch/arm/mach-imx/pm-imx6.c @@ -337,9 +337,11 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode) * * Note that IRQ #32 is GIC SPI #0. */ - imx_gpc_hwirq_unmask(0); + if (mode != WAIT_CLOCKED) + imx_gpc_hwirq_unmask(0); writel_relaxed(val, ccm_base + CLPCR); - imx_gpc_hwirq_mask(0); + if (mode != WAIT_CLOCKED) + imx_gpc_hwirq_mask(0); return 0; } -- 2.14.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active 2017-12-30 13:53 [PATCH 1/2] ARM: imx: no unmask/mask GINT for WAIT_CLOCKED Peng Fan @ 2017-12-30 13:53 ` Peng Fan 2018-01-24 1:35 ` Peng Fan 2018-01-24 11:16 ` Lucas Stach 0 siblings, 2 replies; 6+ messages in thread From: Peng Fan @ 2017-12-30 13:53 UTC (permalink / raw) To: linux-arm-kernel There are two states in i.MX6Q cpuidle driver. state[1]: ARM WFI mode state[2]: i.MX6Q WAIT mode Take i.MX6DL as example, think out such a case: 1. CPU0/1 both run at normal mode 2. On CPU0, `sleep 1` is executed. And there are no workload on CPU1. 3. CPU0 first runs into state[2] and 'wfi' instruction. Switched to use GPT broadcast. 4. CPU1 runs into state[2] and configure CCM to WAIT MODE, then 'wfi' instruction. Now arm_clk and local timer clock are shutdown. Switched to use GPT broadcast 5. GPT broadcast timer interrupt comes to GPC/GIC, then CPU0 wakes up. CPU0 switched to use arm local timer. CPU1 is still sleeping. 6. No workload on CPU0, CPU0 runs into state[1]. But CCM register is still not restored to Normal RUN mode. 'wfi' + CCM WAIT will cause arm_clk and arm core clk. Now CPU0 stops, which is not correct. So, need to make sure CCM configured to RUN mode when any cpu exit state[2]. In this patch, When CPU exits state[2], it configures CCM to RUN mode. When all CPUs enters state[2], the last CPU needs to check whether it's ok to configure CCM to WAIT mode or not. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- V1: This is to upstream patch: http://git.freescale.com/git/cgit.cgi/imx/linux-imx.git/commit/?h=imx_4.9.11_1.0.0_ga&id=0d980646ee068b92db71fd5e4e4efcbc33749cbd arch/arm/mach-imx/cpuidle-imx6q.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c index bfeb25aaf9a2..4d342e2fdfe6 100644 --- a/arch/arm/mach-imx/cpuidle-imx6q.c +++ b/arch/arm/mach-imx/cpuidle-imx6q.c @@ -30,6 +30,8 @@ static int imx6q_enter_wait(struct cpuidle_device *dev, if (!spin_trylock(&master_lock)) goto idle; imx6_set_lpm(WAIT_UNCLOCKED); + if (atomic_read(&master) != num_online_cpus()) + imx6_set_lpm(WAIT_CLOCKED); cpu_do_idle(); imx6_set_lpm(WAIT_CLOCKED); spin_unlock(&master_lock); @@ -41,6 +43,7 @@ static int imx6q_enter_wait(struct cpuidle_device *dev, done: atomic_dec(&master); + imx6_set_lpm(WAIT_CLOCKED); return index; } -- 2.14.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active 2017-12-30 13:53 ` [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active Peng Fan @ 2018-01-24 1:35 ` Peng Fan 2018-01-24 11:16 ` Lucas Stach 1 sibling, 0 replies; 6+ messages in thread From: Peng Fan @ 2018-01-24 1:35 UTC (permalink / raw) To: linux-arm-kernel Hi, For the two patch, ping.. On Sat, Dec 30, 2017 at 09:53:19PM +0800, Peng Fan wrote: >There are two states in i.MX6Q cpuidle driver. >state[1]: ARM WFI mode >state[2]: i.MX6Q WAIT mode > >Take i.MX6DL as example, think out such a case: >1. CPU0/1 both run at normal mode >2. On CPU0, `sleep 1` is executed. And there are no workload on CPU1. >3. CPU0 first runs into state[2] and 'wfi' instruction. Switched to use > GPT broadcast. >4. CPU1 runs into state[2] and configure CCM to WAIT MODE, > then 'wfi' instruction. Now arm_clk and local timer clock are > shutdown. Switched to use GPT broadcast >5. GPT broadcast timer interrupt comes to GPC/GIC, then CPU0 wakes up. > CPU0 switched to use arm local timer. CPU1 is still sleeping. >6. No workload on CPU0, CPU0 runs into state[1]. But CCM register > is still not restored to Normal RUN mode. 'wfi' + CCM WAIT will > cause arm_clk and arm core clk. > Now CPU0 stops, which is not correct. > >So, need to make sure CCM configured to RUN mode when any cpu exit >state[2]. > >In this patch, >When CPU exits state[2], it configures CCM to RUN mode. >When all CPUs enters state[2], the last CPU needs to check >whether it's ok to configure CCM to WAIT mode or not. > >Signed-off-by: Peng Fan <peng.fan@nxp.com> >--- > >V1: > This is to upstream patch: > http://git.freescale.com/git/cgit.cgi/imx/linux-imx.git/commit/?h=imx_4.9.11_1.0.0_ga&id=0d980646ee068b92db71fd5e4e4efcbc33749cbd > > arch/arm/mach-imx/cpuidle-imx6q.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach-imx/cpuidle-imx6q.c >index bfeb25aaf9a2..4d342e2fdfe6 100644 >--- a/arch/arm/mach-imx/cpuidle-imx6q.c >+++ b/arch/arm/mach-imx/cpuidle-imx6q.c >@@ -30,6 +30,8 @@ static int imx6q_enter_wait(struct cpuidle_device *dev, > if (!spin_trylock(&master_lock)) > goto idle; > imx6_set_lpm(WAIT_UNCLOCKED); >+ if (atomic_read(&master) != num_online_cpus()) >+ imx6_set_lpm(WAIT_CLOCKED); > cpu_do_idle(); > imx6_set_lpm(WAIT_CLOCKED); > spin_unlock(&master_lock); >@@ -41,6 +43,7 @@ static int imx6q_enter_wait(struct cpuidle_device *dev, > done: > atomic_dec(&master); > >+ imx6_set_lpm(WAIT_CLOCKED); > return index; > } > >-- >2.14.1 > Thanks, Peng. -- ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active 2017-12-30 13:53 ` [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active Peng Fan 2018-01-24 1:35 ` Peng Fan @ 2018-01-24 11:16 ` Lucas Stach 2018-01-25 2:19 ` Peng Fan 1 sibling, 1 reply; 6+ messages in thread From: Lucas Stach @ 2018-01-24 11:16 UTC (permalink / raw) To: linux-arm-kernel Hi Peng, Am Samstag, den 30.12.2017, 21:53 +0800 schrieb Peng Fan: > There are two states in i.MX6Q cpuidle driver. > state[1]: ARM WFI mode > state[2]: i.MX6Q WAIT mode > > Take i.MX6DL as example, think out such a case: > 1. CPU0/1 both run at normal mode > 2. On CPU0, `sleep 1` is executed. And there are no workload on CPU1. > 3. CPU0 first runs into state[2] and 'wfi' instruction. Switched to > use > ???GPT broadcast. > 4. CPU1 runs into state[2] and configure CCM to WAIT MODE, > ???then 'wfi' instruction. Now arm_clk and local timer clock are > ???shutdown. Switched to use GPT broadcast > 5. GPT broadcast timer interrupt comes to GPC/GIC, then CPU0 wakes > up. > ???CPU0 switched to use arm local timer. CPU1 is still sleeping. > 6. No workload on CPU0, CPU0 runs into state[1]. But CCM register > ???is still not restored to Normal RUN mode. 'wfi' + CCM WAIT will > ???cause arm_clk and arm core clk. > ???Now CPU0 stops, which is not correct. > > So, need to make sure CCM configured to RUN mode when any cpu exit > state[2]. > > In this patch, > When CPU exits state[2], it configures CCM to RUN mode. > When all CPUs enters state[2], the last CPU needs to check > whether it's ok to configure CCM to WAIT mode or not. To me this patch seems like we are adding duct tape to fix the issue.? It seems the whole master_lock thing doesn't properly work when the CPU that is woken up by the timer broadcast isn't the last CPU going to sleep. We should probably try to come up with a way to simplify this whole master dance, instead of adding yet more complexity. Regards, Lucas > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V1: > ?This is to upstream patch: > ?http://git.freescale.com/git/cgit.cgi/imx/linux-imx.git/commit/?h=im > x_4.9.11_1.0.0_ga&id=0d980646ee068b92db71fd5e4e4efcbc33749cbd > > ?arch/arm/mach-imx/cpuidle-imx6q.c | 3 +++ > ?1 file changed, 3 insertions(+) > > diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach- > imx/cpuidle-imx6q.c > index bfeb25aaf9a2..4d342e2fdfe6 100644 > --- a/arch/arm/mach-imx/cpuidle-imx6q.c > +++ b/arch/arm/mach-imx/cpuidle-imx6q.c > @@ -30,6 +30,8 @@ static int imx6q_enter_wait(struct cpuidle_device > *dev, > ? if (!spin_trylock(&master_lock)) > ? goto idle; > ? imx6_set_lpm(WAIT_UNCLOCKED); > + if (atomic_read(&master) != num_online_cpus()) > + imx6_set_lpm(WAIT_CLOCKED); > ? cpu_do_idle(); > ? imx6_set_lpm(WAIT_CLOCKED); > ? spin_unlock(&master_lock); > @@ -41,6 +43,7 @@ static int imx6q_enter_wait(struct cpuidle_device > *dev, > ?done: > ? atomic_dec(&master); > ? > + imx6_set_lpm(WAIT_CLOCKED); > ? return index; > ?} > ? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active 2018-01-24 11:16 ` Lucas Stach @ 2018-01-25 2:19 ` Peng Fan 2018-01-25 11:52 ` Fabio Estevam 0 siblings, 1 reply; 6+ messages in thread From: Peng Fan @ 2018-01-25 2:19 UTC (permalink / raw) To: linux-arm-kernel Hi Lucas, On Wed, Jan 24, 2018 at 12:16:25PM +0100, Lucas Stach wrote: >Hi Peng, > >Am Samstag, den 30.12.2017, 21:53 +0800 schrieb Peng Fan: >> There are two states in i.MX6Q cpuidle driver. >> state[1]: ARM WFI mode >> state[2]: i.MX6Q WAIT mode >> >> Take i.MX6DL as example, think out such a case: >> 1. CPU0/1 both run at normal mode >> 2. On CPU0, `sleep 1` is executed. And there are no workload on CPU1. >> 3. CPU0 first runs into state[2] and 'wfi' instruction. Switched to >> use >> ??????GPT broadcast. >> 4. CPU1 runs into state[2] and configure CCM to WAIT MODE, >> ??????then 'wfi' instruction. Now arm_clk and local timer clock are >> ??????shutdown. Switched to use GPT broadcast >> 5. GPT broadcast timer interrupt comes to GPC/GIC, then CPU0 wakes >> up. >> ??????CPU0 switched to use arm local timer. CPU1 is still sleeping. >> 6. No workload on CPU0, CPU0 runs into state[1]. But CCM register >> ??????is still not restored to Normal RUN mode. 'wfi' + CCM WAIT will >> ??????cause arm_clk and arm core clk. >> ??????Now CPU0 stops, which is not correct. >> >> So, need to make sure CCM configured to RUN mode when any cpu exit >> state[2]. >> >> In this patch, >> When CPU exits state[2], it configures CCM to RUN mode. >> When all CPUs enters state[2], the last CPU needs to check >> whether it's ok to configure CCM to WAIT mode or not. > >To me this patch seems like we are adding duct tape to fix the issue.?? > >It seems the whole master_lock thing doesn't properly work when the CPU >that is woken up by the timer broadcast isn't the last CPU going to >sleep. We should probably try to come up with a way to simplify this >whole master dance, instead of adding yet more complexity. The master_lock is used to protect multiple CPUS runs into State[1]. It could not be used to protect the CPU runs into State[0]. The patch is to make sure that before arm clk turned off, need to check whether all the cpus are in State[1]. And the cpu who first wake up from State[1], need to configure CCM to RUN mode. This patch has been integrated into vendor kernel and fixed customer's issue. I could not think out of a better solution to restructure the master lock, and this may introduce new issues. Thanks, Peng. > >Regards, >Lucas > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> >> --- >> >> V1: >> ??This is to upstream patch: >> ??http://git.freescale.com/git/cgit.cgi/imx/linux-imx.git/commit/?h=im >> x_4.9.11_1.0.0_ga&id=0d980646ee068b92db71fd5e4e4efcbc33749cbd >> >> ??arch/arm/mach-imx/cpuidle-imx6q.c | 3 +++ >> ??1 file changed, 3 insertions(+) >> >> diff --git a/arch/arm/mach-imx/cpuidle-imx6q.c b/arch/arm/mach- >> imx/cpuidle-imx6q.c >> index bfeb25aaf9a2..4d342e2fdfe6 100644 >> --- a/arch/arm/mach-imx/cpuidle-imx6q.c >> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c >> @@ -30,6 +30,8 @@ static int imx6q_enter_wait(struct cpuidle_device >> *dev, >> ?? if (!spin_trylock(&master_lock)) >> ?? goto idle; >> ?? imx6_set_lpm(WAIT_UNCLOCKED); >> + if (atomic_read(&master) != num_online_cpus()) >> + imx6_set_lpm(WAIT_CLOCKED); >> ?? cpu_do_idle(); >> ?? imx6_set_lpm(WAIT_CLOCKED); >> ?? spin_unlock(&master_lock); >> @@ -41,6 +43,7 @@ static int imx6q_enter_wait(struct cpuidle_device >> *dev, >> ??done: >> ?? atomic_dec(&master); >> ?? >> + imx6_set_lpm(WAIT_CLOCKED); >> ?? return index; >> ??} >> ?? -- ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active 2018-01-25 2:19 ` Peng Fan @ 2018-01-25 11:52 ` Fabio Estevam 0 siblings, 0 replies; 6+ messages in thread From: Fabio Estevam @ 2018-01-25 11:52 UTC (permalink / raw) To: linux-arm-kernel Hi Peng, On Thu, Jan 25, 2018 at 12:19 AM, Peng Fan <van.freenix@gmail.com> wrote: > This patch has been integrated into vendor kernel and fixed customer's issue. How does the issue manifest exactly? How can we reproduce the issue? ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-25 11:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-30 13:53 [PATCH 1/2] ARM: imx: no unmask/mask GINT for WAIT_CLOCKED Peng Fan 2017-12-30 13:53 ` [PATCH 2/2] ARM: imx: cpuidle-imx6q: configure CCM to RUN mode when CPU is active Peng Fan 2018-01-24 1:35 ` Peng Fan 2018-01-24 11:16 ` Lucas Stach 2018-01-25 2:19 ` Peng Fan 2018-01-25 11:52 ` Fabio Estevam
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).