linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: shinya.kuribayashi.px@renesas.com (Shinya Kuribayashi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/2] ARM: delay: allow timer-based delay implementation to be selected
Date: Thu, 19 Jul 2012 21:43:40 +0900	[thread overview]
Message-ID: <500800FC.40002@renesas.com> (raw)
In-Reply-To: <20120717090541.GA14624@mudshark.cambridge.arm.com>

Hi Will,

Have not gone through your new mail yet, sorry..

On 7/17/2012 6:05 PM, Will Deacon wrote:
> I assume the reason you've done it like this is because your timer isn't up
> and running until after the delay loop has been calibrated? In which case,
> I'd really rather not duplicate the calibration here -- is there no way you
> can ensure the timer is available earlier? If not, then I'm not sure we
> should be using it as the delay source.

Of course not :-)  My timers get started and running from time_init()
point, before calibrate_delay().

And don't get me wrong, I just tried to provide a working example to
skip calibrate_delay() in response to Santosh request:

> Thanks for the detailed explanation. CPU clock detection is indeed the
> nit way to skip the calibration overhead and this was one of the comment
> when I tried to push the skipping of calibration for secondary CPUs.
> 
> Looks like you have a working patch for the clock detection. Will
> you able to post that patch so that this long pending calibration
> for secondary CPUs gets optimized.

... with keeping the following points in mind:

1. skip the calibration for secondary CPUs (that is, for SMP use)

2. can work without init_current_timer_delay() help; calculating lpj
   from the CPU clock speed, not from current timer

If I understand Santosh request correctly, he requested a patch that
could be tested on non-A15 SMP systems (may be A9).  We know that A15
comes with the architected timer and does not require any additional
patches apart from already provided ones.  Also he said he gave a try
to "the suggested change + two patches" and confirmed it worked.

As for lpj_early variable, I just wanted to reserve lpj_fine for real
"timer" use, and did not want to mix with CPU frequency thing.  That
being said, I have to admit that rewriting using lpj_fine looks much
simpler, and should have done so from the beginning:

diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
index 395d5fb..b9874a3 100644
--- a/arch/arm/lib/delay.c
+++ b/arch/arm/lib/delay.c
@@ -65,6 +65,13 @@ void __init init_current_timer_delay(unsigned long freq)
 	arm_delay_ops.udelay		= __timer_udelay;
 }
 
+void __init calibrate_delay_early(unsigned long rate)
+{
+	pr_info("Calibrating delay using CPU frequency.. %lu Hz\n", rate);
+	lpj_fine = (rate + (HZ/2)) / HZ;
+	loops_per_jiffy = lpj_fine;
+}
+
 unsigned long __cpuinit calibrate_delay_is_known(void)
 {
 	return lpj_fine;
_

Anyway, my goal was to make calibrate_delay_is_known() work to skip
the calibration for secondary CPUs, whether init_current_timer_delay()
was involved or not.

> I also think you need to consider:
> 
>   1. Can your timer change frequency at runtime? [due to PM etc]
>   2.
>      i. Is its clock domain guaranteed to tick as long as the CPU is up?
>     ii. If it's in a separate power domain to the CPU, is that ever shut off
>         while the CPU is up?
> 
> If the answer to any of those is `yes', then I also think it's questionable
> whether it's worthwhile using it for delay.

Good point.  They all strongly suggest a need for current timer help.

At the same time, however, I wonder is it possible for runtime PM to
change frequency, or stop clock supply to the CPU core, or shut-down
its power domain _unnoticed_ by the CPU core? I do not think so.  It
may be easy from the hardware perspective, but it should not from the
software standpoint.

One thing we have to be careful when make use of the CPU clock speed
to skip the calibration on secondary CPUs is, the clock speed of each
CPU core when it enters / leaves from suspend.  And as long as it is
_predictable_, current calibrate_delay() & calibrate_delay_is_known()
infrastructure we have is going to work great.  On secondary CPUs,
loops_per_jiffy gets set up using calibrate_delay_is_known() at the
first calibration time, then stored to per_cpu(cpu_loops_per_jiffy).
Afterward, per_cpu(cpu_loops_per_jiffy) will be used.  If this works
for Santosh / OMAPs, it woudl be best, the way to go.

On the other hand, if it's unpredictable, we need yet another hacks
to 1) skip the calibration for secondary CPUs, and 2) the way to load
an appropriate value to global loops_per_jiffy, every time each CPU
gets started.

[ And at the point udelay() relies on the global loops_per_jiffy,
  this scenario doesn't work.  Primary reason to use current timer! ]

> Updating loops_per_jiffy from init_current_timer_delay is reasonable if
> people rely on these delays prior to calibration and there isn't a compromise
> value for lpj, but all this _early stuff is really horrible. Just make the
> thing tick before calibration occurs (we don't care about interrupts).

Lastly, would like to update examples of use cases.

/* For UP systems, or SMP systems without dynamic CPU freq scaling */
your_timer_init(void)
{
        unsigned long rate;

        rate = get_CPU_frequency();
        calibrate_delay_early(rate);
}

After calibrate_delay() is processed, loops_per_jiffy is supposed be
under the control of cpufreq, if required.

/* For SMP systems with dynamic CPU freq scaling */
your_timer_init(void)
{
        unsigned long rate;

        rate = get_Timer_frequency();
        init_current_timer_delay(rate);
}

You don't have to use calibrate_delay_early() in this case, of course.
My privious example was not clear on this point.

--
Shinya Kuribayashi
Renesas Electronics

  reply	other threads:[~2012-07-19 12:43 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-29 17:33 [PATCH v2 0/2] Use architected timers for delay loop Will Deacon
2012-06-29 17:33 ` [PATCH v2 1/2] ARM: arch timer: implement read_current_timer and get_cycles Will Deacon
2012-07-02 19:14   ` Stephen Boyd
2012-07-05 12:35   ` Shinya Kuribayashi
2012-07-05 12:59     ` Will Deacon
2012-06-29 17:33 ` [PATCH v2 2/2] ARM: delay: allow timer-based delay implementation to be selected Will Deacon
2012-07-02 19:14   ` Stephen Boyd
2012-07-02 21:53     ` Will Deacon
2012-07-03 12:09   ` Shinya Kuribayashi
2012-07-04 15:36     ` Will Deacon
2012-07-05 12:12       ` Shinya Kuribayashi
2012-07-05 12:56         ` Will Deacon
2012-07-05 16:51           ` Stephen Boyd
2012-07-05 13:06   ` Shinya Kuribayashi
2012-07-05 14:15     ` Will Deacon
2012-07-12  7:33   ` Shinya Kuribayashi
2012-07-12  8:44     ` Will Deacon
2012-07-12  9:35       ` Shinya Kuribayashi
2012-07-12 16:40         ` Stephen Boyd
2012-07-13  2:16           ` Shinya Kuribayashi
2012-07-13  8:57             ` Will Deacon
2012-07-13 10:48               ` Shilimkar, Santosh
2012-07-13 11:13                 ` Will Deacon
2012-07-13 12:04                   ` Shilimkar, Santosh
2012-07-13 12:08                     ` Will Deacon
2012-07-13 12:14                       ` Shilimkar, Santosh
2012-07-13 12:23                         ` Will Deacon
2012-07-13 12:28                           ` Shilimkar, Santosh
2012-07-17  3:10                   ` Shinya Kuribayashi
2012-07-17  6:11                     ` Shilimkar, Santosh
2012-07-17  7:42                       ` Shinya Kuribayashi
2012-07-17  9:05                         ` Will Deacon
2012-07-19 12:43                           ` Shinya Kuribayashi [this message]
2012-07-18 17:52                         ` Will Deacon
2012-07-19 15:19                           ` Jonathan Austin
2012-07-20 10:17                             ` Will Deacon
2012-07-24  9:06                               ` Shinya Kuribayashi
2012-07-24  9:15                                 ` Will Deacon

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=500800FC.40002@renesas.com \
    --to=shinya.kuribayashi.px@renesas.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).