All of lore.kernel.org
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] clockevents: Prefer clockevents that don't suffer from FEAT_C3_STOP
Date: Thu, 22 Aug 2013 13:33:15 -0400	[thread overview]
Message-ID: <52164B5B.5010902@ti.com> (raw)
In-Reply-To: <1377191201-14696-2-git-send-email-sboyd@codeaurora.org>

On Thursday 22 August 2013 01:06 PM, Stephen Boyd wrote:
> On some ARM systems there are two sets of per-cpu timers: the TWD
> timers and the global timers. The TWD timers are rated at 350 and
> the global timers are rated at 300 but the TWD timers suffer from
> FEAT_C3_STOP while the arm global timers don't. The tick device
> selection logic doesn't consider the FEAT_C3_STOP flag so we'll
> always end up with the TWD as the tick device although we don't
> want that.
> 
> Extend the preference logic to take the FEAT_C3_STOP flag into
> account and always prefer tick devices that don't suffer from
> FEAT_C3_STOP even if their rating is lower than tick devices that
> do suffer from FEAT_C3_STOP. This will remove the need to do any
> broadcasting on such ARM systems.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  kernel/time/tick-common.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index 64522ec..3ae437d 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -244,12 +244,22 @@ static bool tick_check_preferred(struct clock_event_device *curdev,
>  			return false;
>  	}
>  
> +	if (!curdev)
> +		return true;
> +
> +	/* Always prefer a tick device that doesn't suffer from FEAT_C3STOP */
> +	if (!(newdev->features & CLOCK_EVT_FEAT_C3STOP) &&
> +			(curdev->features & CLOCK_EVT_FEAT_C3STOP))
> +		return true;
> +	if ((newdev->features & CLOCK_EVT_FEAT_C3STOP) &&
> +			!(curdev->features & CLOCK_EVT_FEAT_C3STOP))
> +		return false;
> +
I don't think preferring the clock-event which doesn't suffer
from FEAT_C3STOP is a good idea if the quality of the time source
is not same. Generally the global timers are slow and far away from
CPU(cycle cost). So as long as we don't get impacted because of low power
states, the tick should run out of local timers which are faster access
as well as high resolution.

Regards,
Santosh

WARNING: multiple messages have this Message-ID (diff)
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Daniel Lezcano" <daniel.lezcano@linaro.org>,
	"Russell King" <linux@arm.linux.org.uk>,
	srinivas.kandagatla@st.com,
	"Michal Simek" <michal.simek@xilinx.com>,
	linux-kernel@vger.kernel.org,
	"Stuart Menefy" <stuart.menefy@st.com>,
	"John Stultz" <john.stultz@linaro.org>,
	"Sören Brinkmann" <soren.brinkmann@xilinx.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] clockevents: Prefer clockevents that don't suffer from FEAT_C3_STOP
Date: Thu, 22 Aug 2013 13:33:15 -0400	[thread overview]
Message-ID: <52164B5B.5010902@ti.com> (raw)
In-Reply-To: <1377191201-14696-2-git-send-email-sboyd@codeaurora.org>

On Thursday 22 August 2013 01:06 PM, Stephen Boyd wrote:
> On some ARM systems there are two sets of per-cpu timers: the TWD
> timers and the global timers. The TWD timers are rated at 350 and
> the global timers are rated at 300 but the TWD timers suffer from
> FEAT_C3_STOP while the arm global timers don't. The tick device
> selection logic doesn't consider the FEAT_C3_STOP flag so we'll
> always end up with the TWD as the tick device although we don't
> want that.
> 
> Extend the preference logic to take the FEAT_C3_STOP flag into
> account and always prefer tick devices that don't suffer from
> FEAT_C3_STOP even if their rating is lower than tick devices that
> do suffer from FEAT_C3_STOP. This will remove the need to do any
> broadcasting on such ARM systems.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  kernel/time/tick-common.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index 64522ec..3ae437d 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -244,12 +244,22 @@ static bool tick_check_preferred(struct clock_event_device *curdev,
>  			return false;
>  	}
>  
> +	if (!curdev)
> +		return true;
> +
> +	/* Always prefer a tick device that doesn't suffer from FEAT_C3STOP */
> +	if (!(newdev->features & CLOCK_EVT_FEAT_C3STOP) &&
> +			(curdev->features & CLOCK_EVT_FEAT_C3STOP))
> +		return true;
> +	if ((newdev->features & CLOCK_EVT_FEAT_C3STOP) &&
> +			!(curdev->features & CLOCK_EVT_FEAT_C3STOP))
> +		return false;
> +
I don't think preferring the clock-event which doesn't suffer
from FEAT_C3STOP is a good idea if the quality of the time source
is not same. Generally the global timers are slow and far away from
CPU(cycle cost). So as long as we don't get impacted because of low power
states, the tick should run out of local timers which are faster access
as well as high resolution.

Regards,
Santosh




  reply	other threads:[~2013-08-22 17:33 UTC|newest]

Thread overview: 142+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20130717210417.GP13667@xsjandreislx>
     [not found] ` <51E72DCA.9070500@codeaurora.org>
     [not found]   ` <f5b76049-7e5b-4dd0-9863-19cfd1d599b9@TX2EHSMHS006.ehs.local>
     [not found]     ` <51E7435B.3060605@codeaurora.org>
     [not found]       ` <ff4d40be-7177-4a55-ab2f-dff11fa18642@DB9EHSMHS009.ehs.local>
     [not found]         ` <51ED8DF2.60600@codeaurora.org>
     [not found]           ` <20130722201348.GI453@xsjandreislx>
2013-07-22 22:41             ` Enable arm_global_timer for Zynq brakes boot Sören Brinkmann
2013-07-22 22:41               ` Sören Brinkmann
2013-07-29 12:51               ` Daniel Lezcano
2013-07-29 12:51                 ` Daniel Lezcano
2013-07-29 15:58                 ` Sören Brinkmann
2013-07-29 15:58                   ` Sören Brinkmann
2013-07-30  0:03                 ` Sören Brinkmann
2013-07-30  0:03                   ` Sören Brinkmann
2013-07-30  8:47                   ` Daniel Lezcano
2013-07-30  8:47                     ` Daniel Lezcano
2013-07-30 22:14                     ` Sören Brinkmann
2013-07-30 22:14                       ` Sören Brinkmann
2013-07-30 22:23                       ` Sören Brinkmann
2013-07-30 22:23                         ` Sören Brinkmann
2013-07-30 22:34                     ` Sören Brinkmann
2013-07-30 22:34                       ` Sören Brinkmann
2013-07-31  7:27                       ` Daniel Lezcano
2013-07-31  7:27                         ` Daniel Lezcano
2013-07-31 16:26                         ` Sören Brinkmann
2013-07-31 16:26                           ` Sören Brinkmann
2013-07-31  9:34                       ` Daniel Lezcano
2013-07-31  9:34                         ` Daniel Lezcano
2013-07-31 16:43                         ` Sören Brinkmann
2013-07-31 16:43                           ` Sören Brinkmann
2013-07-31 20:49                       ` Daniel Lezcano
2013-07-31 20:49                         ` Daniel Lezcano
2013-07-31 20:58                         ` Sören Brinkmann
2013-07-31 20:58                           ` Sören Brinkmann
2013-07-31 21:08                           ` Daniel Lezcano
2013-07-31 21:08                             ` Daniel Lezcano
2013-07-31 22:18                             ` Sören Brinkmann
2013-07-31 22:18                               ` Sören Brinkmann
2013-07-31 23:01                               ` Daniel Lezcano
2013-07-31 23:01                                 ` Daniel Lezcano
2013-07-31 23:38                                 ` Sören Brinkmann
2013-07-31 23:38                                   ` Sören Brinkmann
2013-08-01 17:29                                   ` Daniel Lezcano
2013-08-01 17:29                                     ` Daniel Lezcano
2013-08-01 17:43                                     ` Sören Brinkmann
2013-08-01 17:43                                       ` Sören Brinkmann
2013-08-01 17:48                                       ` Daniel Lezcano
2013-08-01 17:48                                         ` Daniel Lezcano
2013-08-06  1:28                                         ` Sören Brinkmann
2013-08-06  1:28                                           ` Sören Brinkmann
2013-08-06  8:46                                           ` Daniel Lezcano
2013-08-06  8:46                                             ` Daniel Lezcano
2013-08-06  9:18                                             ` Michal Simek
2013-08-06  9:18                                               ` Michal Simek
2013-08-06 12:30                                               ` Daniel Lezcano
2013-08-06 12:30                                                 ` Daniel Lezcano
2013-08-06 12:41                                                 ` Michal Simek
2013-08-06 12:41                                                   ` Michal Simek
2013-08-06 13:08                                                   ` Daniel Lezcano
2013-08-06 13:08                                                     ` Daniel Lezcano
2013-08-06 13:18                                                     ` Michal Simek
2013-08-06 13:18                                                       ` Michal Simek
2013-08-06 16:09                                                       ` Daniel Lezcano
2013-08-06 16:09                                                         ` Daniel Lezcano
2013-08-06 16:13                                                         ` Sören Brinkmann
2013-08-06 16:13                                                           ` Sören Brinkmann
2013-08-06 16:25                                                         ` Sören Brinkmann
2013-08-06 16:25                                                           ` Sören Brinkmann
2013-08-06 16:18                                             ` Sören Brinkmann
2013-08-06 16:18                                               ` Sören Brinkmann
2013-08-08 17:11                                         ` Sören Brinkmann
2013-08-08 17:11                                           ` Sören Brinkmann
2013-08-08 17:16                                           ` Mark Rutland
2013-08-08 17:16                                             ` Mark Rutland
2013-08-08 17:22                                             ` Stephen Boyd
2013-08-08 17:22                                               ` Stephen Boyd
2013-08-09 10:58                                               ` Mark Rutland
2013-08-09 10:58                                                 ` Mark Rutland
2013-08-08 17:22                                             ` Sören Brinkmann
2013-08-08 17:22                                               ` Sören Brinkmann
2013-08-09 10:32                                           ` Srinivas KANDAGATLA
2013-08-09 10:32                                             ` Srinivas KANDAGATLA
2013-08-09 14:19                                             ` Daniel Lezcano
2013-08-09 14:19                                               ` Daniel Lezcano
2013-08-09 17:27                                               ` Stephen Boyd
2013-08-09 17:27                                                 ` Stephen Boyd
2013-08-09 17:48                                                 ` Sören Brinkmann
2013-08-09 17:48                                                   ` Sören Brinkmann
2013-08-09 18:45                                                   ` Stephen Boyd
2013-08-09 18:45                                                     ` Stephen Boyd
2013-08-12 10:53                                                 ` Daniel Lezcano
2013-08-12 10:53                                                   ` Daniel Lezcano
2013-08-12 16:23                                                   ` Stephen Boyd
2013-08-12 16:23                                                     ` Stephen Boyd
2013-08-12 16:53                                                     ` Daniel Lezcano
2013-08-12 16:53                                                       ` Daniel Lezcano
2013-08-12 16:03                                                 ` Sören Brinkmann
2013-08-12 16:03                                                   ` Sören Brinkmann
2013-08-12 16:08                                                   ` Daniel Lezcano
2013-08-12 16:08                                                     ` Daniel Lezcano
2013-08-12 16:17                                                     ` Sören Brinkmann
2013-08-12 16:17                                                       ` Sören Brinkmann
2013-08-12 16:20                                                   ` Stephen Boyd
2013-08-12 16:20                                                     ` Stephen Boyd
2013-08-12 16:24                                                     ` Sören Brinkmann
2013-08-12 16:24                                                       ` Sören Brinkmann
2013-08-12 16:40                                                       ` Stephen Boyd
2013-08-12 16:40                                                         ` Stephen Boyd
2013-08-12 16:43                                                         ` Sören Brinkmann
2013-08-12 16:43                                                           ` Sören Brinkmann
2013-08-12 16:32                                                     ` Sören Brinkmann
2013-08-12 16:32                                                       ` Sören Brinkmann
2013-08-12 16:49                                                       ` Daniel Lezcano
2013-08-12 16:49                                                         ` Daniel Lezcano
2013-08-12 16:53                                                         ` Sören Brinkmann
2013-08-12 16:53                                                           ` Sören Brinkmann
2013-08-12 17:02                                                           ` Daniel Lezcano
2013-08-12 17:02                                                             ` Daniel Lezcano
2013-08-16 17:28                                                             ` Sören Brinkmann
2013-08-16 17:28                                                               ` Sören Brinkmann
2013-08-19 23:00                                                               ` Stephen Boyd
2013-08-19 23:00                                                                 ` Stephen Boyd
2013-08-19 23:30                                                                 ` Sören Brinkmann
2013-08-19 23:30                                                                   ` Sören Brinkmann
2013-08-20  0:57                                                                   ` Stephen Boyd
2013-08-20  0:57                                                                     ` Stephen Boyd
2013-08-20 15:13                                                                     ` Daniel Lezcano
2013-08-20 15:13                                                                       ` Daniel Lezcano
2013-08-22 17:06                                                                       ` [PATCH 1/2] tick: broadcast: Deny per-cpu clockevents from being broadcast sources Stephen Boyd
2013-08-22 17:06                                                                         ` Stephen Boyd
2013-08-22 17:06                                                                         ` [PATCH 2/2] clockevents: Prefer clockevents that don't suffer from FEAT_C3_STOP Stephen Boyd
2013-08-22 17:06                                                                           ` Stephen Boyd
2013-08-22 17:33                                                                           ` Santosh Shilimkar [this message]
2013-08-22 17:33                                                                             ` Santosh Shilimkar
2013-08-22 17:40                                                                             ` Stephen Boyd
2013-08-22 17:40                                                                               ` Stephen Boyd
2013-08-22 17:48                                                                               ` Santosh Shilimkar
2013-08-22 17:48                                                                                 ` Santosh Shilimkar
2013-08-22 18:31                                                                                 ` Stephen Boyd
2013-08-22 18:31                                                                                   ` Stephen Boyd
2013-08-22 21:06                                                                                   ` Santosh Shilimkar
2013-08-22 21:06                                                                                     ` Santosh Shilimkar
2013-08-22 23:38                                                                         ` [PATCH 1/2] tick: broadcast: Deny per-cpu clockevents from being broadcast sources Sören Brinkmann
2013-08-22 23:38                                                                           ` Sören Brinkmann
2013-09-05 16:53                                                                         ` Sören Brinkmann
2013-09-05 16:53                                                                           ` Sören Brinkmann
2013-08-09 16:03                                             ` Enable arm_global_timer for Zynq brakes boot Sören Brinkmann
2013-08-09 16:03                                               ` Sören Brinkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52164B5B.5010902@ti.com \
    --to=santosh.shilimkar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.