arch_timer: Move delay timer to drivers clocksource
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Now arch timer is registerd using generic sched timer, delay timer registration is the only part remaining in arch ports. Move this part to drivers clocksource and remove arch timer from arch ports.
--- arch/arm/include/asm/arch_timer.h | 1 - arch/arm/kernel/Makefile | 1 - arch/arm/kernel/arch_timer.c | 44 ---------------------------------- arch/arm64/include/asm/arch_timer.h | 5 ---- arch/arm64/include/asm/delay.h | 32 ++++++++++++++++++++++++ arch/arm64/include/asm/timex.h | 5 +-- arch/arm64/kernel/time.c | 9 ------- arch/arm64/lib/delay.c | 26 ++++++++++++++++++++ drivers/clocksource/arm_arch_timer.c | 12 ++++++++- 9 files changed, 71 insertions(+), 64 deletions(-) delete mode 100644 arch/arm/kernel/arch_timer.c create mode 100644 arch/arm64/include/asm/delay.h
arch/arm/include/asm/arch_timer.h +0 -1
@@ -10,7 +10,6 @@#include <clocksource/arm_arch_timer.h> #ifdef CONFIG_ARM_ARCH_TIMER -int arch_timer_arch_init (void );/* * These register accessors are marked inline so the compiler can
arch/arm/kernel/Makefile +0 -1
@@ -45,7 +45,6 @@ obj-$(CONFIG_SMP) += smp_tlb.o endif obj-$(CONFIG_HAVE_ARM_SCU) += smp_scu.o obj-$(CONFIG_HAVE_ARM_TWD) += smp_twd.o -obj-$(CONFIG_ARM_ARCH_TIMER) += arch_timer.o obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o
arch/arm/kernel/arch_timer.c deleted +0 -44
44 lines not on this page · raw message
arch/arm64/include/asm/arch_timer.h +0 -5
@@ -145,9 +145,4 @@ static inline u64 arch_counter_get_cntvct(void) return cval; } -static inline int arch_timer_arch_init (void ) -{ -return 0 ; -} -#endif
arch/arm64/include/asm/delay.h new file +32 -0
@@ -0,0 +1,32 @@ + +/* +* Copyright (c) 2014, NVIDIA Corporation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify it +* under the terms and conditions of the GNU General Public License, +* version 2, as published by the Free Software Foundation. +* +* This program is distributed in the hope it will be useful, but WITHOUT +* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +* more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef __ASM_ARM64_DELAY_H +#define __ASM_ARM64_DELAY_H + +#include <asm-generic/delay.h> + +struct delay_timer { +unsigned long (*read_current_timer)(void ); +unsigned long freq; +}; + +/* Delay-loop timer registration. */ +#define ARCH_HAS_READ_CURRENT_TIMER +extern void register_current_timer_delay (const struct delay_timer *timer); + +#endif /* defined(_ARM64_DELAY_H) */
arch/arm64/include/asm/timex.h +2 -3
@@ -16,13 +16,12 @@#ifndef __ASM_TIMEX_H#define __ASM_TIMEX_H -#include <asm/arch_timer.h> -/* * Use the current timer as a cycle counter since this is what we use for * the delay loop. */ -#define get_cycles () arch_counter_get_cntvct() +typedef unsigned long cycles_t ; +#define get_cycles () ({ cycles_t c; read_current_timer(&c) ? 0 : c; })#include <asm-generic/timex.h>
arch/arm64/kernel/time.c +0 -9
@@ -63,14 +63,5 @@ EXPORT_SYMBOL(profile_pc);void __init time_init (void ) { -u32 arch_timer_rate; -clocksource_of_init (); - - arch_timer_rate =arch_timer_get_rate (); -if (!arch_timer_rate) -panic ("Unable to initialise architected timer.\n" ); - -/* Calibrate the delay loop directly */ - lpj_fine = arch_timer_rate /HZ ; }
arch/arm64/lib/delay.c +26 -0
@@ -24,6 +24,19 @@#include <linux/module.h> #include <linux/timex.h> +static const struct delay_timer *delay_timer; +static bool delay_calibrated; + +int read_current_timer (unsigned long *timer_val) +{ +if (!delay_timer) +return -ENXIO ; + + *timer_val = delay_timer->read_current_timer (); +return 0 ; +} +EXPORT_SYMBOL (read_current_timer); +void __delay (unsigned long cycles) {cycles_t start =get_cycles ();
@@ -53,3 +66,16 @@ void __ndelay(unsigned long nsecs)__const_udelay (nsecs *0x5UL );/* 2**32 / 1000000000 (rounded up) */ }EXPORT_SYMBOL (__ndelay); + +void register_current_timer_delay (const struct delay_timer *timer) +{ +if (!delay_calibrated) { +pr_info ("Switching to timer-based delay loop\n" ); + delay_timer = timer; + lpj_fine = timer->freq /HZ ; + + delay_calibrated = true; + }else { +pr_info ("Ignoring duplicate/late registration of read_current_timer delay\n" ); + } +}
drivers/clocksource/arm_arch_timer.c +11 -1
@@ -422,6 +422,16 @@ struct timecounter *arch_timer_get_timecounter(void)return &timecounter; } +static struct delay_timer arch_delay_timer; + +static void __init arch_delay_timer_register (void ) +{ +/* Use the architected timer for the delay loop. */ + arch_delay_timer.read_current_timer =arch_timer_read_counter (); + arch_delay_timer.freq = arch_timer_rate; +register_current_timer_delay (&arch_delay_timer); +} +static void __init arch_counter_register (unsigned type) {u64 start_count;
@@ -630,7 +640,7 @@ static void __init arch_timer_common_init(void)arch_timer_banner (arch_timers_present);arch_counter_register (arch_timers_present); -arch_timer_arch_init (); +arch_delay_timer_register (); }static void __init arch_timer_init (struct device_node *np)
Signature · 1 line
1.7.4.1
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 134 lines · Prashant Gaikwad
Now arch timer is registerd using generic sched timer, delay timer registration is the only part remaining in arch ports. Move this part to drivers clocksource and remove arch timer from arch ports.
Signed-off-by Prashant Gaikwad <[email protected]>--- arch/arm/include/asm/arch_timer.h | 1 - arch/arm/kernel/Makefile | 1 - arch/arm/kernel/arch_timer.c | 44 ---------------------------------- arch/arm64/include/asm/arch_timer.h | 5 ---- arch/arm64/include/asm/delay.h | 32 ++++++++++++++++++++++++ arch/arm64/include/asm/timex.h | 5 +-- arch/arm64/kernel/time.c | 9 ------- arch/arm64/lib/delay.c | 26 ++++++++++++++++++++ drivers/clocksource/arm_arch_timer.c | 12 ++++++++- 9 files changed, 71 insertions(+), 64 deletions(-) delete mode 100644 arch/arm/kernel/arch_timer.c create mode 100644 arch/arm64/include/asm/delay.harch/arm/include/asm/arch_timer.h +0
-1@@ -10,7 +10,6 @@#include <clocksource/arm_arch_timer.h> #ifdef CONFIG_ARM_ARCH_TIMER -int arch_timer_arch_init (void );/* * These register accessors are marked inline so the compiler can arch/arm/kernel/Makefile +0
-1@@ -45,7 +45,6 @@ obj-$(CONFIG_SMP) += smp_tlb.o endif obj-$(CONFIG_HAVE_ARM_SCU) += smp_scu.o obj-$(CONFIG_HAVE_ARM_TWD) += smp_twd.o -obj-$(CONFIG_ARM_ARCH_TIMER) += arch_timer.o obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.oarch/arm/kernel/arch_timer.c deleted +0
-44@@ -1,44 +0,0 @@ -/* -* linux/arch/arm/kernel/arch_timer.c -* -* Copyright (C) 2011 ARM Ltd. -* All Rights Reserved -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -*/ -#include <linux/init.h> -#include <linux/types.h> -#include <linux/errno.h> - -#include <asm/delay.h> - -#include <clocksource/arm_arch_timer.h> - -static unsigned long arch_timer_read_counter_long (void ) -{ -return arch_timer_read_counter (); -} - -static struct delay_timer arch_delay_timer; - -static void __init arch_timer_delay_timer_register (void ) -{ -/* Use the architected timer for the delay loop. */ - arch_delay_timer.read_current_timer = arch_timer_read_counter_long; - arch_delay_timer.freq =arch_timer_get_rate (); -register_current_timer_delay (&arch_delay_timer); -} - -int __init arch_timer_arch_init (void ) -{ -u32 arch_timer_rate =arch_timer_get_rate (); - -if (arch_timer_rate ==0 ) -return -ENXIO ; - -arch_timer_delay_timer_register (); - -return 0 ; -}arch/arm64/include/asm/arch_timer.h +0
-5@@ -145,9 +145,4 @@ static inline u64 arch_counter_get_cntvct(void) return cval; } -static inline int arch_timer_arch_init (void ) -{ -return 0 ; -} -#endif arch/arm64/include/asm/delay.h new file +20
-0@@ -0,0 +1,32 @@ + +/* +* Copyright (c) 2014, NVIDIA Corporation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify it +* under the terms and conditions of the GNU General Public License, +* version 2, as published by the Free Software Foundation. +* +* This program is distributed in the hope it will be useful, but WITHOUT +* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +* more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef __ASM_ARM64_DELAY_H +#define __ASM_ARM64_DELAY_H +
arch/arm64/include/asm/delay.h line 30
Quoted 10 lines
arch/arm64/include/asm/delay.h new file +10
-0+#include <asm-generic/delay.h> + +struct delay_timer { +unsigned long (*read_current_timer)(void ); +unsigned long freq; +}; + +/* Delay-loop timer registration. */ +#define ARCH_HAS_READ_CURRENT_TIMER +extern void register_current_timer_delay (const struct delay_timer *timer);
Can't all but the define be moved to a common location?
Quoted 9 lines
arch/arm64/include/asm/delay.h new file +2
-0+ +#endif /* defined(_ARM64_DELAY_H) */ arch/arm64/include/asm/timex.h +0
-0@@ -16,13 +16,12 @@#ifndef __ASM_TIMEX_H#define __ASM_TIMEX_H
arch/arm64/include/asm/timex.h line 24
Quoted 10 lines
arch/arm64/include/asm/timex.h +2
-3-#include <asm/arch_timer.h> -/* * Use the current timer as a cycle counter since this is what we use for * the delay loop. */ -#define get_cycles () arch_counter_get_cntvct() +typedef unsigned long cycles_t ; +#define get_cycles () ({ cycles_t c; read_current_timer(&c) ? 0 : c; })
Could be in a common location conditional on ARCH_HAS_READ_CURRENT_TIMER.
Quoted 32 lines
arch/arm64/include/asm/timex.h +0
-0#include <asm-generic/timex.h> arch/arm64/kernel/time.c +0
-9@@ -63,14 +63,5 @@ EXPORT_SYMBOL(profile_pc);void __init time_init (void ) { -u32 arch_timer_rate; -clocksource_of_init (); - - arch_timer_rate =arch_timer_get_rate (); -if (!arch_timer_rate) -panic ("Unable to initialise architected timer.\n" ); - -/* Calibrate the delay loop directly */ - lpj_fine = arch_timer_rate /HZ ; }arch/arm64/lib/delay.c +3
-0@@ -24,6 +24,19 @@#include <linux/module.h> #include <linux/timex.h> +static const struct delay_timer *delay_timer; +static bool delay_calibrated; +
arch/arm64/lib/delay.c line 39
Quoted 10 lines
arch/arm64/lib/delay.c +10
-0+int read_current_timer (unsigned long *timer_val) +{ +if (!delay_timer) +return -ENXIO ; + + *timer_val = delay_timer->read_current_timer (); +return 0 ; +} +EXPORT_SYMBOL (read_current_timer); +
This too could be common.
Quoted 58 lines
arch/arm64/lib/delay.c +13
-0void __delay (unsigned long cycles) {cycles_t start =get_cycles ();@@ -53,3 +66,16 @@ void __ndelay(unsigned long nsecs)__const_udelay (nsecs *0x5UL );/* 2**32 / 1000000000 (rounded up) */ }EXPORT_SYMBOL (__ndelay); + +void register_current_timer_delay (const struct delay_timer *timer) +{ +if (!delay_calibrated) { +pr_info ("Switching to timer-based delay loop\n" ); + delay_timer = timer; + lpj_fine = timer->freq /HZ ; + + delay_calibrated = true; + }else { +pr_info ("Ignoring duplicate/late registration of read_current_timer delay\n" ); + } +}drivers/clocksource/arm_arch_timer.c +11
-2@@ -422,6 +422,16 @@ struct timecounter *arch_timer_get_timecounter(void)return &timecounter; } +static struct delay_timer arch_delay_timer; + +static void __init arch_delay_timer_register (void ) +{ +/* Use the architected timer for the delay loop. */ + arch_delay_timer.read_current_timer =arch_timer_read_counter (); + arch_delay_timer.freq = arch_timer_rate; +register_current_timer_delay (&arch_delay_timer); +} +static void __init arch_counter_register (unsigned type) {u64 start_count;@@ -630,7 +640,7 @@ static void __init arch_timer_common_init(void)arch_timer_banner (arch_timers_present);arch_counter_register (arch_timers_present); -arch_timer_arch_init (); +arch_delay_timer_register (); }static void __init arch_timer_init (struct device_node *np) --1.7.4.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 6 lines · Rob Herring
Quoted 5 lines · Prashant Gaikwad
Now arch timer is registerd using generic sched timer, delay timer registration is the only part remaining in arch ports. Move this part to drivers clocksource and remove arch timer from arch ports.
Quoted 14 lines
Quoted 12 lines
Signed-off-by Prashant Gaikwad <[email protected]>--- arch/arm/include/asm/arch_timer.h | 1 - arch/arm/kernel/Makefile | 1 - arch/arm/kernel/arch_timer.c | 44 ---------------------------------- arch/arm64/include/asm/arch_timer.h | 5 ---- arch/arm64/include/asm/delay.h | 32 ++++++++++++++++++++++++ arch/arm64/include/asm/timex.h | 5 +-- arch/arm64/kernel/time.c | 9 ------- arch/arm64/lib/delay.c | 26 ++++++++++++++++++++ drivers/clocksource/arm_arch_timer.c | 12 ++++++++- 9 files changed, 71 insertions(+), 64 deletions(-)Quoted 2 lines
delete mode 100644
arch/arm/kernel/arch_timer.ccreate mode 100644arch/arm64/include/asm/delay.h
Thanks for the comments Rob!! I was planning to do that in later patch. Is that fine?
Quoted 249 lines
Quoted 114 lines
arch/arm/include/asm/arch_timer.h +0
-1@@ -10,7 +10,6 @@#include <clocksource/arm_arch_timer.h> #ifdef CONFIG_ARM_ARCH_TIMER -int arch_timer_arch_init (void );/* * These register accessors are marked inline so the compiler can arch/arm/kernel/Makefile +0
-1@@ -45,7 +45,6 @@ obj-$(CONFIG_SMP) += smp_tlb.o endif obj-$(CONFIG_HAVE_ARM_SCU) += smp_scu.o obj-$(CONFIG_HAVE_ARM_TWD) += smp_twd.o -obj-$(CONFIG_ARM_ARCH_TIMER) += arch_timer.o obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.oarch/arm/kernel/arch_timer.c deleted +0
-44@@ -1,44 +0,0 @@ -/* -* linux/arch/arm/kernel/arch_timer.c -* -* Copyright (C) 2011 ARM Ltd. -* All Rights Reserved -* -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License version 2 as -* published by the Free Software Foundation. -*/ -#include <linux/init.h> -#include <linux/types.h> -#include <linux/errno.h> - -#include <asm/delay.h> - -#include <clocksource/arm_arch_timer.h> - -static unsigned long arch_timer_read_counter_long (void ) -{ -return arch_timer_read_counter (); -} - -static struct delay_timer arch_delay_timer; - -static void __init arch_timer_delay_timer_register (void ) -{ -/* Use the architected timer for the delay loop. */ - arch_delay_timer.read_current_timer = arch_timer_read_counter_long; - arch_delay_timer.freq =arch_timer_get_rate (); -register_current_timer_delay (&arch_delay_timer); -} - -int __init arch_timer_arch_init (void ) -{ -u32 arch_timer_rate =arch_timer_get_rate (); - -if (arch_timer_rate ==0 ) -return -ENXIO ; - -arch_timer_delay_timer_register (); - -return 0 ; -}arch/arm64/include/asm/arch_timer.h +0
-5@@ -145,9 +145,4 @@ static inline u64 arch_counter_get_cntvct(void) return cval; } -static inline int arch_timer_arch_init (void ) -{ -return 0 ; -} -#endif arch/arm64/include/asm/delay.h new file +20
-0@@ -0,0 +1,32 @@ + +/* +* Copyright (c) 2014, NVIDIA Corporation. All rights reserved. +* +* This program is free software; you can redistribute it and/or modify it +* under the terms and conditions of the GNU General Public License, +* version 2, as published by the Free Software Foundation. +* +* This program is distributed in the hope it will be useful, but WITHOUT +* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +* more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef __ASM_ARM64_DELAY_H +#define __ASM_ARM64_DELAY_H +arch/arm64/include/asm/delay.h line 30
Quoted 10 lines
arch/arm64/include/asm/delay.h new file +10
-0+#include <asm-generic/delay.h> + +struct delay_timer { +unsigned long (*read_current_timer)(void ); +unsigned long freq; +}; + +/* Delay-loop timer registration. */ +#define ARCH_HAS_READ_CURRENT_TIMER +extern void register_current_timer_delay (const struct delay_timer *timer);Can't all but the define be moved to a common location?
Quoted 9 lines
arch/arm64/include/asm/delay.h new file +2
-0+ +#endif /* defined(_ARM64_DELAY_H) */ arch/arm64/include/asm/timex.h +0
-0@@ -16,13 +16,12 @@#ifndef __ASM_TIMEX_H#define __ASM_TIMEX_Harch/arm64/include/asm/timex.h line 24
Quoted 10 lines
arch/arm64/include/asm/timex.h +2
-3-#include <asm/arch_timer.h> -/* * Use the current timer as a cycle counter since this is what we use for * the delay loop. */ -#define get_cycles () arch_counter_get_cntvct() +typedef unsigned long cycles_t ; +#define get_cycles () ({ cycles_t c; read_current_timer(&c) ? 0 : c; })Could be in a common location conditional on
ARCH_HAS_READ_CURRENT_TIMER.Quoted 32 lines
arch/arm64/include/asm/timex.h +0
-0#include <asm-generic/timex.h> arch/arm64/kernel/time.c +0
-9@@ -63,14 +63,5 @@ EXPORT_SYMBOL(profile_pc);void __init time_init (void ) { -u32 arch_timer_rate; -clocksource_of_init (); - - arch_timer_rate =arch_timer_get_rate (); -if (!arch_timer_rate) -panic ("Unable to initialise architected timer.\n" ); - -/* Calibrate the delay loop directly */ - lpj_fine = arch_timer_rate /HZ ; }arch/arm64/lib/delay.c +3
-0@@ -24,6 +24,19 @@#include <linux/module.h> #include <linux/timex.h> +static const struct delay_timer *delay_timer; +static bool delay_calibrated; +arch/arm64/lib/delay.c line 39
Quoted 10 lines
arch/arm64/lib/delay.c +10
-0+int read_current_timer (unsigned long *timer_val) +{ +if (!delay_timer) +return -ENXIO ; + + *timer_val = delay_timer->read_current_timer (); +return 0 ; +} +EXPORT_SYMBOL (read_current_timer); +This too could be common.
Quoted 58 lines
arch/arm64/lib/delay.c +13
-0void __delay (unsigned long cycles) {cycles_t start =get_cycles ();@@ -53,3 +66,16 @@ void __ndelay(unsigned long nsecs)__const_udelay (nsecs *0x5UL );/* 2**32 / 1000000000 (rounded up) */ }EXPORT_SYMBOL (__ndelay); + +void register_current_timer_delay (const struct delay_timer *timer) +{ +if (!delay_calibrated) { +pr_info ("Switching to timer-based delay loop\n" ); + delay_timer = timer; + lpj_fine = timer->freq /HZ ; + + delay_calibrated = true; + }else { +pr_info ("Ignoring duplicate/late registration of read_current_timer delay\n" ); + } +}drivers/clocksource/arm_arch_timer.c +11
-2@@ -422,6 +422,16 @@ struct timecounter *arch_timer_get_timecounter(void)return &timecounter; } +static struct delay_timer arch_delay_timer; + +static void __init arch_delay_timer_register (void ) +{ +/* Use the architected timer for the delay loop. */ + arch_delay_timer.read_current_timer =arch_timer_read_counter (); + arch_delay_timer.freq = arch_timer_rate; +register_current_timer_delay (&arch_delay_timer); +} +static void __init arch_counter_register (unsigned type) {u64 start_count;@@ -630,7 +640,7 @@ static void __init arch_timer_common_init(void)arch_timer_banner (arch_timers_present);arch_counter_register (arch_timers_present); -arch_timer_arch_init (); +arch_delay_timer_register (); }static void __init arch_timer_init (struct device_node *np) --1.7.4.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
In reply to Prashant GaikwadHello,
Quoted 4 lines · Prashant Gaikwad
Now arch timer is registerd using generic sched timer, delay timer registration is the only part remaining in arch ports. Move this part to drivers clocksource and remove arch timer from arch ports.
What's the advantage in doing this? I'd have thought consolidation, but...
Quoted 12 lines
Signed-off-by Prashant Gaikwad <[email protected]>--- arch/arm/include/asm/arch_timer.h | 1 - arch/arm/kernel/Makefile | 1 - arch/arm/kernel/arch_timer.c | 44 ---------------------------------- arch/arm64/include/asm/arch_timer.h | 5 ---- arch/arm64/include/asm/delay.h | 32 ++++++++++++++++++++++++ arch/arm64/include/asm/timex.h | 5 +-- arch/arm64/kernel/time.c | 9 ------- arch/arm64/lib/delay.c | 26 ++++++++++++++++++++ drivers/clocksource/arm_arch_timer.c | 12 ++++++++- 9 files changed, 71 insertions(+), 64 deletions(-)
... that's a positive diffstat! I also think that delaying the delay loop initialisation for arm64 could be problematic, since we don't have anything to fall back on (like the busy-loop on ARM) in case of early *delay calls.
What happens if I call udelay on arm64 before the counter has registered?
Will
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 8 lines · Will Deacon
Hello,
Quoted 4 lines · Prashant Gaikwad
Now arch timer is registerd using generic sched timer, delay timer registration is the only part remaining in arch ports. Move this part to drivers clocksource and remove arch timer from arch ports.
What's the advantage in doing this? I'd have thought consolidation, but...
Primary objective of doing this to use other timers on SoC instead of arch timers. It is observed that on ARM implementations of ARMv8 architecture context of per CPU logic of arch timer is not preserved when CPU is powered down and reset is held during powerdown.
This makes arch timers unusable as wake events for some CPU idle states where CPU core is powered down.
So we have two options:
1. Use system timers as broadcast timers and save/restore arch timers context actross these CPU idle states.
2. Disable arch timer and use system timers.
This patch will help to implement 2nd option.
Quoted 12 lines
Quoted 12 lines
Signed-off-by Prashant Gaikwad <[email protected]>--- arch/arm/include/asm/arch_timer.h | 1 - arch/arm/kernel/Makefile | 1 - arch/arm/kernel/arch_timer.c | 44 ---------------------------------- arch/arm64/include/asm/arch_timer.h | 5 ---- arch/arm64/include/asm/delay.h | 32 ++++++++++++++++++++++++ arch/arm64/include/asm/timex.h | 5 +-- arch/arm64/kernel/time.c | 9 ------- arch/arm64/lib/delay.c | 26 ++++++++++++++++++++ drivers/clocksource/arm_arch_timer.c | 12 ++++++++- 9 files changed, 71 insertions(+), 64 deletions(-)
Quoted 5 lines
... that's a positive diffstat! I also think that delaying the delay loop initialisation for arm64 could be problematic, since we don't have anything to fall back on (like the busy-loop on ARM) in case of early *delay calls.
What happens if I call udelay on arm64 before the counter has registered?
I will fix this in next patch. Do you have plan to add busy-loop for arm64? Or I will have to initialize read counter function early.
Quoted 1 line
Will
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 13 lines · Prashant Gaikwad
Quoted 6 lines · Will Deacon
Quoted 4 lines · Prashant Gaikwad
Now arch timer is registerd using generic sched timer, delay timer registration is the only part remaining in arch ports. Move this part to drivers clocksource and remove arch timer from arch ports.
What's the advantage in doing this? I'd have thought consolidation, but...
Primary objective of doing this to use other timers on SoC instead of arch timers. It is observed that on ARM implementations of ARMv8 architecture context of per CPU logic of arch timer is not preserved when CPU is powered down and reset is held during powerdown.
Quoted 7 lines
This makes arch timers unusable as wake events for some CPU idle states where CPU core is powered down.
So we have two options:
1. Use system timers as broadcast timers and save/restore arch timers context actross these CPU idle states.
2. Disable arch timer and use system timers.
Why can't you use the C3STOP feature so that the arch-timer isn't used when you go idle?
There are several good reasons to use the architected timers when you can:
- KVM requires them, as they have hardware support for virtualisation
- The VDSO can make use of them to implement efficient gtod in userspace
- They are per-cpu, so no need to broadcasting in software
- They can generate events and allow userspace to implement efficient polling loops
Quoted 4 lines
Quoted 1 line
What happens if I call udelay on arm64 before the counter has registered?
I will fix this in next patch. Do you have plan to add busy-loop for arm64? Or I will have to initialize read counter function early.
Well, I assume the virtual counter (cntvct) still ticks on your system? If so, I would suggest having that as the default delay implementation and allowing it to be over-ridden later on.
The part I don't understand is why being `unusable as wake events' affects the delay loop. All we need for that is the counter, the timers are irrelevant.
Will
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 2 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when you go idle?
That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs. Does anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
--Antti
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 6 lines · Antti Miettinen
Quoted 2 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when you go idle?
That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the CLOCK_EVT_DYNIRQ flag on the broadcast timer.
Quoted 3 lines
Does
anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 8 lines · Daniel Lezcano
Quoted 5 lines · Antti Miettinen
Quoted 2 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when you go idle?
That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.
Instead of falling back on broadcast timer, is it possible to fall back on other per-CPU timer which is preserved across idle state?
Quoted 5 lines
Quoted 3 lines
Does
anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 6 lines · Prashant Gaikwad
Quoted 5 lines · Daniel Lezcano
Quoted 4 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back on other per-CPU timer which is preserved across idle state?
Is it what you are looking for ?
http://lwn.net/Articles/580568/
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
Does
anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 9 lines · Daniel Lezcano
Quoted 8 lines · Prashant Gaikwad
Quoted 7 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back on other per-CPU timer which is preserved across idle state?
Is it what you are looking for ?
If I understand correctly these patches enables us to use per-CPU timers as broadcast timers. I do not want to use broadcast timer.
If I have 2 per-CPU timers T1 and T2, T1 is not preserved across idle state and T2 is preserved. And I want to use T1 as scheduler timer.
Can I do following for idle state?
Idle entry:
clockevents_shutdown(T1);
clockevents_set_mode(T2, ONESHOT);
clockevents_program_event(T2, next_event);Idle exit:
clockevents_shutdown(T2);
clockevents_set_mode(T1, ONESHOT);Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Does
anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 14 lines · Prashant Gaikwad
Quoted 13 lines · Daniel Lezcano
Quoted 12 lines · Prashant Gaikwad
Quoted 9 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back on other per-CPU timer which is preserved across idle state?
Quoted 7 lines
Quoted 4 lines
Is it what you are looking for ?
If I understand correctly these patches enables us to use per-CPU timers as broadcast timers. I do not want to use broadcast timer.
Why ?
Quoted 22 lines
If I have 2 per-CPU timers T1 and T2, T1 is not preserved across idle state and T2 is preserved. And I want to use T1 as scheduler timer. Can I do following for idle state?
Idle entry:
clockevents_shutdown(T1); clockevents_set_mode(T2, ONESHOT); clockevents_program_event(T2, next_event);Idle exit:
clockevents_shutdown(T2); clockevents_set_mode(T1, ONESHOT);Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Does
anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 15 lines · Daniel Lezcano
Quoted 14 lines · Prashant Gaikwad
Quoted 13 lines · Daniel Lezcano
Quoted 12 lines · Prashant Gaikwad
Quoted 9 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back on other per-CPU timer which is preserved across idle state?
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Is it what you are looking for ?
If I understand correctly these patches enables us to use per-CPU timers as broadcast timers. I do not want to use broadcast timer.
Why ?
For some idle states it may be required to change the timer when entering idle state to adjust the exit latency.
It can be done for broadcast timer too but following scenario will not work
1. CPU1 enters in idle state:
Broadcast timer next event is in 2ms, CPU latency is 50us. So
we change the broadcast timer to send event after (2ms - 50us).
2. After 1ms CPU2 enters in idle state:
Next event is 5ms. Broadcast timer is already programmed to <
(5ms -50us) so we do nothing.
3. CPU1 exits from idle state because of timer interrupt
4. Broadcast event handler:
- Timer event is handled and CPU1 is switched back to local timer.
- Next CPU is CPU2 and next event for it is 4ms. So brodcast timer
is programmed to 4ms.
We can not change brodcast timer here to adjust delay caused by CPU exit latency.
CPU idle governors does help to solve the latency issue. I was thinking this from sub-states perspective which are not exposed to CPU idle governor.
Solution for this could be to expose those states to CPU idle governor but just wanted to know if we can use timers this way?
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake event
for C2.
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
Quoted 19 lines
Quoted 18 lines
If I have 2 per-CPU timers T1 and T2, T1 is not preserved across idle state and T2 is preserved. And I want to use T1 as scheduler timer. Can I do following for idle state?
Idle entry:
clockevents_shutdown(T1); clockevents_set_mode(T2, ONESHOT); clockevents_program_event(T2, next_event);Idle exit:
clockevents_shutdown(T2); clockevents_set_mode(T1, ONESHOT);Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Does
anyone have patches for using a CPU local timer as a fallback for C3STOP timers?
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 6 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Quoted 8 lines
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the clockevents_notify() caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 13 lines · Stephen Boyd
Quoted 12 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.
Quoted 8 lines
Quoted 2 lines
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the
clockevents_notify()caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.
IMO, there are different solutions:
1. extend the C3STOP to C1STOP, C2STOP, etc ... and pass the idle state to the time framework where these flags are checked against. I don't like this approach but it is feasible.
2. use the generic power domain. When the power domain is shutdown via the cpuidle backend driver, it switches the timer.
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 23 lines · Daniel Lezcano
Quoted 13 lines · Stephen Boyd
Quoted 12 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.Quoted 8 lines
Quoted 2 lines
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the
clockevents_notify()caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.
Quoted 8 lines
IMO, there are different solutions:
1. extend the C3STOP to C1STOP, C2STOP, etc ... and pass the idle state to the time framework where these flags are checked against. I don't like this approach but it is feasible.
2. use the generic power domain. When the power domain is shutdown via the cpuidle backend driver, it switches the timer.
IMO, 2 is the way forward. It is the only solution that links resources to the reason they need maintainance (ie power management). I am writing v2 of C-state proposal where power domains are explicitly associated with devices (eg arch timers), and 2 fits well with this approach.
Lorenzo
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 30 lines · Lorenzo Pieralisi
Quoted 29 lines · Daniel Lezcano
Quoted 13 lines · Stephen Boyd
Quoted 12 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.Quoted 8 lines
Quoted 2 lines
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the
clockevents_notify()caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.IMO, there are different solutions:
1. extend the C3STOP to C1STOP, C2STOP, etc ... and pass the idle state to the time framework where these flags are checked against. I don't like this approach but it is feasible.
Quoted 7 lines
Quoted 2 lines
2. use the generic power domain. When the power domain is shutdown via the cpuidle backend driver, it switches the timer.
IMO, 2 is the way forward. It is the only solution that links resources to the reason they need maintainance (ie power management). I am writing v2 of C-state proposal where power domains are explicitly associated with devices (eg arch timers), and 2 fits well with this approach.
Thanks Lorenzo for your feedback.
-- Daniel
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
In reply to Daniel LezcanoQuoted 20 lines · Daniel Lezcano
Quoted 12 lines · Stephen Boyd
Quoted 11 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.Quoted 7 lines
Quoted 2 lines
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the
clockevents_notify()caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.
Quoted 8 lines
IMO, there are different solutions:
1. extend the C3STOP to C1STOP, C2STOP, etc ... and pass the idle state to the time framework where these flags are checked against. I don't like this approach but it is feasible.
2. use the generic power domain. When the power domain is shutdown via the cpuidle backend driver, it switches the timer.
I am aware of a way to attach idle state to GenPD where we enable an idle state when that power domain is turned off but not the other way where domain is shutdown via CPU idle driver. How do we do it?
Even though we shutdown power domain via cpuidle driver this still has to happen from CPU idle state, is that correct assumption? and we switch the timer here. So we still need a way to switch timer from CPU idle state. Hence the question remains is how to switch timers from idle state?
Quoted 2 lines
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 30 lines · Prashant Gaikwad
Quoted 20 lines · Daniel Lezcano
Quoted 12 lines · Stephen Boyd
Quoted 11 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.Quoted 7 lines
Quoted 2 lines
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the
clockevents_notify()caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.Quoted 8 lines
IMO, there are different solutions:
1. extend the C3STOP to C1STOP, C2STOP, etc ... and pass the idle state to the time framework where these flags are checked against. I don't like this approach but it is feasible.
2. use the generic power domain. When the power domain is shutdown via the cpuidle backend driver, it switches the timer.
Quoted 8 lines
I am aware of a way to attach idle state to GenPD where we enable an idle state when that power domain is turned off but not the other way where domain is shutdown via CPU idle driver. How do we do it?
Even though we shutdown power domain via cpuidle driver this still has to happen from CPU idle state, is that correct assumption? and we switch the timer here. So we still need a way to switch timer from CPU idle state. Hence the question remains is how to switch timers from idle state?
You can effectively attach a power domain to a cpuidle state but that wasn't the point.
What I meant is to create a generic power domain which maps the power domain of the idle state. When the power domain is shutdown, the callback of the genpd will switch to the timer.
I can't give too much details because I am not used to this code but maybe it is a good solution for your specific case.
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 41 lines · Daniel Lezcano
Quoted 29 lines · Prashant Gaikwad
Quoted 20 lines · Daniel Lezcano
Quoted 12 lines · Stephen Boyd
Quoted 11 lines · Prashant Gaikwad
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake
event for C2.Quoted 7 lines
Quoted 2 lines
Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
We have the same situation on MSM. I've been thinking about proposing we allow multiple broadcast timers to exist in the system and then have the
clockevents_notify()caller indicate which C state is being entered. The broadcast timers would need to indicate which C state they don't work in though.Quoted 8 lines
IMO, there are different solutions:
1. extend the C3STOP to C1STOP, C2STOP, etc ... and pass the idle state to the time framework where these flags are checked against. I don't like this approach but it is feasible.
2. use the generic power domain. When the power domain is shutdown via the cpuidle backend driver, it switches the timer.
Quoted 8 lines
I am aware of a way to attach idle state to GenPD where we enable an idle state when that power domain is turned off but not the other way where domain is shutdown via CPU idle driver. How do we do it?
Even though we shutdown power domain via cpuidle driver this still has to happen from CPU idle state, is that correct assumption? and we switch the timer here. So we still need a way to switch timer from CPU idle state. Hence the question remains is how to switch timers from idle state?
You can effectively attach a power domain to a cpuidle state but that wasn't the point.
Quoted 7 lines
What I meant is to create a generic power domain which maps the power domain of the idle state. When the power domain is shutdown, the callback of the genpd will switch to the timer.
I can't give too much details because I am not used to this code but maybe it is a good solution for your specific case.
Somehow this is not mapping to my use case. We are using generic power domains with CPU idle states.
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
In reply to Prashant GaikwadQuoted 2 lines · Prashant Gaikwad
For some idle states it may be required to change the timer when entering idle state to adjust the exit latency.
Would it work to set up a separate software timer to account for the exit latency?
--Antti
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
In reply to Prashant GaikwadQuoted 41 lines · Prashant Gaikwad
Quoted 16 lines · Daniel Lezcano
Quoted 15 lines · Prashant Gaikwad
Quoted 14 lines · Daniel Lezcano
Quoted 13 lines · Prashant Gaikwad
Quoted 9 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back
on other per-CPU timer which is preserved across idle state?Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Is it what you are looking for ?
If I understand correctly these patches enables us to use per-CPU timers as broadcast timers. I do not want to use broadcast timer.
Why ?
For some idle states it may be required to change the timer when entering idle state to adjust the exit latency.
It can be done for broadcast timer too but following scenario will not work
1. CPU1 enters in idle state:
Broadcast timer next event is in 2ms, CPU latency is 50us. So
we change the broadcast timer to send event after (2ms - 50us).2. After 1ms CPU2 enters in idle state:
Next event is 5ms. Broadcast timer is already programmed to <
(5ms -50us) so we do nothing.3. CPU1 exits from idle state because of timer interrupt
Quoted 7 lines
4. Broadcast event handler:
- Timer event is handled and CPU1 is switched back to local timer.
- Next CPU is CPU2 and next event for it is 4ms. So brodcast timer
is programmed to 4ms.We can not change brodcast timer here to adjust delay caused by CPU exit latency.
Thanks for the detailed explanation. IIUC, this not only related to your hardware only but with how is implemented the broadcast timer, no ?
I think there is a similar need with the scheduler when it needs to know what is the idlest cpu. One thing the scheduler wants to know is the wakeup latency in order to choose the cpu in the shallowest state.
Quoted 3 lines
CPU idle governors does help to solve the latency issue. I was thinking this from sub-states perspective which are not exposed to CPU idle governor.
Could you elaborate what you mean by these sub-states ? Is it related to the cpuidle backend drivers choosing an intermediate state different from the one the governor choose ?
Quoted 2 lines
Solution for this could be to expose those states to CPU idle governor but just wanted to know if we can use timers this way
IMO, that should be studied in a larger scope including the scheduler.
Quoted 5 lines
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Quoted 21 lines
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake event
for C2.Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
Quoted 12 lines
Quoted 12 lines
If I have 2 per-CPU timers T1 and T2, T1 is not preserved across idle state and T2 is preserved. And I want to use T1 as scheduler timer. Can I do following for idle state?
Idle entry:
clockevents_shutdown(T1); clockevents_set_mode(T2, ONESHOT); clockevents_program_event(T2, next_event);Idle exit:
clockevents_shutdown(T2); clockevents_set_mode(T1, ONESHOT);
See answer to Stephen.
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 46 lines · Daniel Lezcano
Quoted 45 lines · Prashant Gaikwad
Quoted 16 lines · Daniel Lezcano
Quoted 15 lines · Prashant Gaikwad
Quoted 14 lines · Daniel Lezcano
Quoted 13 lines · Prashant Gaikwad
Quoted 9 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back
on other per-CPU timer which is preserved across idle state?Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Is it what you are looking for ?
If I understand correctly these patches enables us to use per-CPU timers as broadcast timers. I do not want to use broadcast timer.
Why ?
For some idle states it may be required to change the timer when entering idle state to adjust the exit latency.
It can be done for broadcast timer too but following scenario will not work
1. CPU1 enters in idle state:
Broadcast timer next event is in 2ms, CPU latency is 50us. So
we change the broadcast timer to send event after (2ms - 50us).2. After 1ms CPU2 enters in idle state:
Next event is 5ms. Broadcast timer is already programmed to <
(5ms -50us) so we do nothing.3. CPU1 exits from idle state because of timer interrupt
4. Broadcast event handler:
- Timer event is handled and CPU1 is switched back to local timer.
- Next CPU is CPU2 and next event for it is 4ms. So brodcast timer
is programmed to 4ms.
Quoted 4 lines
Quoted 2 lines
We can not change brodcast timer here to adjust delay caused by CPU exit latency.
Thanks for the detailed explanation. IIUC, this not only related to your hardware only but with how is implemented the broadcast timer, no ?
Yes.
Quoted 10 lines
I think there is a similar need with the scheduler when it needs to know what is the idlest cpu. One thing the scheduler wants to know is the wakeup latency in order to choose the cpu in the shallowest state.
Quoted 3 lines
CPU idle governors does help to solve the latency issue. I was thinking this from sub-states perspective which are not exposed to CPU idle governor.
Could you elaborate what you mean by these sub-states ? Is it related to the cpuidle backend drivers choosing an intermediate state different from the one the governor choose ?
Yes.
Quoted 4 lines
Quoted 2 lines
Solution for this could be to expose those states to CPU idle governor but just wanted to know if we can use timers this way
IMO, that should be studied in a larger scope including the scheduler.
Is this about the per-core timer switching as proposed below ...
Idle entry:
clockevents_shutdown(T1);
clockevents_set_mode(T2, ONESHOT);
clockevents_program_event(T2, next_event - latency);Idle exit:
clockevents_shutdown(T2);
clockevents_set_mode(T1, ONESHOT);... or about overall approach for this requirement?
Quoted 29 lines
Quoted 5 lines
Another requirement:
We have 3 timers T1, T2, T3 used as wake events for 3 idle states C1, C2, C3 respectively.
Quoted 21 lines
Rating of T2 is better than T3. If I register T2 and T3 both as broadcast timers then T3 will not be used. But ...
- T2 is not preserved in C3 idle state.
- T3 resolution is very poor (ms) and can not be used as wake event
for C2.Possible solution, register only T3 as broadcast device and use T2 as per-CPU fallback timer.
Quoted 12 lines
Quoted 12 lines
If I have 2 per-CPU timers T1 and T2, T1 is not preserved across idle state and T2 is preserved. And I want to use T1 as scheduler timer. Can I do following for idle state?
Idle entry:
clockevents_shutdown(T1); clockevents_set_mode(T2, ONESHOT); clockevents_program_event(T2, next_event);Idle exit:
clockevents_shutdown(T2); clockevents_set_mode(T1, ONESHOT);See answer to Stephen.
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
Quoted 49 lines · Prashant Gaikwad
Quoted 48 lines · Daniel Lezcano
Quoted 47 lines · Prashant Gaikwad
Quoted 16 lines · Daniel Lezcano
Quoted 15 lines · Prashant Gaikwad
Quoted 14 lines · Daniel Lezcano
Quoted 13 lines · Prashant Gaikwad
Quoted 9 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 3 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when
you go idle?That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.Instead of falling back on broadcast timer, is it possible to fall back
on other per-CPU timer which is preserved across idle state?Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Is it what you are looking for ?
If I understand correctly these patches enables us to use per-CPU timers
as broadcast timers. I do not want to use broadcast timer.Why ?
For some idle states it may be required to change the timer when entering idle state to adjust the exit latency.
It can be done for broadcast timer too but following scenario will not work
1. CPU1 enters in idle state:
Broadcast timer next event is in 2ms, CPU latency is 50us. So
we change the broadcast timer to send event after (2ms - 50us).2. After 1ms CPU2 enters in idle state:
Next event is 5ms. Broadcast timer is already programmed to <
(5ms -50us) so we do nothing.3. CPU1 exits from idle state because of timer interrupt
4. Broadcast event handler:
- Timer event is handled and CPU1 is switched back to local timer.
- Next CPU is CPU2 and next event for it is 4ms. So brodcast timer
is programmed to 4ms.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
We can not change brodcast timer here to adjust delay caused by CPU exit latency.
Thanks for the detailed explanation. IIUC, this not only related to your hardware only but with how is implemented the broadcast timer, no ?
Yes.
Hmm, interesting.
Quoted 5 lines
Quoted 5 lines
I think there is a similar need with the scheduler when it needs to know what is the idlest cpu. One thing the scheduler wants to know is the wakeup latency in order to choose the cpu in the shallowest state.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
CPU idle governors does help to solve the latency issue. I was thinking this from sub-states perspective which are not exposed to CPU idle governor.
Could you elaborate what you mean by these sub-states ? Is it related to the cpuidle backend drivers choosing an intermediate state different from the one the governor choose ?
Yes.
Ok.
Is there any tool to measure how the timers are far from the expected expiration time ? It would be interesting to do some measurements on this with and without cpuidle. That would help to check the future improvements.
Quoted 12 lines
Quoted 4 lines
Quoted 2 lines
Solution for this could be to expose those states to CPU idle governor but just wanted to know if we can use timers this way
IMO, that should be studied in a larger scope including the scheduler.
Is this about the per-core timer switching as proposed below ...
Idle entry:
clockevents_shutdown(T1); clockevents_set_mode(T2, ONESHOT); clockevents_program_event(T2, next_event - latency);
Quoted 5 lines
Idle exit:
clockevents_shutdown(T2); clockevents_set_mode(T1, ONESHOT);... or about overall approach for this requirement?
It is about the overall approach.
Signature · 5 lines
<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
PATCH 1/1 arch_timer: Move delay timer to drivers clocksource
In reply to Daniel LezcanoQuoted 10 lines · Daniel Lezcano
Quoted 6 lines · Antti Miettinen
Quoted 2 lines · Will Deacon
Why can't you use the C3STOP feature so that the arch-timer isn't used when you go idle?
That would mean falling back to broadcast timer, right? That's not necessarily on the local CPU so wakeups would often wake two CPUs.
You can prevent that if the hardware supports it with the
CLOCK_EVT_DYNIRQflag on the broadcast timer.
CLOCK_EVT_FEAT_DYNIRQ seems to be the flag. Cool, thanks.
--Antti