All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
200 Hz. Unfortunately in case of multiplatform image this affects also
other platforms when Exynos is enabled.
This looks like an very old legacy code, dating back to initial
upstreaming of S3C24xx. Probably it was required for s3c24xx timer
driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
unused plat-samsung/time.c").
Since then, this fixed 200 Hz spread everywhere, including out-of-tree
Samsung kernels (SoC vendor's and Tizen's). I believe this choice
was rather an effect of coincidence instead of conscious choice.
Exynos uses its own MCT or arch timer and can work with all HZ values.
Older platforms use newer Samsung PWM timer driver which should handle
down to 100 Hz.
Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
other values.
Reported-by: Lee Jones <[email protected]>
[Dropping 200_HZ from S3C/S5P suggested by Arnd]
Reported-by: Arnd Bergmann <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Cc: Kukjin Kim <[email protected]>
Tested-by: Javier Martinez Canillas <[email protected]>
---
Tested on Exynos5422 and Exynos5800 (by Javier). It would be
appreciated if anyone could test it on S3C24xx or S5PV210.
Changes since v1:
1. Add Javier's tested-by.
2. Drop HZ_FIXED also from ARCH_S5PV210 and ARCH_S3C24XX after Arnd
suggestions and analysis.
---
arch/arm/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529fdffab..ced2e08a9d08 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1496,8 +1496,7 @@ source kernel/Kconfig.preempt
config HZ_FIXED
int
- default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
- ARCH_S5PV210 || ARCH_EXYNOS4
+ default 200 if ARCH_EBSA110
default 128 if SOC_AT91RM9200
default 0
--
2.7.4
2016. 11. 18. 16:16 Krzysztof Kozlowski <[email protected]> wrote:
> All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> 200 Hz. Unfortunately in case of multiplatform image this affects also
> other platforms when Exynos is enabled.
>
> This looks like an very old legacy code, dating back to initial
> upstreaming of S3C24xx. Probably it was required for s3c24xx timer
> driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> unused plat-samsung/time.c").
>
> Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> Samsung kernels (SoC vendor's and Tizen's). I believe this choice
> was rather an effect of coincidence instead of conscious choice.
>
> Exynos uses its own MCT or arch timer and can work with all HZ values.
> Older platforms use newer Samsung PWM timer driver which should handle
> down to 100 Hz.
>
> Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> other values.
>
> Reported-by: Lee Jones <[email protected]>
> [Dropping 200_HZ from S3C/S5P suggested by Arnd]
> Reported-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> Cc: Kukjin Kim <[email protected]>
Acked-by: Kukjin Kim <[email protected]>
> Tested-by: Javier Martinez Canillas <[email protected]>
>
> ---
>
> Tested on Exynos5422 and Exynos5800 (by Javier). It would be
> appreciated if anyone could test it on S3C24xx or S5PV210.
>
> Changes since v1:
> 1. Add Javier's tested-by.
> 2. Drop HZ_FIXED also from ARCH_S5PV210 and ARCH_S3C24XX after Arnd
> suggestions and analysis.
> ---
> arch/arm/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b5d529fdffab..ced2e08a9d08 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1496,8 +1496,7 @@ source kernel/Kconfig.preempt
>
> config HZ_FIXED
> int
> - default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> - ARCH_S5PV210 || ARCH_EXYNOS4
> + default 200 if ARCH_EBSA110
> default 128 if SOC_AT91RM9200
> default 0
>
> --
> 2.7.4
>
On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
> All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> 200 Hz. Unfortunately in case of multiplatform image this affects also
> other platforms when Exynos is enabled.
>
> This looks like an very old legacy code, dating back to initial
> upstreaming of S3C24xx. Probably it was required for s3c24xx timer
> driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> unused plat-samsung/time.c").
>
> Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> Samsung kernels (SoC vendor's and Tizen's). I believe this choice
> was rather an effect of coincidence instead of conscious choice.
>
> Exynos uses its own MCT or arch timer and can work with all HZ values.
> Older platforms use newer Samsung PWM timer driver which should handle
> down to 100 Hz.
>
> Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> other values.
>
> Reported-by: Lee Jones <[email protected]>
> [Dropping 200_HZ from S3C/S5P suggested by Arnd]
> Reported-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Tested-by: Javier Martinez Canillas <[email protected]>
>
Acked-by: Arnd Bergmann <[email protected]>
Maybe add a paragraph about the specific problem:
"On s3c24xx, the PWM counter is only 16 bit wide, and with the
typical 12MHz input clock that overflows every 5.5ms. This works
with HZ=200 or higher but not with HZ=100 which needs a 10ms
interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
the counter is 32 bits and does not have this problem.
The new samsung_pwm_timer driver solves the problem by scaling
the input clock by a factor of 50 on s3c24xx, which makes it
less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
fewer wakeups".
Arnd
On 11/18/2016 09:46 AM, Arnd Bergmann wrote:
> On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
>> > All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
>> > 200 Hz. Unfortunately in case of multiplatform image this affects also
>> > other platforms when Exynos is enabled.
>> >
>> > This looks like an very old legacy code, dating back to initial
>> > upstreaming of S3C24xx. Probably it was required for s3c24xx timer
>> > driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
>> > unused plat-samsung/time.c").
>> >
>> > Since then, this fixed 200 Hz spread everywhere, including out-of-tree
>> > Samsung kernels (SoC vendor's and Tizen's). I believe this choice
>> > was rather an effect of coincidence instead of conscious choice.
>> >
>> > Exynos uses its own MCT or arch timer and can work with all HZ values.
>> > Older platforms use newer Samsung PWM timer driver which should handle
>> > down to 100 Hz.
>> >
>> > Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
>> > A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
>> > other values.
>> >
>> > Reported-by: Lee Jones <[email protected]>
>> > [Dropping 200_HZ from S3C/S5P suggested by Arnd]
>> > Reported-by: Arnd Bergmann <[email protected]>
>> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> > Cc: Kukjin Kim <[email protected]>
>> > Tested-by: Javier Martinez Canillas <[email protected]>
>> >
> Acked-by: Arnd Bergmann <[email protected]>
>
> Maybe add a paragraph about the specific problem:
>
> "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> typical 12MHz input clock that overflows every 5.5ms. This works
> with HZ=200 or higher but not with HZ=100 which needs a 10ms
> interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> the counter is 32 bits and does not have this problem.
> The new samsung_pwm_timer driver solves the problem by scaling
> the input clock by a factor of 50 on s3c24xx, which makes it
> less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> fewer wakeups".
I've tested on S3C2440 SoC based board and I didn't notice any
issues with HZ=100.
Clock frequencies look a bit different because AFAIU MPLL
clock is mostly used as a root clock. The 12 MHz oscillator clock
is used a root clock for the MPLL.
refclk: 12000 kHz
mpll: 405000 kHz
upll: 48000 kHz
fclk: 405000 kHz
hclk: 101250 kHz
pclk: 50625 kHz
So frequency of the timer block's source clock (PCLK) is 50.625 MHz.
This is further divided by 50 in the prescaler as you pointed out.
So the 16-bit is clocked with 1012500 Hz clock. I added some printks
to verify this.
Here is boot log for HZ=200: http://pastebin.com/JuWZdYwh
and HZ=100 http://pastebin.com/HnDnBfhc
samsung_clocksource_init:351 pclk: 50625000, timer clock_rate: 1012500
sched_clock: 16 bits at 1012kHz, resolution 987ns, wraps every 32362962ns
I just don't understand why the log says timer overflow is every 32.362 ms
and not twice this value (65536 * 1/1012500).
--
Thanks,
Sylwester
On Fri, Nov 18, 2016 at 11:43:14AM +0100, Sylwester Nawrocki wrote:
> On 11/18/2016 09:46 AM, Arnd Bergmann wrote:
> > On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
> >> > All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> >> > 200 Hz. Unfortunately in case of multiplatform image this affects also
> >> > other platforms when Exynos is enabled.
> >> >
> >> > This looks like an very old legacy code, dating back to initial
> >> > upstreaming of S3C24xx. Probably it was required for s3c24xx timer
> >> > driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> >> > unused plat-samsung/time.c").
> >> >
> >> > Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> >> > Samsung kernels (SoC vendor's and Tizen's). I believe this choice
> >> > was rather an effect of coincidence instead of conscious choice.
> >> >
> >> > Exynos uses its own MCT or arch timer and can work with all HZ values.
> >> > Older platforms use newer Samsung PWM timer driver which should handle
> >> > down to 100 Hz.
> >> >
> >> > Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> >> > A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> >> > other values.
> >> >
> >> > Reported-by: Lee Jones <[email protected]>
> >> > [Dropping 200_HZ from S3C/S5P suggested by Arnd]
> >> > Reported-by: Arnd Bergmann <[email protected]>
> >> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >> > Cc: Kukjin Kim <[email protected]>
> >> > Tested-by: Javier Martinez Canillas <[email protected]>
> >> >
> > Acked-by: Arnd Bergmann <[email protected]>
> >
> > Maybe add a paragraph about the specific problem:
> >
> > "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> > typical 12MHz input clock that overflows every 5.5ms. This works
> > with HZ=200 or higher but not with HZ=100 which needs a 10ms
> > interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> > the counter is 32 bits and does not have this problem.
> > The new samsung_pwm_timer driver solves the problem by scaling
> > the input clock by a factor of 50 on s3c24xx, which makes it
> > less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> > fewer wakeups".
>
> I've tested on S3C2440 SoC based board and I didn't notice any
> issues with HZ=100.
>
> Clock frequencies look a bit different because AFAIU MPLL
> clock is mostly used as a root clock. The 12 MHz oscillator clock
> is used a root clock for the MPLL.
>
> refclk: 12000 kHz
> mpll: 405000 kHz
> upll: 48000 kHz
> fclk: 405000 kHz
> hclk: 101250 kHz
> pclk: 50625 kHz
>
> So frequency of the timer block's source clock (PCLK) is 50.625 MHz.
> This is further divided by 50 in the prescaler as you pointed out.
>
> So the 16-bit is clocked with 1012500 Hz clock. I added some printks
> to verify this.
>
> Here is boot log for HZ=200: http://pastebin.com/JuWZdYwh
> and HZ=100 http://pastebin.com/HnDnBfhc
>
> samsung_clocksource_init:351 pclk: 50625000, timer clock_rate: 1012500
> sched_clock: 16 bits at 1012kHz, resolution 987ns, wraps every 32362962ns
Thanks for tests! I really appreciate it.
> I just don't understand why the log says timer overflow is every 32.362 ms
> and not twice this value (65536 * 1/1012500).
The answer could be at kernel/time/clocksource.c:
* NOTE: This function includes a safety margin of 50%, in other words, we
* return half the number of nanoseconds the hardware counter can technically
* cover.
Best regards,
Krzysztof
On Fri, 18 Nov 2016, Krzysztof Kozlowski wrote:
> All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> 200 Hz. Unfortunately in case of multiplatform image this affects also
> other platforms when Exynos is enabled.
>
> This looks like an very old legacy code, dating back to initial
> upstreaming of S3C24xx. Probably it was required for s3c24xx timer
> driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> unused plat-samsung/time.c").
>
> Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> Samsung kernels (SoC vendor's and Tizen's). I believe this choice
> was rather an effect of coincidence instead of conscious choice.
>
> Exynos uses its own MCT or arch timer and can work with all HZ values.
> Older platforms use newer Samsung PWM timer driver which should handle
> down to 100 Hz.
>
> Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> other values.
>
> Reported-by: Lee Jones <[email protected]>
Acked-by: Lee Jones <[email protected]>
> [Dropping 200_HZ from S3C/S5P suggested by Arnd]
> Reported-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Tested-by: Javier Martinez Canillas <[email protected]>
>
> ---
>
> Tested on Exynos5422 and Exynos5800 (by Javier). It would be
> appreciated if anyone could test it on S3C24xx or S5PV210.
>
> Changes since v1:
> 1. Add Javier's tested-by.
> 2. Drop HZ_FIXED also from ARCH_S5PV210 and ARCH_S3C24XX after Arnd
> suggestions and analysis.
> ---
> arch/arm/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b5d529fdffab..ced2e08a9d08 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1496,8 +1496,7 @@ source kernel/Kconfig.preempt
>
> config HZ_FIXED
> int
> - default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> - ARCH_S5PV210 || ARCH_EXYNOS4
> + default 200 if ARCH_EBSA110
> default 128 if SOC_AT91RM9200
> default 0
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
2016-11-18 17:46 GMT+09:00 Arnd Bergmann <[email protected]>:
> Maybe add a paragraph about the specific problem:
>
> "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> typical 12MHz input clock that overflows every 5.5ms. This works
> with HZ=200 or higher but not with HZ=100 which needs a 10ms
> interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> the counter is 32 bits and does not have this problem.
> The new samsung_pwm_timer driver solves the problem by scaling
> the input clock by a factor of 50 on s3c24xx, which makes it
> less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> fewer wakeups".
One thing to correct here is that the typical clock is PCLK, which is
derived from one of the PLLs and AFAIR is between 33-66 MHz on
s3c24xx. Technically you can drive the PWM block from an external
clock (12 MHz for some board-file based boards), but for simplicity
this functionality was omitted in the new PWM timer driver used for DT
boards (which worked fine with the PWM driven by PCLK).
Also I'm wondering if the divisor we use right now for 16-bit timers
isn't too small, since it gives us a really short wraparound time,
which means getting more timer interrupts for longer intervals, kind
of defeating the benefit of tickless mode. However, AFAICT it doesn't
affect the HZ problem.
Best regards.
Tomasz
On Mon, Nov 21, 2016 at 08:59:06AM +0000, Ben Dooks wrote:
> On 21/11/16 06:01, Tomasz Figa wrote:
> >2016-11-18 17:46 GMT+09:00 Arnd Bergmann <[email protected]>:
> >>Maybe add a paragraph about the specific problem:
> >>
> >>"On s3c24xx, the PWM counter is only 16 bit wide, and with the
> >>typical 12MHz input clock that overflows every 5.5ms. This works
> >>with HZ=200 or higher but not with HZ=100 which needs a 10ms
> >>interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> >>the counter is 32 bits and does not have this problem.
> >>The new samsung_pwm_timer driver solves the problem by scaling
> >>the input clock by a factor of 50 on s3c24xx, which makes it
> >>less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> >>fewer wakeups".
> >
> >One thing to correct here is that the typical clock is PCLK, which is
> >derived from one of the PLLs and AFAIR is between 33-66 MHz on
> >s3c24xx. Technically you can drive the PWM block from an external
> >clock (12 MHz for some board-file based boards), but for simplicity
> >this functionality was omitted in the new PWM timer driver used for DT
> >boards (which worked fine with the PWM driven by PCLK).
>
> Given it was a clock mux option, that would not have been difficult to
> acheive. However these platforms are now so old people don't care, I
> think all my pre-armv7 stuff is now in a box.
However, there are still s3c2410 machines running out there... so we
should do our best not to break them.
> The use of the 12MHz input was to give something to run PWM timers from
> that wasn't interrupted by cpu frequency scaling as PCLK generally is
> half HCLK which is divided down from the core CPU clock.
...
> The original implementation was to go for the best accuracy from the
> timer at the expense of 200 irqs per second instead of the usual 100.
This sounds like a good enough reason not to switch away from using 200Hz
and the 12MHz input.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
On 21/11/16 06:01, Tomasz Figa wrote:
> 2016-11-18 17:46 GMT+09:00 Arnd Bergmann <[email protected]>:
>> Maybe add a paragraph about the specific problem:
>>
>> "On s3c24xx, the PWM counter is only 16 bit wide, and with the
>> typical 12MHz input clock that overflows every 5.5ms. This works
>> with HZ=200 or higher but not with HZ=100 which needs a 10ms
>> interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
>> the counter is 32 bits and does not have this problem.
>> The new samsung_pwm_timer driver solves the problem by scaling
>> the input clock by a factor of 50 on s3c24xx, which makes it
>> less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
>> fewer wakeups".
>
> One thing to correct here is that the typical clock is PCLK, which is
> derived from one of the PLLs and AFAIR is between 33-66 MHz on
> s3c24xx. Technically you can drive the PWM block from an external
> clock (12 MHz for some board-file based boards), but for simplicity
> this functionality was omitted in the new PWM timer driver used for DT
> boards (which worked fine with the PWM driven by PCLK).
Given it was a clock mux option, that would not have been difficult to
acheive. However these platforms are now so old people don't care, I
think all my pre-armv7 stuff is now in a box.
The use of the 12MHz input was to give something to run PWM timers from
that wasn't interrupted by cpu frequency scaling as PCLK generally is
half HCLK which is divided down from the core CPU clock.
(Later devices had multiple PLL sources so you didn't have to have the
CPU fed from the same clock as the peripherals)
> Also I'm wondering if the divisor we use right now for 16-bit timers
> isn't too small, since it gives us a really short wraparound time,
> which means getting more timer interrupts for longer intervals, kind
> of defeating the benefit of tickless mode. However, AFAICT it doesn't
> affect the HZ problem.
The original implementation was to go for the best accuracy from the
timer at the expense of 200 irqs per second instead of the usual 100.
> Best regards.
> Tomasz
>
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
On Mon, Nov 21, 2016 at 03:01:57PM +0900, Tomasz Figa wrote:
> One thing to correct here is that the typical clock is PCLK, which is
> derived from one of the PLLs and AFAIR is between 33-66 MHz on
> s3c24xx. Technically you can drive the PWM block from an external
> clock (12 MHz for some board-file based boards), but for simplicity
> this functionality was omitted in the new PWM timer driver used for DT
> boards (which worked fine with the PWM driven by PCLK).
So that's a knowingly-made regression for s3c24xx then. Pretty
disgusting developer behavior to do that, IMHO.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.