Quite a few systems have deeper idle states where the local timer tick
stops to fire, that rely on the GENERIC_CLOCKEVENTS_BROADCAST
infrastructure to switch to a global (non-stopping) clockevent device.
This is also true on UP systems, so select GENERIC_CLOCKEVENTS_BROADCAST
unconditionally.
Instead only select ARCH_HAS_TICK_BROADCAST if SMP is present, as this
is only implemented for SMP systems.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
arch/arm/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Hi Russell,
can you please take a look at this and tell if there is something wrong
about it, or if you think it can go in?
Without this patch the kernel is unable to use the deeper CPU idle
states on i.MX6S if it isn't built with SMP support enabled.
Regards,
Lucas
Am Freitag, den 08.07.2016, 17:58 +0200 schrieb Lucas Stach:
quoted hunk
Quite a few systems have deeper idle states where the local timer tick
stops to fire, that rely on the GENERIC_CLOCKEVENTS_BROADCAST
infrastructure to switch to a global (non-stopping) clockevent device.
This is also true on UP systems, so select GENERIC_CLOCKEVENTS_BROADCAST
unconditionally.
Instead only select ARCH_HAS_TICK_BROADCAST if SMP is present, as this
is only implemented for SMP systems.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
arch/arm/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: 2016-09-16 10:20:51
On Fri, Sep 16, 2016 at 11:32:20AM +0200, Lucas Stach wrote:
Hi Russell,
can you please take a look at this and tell if there is something wrong
about it, or if you think it can go in?
Without this patch the kernel is unable to use the deeper CPU idle
states on i.MX6S if it isn't built with SMP support enabled.
Without spending a lot of time digging into this code, working out what
these configuration symbols do, I frankly don't know if it's a sensible
thing to do or not. I guess tglx knows this code like the back of his
hand, and would be in a better position to comment.
There's two factors there:
1. the amount of undocumented code in the kernel needing the code to be
read and understood to understand various aspects of it from the
architecture point of view.
2. your architecture maintainer has done very little actual platform
development (not through his own choice) for the last 10 or so years,
which means he's not had to dig into these areas.
So, I'm afraid I feel less than qualified on this at the moment. I'll
try to look into this and talk to tglx to work out whether it's a
sensible change. As I never had a reply to my last email to tglx about
threaded interrupts vs tasklets (for a reported performance regression
in my crypto patches already queued for the next merge window) I'm not
hoping for much help.
--
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.
From: Thomas Gleixner <hidden> Date: 2016-09-16 10:30:05
On Fri, 16 Sep 2016, Russell King - ARM Linux wrote:
On Fri, Sep 16, 2016 at 11:32:20AM +0200, Lucas Stach wrote:
quoted
Hi Russell,
can you please take a look at this and tell if there is something wrong
about it, or if you think it can go in?
Without this patch the kernel is unable to use the deeper CPU idle
states on i.MX6S if it isn't built with SMP support enabled.
Without spending a lot of time digging into this code, working out what
these configuration symbols do, I frankly don't know if it's a sensible
thing to do or not. I guess tglx knows this code like the back of his
hand, and would be in a better position to comment.
There's two factors there:
1. the amount of undocumented code in the kernel needing the code to be
read and understood to understand various aspects of it from the
architecture point of view.
Yeah. I know that this stuff lacks documentation.
2. your architecture maintainer has done very little actual platform
development (not through his own choice) for the last 10 or so years,
which means he's not had to dig into these areas.
So, I'm afraid I feel less than qualified on this at the moment. I'll
try to look into this and talk to tglx to work out whether it's a
sensible change.
The broadcast feature has the following functionality:
It lets you use fast accessible (cpu local) timers for normal operation
and in case of deep idle sleeps where the cpu local timer stops switch
to broadcast operation, which arms a slower to access (global) timer
device which does not stop in deep idle.
On UP we usually just use the global device, but on SMP we need the cpu
local timers for normal (non idle) operation. Though there is no reason
to restrict this UP from a technical POV especially when the local
timer is way cheaper to access than the other device.
Hope that helps.
As I never had a reply to my last email to tglx about threaded interrupts
vs tasklets (for a reported performance regression in my crypto patches
already queued for the next merge window) I'm not hoping for much help.
Sorry, got lost in my mail backlog. Looking at it next.
Thanks,
tglx
Am Freitag, den 16.09.2016, 12:30 +0200 schrieb Thomas Gleixner:
On Fri, 16 Sep 2016, Russell King - ARM Linux wrote:
quoted
On Fri, Sep 16, 2016 at 11:32:20AM +0200, Lucas Stach wrote:
quoted
Hi Russell,
can you please take a look at this and tell if there is something wrong
about it, or if you think it can go in?
Without this patch the kernel is unable to use the deeper CPU idle
states on i.MX6S if it isn't built with SMP support enabled.
Without spending a lot of time digging into this code, working out what
these configuration symbols do, I frankly don't know if it's a sensible
thing to do or not. I guess tglx knows this code like the back of his
hand, and would be in a better position to comment.
There's two factors there:
1. the amount of undocumented code in the kernel needing the code to be
read and understood to understand various aspects of it from the
architecture point of view.
Yeah. I know that this stuff lacks documentation.
quoted
2. your architecture maintainer has done very little actual platform
development (not through his own choice) for the last 10 or so years,
which means he's not had to dig into these areas.
So, I'm afraid I feel less than qualified on this at the moment. I'll
try to look into this and talk to tglx to work out whether it's a
sensible change.
The broadcast feature has the following functionality:
It lets you use fast accessible (cpu local) timers for normal operation
and in case of deep idle sleeps where the cpu local timer stops switch
to broadcast operation, which arms a slower to access (global) timer
device which does not stop in deep idle.
On UP we usually just use the global device, but on SMP we need the cpu
local timers for normal (non idle) operation. Though there is no reason
to restrict this UP from a technical POV especially when the local
timer is way cheaper to access than the other device.
Hope that helps.
What I take from this is that the i.MX6 decision to use the local timer
for normal operation on UP is sane, as it's both cheaper to access, as
well as providing higher accuracy.
In fact this behavior has been explicitly enabled by a patch "ARM: imx:
always use TWD on IMX6Q" from Sebastian Andrzej Siewior to get better
responsiveness in cyclictest.
So the subject patch is actually a regression fix, as we now need the
timer broadcast infrastructure to be able to use the deeper idle states,
instead of having them enabled by always using the global timer on UP.
Russell, okay to put it in the patch system?
Regards,
Lucas