Re: about system time incorrect after changing cpu frequency

Re: about system time incorrect after changing cpu frequency

[Viresh Kumar]

On Mon, Aug 31, 2015 at 7:33 PM, vichy <vichy.kuo@gmail.com> wrote:
> hi all:
> My platform is like below:
> 1. single core Cortex A9
> 2. use global timer for system timer
>
> after I porting cpu frequency driver based on snow ball, the cpu
> frequency did change as I expected.
> But the system time is incorrect( since pherial clk is got from cpu frequency)
>
> for example:
> a) cpu 1G (pherial clk = 250M) --> sleep 1 sec (OK)
> b) cpu 500M  (pherial clk = 125M) --> sleep 1 sec (will be measured as 2 sec)
>
> I try to call below 2 functions to change the frequency of clocksource
> and clockevent, but the above b) sleep time is still incorrect when
> cpu runs in 500Mhz.
>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
>
> in Arm cortex A9 single core system with Global timer as system timer,
> is there any kernel api to change system timer period when cpu/pherial
> frequency change?
>
> appreciate your kind help in advance,

The list cpufreq@vger.kernel.org is the wrong list for posting cpufreq queries
as we have moved to Linux PM list <linux-pm@vger.kernel.org> list now.

Try unsetting CPUFREQ_CONST_LOOPS flag in your driver, if you have it
set.

Re: about system time incorrect after changing cpu frequency

[vichy]

hi Viresh:


2015-09-01 11:57 GMT+08:00 Viresh Kumar <viresh.kumar@linaro.org>:
> On Mon, Aug 31, 2015 at 7:33 PM, vichy <vichy.kuo@gmail.com> wrote:
>> hi all:
>> My platform is like below:
>> 1. single core Cortex A9
>> 2. use global timer for system timer
>>
>> after I porting cpu frequency driver based on snow ball, the cpu
>> frequency did change as I expected.
>> But the system time is incorrect( since pherial clk is got from cpu frequency)
>>
>> for example:
>> a) cpu 1G (pherial clk = 250M) --> sleep 1 sec (OK)
>> b) cpu 500M  (pherial clk = 125M) --> sleep 1 sec (will be measured as 2 sec)
>>
>> I try to call below 2 functions to change the frequency of clocksource
>> and clockevent, but the above b) sleep time is still incorrect when
>> cpu runs in 500Mhz.
>>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
>>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
>>
>> in Arm cortex A9 single core system with Global timer as system timer,
>> is there any kernel api to change system timer period when cpu/pherial
>> frequency change?
>>
>> appreciate your kind help in advance,
>
> The list cpufreq@vger.kernel.org is the wrong list for posting cpufreq queries
> as we have moved to Linux PM list <linux-pm@vger.kernel.org> list now.
>
> Try unsetting CPUFREQ_CONST_LOOPS flag in your driver, if you have it
> set.

I DIDN'T set the CPUFREQ_CONST_LOOPS when I register my cpufreq driver
I pasted my cpufreq driver declariation as below:

static struct cpufreq_driver plat_cpufreq_driver = {
    .flags  = CPUFREQ_STICKY,
    .verify = plat_cpufreq_verify_speed,
    .target = plat_cpufreq_target,
    .get    = plat_cpufreq_getspeed,
    .init   = plat_cpufreq_init,
    .name   = "plat-cpufreq",
    .attr   = plat_cpufreq_attr,
};


I have traced the kernel code
if I guess correctly, the sleep accurate is based on jiffies and
tick_handle_periodic will periodically update the next event interval

void tick_handle_periodic
-->
  for (;;) {
          if (!clockevents_program_event(dev, next, false))
              return;
          /*
           * Have to be careful here. If we're in oneshot mode,
           * before we call tick_periodic() in a loop, we need
           * to be sure we're using a real hardware clocksource.
           * Otherwise we could get trapped in an infinite
           * loop, as the tick_periodic() increments jiffies,
           * when then will increment time, posibly causing
           * the loop to trigger again and again.
           */
          if (timekeeping_valid_for_hres())
              tick_periodic(cpu);
          next = ktime_add(next, tick_period);
      }

and in clockevents_program_event, we will use mult, shift to calculate
the cycles need for global timer triggering next interrupt event.

    clc = ((unsigned long long) delta * dev->mult) >> dev->shift;
    rc = dev->set_next_event((unsigned long) clc, dev);

belwo is my tick device information in /proc/timer_list
and multi did change to 1/2 when I change cpu freq from 1G to 500Mhz.
when cpu run 1GHz

except multi and shift, is there any place I need to take care for
system timer accurate?
appreciate your kind help,

Tick Device: mode:     1
Per CPU device: 0
Clock Event Device: arm_global_timer
 max_delta_ns:   17043521021
 min_delta_ns:   1000
 mult:           541165879
 shift:          31
 mode:           3
 next_event:     2176344000000 nsecs
 set_next_event: gt_clockevent_set_next_event
 set_mode:       gt_clockevent_set_mode
 event_handler:  hrtimer_interrupt
 retries:        0

when cpu run in 500Mhz
Tick Device: mode:     1
Per CPU device: 0
Clock Event Device: arm_global_timer
 max_delta_ns:   34087041979
 min_delta_ns:   1000
 mult:           270582940
 shift:          31
 mode:           3
 next_event:     2230100000000 nsecs
 set_next_event: gt_clockevent_set_next_event
 set_mode:       gt_clockevent_set_mode
 event_handler:  hrtimer_interrupt
 retries:        0

Re: about system time incorrect after changing cpu frequency

[Viresh Kumar]

On 01-09-15, 13:36, vichy wrote:
> >> I try to call below 2 functions to change the frequency of clocksource
> >> and clockevent, but the above b) sleep time is still incorrect when
> >> cpu runs in 500Mhz.
> >>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
> >>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);

How and when were you calling them? What kernel version is it ?

> Clock Event Device: arm_global_timer

This driver doesn't have support to update clkevt device's freq. You
may need to modify that based on how arch/arm/kernel/smp_twd.c is
updated. look for: clockevents_update_freq().

-- 
viresh

Re: about system time incorrect after changing cpu frequency

[Jisheng Zhang]

On Tue, 1 Sep 2015 11:32:36 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:

> On 01-09-15, 13:36, vichy wrote:
> > >> I try to call below 2 functions to change the frequency of clocksource
> > >> and clockevent, but the above b) sleep time is still incorrect when
> > >> cpu runs in 500Mhz.
> > >>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
> > >>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
> 
> How and when were you calling them? What kernel version is it ?
> 
> > Clock Event Device: arm_global_timer
> 
> This driver doesn't have support to update clkevt device's freq. You
> may need to modify that based on how arch/arm/kernel/smp_twd.c is
> updated. look for: clockevents_update_freq().
> 

This can only help the clockevent. Even with this cpufreq notifier
update, the globaltimer clocksource still can't hold correct/accurate time.

I think the arm global timer is not suitable for clocksource if cpufreq is
enabled.

Re: about system time incorrect after changing cpu frequency

[vichy]

hi Jisheng:

2015-09-01 14:39 GMT+08:00 Jisheng Zhang <jszhang@marvell.com>:
> On Tue, 1 Sep 2015 11:32:36 +0530
> Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
>> On 01-09-15, 13:36, vichy wrote:
>> > >> I try to call below 2 functions to change the frequency of clocksource
>> > >> and clockevent, but the above b) sleep time is still incorrect when
>> > >> cpu runs in 500Mhz.
>> > >>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
>> > >>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
>>
>> How and when were you calling them? What kernel version is it ?
>>
>> > Clock Event Device: arm_global_timer
>>
>> This driver doesn't have support to update clkevt device's freq. You
>> may need to modify that based on how arch/arm/kernel/smp_twd.c is
>> updated. look for: clockevents_update_freq().
>>
>
> This can only help the clockevent. Even with this cpufreq notifier
> update, the globaltimer clocksource still can't hold correct/accurate time.
Why even global timer add clockevents_update_freq, it stilll CANNOT
hold currect/accurate time?
When I trace the kernel, clockevents_program_event will calculate the
cycles with mult/shift then put in comparator.
if we put the write mult/shift, the global  timer should hold
correct/accurate time under different phepherial clock, right?

Appreciate all your kind help,

Re: about system time incorrect after changing cpu frequency

[vichy]

hi Viresh:

2015-09-01 14:02 GMT+08:00 Viresh Kumar <viresh.kumar@linaro.org>:
> On 01-09-15, 13:36, vichy wrote:
>> >> I try to call below 2 functions to change the frequency of clocksource
>> >> and clockevent, but the above b) sleep time is still incorrect when
>> >> cpu runs in 500Mhz.
>> >>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
>> >>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
>
> How and when were you calling them? What kernel version is it ?
Belwo is what I did in my cpufreq driver and my kernel version is 3.0.33.
plat_cpufreq_target()
{
........
    cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
    PLAT_Mpll(freqs.new / 1000);
    PLAT_gt_change_freq();
    /* post change notification */
    cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
....
}

and I add  PLAT_gt_change_freq in arm_global_timer.c like below:
 PLAT_gt_change_freq calling() {
    gt_clk_rate = get_periph_clk();
    clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
    __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
}


I also do another experiment that DIDN'T change any REAL HW setting
but change mult/shift to the half value like below:
(But I measure out the "sleep 1" --> still 1secs, even I modify mult/shift)
(in my opinion, the "seep 1" I measured should be 500ms, since HW NO
change but mult/shift change)
plat_cpufreq_target()
{
........
    cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
   //    PLAT_Mpll(freqs.new / 1000); //NO HW FREQ change
     PLAT_gt_change_freq();
    /* post change notification */
    cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
....
}

PLAT_gt_change_freq in arm_global_timer.c like below:
 PLAT_gt_change_freq calling() {
    gt_clk_rate = get_periph_clk()/2; //purposely chage 1/2
    clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);
    __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);
}

>
>> Clock Event Device: arm_global_timer
>
> This driver doesn't have support to update clkevt device's freq. You
> may need to modify that based on how arch/arm/kernel/smp_twd.c is
> updated. look for: clockevents_update_freq().
I have check the latest kernel about arch/arm/kernel/smp_twd.c
it almost do the same thing like I did, except it use cpufreq notifier
for calling clockevents_update_freq like below:
(the final target of them is the same --> clling
clockevents_update_freq to change mult/shift.

static void twd_update_frequency(void *data)
{
    twd_timer_rate = clk_get_rate(twd_clk);

    clockevents_update_freq(raw_cpu_ptr(twd_evt), twd_timer_rate);
}

I know so far arm global timer seems not support cpu frequcy chage flow.
But like my experiment above to fake change gt_clk_rate, the sleep
should be 1/2 of the original.

if smp_twd call twd_update_frequency to change multi/shift can work,
why purposely adding  clockevents_update_freq in global timer not
working?

or there is something else except Mult/shift need to be modify when
gt_clk_rate changed?

appreciate your help in advance,
>
> --
> viresh

Re: about system time incorrect after changing cpu frequency

[Viresh Kumar]

On 01-09-15, 15:21, vichy wrote:
> and I add  PLAT_gt_change_freq in arm_global_timer.c like below:
>  PLAT_gt_change_freq calling() {
>     gt_clk_rate = get_periph_clk();
>     clockevents_update_freq(this_cpu_ptr(gt_evt), gt_clk_rate);

In earlier versions of kernel, this had a problem that it works only
for ONESHOT mode and simply returns in PERIODIC mode. I do see that
your's mode is ONESHOT only, and so it should have worked.

Anyway, few prints into that routine to see where we are reaching
might help.

>     __clocksource_updatefreq_hz(&gt_clocksource, gt_clk_rate);

AFAIR, there is a difference between how different mdelay/udelay are
implemented. One of them is using the loops_per_jiffie variable to
calculate the time, other uses clkevt device. I am not 100% sure about
the theory, but that's what I remember. So, have a look from that
perspective as well..

-- 
viresh