* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 22:08 [PATCH] OMAP CPUIDLE: CPU Idle latency measurement vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
@ 2010-08-27 9:46 ` Jean Pihet
2010-08-27 10:00 ` Sripathy, Vishwanath
2010-08-27 9:58 ` Silesh C V
` (3 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Jean Pihet @ 2010-08-27 9:46 UTC (permalink / raw)
To: vishwanath.sripathy; +Cc: linux-omap, linaro-dev
Hi,
On Sat, Aug 28, 2010 at 12:08 AM, <vishwanath.sripathy@linaro.org> wrote:
> From: Vishwanath BS <vishwanath.sripathy@linaro.org>
>
> This patch has instrumentation code for measuring latencies for
> various CPUIdle C states for OMAP. Idea here is to capture the
> timestamp at various phases of CPU Idle and then compute the sw
> latency for various c states. For OMAP, 32k clock is chosen as
> reference clock this as is an always on clock. wkup domain memory
> (scratchpad memory) is used for storing timestamps. One can see the
> worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
> in .config). This information can be used to correctly configure cpu idle
> latencies for various C states after adding HW latencies for each of
> these sw latencies.
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>
> THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>
> Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> Cc: linaro-dev@lists.linaro.org
> ---
...
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + sleep_time = omap3_sram_get_sleep_time();
> + wkup_time = omap3_sram_get_wkup_time();
> +
> + /* take care of overflow */
> + if (postidle_time < preidle_time)
> + postidle_time += (u32) 0xffffffff;
> + if (wkup_time < sleep_time)
> + wkup_time += (u32) 0xffffffff;
> +
> + idle_time = postidle_time - preidle_time;
> + total_sleep_time = wkup_time - sleep_time;
> + latency = idle_time - total_sleep_time;
> + sleep_time = omap3_sram_get_sleep_time();
> + wkup_time = omap3_sram_get_wkup_time();
Is it needed to re-read the sleep_time and wkup_time values from the scratchpad?
What about the 32k timer overflow? Could that cause the sleep_latency
and/or wkup_latency to be <0?
> +
> + /* calculate average latency after ignoring sprious ones */
> + if ((total_sleep_time > 0) && (latency > state->actual_latency)
> + && (latency >= 0)) {
> + state->actual_latency = CONVERT_32K_USEC(latency);
> + latency = (sleep_time - preidle_time);
Risk of overflow?
> + state->sleep_latency = CONVERT_32K_USEC(latency);
> + latency = postidle_time - wkup_time;
Risk of overflow?
> + state->wkup_latency = CONVERT_32K_USEC(latency);
> + }
> +#endif
> +
> return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
> }
>
Jean
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 22:08 [PATCH] OMAP CPUIDLE: CPU Idle latency measurement vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
2010-08-27 9:46 ` Jean Pihet
@ 2010-08-27 9:58 ` Silesh C V
2010-08-27 10:00 ` Sripathy, Vishwanath
2010-08-27 10:25 ` Cousson, Benoit
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Silesh C V @ 2010-08-27 9:58 UTC (permalink / raw)
To: vishwanath.sripathy; +Cc: linux-omap, linaro-dev
g 28, 2010 at 3:38 AM, <vishwanath.sripathy@linaro.org> wrote:
> From: Vishwanath BS <vishwanath.sripathy@linaro.org>
>
> This patch has instrumentation code for measuring latencies for
> various CPUIdle C states for OMAP. Idea here is to capture the
> timestamp at various phases of CPU Idle and then compute the sw
> latency for various c states. For OMAP, 32k clock is chosen as
> reference clock this as is an always on clock. wkup domain memory
> (scratchpad memory) is used for storing timestamps. One can see the
> worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
> in .config). This information can be used to correctly configure cpu idle
> latencies for various C states after adding HW latencies for each of
> these sw latencies.
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>
> THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>
> Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> Cc: linaro-dev@lists.linaro.org
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 58 ++++++++++++++++--
> arch/arm/mach-omap2/pm.h | 5 ++
> arch/arm/mach-omap2/sleep34xx.S | 121 +++++++++++++++++++++++++++++++++++++
> drivers/cpuidle/Kconfig | 5 ++
> drivers/cpuidle/sysfs.c | 16 +++++-
> include/linux/cpuidle.h | 3 +
> 6 files changed, 202 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 3d3d035..398bef8
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -25,6 +25,7 @@
> #include <linux/sched.h>
> #include <linux/cpuidle.h>
>
> +#include <linux/clk.h>
> #include <plat/prcm.h>
> #include <plat/irqs.h>
> #include <plat/powerdomain.h>
> @@ -86,6 +87,11 @@ static struct cpuidle_params cpuidle_params_table[] = {
> {1, 10000, 30000, 300000},
> };
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> +static struct clk *clk_32k;
> +#define CONVERT_32K_USEC(lat) (lat * (USEC_PER_SEC/clk_get_rate(clk_32k)))
> +#endif
> +
> static int omap3_idle_bm_check(void)
> {
> if (!omap3_can_sleep())
> @@ -115,21 +121,28 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
> * Called from the CPUidle framework to program the device to the
> * specified target state selected by the governor.
> */
> +
> static int omap3_enter_idle(struct cpuidle_device *dev,
> struct cpuidle_state *state)
> {
> struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
> struct timespec ts_preidle, ts_postidle, ts_idle;
> u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
> +#ifdef CONFIG_CPU_IDLE_PROF
> + int idle_time, latency;
> + long sleep_time, wkup_time, total_sleep_time;
> + long preidle_time, postidle_time;
> +#endif
>
> current_cx_state = *cx;
>
> - /* Used to keep track of the total time in idle */
> - getnstimeofday(&ts_preidle);
> -
> local_irq_disable();
> local_fiq_disable();
> -
> + /* Used to keep track of the total time in idle */
> + getnstimeofday(&ts_preidle);
> +#ifdef CONFIG_CPU_IDLE_PROF
> + preidle_time = omap3_sram_get_32k_tick();
> +#endif
> pwrdm_set_next_pwrst(mpu_pd, mpu_state);
> pwrdm_set_next_pwrst(core_pd, core_state);
>
> @@ -153,9 +166,39 @@ return_sleep_time:
> getnstimeofday(&ts_postidle);
> ts_idle = timespec_sub(ts_postidle, ts_preidle);
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + postidle_time = omap3_sram_get_32k_tick();
> +#endif
> local_irq_enable();
> local_fiq_enable();
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + sleep_time = omap3_sram_get_sleep_time();
> + wkup_time = omap3_sram_get_wkup_time();
> +
> + /* take care of overflow */
> + if (postidle_time < preidle_time)
> + postidle_time += (u32) 0xffffffff;
> + if (wkup_time < sleep_time)
> + wkup_time += (u32) 0xffffffff;
> +
> + idle_time = postidle_time - preidle_time;
> + total_sleep_time = wkup_time - sleep_time;
> + latency = idle_time - total_sleep_time;
> + sleep_time = omap3_sram_get_sleep_time();
> + wkup_time = omap3_sram_get_wkup_time();
> +
> + /* calculate average latency after ignoring sprious ones */
> + if ((total_sleep_time > 0) && (latency > state->actual_latency)
> + && (latency >= 0)) {
> + state->actual_latency = CONVERT_32K_USEC(latency);
> + latency = (sleep_time - preidle_time);
> + state->sleep_latency = CONVERT_32K_USEC(latency);
> + latency = postidle_time - wkup_time;
> + state->wkup_latency = CONVERT_32K_USEC(latency);
> + }
> +#endif
> +
> return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
> }
>
> @@ -423,7 +466,9 @@ int __init omap3_idle_init(void)
> struct omap3_processor_cx *cx;
> struct cpuidle_state *state;
> struct cpuidle_device *dev;
> -
> +#ifdef CONFIG_CPU_IDLE_PROF
> + static struct device dummy_device;
> +#endif
> mpu_pd = pwrdm_lookup("mpu_pwrdm");
> core_pd = pwrdm_lookup("core_pwrdm");
>
> @@ -456,6 +501,9 @@ int __init omap3_idle_init(void)
>
> omap3_cpuidle_update_states();
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + clk_32k = clk_get(&dummy_device, "wkup_32k_fck");
> +#endif
> if (cpuidle_register_device(dev)) {
> printk(KERN_ERR "%s: CPUidle register device failed\n",
> __func__);
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 3de6ece..e62e87d 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -82,4 +82,9 @@ extern unsigned int save_secure_ram_context_sz;
> extern unsigned int omap24xx_cpu_suspend_sz;
> extern unsigned int omap34xx_cpu_suspend_sz;
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> +extern u32 omap3_sram_get_wkup_time();
> +extern u32 omap3_sram_get_sleep_time();
> +extern u32 omap3_sram_get_32k_tick();
> +#endif
> #endif
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index d522cd7..8dec5ef 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -59,6 +59,20 @@
> #define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
> #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
>
> +#define TIMER_32K_SYNC_P 0x48320010
> +#define TIMER_32K_SYNC OMAP2_L4_IO_ADDRESS(TIMER_32K_SYNC_P)
> +
> +#define SCRATCHPAD_SLEEP_TIME_OFFSET 0x9f8
> +#define SCRATCHPAD_WKUP_TIME_OFFSET 0x9fc
> +#define SCRATCHPAD_SLEEP_TIME OMAP343X_CTRL_REGADDR(SCRATCHPAD_SLEEP_TIME_OFFSET)
> +#define SCRATCHPAD_WKUP_TIME OMAP343X_CTRL_REGADDR(SCRATCHPAD_WKUP_TIME_OFFSET)
> +#define SCRATCHPAD_WKUP_TIME_P OMAP343X_CTRL_BASE + SCRATCHPAD_WKUP_TIME_OFFSET
> +
> +#define CM_ICLKEN_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_ICLKEN)
> +#define CM_ICLKEN_WKUP_P OMAP3430_CM_BASE + WKUP_MOD + CM_ICLKEN
> +#define CM_IDLEST_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_IDLEST)
> +#define CM_IDLEST_WKUP_P OMAP3430_CM_BASE + WKUP_MOD + CM_IDLEST
> +
> .text
> /* Function to aquire the semaphore in scratchpad */
> ENTRY(lock_scratchpad_sem)
> @@ -183,7 +197,31 @@ api_params:
> .word 0x4, 0x0, 0x0, 0x1, 0x1
> ENTRY(save_secure_ram_context_sz)
> .word . - save_secure_ram_context
> +#ifdef CONFIG_CPU_IDLE_PROF
> +ENTRY(omap3_sram_get_wkup_time)
> + stmfd sp!, {lr} @ save registers on stack
> + ldr r0, wkup_time
> + ldr r0, [r0]
> + ldmfd sp!, {pc} @ restore regs and return
> +ENTRY(omap3_sram_get_wkup_time_sz)
> + .word . - omap3_sram_get_wkup_time
> +
> +ENTRY(omap3_sram_get_sleep_time)
> + stmfd sp!, {lr} @ save registers on stack
> + ldr r0, sleep_time
> + ldr r0, [r0]
> + ldmfd sp!, {pc} @ restore regs and return
> +ENTRY(omap3_sram_get_sleep_time_sz)
> + .word . - omap3_sram_get_sleep_time
>
> +ENTRY(omap3_sram_get_32k_tick)
> + stmfd sp!, {lr} @ save registers on stack
> + ldr r0, sync_32k_timer
> + ldr r0, [r0]
> + ldmfd sp!, {pc} @ restore regs and return
> +ENTRY(omap3_sram_get_32k_tick_sz)
> + .word . - omap3_sram_get_32k_tick
> +#endif
> /*
> * Forces OMAP into idle state
> *
> @@ -207,6 +245,13 @@ loop:
> cmp r1, #0x0
> /* If context save is required, do that and execute wfi */
> bne save_context_wfi
> +
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, sleep_time
> + str r5, [r6]
> +#endif
> /* Data memory barrier and Data sync barrier */
> mov r1, #0
> mcr p15, 0, r1, c7, c10, 4
> @@ -224,8 +269,25 @@ loop:
> nop
> nop
> nop
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, iclken_wkup
> + ldr r5, [r4]
> + orr r5, r5, #0x4
> + str r5, [r4]
> + ldr r4, idlest_wkup
> +wait_idlest:
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x0
> + bne wait_idlest
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, wkup_time
> + str r5, [r6]
> +#endif
> bl wait_sdrc_ok
>
> +
> ldmfd sp!, {r0-r12, pc} @ restore regs and return
> restore_es3:
> /*b restore_es3*/ @ Enable to debug restore code
> @@ -247,6 +309,23 @@ copy_to_sram:
> blx r1
> restore:
> /* b restore*/ @ Enable to debug restore code
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, iclken_wkup_p
> + ldr r5, [r4]
> + orr r5, r5, #0x4
> + str r5, [r4]
> + ldr r4, idlest_wkup_p
> +wait_idlest1:
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x0
> + bne wait_idlest1
> + ldr r4, sync_32k_timer_p
> + ldr r5, [r4]
> + ldr r6, wkup_time_p
> + str r5, [r6]
> +#endif
> +
> /* Check what was the reason for mpu reset and store the reason in r9*/
> /* 1 - Only L1 and logic lost */
> /* 2 - Only L2 lost - In this case, we wont be here */
> @@ -587,6 +666,12 @@ finished:
> mcr p15, 2, r10, c0, c0, 0
> isb
> skip_l2_inval:
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, sleep_time
> + str r5, [r6]
> +#endif
> /* Data memory barrier and Data sync barrier */
> mov r1, #0
> mcr p15, 0, r1, c7, c10, 4
> @@ -603,6 +688,22 @@ skip_l2_inval:
> nop
> nop
> nop
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, iclken_wkup
> + ldr r5, [r4]
> + orr r5, r5, #0x4
> + str r5, [r4]
> + ldr r4, idlest_wkup
> +wait_idlest2:
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x0
> + bne wait_idlest2
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, wkup_time
> + str r5, [r6]
> +#endif
> bl wait_sdrc_ok
> /* restore regs and return */
> ldmfd sp!, {r0-r12, pc}
> @@ -668,5 +769,25 @@ cache_pred_disable_mask:
> .word 0xFFFFE7FB
> control_stat:
> .word CONTROL_STAT
> +#ifdef CONFIG_CPU_IDLE_PROF
> +sync_32k_timer:
> + .word TIMER_32K_SYNC
> +sync_32k_timer_p:
> + .word TIMER_32K_SYNC_P
> +sleep_time:
> + .word SCRATCHPAD_SLEEP_TIME
> +wkup_time:
> + .word SCRATCHPAD_WKUP_TIME
> +wkup_time_p:
> + .word SCRATCHPAD_WKUP_TIME_P
> +iclken_wkup:
> + .word CM_ICLKEN_WKUP
> +iclken_wkup_p:
> + .word CM_ICLKEN_WKUP_P
> +idlest_wkup:
> + .word CM_IDLEST_WKUP
> +idlest_wkup_p:
> + .word CM_IDLEST_WKUP_P
> +#endif
> ENTRY(omap34xx_cpu_suspend_sz)
> .word . - omap34xx_cpu_suspend
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 7dbc4a8..147456d 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -18,3 +18,8 @@ config CPU_IDLE_GOV_MENU
> bool
> depends on CPU_IDLE && NO_HZ
> default y
> +
> +config CPU_IDLE_PROF
> + bool
Should not this be something like bool "CPU idle profiling " so that
this entry shows up in
'make menuconfig' ? and this should have dependency on ARCH_OMAP3 also ?
> + depends on CPU_IDLE
> + default n
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 0310ffa..a3e9db1 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -249,6 +249,11 @@ define_show_state_ull_function(usage)
> define_show_state_ull_function(time)
> define_show_state_str_function(name)
> define_show_state_str_function(desc)
> +#ifdef CONFIG_CPU_IDLE_PROF
> +define_show_state_function(actual_latency)
> +define_show_state_function(sleep_latency)
> +define_show_state_function(wkup_latency)
> +#endif
>
> define_one_state_ro(name, show_state_name);
> define_one_state_ro(desc, show_state_desc);
> @@ -256,7 +261,11 @@ define_one_state_ro(latency, show_state_exit_latency);
> define_one_state_ro(power, show_state_power_usage);
> define_one_state_ro(usage, show_state_usage);
> define_one_state_ro(time, show_state_time);
> -
> +#ifdef CONFIG_CPU_IDLE_PROF
> +define_one_state_ro(actual_latency, show_state_actual_latency);
> +define_one_state_ro(sleep_latency, show_state_sleep_latency);
> +define_one_state_ro(wkup_latency, show_state_wkup_latency);
> +#endif
> static struct attribute *cpuidle_state_default_attrs[] = {
> &attr_name.attr,
> &attr_desc.attr,
> @@ -264,6 +273,11 @@ static struct attribute *cpuidle_state_default_attrs[] = {
> &attr_power.attr,
> &attr_usage.attr,
> &attr_time.attr,
> +#ifdef CONFIG_CPU_IDLE_PROF
> + &attr_actual_latency.attr,
> + &attr_sleep_latency.attr,
> + &attr_wkup_latency.attr,
> +#endif
> NULL
> };
>
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 55215cc..6474f6a 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -43,6 +43,9 @@ struct cpuidle_state {
>
> int (*enter) (struct cpuidle_device *dev,
> struct cpuidle_state *state);
> +#ifdef CONFIG_CPU_IDLE_PROF
> + u32 actual_latency, sleep_latency, wkup_latency;
> +#endif
> };
>
> /* Idle State Flags */
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Silesh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 9:46 ` Jean Pihet
@ 2010-08-27 10:00 ` Sripathy, Vishwanath
0 siblings, 0 replies; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-08-27 10:00 UTC (permalink / raw)
To: Jean Pihet, vishwanath.sripathy@linaro.org
Cc: linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Jean Pihet
> Sent: Friday, August 27, 2010 3:17 PM
> To: vishwanath.sripathy@linaro.org
> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> Hi,
>
> On Sat, Aug 28, 2010 at 12:08 AM, <vishwanath.sripathy@linaro.org> wrote:
> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >
> > This patch has instrumentation code for measuring latencies for
> > various CPUIdle C states for OMAP. Idea here is to capture the
> > timestamp at various phases of CPU Idle and then compute the sw
> > latency for various c states. For OMAP, 32k clock is chosen as
> > reference clock this as is an always on clock. wkup domain memory
> > (scratchpad memory) is used for storing timestamps. One can see the
> > worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
> > in .config). This information can be used to correctly configure cpu idle
> > latencies for various C states after adding HW latencies for each of
> > these sw latencies.
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> >
> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> > Cc: linaro-dev@lists.linaro.org
> > ---
> ...
>
> >
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + sleep_time = omap3_sram_get_sleep_time();
> > + wkup_time = omap3_sram_get_wkup_time();
> > +
> > + /* take care of overflow */
> > + if (postidle_time < preidle_time)
> > + postidle_time += (u32) 0xffffffff;
> > + if (wkup_time < sleep_time)
> > + wkup_time += (u32) 0xffffffff;
> > +
> > + idle_time = postidle_time - preidle_time;
> > + total_sleep_time = wkup_time - sleep_time;
> > + latency = idle_time - total_sleep_time;
> > + sleep_time = omap3_sram_get_sleep_time();
> > + wkup_time = omap3_sram_get_wkup_time();
> Is it needed to re-read the sleep_time and wkup_time values from the scratchpad?
Sleep and wake up time stamps were taken just before and after executing wfi and stored in scratchpad wkup memory. These values are used while computing actual latency.
> What about the 32k timer overflow? Could that cause the sleep_latency
> and/or wkup_latency to be <0?
Overflow issues are taken care while computing idle_time and total_sleep_time in code below.
+ if (postidle_time < preidle_time)
+ postidle_time += (u32) 0xffffffff;
+ if (wkup_time < sleep_time)
+ wkup_time += (u32) 0xffffffff;
>
> > +
> > + /* calculate average latency after ignoring sprious ones */
> > + if ((total_sleep_time > 0) && (latency > state->actual_latency)
> > + && (latency >= 0)) {
> > + state->actual_latency = CONVERT_32K_USEC(latency);
> > + latency = (sleep_time - preidle_time);
> Risk of overflow?
Yes I just realized that overflow can cause sleep_latency and wkup_latency to turn negative. Thanks for pointing it. Will fix it in next version.
>
> > + state->sleep_latency = CONVERT_32K_USEC(latency);
> > + latency = postidle_time - wkup_time;
> Risk of overflow?
Agreed. Will fix it.
Vishwa
>
> > + state->wkup_latency = CONVERT_32K_USEC(latency);
> > + }
> > +#endif
> > +
> > return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
> > }
> >
>
> Jean
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 9:58 ` Silesh C V
@ 2010-08-27 10:00 ` Sripathy, Vishwanath
0 siblings, 0 replies; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-08-27 10:00 UTC (permalink / raw)
To: C V, Silesh, vishwanath.sripathy@linaro.org
Cc: linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of C V, Silesh
> Sent: Friday, August 27, 2010 3:28 PM
> To: vishwanath.sripathy@linaro.org
> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> g 28, 2010 at 3:38 AM, <vishwanath.sripathy@linaro.org> wrote:
> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >
> > This patch has instrumentation code for measuring latencies for
> > various CPUIdle C states for OMAP. Idea here is to capture the
> > timestamp at various phases of CPU Idle and then compute the sw
> > latency for various c states. For OMAP, 32k clock is chosen as
> > reference clock this as is an always on clock. wkup domain memory
> > (scratchpad memory) is used for storing timestamps. One can see the
> > worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
> > in .config). This information can be used to correctly configure cpu idle
> > latencies for various C states after adding HW latencies for each of
> > these sw latencies.
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> >
> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> > Cc: linaro-dev@lists.linaro.org
> > ---
> > arch/arm/mach-omap2/cpuidle34xx.c | 58 ++++++++++++++++--
> > arch/arm/mach-omap2/pm.h | 5 ++
> > arch/arm/mach-omap2/sleep34xx.S | 121
> +++++++++++++++++++++++++++++++++++++
> > drivers/cpuidle/Kconfig | 5 ++
> > drivers/cpuidle/sysfs.c | 16 +++++-
> > include/linux/cpuidle.h | 3 +
> > 6 files changed, 202 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-
> omap2/cpuidle34xx.c
> > index 3d3d035..398bef8
> > --- a/arch/arm/mach-omap2/cpuidle34xx.c
> > +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> > @@ -25,6 +25,7 @@
> > #include <linux/sched.h>
> > #include <linux/cpuidle.h>
> >
> > +#include <linux/clk.h>
> > #include <plat/prcm.h>
> > #include <plat/irqs.h>
> > #include <plat/powerdomain.h>
> > @@ -86,6 +87,11 @@ static struct cpuidle_params cpuidle_params_table[] = {
> > {1, 10000, 30000, 300000},
> > };
> >
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > +static struct clk *clk_32k;
> > +#define CONVERT_32K_USEC(lat) (lat * (USEC_PER_SEC/clk_get_rate(clk_32k)))
> > +#endif
> > +
> > static int omap3_idle_bm_check(void)
> > {
> > if (!omap3_can_sleep())
> > @@ -115,21 +121,28 @@ static int _cpuidle_deny_idle(struct powerdomain
> *pwrdm,
> > * Called from the CPUidle framework to program the device to the
> > * specified target state selected by the governor.
> > */
> > +
> > static int omap3_enter_idle(struct cpuidle_device *dev,
> > struct cpuidle_state *state)
> > {
> > struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
> > struct timespec ts_preidle, ts_postidle, ts_idle;
> > u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + int idle_time, latency;
> > + long sleep_time, wkup_time, total_sleep_time;
> > + long preidle_time, postidle_time;
> > +#endif
> >
> > current_cx_state = *cx;
> >
> > - /* Used to keep track of the total time in idle */
> > - getnstimeofday(&ts_preidle);
> > -
> > local_irq_disable();
> > local_fiq_disable();
> > -
> > + /* Used to keep track of the total time in idle */
> > + getnstimeofday(&ts_preidle);
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + preidle_time = omap3_sram_get_32k_tick();
> > +#endif
> > pwrdm_set_next_pwrst(mpu_pd, mpu_state);
> > pwrdm_set_next_pwrst(core_pd, core_state);
> >
> > @@ -153,9 +166,39 @@ return_sleep_time:
> > getnstimeofday(&ts_postidle);
> > ts_idle = timespec_sub(ts_postidle, ts_preidle);
> >
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + postidle_time = omap3_sram_get_32k_tick();
> > +#endif
> > local_irq_enable();
> > local_fiq_enable();
> >
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + sleep_time = omap3_sram_get_sleep_time();
> > + wkup_time = omap3_sram_get_wkup_time();
> > +
> > + /* take care of overflow */
> > + if (postidle_time < preidle_time)
> > + postidle_time += (u32) 0xffffffff;
> > + if (wkup_time < sleep_time)
> > + wkup_time += (u32) 0xffffffff;
> > +
> > + idle_time = postidle_time - preidle_time;
> > + total_sleep_time = wkup_time - sleep_time;
> > + latency = idle_time - total_sleep_time;
> > + sleep_time = omap3_sram_get_sleep_time();
> > + wkup_time = omap3_sram_get_wkup_time();
> > +
> > + /* calculate average latency after ignoring sprious ones */
> > + if ((total_sleep_time > 0) && (latency > state->actual_latency)
> > + && (latency >= 0)) {
> > + state->actual_latency = CONVERT_32K_USEC(latency);
> > + latency = (sleep_time - preidle_time);
> > + state->sleep_latency = CONVERT_32K_USEC(latency);
> > + latency = postidle_time - wkup_time;
> > + state->wkup_latency = CONVERT_32K_USEC(latency);
> > + }
> > +#endif
> > +
> > return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
> > }
> >
> > @@ -423,7 +466,9 @@ int __init omap3_idle_init(void)
> > struct omap3_processor_cx *cx;
> > struct cpuidle_state *state;
> > struct cpuidle_device *dev;
> > -
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + static struct device dummy_device;
> > +#endif
> > mpu_pd = pwrdm_lookup("mpu_pwrdm");
> > core_pd = pwrdm_lookup("core_pwrdm");
> >
> > @@ -456,6 +501,9 @@ int __init omap3_idle_init(void)
> >
> > omap3_cpuidle_update_states();
> >
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + clk_32k = clk_get(&dummy_device, "wkup_32k_fck");
> > +#endif
> > if (cpuidle_register_device(dev)) {
> > printk(KERN_ERR "%s: CPUidle register device failed\n",
> > __func__);
> > diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> > index 3de6ece..e62e87d 100644
> > --- a/arch/arm/mach-omap2/pm.h
> > +++ b/arch/arm/mach-omap2/pm.h
> > @@ -82,4 +82,9 @@ extern unsigned int save_secure_ram_context_sz;
> > extern unsigned int omap24xx_cpu_suspend_sz;
> > extern unsigned int omap34xx_cpu_suspend_sz;
> >
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > +extern u32 omap3_sram_get_wkup_time();
> > +extern u32 omap3_sram_get_sleep_time();
> > +extern u32 omap3_sram_get_32k_tick();
> > +#endif
> > #endif
> > diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-
> omap2/sleep34xx.S
> > index d522cd7..8dec5ef 100644
> > --- a/arch/arm/mach-omap2/sleep34xx.S
> > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > @@ -59,6 +59,20 @@
> > #define SDRC_DLLA_STATUS_V
> OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
> > #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
> >
> > +#define TIMER_32K_SYNC_P 0x48320010
> > +#define TIMER_32K_SYNC OMAP2_L4_IO_ADDRESS(TIMER_32K_SYNC_P)
> > +
> > +#define SCRATCHPAD_SLEEP_TIME_OFFSET 0x9f8
> > +#define SCRATCHPAD_WKUP_TIME_OFFSET 0x9fc
> > +#define SCRATCHPAD_SLEEP_TIME
> OMAP343X_CTRL_REGADDR(SCRATCHPAD_SLEEP_TIME_OFFSET)
> > +#define SCRATCHPAD_WKUP_TIME
> OMAP343X_CTRL_REGADDR(SCRATCHPAD_WKUP_TIME_OFFSET)
> > +#define SCRATCHPAD_WKUP_TIME_P OMAP343X_CTRL_BASE +
> SCRATCHPAD_WKUP_TIME_OFFSET
> > +
> > +#define CM_ICLKEN_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_ICLKEN)
> > +#define CM_ICLKEN_WKUP_P OMAP3430_CM_BASE + WKUP_MOD +
> CM_ICLKEN
> > +#define CM_IDLEST_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_IDLEST)
> > +#define CM_IDLEST_WKUP_P OMAP3430_CM_BASE + WKUP_MOD +
> CM_IDLEST
> > +
> > .text
> > /* Function to aquire the semaphore in scratchpad */
> > ENTRY(lock_scratchpad_sem)
> > @@ -183,7 +197,31 @@ api_params:
> > .word 0x4, 0x0, 0x0, 0x1, 0x1
> > ENTRY(save_secure_ram_context_sz)
> > .word . - save_secure_ram_context
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > +ENTRY(omap3_sram_get_wkup_time)
> > + stmfd sp!, {lr} @ save registers on stack
> > + ldr r0, wkup_time
> > + ldr r0, [r0]
> > + ldmfd sp!, {pc} @ restore regs and return
> > +ENTRY(omap3_sram_get_wkup_time_sz)
> > + .word . - omap3_sram_get_wkup_time
> > +
> > +ENTRY(omap3_sram_get_sleep_time)
> > + stmfd sp!, {lr} @ save registers on stack
> > + ldr r0, sleep_time
> > + ldr r0, [r0]
> > + ldmfd sp!, {pc} @ restore regs and return
> > +ENTRY(omap3_sram_get_sleep_time_sz)
> > + .word . - omap3_sram_get_sleep_time
> >
> > +ENTRY(omap3_sram_get_32k_tick)
> > + stmfd sp!, {lr} @ save registers on stack
> > + ldr r0, sync_32k_timer
> > + ldr r0, [r0]
> > + ldmfd sp!, {pc} @ restore regs and return
> > +ENTRY(omap3_sram_get_32k_tick_sz)
> > + .word . - omap3_sram_get_32k_tick
> > +#endif
> > /*
> > * Forces OMAP into idle state
> > *
> > @@ -207,6 +245,13 @@ loop:
> > cmp r1, #0x0
> > /* If context save is required, do that and execute wfi */
> > bne save_context_wfi
> > +
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + ldr r4, sync_32k_timer
> > + ldr r5, [r4]
> > + ldr r6, sleep_time
> > + str r5, [r6]
> > +#endif
> > /* Data memory barrier and Data sync barrier */
> > mov r1, #0
> > mcr p15, 0, r1, c7, c10, 4
> > @@ -224,8 +269,25 @@ loop:
> > nop
> > nop
> > nop
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + ldr r4, iclken_wkup
> > + ldr r5, [r4]
> > + orr r5, r5, #0x4
> > + str r5, [r4]
> > + ldr r4, idlest_wkup
> > +wait_idlest:
> > + ldr r5, [r4]
> > + and r5, r5, #0x4
> > + cmp r5, #0x0
> > + bne wait_idlest
> > + ldr r4, sync_32k_timer
> > + ldr r5, [r4]
> > + ldr r6, wkup_time
> > + str r5, [r6]
> > +#endif
> > bl wait_sdrc_ok
> >
> > +
> > ldmfd sp!, {r0-r12, pc} @ restore regs and return
> > restore_es3:
> > /*b restore_es3*/ @ Enable to debug restore code
> > @@ -247,6 +309,23 @@ copy_to_sram:
> > blx r1
> > restore:
> > /* b restore*/ @ Enable to debug restore code
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + ldr r4, iclken_wkup_p
> > + ldr r5, [r4]
> > + orr r5, r5, #0x4
> > + str r5, [r4]
> > + ldr r4, idlest_wkup_p
> > +wait_idlest1:
> > + ldr r5, [r4]
> > + and r5, r5, #0x4
> > + cmp r5, #0x0
> > + bne wait_idlest1
> > + ldr r4, sync_32k_timer_p
> > + ldr r5, [r4]
> > + ldr r6, wkup_time_p
> > + str r5, [r6]
> > +#endif
> > +
> > /* Check what was the reason for mpu reset and store the reason in r9*/
> > /* 1 - Only L1 and logic lost */
> > /* 2 - Only L2 lost - In this case, we wont be here */
> > @@ -587,6 +666,12 @@ finished:
> > mcr p15, 2, r10, c0, c0, 0
> > isb
> > skip_l2_inval:
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + ldr r4, sync_32k_timer
> > + ldr r5, [r4]
> > + ldr r6, sleep_time
> > + str r5, [r6]
> > +#endif
> > /* Data memory barrier and Data sync barrier */
> > mov r1, #0
> > mcr p15, 0, r1, c7, c10, 4
> > @@ -603,6 +688,22 @@ skip_l2_inval:
> > nop
> > nop
> > nop
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + ldr r4, iclken_wkup
> > + ldr r5, [r4]
> > + orr r5, r5, #0x4
> > + str r5, [r4]
> > + ldr r4, idlest_wkup
> > +wait_idlest2:
> > + ldr r5, [r4]
> > + and r5, r5, #0x4
> > + cmp r5, #0x0
> > + bne wait_idlest2
> > + ldr r4, sync_32k_timer
> > + ldr r5, [r4]
> > + ldr r6, wkup_time
> > + str r5, [r6]
> > +#endif
> > bl wait_sdrc_ok
> > /* restore regs and return */
> > ldmfd sp!, {r0-r12, pc}
> > @@ -668,5 +769,25 @@ cache_pred_disable_mask:
> > .word 0xFFFFE7FB
> > control_stat:
> > .word CONTROL_STAT
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > +sync_32k_timer:
> > + .word TIMER_32K_SYNC
> > +sync_32k_timer_p:
> > + .word TIMER_32K_SYNC_P
> > +sleep_time:
> > + .word SCRATCHPAD_SLEEP_TIME
> > +wkup_time:
> > + .word SCRATCHPAD_WKUP_TIME
> > +wkup_time_p:
> > + .word SCRATCHPAD_WKUP_TIME_P
> > +iclken_wkup:
> > + .word CM_ICLKEN_WKUP
> > +iclken_wkup_p:
> > + .word CM_ICLKEN_WKUP_P
> > +idlest_wkup:
> > + .word CM_IDLEST_WKUP
> > +idlest_wkup_p:
> > + .word CM_IDLEST_WKUP_P
> > +#endif
> > ENTRY(omap34xx_cpu_suspend_sz)
> > .word . - omap34xx_cpu_suspend
> > diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> > index 7dbc4a8..147456d 100644
> > --- a/drivers/cpuidle/Kconfig
> > +++ b/drivers/cpuidle/Kconfig
> > @@ -18,3 +18,8 @@ config CPU_IDLE_GOV_MENU
> > bool
> > depends on CPU_IDLE && NO_HZ
> > default y
> > +
> > +config CPU_IDLE_PROF
> > + bool
>
> Should not this be something like bool "CPU idle profiling " so that
> this entry shows up in
> 'make menuconfig' ? and this should have dependency on ARCH_OMAP3 also ?
Thanks. Will fix it in V2.
Vishwa
>
> > + depends on CPU_IDLE
> > + default n
> > diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> > index 0310ffa..a3e9db1 100644
> > --- a/drivers/cpuidle/sysfs.c
> > +++ b/drivers/cpuidle/sysfs.c
> > @@ -249,6 +249,11 @@ define_show_state_ull_function(usage)
> > define_show_state_ull_function(time)
> > define_show_state_str_function(name)
> > define_show_state_str_function(desc)
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > +define_show_state_function(actual_latency)
> > +define_show_state_function(sleep_latency)
> > +define_show_state_function(wkup_latency)
> > +#endif
> >
> > define_one_state_ro(name, show_state_name);
> > define_one_state_ro(desc, show_state_desc);
> > @@ -256,7 +261,11 @@ define_one_state_ro(latency, show_state_exit_latency);
> > define_one_state_ro(power, show_state_power_usage);
> > define_one_state_ro(usage, show_state_usage);
> > define_one_state_ro(time, show_state_time);
> > -
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > +define_one_state_ro(actual_latency, show_state_actual_latency);
> > +define_one_state_ro(sleep_latency, show_state_sleep_latency);
> > +define_one_state_ro(wkup_latency, show_state_wkup_latency);
> > +#endif
> > static struct attribute *cpuidle_state_default_attrs[] = {
> > &attr_name.attr,
> > &attr_desc.attr,
> > @@ -264,6 +273,11 @@ static struct attribute *cpuidle_state_default_attrs[] = {
> > &attr_power.attr,
> > &attr_usage.attr,
> > &attr_time.attr,
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + &attr_actual_latency.attr,
> > + &attr_sleep_latency.attr,
> > + &attr_wkup_latency.attr,
> > +#endif
> > NULL
> > };
> >
> > diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> > index 55215cc..6474f6a 100644
> > --- a/include/linux/cpuidle.h
> > +++ b/include/linux/cpuidle.h
> > @@ -43,6 +43,9 @@ struct cpuidle_state {
> >
> > int (*enter) (struct cpuidle_device *dev,
> > struct cpuidle_state *state);
> > +#ifdef CONFIG_CPU_IDLE_PROF
> > + u32 actual_latency, sleep_latency, wkup_latency;
> > +#endif
> > };
> >
> > /* Idle State Flags */
> > --
> > 1.7.0.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
>
> --
> Silesh
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 22:08 [PATCH] OMAP CPUIDLE: CPU Idle latency measurement vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
2010-08-27 9:46 ` Jean Pihet
2010-08-27 9:58 ` Silesh C V
@ 2010-08-27 10:25 ` Cousson, Benoit
2010-08-27 13:04 ` Jean Pihet
[not found] ` <4C7792AD.80804-l0cyMroinI0@public.gmane.org>
2010-08-27 19:15 ` Kevin Hilman
[not found] ` <1282946913-26659-1-git-send-email-vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
4 siblings, 2 replies; 23+ messages in thread
From: Cousson, Benoit @ 2010-08-27 10:25 UTC (permalink / raw)
To: vishwanath.sripathy@linaro.org, Sripathy, Vishwanath
Cc: linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org,
Jean Pihet, Nicole Chalhoub, Vincent Bour
Hi Vishwa,
On 8/28/2010 12:08 AM, vishwanath.sripathy@linaro.org wrote:
> From: Vishwanath BS<vishwanath.sripathy@linaro.org>
>
> This patch has instrumentation code for measuring latencies for
> various CPUIdle C states for OMAP. Idea here is to capture the
> timestamp at various phases of CPU Idle and then compute the sw
> latency for various c states. For OMAP, 32k clock is chosen as
> reference clock this as is an always on clock. wkup domain memory
> (scratchpad memory) is used for storing timestamps. One can see the
> worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
> in .config). This information can be used to correctly configure cpu idle
> latencies for various C states after adding HW latencies for each of
> these sw latencies.
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
FYI, Jean is currently working on using standard Linux probes in order
to instrument CPUIdle / CPUfreq. I'm not sure it is doable, but it might
better to use standard method to do that instead of purely OMAP3
specific stuff. This code will not scale easily on OMAP4.
Do you have a dump of the latency you measured do far?
> THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>
> Signed-off-by: Vishwanath BS<vishwanath.sripathy@linaro.org>
> Cc: linaro-dev@lists.linaro.org
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 58 ++++++++++++++++--
> arch/arm/mach-omap2/pm.h | 5 ++
> arch/arm/mach-omap2/sleep34xx.S | 121 +++++++++++++++++++++++++++++++++++++
> drivers/cpuidle/Kconfig | 5 ++
> drivers/cpuidle/sysfs.c | 16 +++++-
> include/linux/cpuidle.h | 3 +
> 6 files changed, 202 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 3d3d035..398bef8
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -25,6 +25,7 @@
> #include<linux/sched.h>
> #include<linux/cpuidle.h>
>
> +#include<linux/clk.h>
> #include<plat/prcm.h>
> #include<plat/irqs.h>
> #include<plat/powerdomain.h>
> @@ -86,6 +87,11 @@ static struct cpuidle_params cpuidle_params_table[] = {
> {1, 10000, 30000, 300000},
> };
>
> +#ifdef CONFIG_CPU_IDLE_PROF
Cannot you use _PROFILING instead? _PROF does not sound very meaningful
for my point of view.
> +static struct clk *clk_32k;
> +#define CONVERT_32K_USEC(lat) (lat * (USEC_PER_SEC/clk_get_rate(clk_32k)))
> +#endif
> +
> static int omap3_idle_bm_check(void)
> {
> if (!omap3_can_sleep())
> @@ -115,21 +121,28 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
> * Called from the CPUidle framework to program the device to the
> * specified target state selected by the governor.
> */
> +
> static int omap3_enter_idle(struct cpuidle_device *dev,
> struct cpuidle_state *state)
> {
> struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
> struct timespec ts_preidle, ts_postidle, ts_idle;
> u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
> +#ifdef CONFIG_CPU_IDLE_PROF
> + int idle_time, latency;
> + long sleep_time, wkup_time, total_sleep_time;
> + long preidle_time, postidle_time;
> +#endif
>
> current_cx_state = *cx;
>
> - /* Used to keep track of the total time in idle */
> - getnstimeofday(&ts_preidle);
> -
> local_irq_disable();
> local_fiq_disable();
> -
> + /* Used to keep track of the total time in idle */
> + getnstimeofday(&ts_preidle);
> +#ifdef CONFIG_CPU_IDLE_PROF
> + preidle_time = omap3_sram_get_32k_tick();
> +#endif
> pwrdm_set_next_pwrst(mpu_pd, mpu_state);
> pwrdm_set_next_pwrst(core_pd, core_state);
>
> @@ -153,9 +166,39 @@ return_sleep_time:
> getnstimeofday(&ts_postidle);
> ts_idle = timespec_sub(ts_postidle, ts_preidle);
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + postidle_time = omap3_sram_get_32k_tick();
> +#endif
> local_irq_enable();
> local_fiq_enable();
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + sleep_time = omap3_sram_get_sleep_time();
> + wkup_time = omap3_sram_get_wkup_time();
> +
> + /* take care of overflow */
> + if (postidle_time< preidle_time)
> + postidle_time += (u32) 0xffffffff;
> + if (wkup_time< sleep_time)
> + wkup_time += (u32) 0xffffffff;
> +
> + idle_time = postidle_time - preidle_time;
> + total_sleep_time = wkup_time - sleep_time;
> + latency = idle_time - total_sleep_time;
> + sleep_time = omap3_sram_get_sleep_time();
> + wkup_time = omap3_sram_get_wkup_time();
> +
> + /* calculate average latency after ignoring sprious ones */
> + if ((total_sleep_time> 0)&& (latency> state->actual_latency)
> +&& (latency>= 0)) {
> + state->actual_latency = CONVERT_32K_USEC(latency);
> + latency = (sleep_time - preidle_time);
> + state->sleep_latency = CONVERT_32K_USEC(latency);
> + latency = postidle_time - wkup_time;
> + state->wkup_latency = CONVERT_32K_USEC(latency);
> + }
> +#endif
> +
> return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
> }
>
> @@ -423,7 +466,9 @@ int __init omap3_idle_init(void)
> struct omap3_processor_cx *cx;
> struct cpuidle_state *state;
> struct cpuidle_device *dev;
> -
> +#ifdef CONFIG_CPU_IDLE_PROF
> + static struct device dummy_device;
> +#endif
> mpu_pd = pwrdm_lookup("mpu_pwrdm");
> core_pd = pwrdm_lookup("core_pwrdm");
>
> @@ -456,6 +501,9 @@ int __init omap3_idle_init(void)
>
> omap3_cpuidle_update_states();
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> + clk_32k = clk_get(&dummy_device, "wkup_32k_fck");
> +#endif
> if (cpuidle_register_device(dev)) {
> printk(KERN_ERR "%s: CPUidle register device failed\n",
> __func__);
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 3de6ece..e62e87d 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -82,4 +82,9 @@ extern unsigned int save_secure_ram_context_sz;
> extern unsigned int omap24xx_cpu_suspend_sz;
> extern unsigned int omap34xx_cpu_suspend_sz;
>
> +#ifdef CONFIG_CPU_IDLE_PROF
> +extern u32 omap3_sram_get_wkup_time();
> +extern u32 omap3_sram_get_sleep_time();
> +extern u32 omap3_sram_get_32k_tick();
> +#endif
> #endif
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index d522cd7..8dec5ef 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -59,6 +59,20 @@
> #define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
> #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
>
> +#define TIMER_32K_SYNC_P 0x48320010
> +#define TIMER_32K_SYNC OMAP2_L4_IO_ADDRESS(TIMER_32K_SYNC_P)
We are trying to get rid of all the direct usage of physical address and
hard coded virtual address translation in the code.
So please do not access the timer like that.
> +
> +#define SCRATCHPAD_SLEEP_TIME_OFFSET 0x9f8
> +#define SCRATCHPAD_WKUP_TIME_OFFSET 0x9fc
> +#define SCRATCHPAD_SLEEP_TIME OMAP343X_CTRL_REGADDR(SCRATCHPAD_SLEEP_TIME_OFFSET)
> +#define SCRATCHPAD_WKUP_TIME OMAP343X_CTRL_REGADDR(SCRATCHPAD_WKUP_TIME_OFFSET)
> +#define SCRATCHPAD_WKUP_TIME_P OMAP343X_CTRL_BASE + SCRATCHPAD_WKUP_TIME_OFFSET
> +
> +#define CM_ICLKEN_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_ICLKEN)
> +#define CM_ICLKEN_WKUP_P OMAP3430_CM_BASE + WKUP_MOD + CM_ICLKEN
> +#define CM_IDLEST_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_IDLEST)
> +#define CM_IDLEST_WKUP_P OMAP3430_CM_BASE + WKUP_MOD + CM_IDLEST
Same issue, do not use any macros based on OMAP2_L4_IO_ADDRESS
translation macros.
> +
> .text
> /* Function to aquire the semaphore in scratchpad */
> ENTRY(lock_scratchpad_sem)
> @@ -183,7 +197,31 @@ api_params:
> .word 0x4, 0x0, 0x0, 0x1, 0x1
> ENTRY(save_secure_ram_context_sz)
> .word . - save_secure_ram_context
> +#ifdef CONFIG_CPU_IDLE_PROF
> +ENTRY(omap3_sram_get_wkup_time)
> + stmfd sp!, {lr} @ save registers on stack
Not needed in your case, you are not using lr in your code.
> + ldr r0, wkup_time
> + ldr r0, [r0]
> + ldmfd sp!, {pc} @ restore regs and return
Not needed either.
> +ENTRY(omap3_sram_get_wkup_time_sz)
> + .word . - omap3_sram_get_wkup_time
> +
> +ENTRY(omap3_sram_get_sleep_time)
> + stmfd sp!, {lr} @ save registers on stack
Ditto
> + ldr r0, sleep_time
> + ldr r0, [r0]
> + ldmfd sp!, {pc} @ restore regs and return
Ditto
> +ENTRY(omap3_sram_get_sleep_time_sz)
> + .word . - omap3_sram_get_sleep_time
>
> +ENTRY(omap3_sram_get_32k_tick)
> + stmfd sp!, {lr} @ save registers on stack
Ditto
> + ldr r0, sync_32k_timer
> + ldr r0, [r0]
> + ldmfd sp!, {pc} @ restore regs and return
Ditto
> +ENTRY(omap3_sram_get_32k_tick_sz)
> + .word . - omap3_sram_get_32k_tick
> +#endif
Why do you need assembly code to do that? Cannot you access directly
these variable from the C code?
> /*
> * Forces OMAP into idle state
> *
> @@ -207,6 +245,13 @@ loop:
> cmp r1, #0x0
> /* If context save is required, do that and execute wfi */
> bne save_context_wfi
> +
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, sleep_time
> + str r5, [r6]
> +#endif
> /* Data memory barrier and Data sync barrier */
> mov r1, #0
> mcr p15, 0, r1, c7, c10, 4
> @@ -224,8 +269,25 @@ loop:
> nop
> nop
> nop
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, iclken_wkup
> + ldr r5, [r4]
> + orr r5, r5, #0x4
> + str r5, [r4]
> + ldr r4, idlest_wkup
> +wait_idlest:
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x0
> + bne wait_idlest
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, wkup_time
> + str r5, [r6]
> +#endif
What are you trying to do exactly in that case? Could you describe that
a little bit?
Regards,
Benoit
> bl wait_sdrc_ok
>
> +
> ldmfd sp!, {r0-r12, pc} @ restore regs and return
> restore_es3:
> /*b restore_es3*/ @ Enable to debug restore code
> @@ -247,6 +309,23 @@ copy_to_sram:
> blx r1
> restore:
> /* b restore*/ @ Enable to debug restore code
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, iclken_wkup_p
> + ldr r5, [r4]
> + orr r5, r5, #0x4
> + str r5, [r4]
> + ldr r4, idlest_wkup_p
> +wait_idlest1:
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x0
> + bne wait_idlest1
> + ldr r4, sync_32k_timer_p
> + ldr r5, [r4]
> + ldr r6, wkup_time_p
> + str r5, [r6]
> +#endif
> +
> /* Check what was the reason for mpu reset and store the reason in r9*/
> /* 1 - Only L1 and logic lost */
> /* 2 - Only L2 lost - In this case, we wont be here */
> @@ -587,6 +666,12 @@ finished:
> mcr p15, 2, r10, c0, c0, 0
> isb
> skip_l2_inval:
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, sleep_time
> + str r5, [r6]
> +#endif
> /* Data memory barrier and Data sync barrier */
> mov r1, #0
> mcr p15, 0, r1, c7, c10, 4
> @@ -603,6 +688,22 @@ skip_l2_inval:
> nop
> nop
> nop
> +#ifdef CONFIG_CPU_IDLE_PROF
> + ldr r4, iclken_wkup
> + ldr r5, [r4]
> + orr r5, r5, #0x4
> + str r5, [r4]
> + ldr r4, idlest_wkup
> +wait_idlest2:
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x0
> + bne wait_idlest2
> + ldr r4, sync_32k_timer
> + ldr r5, [r4]
> + ldr r6, wkup_time
> + str r5, [r6]
> +#endif
> bl wait_sdrc_ok
> /* restore regs and return */
> ldmfd sp!, {r0-r12, pc}
> @@ -668,5 +769,25 @@ cache_pred_disable_mask:
> .word 0xFFFFE7FB
> control_stat:
> .word CONTROL_STAT
> +#ifdef CONFIG_CPU_IDLE_PROF
> +sync_32k_timer:
> + .word TIMER_32K_SYNC
> +sync_32k_timer_p:
> + .word TIMER_32K_SYNC_P
> +sleep_time:
> + .word SCRATCHPAD_SLEEP_TIME
> +wkup_time:
> + .word SCRATCHPAD_WKUP_TIME
> +wkup_time_p:
> + .word SCRATCHPAD_WKUP_TIME_P
> +iclken_wkup:
> + .word CM_ICLKEN_WKUP
> +iclken_wkup_p:
> + .word CM_ICLKEN_WKUP_P
> +idlest_wkup:
> + .word CM_IDLEST_WKUP
> +idlest_wkup_p:
> + .word CM_IDLEST_WKUP_P
> +#endif
> ENTRY(omap34xx_cpu_suspend_sz)
> .word . - omap34xx_cpu_suspend
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 7dbc4a8..147456d 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -18,3 +18,8 @@ config CPU_IDLE_GOV_MENU
> bool
> depends on CPU_IDLE&& NO_HZ
> default y
> +
> +config CPU_IDLE_PROF
> + bool
> + depends on CPU_IDLE
> + default n
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 0310ffa..a3e9db1 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -249,6 +249,11 @@ define_show_state_ull_function(usage)
> define_show_state_ull_function(time)
> define_show_state_str_function(name)
> define_show_state_str_function(desc)
> +#ifdef CONFIG_CPU_IDLE_PROF
> +define_show_state_function(actual_latency)
> +define_show_state_function(sleep_latency)
> +define_show_state_function(wkup_latency)
> +#endif
>
> define_one_state_ro(name, show_state_name);
> define_one_state_ro(desc, show_state_desc);
> @@ -256,7 +261,11 @@ define_one_state_ro(latency, show_state_exit_latency);
> define_one_state_ro(power, show_state_power_usage);
> define_one_state_ro(usage, show_state_usage);
> define_one_state_ro(time, show_state_time);
> -
> +#ifdef CONFIG_CPU_IDLE_PROF
> +define_one_state_ro(actual_latency, show_state_actual_latency);
> +define_one_state_ro(sleep_latency, show_state_sleep_latency);
> +define_one_state_ro(wkup_latency, show_state_wkup_latency);
> +#endif
> static struct attribute *cpuidle_state_default_attrs[] = {
> &attr_name.attr,
> &attr_desc.attr,
> @@ -264,6 +273,11 @@ static struct attribute *cpuidle_state_default_attrs[] = {
> &attr_power.attr,
> &attr_usage.attr,
> &attr_time.attr,
> +#ifdef CONFIG_CPU_IDLE_PROF
> +&attr_actual_latency.attr,
> +&attr_sleep_latency.attr,
> +&attr_wkup_latency.attr,
> +#endif
> NULL
> };
>
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 55215cc..6474f6a 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -43,6 +43,9 @@ struct cpuidle_state {
>
> int (*enter) (struct cpuidle_device *dev,
> struct cpuidle_state *state);
> +#ifdef CONFIG_CPU_IDLE_PROF
> + u32 actual_latency, sleep_latency, wkup_latency;
> +#endif
> };
>
> /* Idle State Flags */
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 10:25 ` Cousson, Benoit
@ 2010-08-27 13:04 ` Jean Pihet
[not found] ` <4C7792AD.80804-l0cyMroinI0@public.gmane.org>
1 sibling, 0 replies; 23+ messages in thread
From: Jean Pihet @ 2010-08-27 13:04 UTC (permalink / raw)
To: Cousson, Benoit
Cc: vishwanath.sripathy@linaro.org, Sripathy, Vishwanath,
linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org,
Jean Pihet, Nicole Chalhoub, Vincent Bour
Benoit,
On Fri, Aug 27, 2010 at 12:25 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
>> This patch has instrumentation code for measuring latencies for
>> various CPUIdle C states for OMAP. Idea here is to capture the
>> timestamp at various phases of CPU Idle and then compute the sw
>> latency for various c states. For OMAP, 32k clock is chosen as
>> reference clock this as is an always on clock. wkup domain memory
>> (scratchpad memory) is used for storing timestamps. One can see the
>> worstcase latencies in below sysfs entries (after enabling
>> CONFIG_CPU_IDLE_PROF
>> in .config). This information can be used to correctly configure cpu idle
>> latencies for various C states after adding HW latencies for each of
>> these sw latencies.
>> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
>> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
>> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>
> FYI, Jean is currently working on using standard Linux probes in order to
> instrument CPUIdle / CPUfreq. I'm not sure it is doable, but it might better
> to use standard method to do that instead of purely OMAP3 specific stuff.
> This code will not scale easily on OMAP4.
The proposed code looks OK to me since it is exporting the cpuidle
latency figures through the generic cpuidle driver (in
drivers/cpuidle/sysfs.c, via
/sys/devices/system/cpu/cpu0/cpuidle/state<n>/).
Once that code gets approved I can (and will) add some trace points in
it, so that all PM related events can be traced vs time.
>
> Do you have a dump of the latency you measured do far?
Jean
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <4C7792AD.80804-l0cyMroinI0@public.gmane.org>
@ 2010-08-27 13:41 ` Shilimkar, Santosh
0 siblings, 0 replies; 23+ messages in thread
From: Shilimkar, Santosh @ 2010-08-27 13:41 UTC (permalink / raw)
To: Cousson, Benoit,
vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Sripathy, Vishwanath
Cc: Chalhoub, Nicole, Jean Pihet,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org, Bour, Vincent
Benoit,
> -----Original Message-----
> From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-omap-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Cousson, Benoit
> Sent: Friday, August 27, 2010 3:56 PM
> To: vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Sripathy, Vishwanath
> Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org; Jean Pihet;
> Chalhoub, Nicole; Bour, Vincent
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> Hi Vishwa,
>
> On 8/28/2010 12:08 AM, vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> > From: Vishwanath BS<vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >
> > This patch has instrumentation code for measuring latencies for
> > various CPUIdle C states for OMAP. Idea here is to capture the
> > timestamp at various phases of CPU Idle and then compute the sw
> > latency for various c states. For OMAP, 32k clock is chosen as
> > reference clock this as is an always on clock. wkup domain memory
> > (scratchpad memory) is used for storing timestamps. One can see the
> > worstcase latencies in below sysfs entries (after enabling
> CONFIG_CPU_IDLE_PROF
> > in .config). This information can be used to correctly configure cpu
> idle
> > latencies for various C states after adding HW latencies for each of
> > these sw latencies.
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>
> FYI, Jean is currently working on using standard Linux probes in order
> to instrument CPUIdle / CPUfreq. I'm not sure it is doable, but it might
> better to use standard method to do that instead of purely OMAP3
> specific stuff. This code will not scale easily on OMAP4.
>
Just discussed how to scale this for all OMAPs. Firstly we need to
get this code to common place instead of tying it to OMAP3/OMAP4
specific low level code. Since on OMAP3, we can push C-functions
on SRAM and for OMAP4 we don't have any limitation, all this
code can be converted to C.
Vishwa is planning to attempt that in next version.
Regards,
santosh
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 22:08 [PATCH] OMAP CPUIDLE: CPU Idle latency measurement vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
` (2 preceding siblings ...)
2010-08-27 10:25 ` Cousson, Benoit
@ 2010-08-27 19:15 ` Kevin Hilman
2010-08-30 12:59 ` Sripathy, Vishwanath
[not found] ` <87bp8nn9yx.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
[not found] ` <1282946913-26659-1-git-send-email-vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
4 siblings, 2 replies; 23+ messages in thread
From: Kevin Hilman @ 2010-08-27 19:15 UTC (permalink / raw)
To: vishwanath.sripathy; +Cc: linux-omap, linaro-dev
vishwanath.sripathy@linaro.org writes:
> From: Vishwanath BS <vishwanath.sripathy@linaro.org>
>
> This patch has instrumentation code for measuring latencies for
> various CPUIdle C states for OMAP. Idea here is to capture the
> timestamp at various phases of CPU Idle and then compute the sw
> latency for various c states. For OMAP, 32k clock is chosen as
> reference clock this as is an always on clock. wkup domain memory
> (scratchpad memory) is used for storing timestamps. One can see the
> worstcase latencies in below sysfs entries (after enabling
> CONFIG_CPU_IDLE_PROF in .config). This information can be used to
> correctly configure cpu idle latencies for various C states after
> adding HW latencies for each of these sw latencies.
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>
> THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>
> Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> Cc: linaro-dev@lists.linaro.org
While I have many problems with the implementation details, I won't go
into them because in general this is the wrong direction for kernel
instrumentation.
This approach adds quite a bit overhead to the idle path itself. With
all the reads/writes from/to the scratchpad(?) and all the multiplications
and divides in every idle path, as well as the wait-for-idlest in both
the sleep and resume paths. The additional overhead added is non trivial.
Basically, I'd like get away from custom instrumentation and measurement
coded inside the kernel itself. This kind of code never stops growing
and morphing into ugliness, and rarely scales well when new SoCs are
added.
With ftrace/perf, we can add tracepoints at specific points and use
external tools to extract and analyze the delays, latencys etc.
The point is to keep the minimum possible in the kernel: just the
tracepoints we're interested in. The rest (calculations, averages,
analysis, etc.) does not need to be in the kernel and can be done easier
and with more powerful tools outside the kernel.
Kevin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <1282946913-26659-1-git-send-email-vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2010-08-27 20:28 ` Amit Kucheria
[not found] ` <20100827202842.GF2352-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
0 siblings, 1 reply; 23+ messages in thread
From: Amit Kucheria @ 2010-08-27 20:28 UTC (permalink / raw)
To: vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw
On 10 Aug 28, vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> From: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> This patch has instrumentation code for measuring latencies for
> various CPUIdle C states for OMAP. Idea here is to capture the
> timestamp at various phases of CPU Idle and then compute the sw
> latency for various c states. For OMAP, 32k clock is chosen as
> reference clock this as is an always on clock. wkup domain memory
> (scratchpad memory) is used for storing timestamps. One can see the
> worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
> in .config). This information can be used to correctly configure cpu idle
> latencies for various C states after adding HW latencies for each of
> these sw latencies.
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>
> THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>
> Signed-off-by: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 58 ++++++++++++++++--
> arch/arm/mach-omap2/pm.h | 5 ++
> arch/arm/mach-omap2/sleep34xx.S | 121 +++++++++++++++++++++++++++++++++++++
> drivers/cpuidle/Kconfig | 5 ++
> drivers/cpuidle/sysfs.c | 16 +++++-
> include/linux/cpuidle.h | 3 +
> 6 files changed, 202 insertions(+), 6 deletions(-)
Vishwa,
You should perhaps cc Len Brown and LKML for V2 to get acceptance for the new
counters in cpuidle
Regards,
Amit
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <20100827202842.GF2352-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
@ 2010-08-27 21:59 ` Kevin Hilman
0 siblings, 0 replies; 23+ messages in thread
From: Kevin Hilman @ 2010-08-27 21:59 UTC (permalink / raw)
To: vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linaro-dev-cunTk1MwBs8s++Sfvej+rw
Amit Kucheria <amit.kucheria-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
> On 10 Aug 28, vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
>> From: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> This patch has instrumentation code for measuring latencies for
>> various CPUIdle C states for OMAP. Idea here is to capture the
>> timestamp at various phases of CPU Idle and then compute the sw
>> latency for various c states. For OMAP, 32k clock is chosen as
>> reference clock this as is an always on clock. wkup domain memory
>> (scratchpad memory) is used for storing timestamps. One can see the
>> worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
>> in .config). This information can be used to correctly configure cpu idle
>> latencies for various C states after adding HW latencies for each of
>> these sw latencies.
>> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
>> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
>> /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>>
>> THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>>
>> Signed-off-by: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
>> ---
>> arch/arm/mach-omap2/cpuidle34xx.c | 58 ++++++++++++++++--
>> arch/arm/mach-omap2/pm.h | 5 ++
>> arch/arm/mach-omap2/sleep34xx.S | 121 +++++++++++++++++++++++++++++++++++++
>> drivers/cpuidle/Kconfig | 5 ++
>> drivers/cpuidle/sysfs.c | 16 +++++-
>> include/linux/cpuidle.h | 3 +
>> 6 files changed, 202 insertions(+), 6 deletions(-)
>
> You should perhaps cc Len Brown and LKML for V2 to get acceptance for the new
> counters in cpuidle
Before a v2, we need to have some discussions about the general
direction of how to best do PM instrumentation. As I said in my review
of this patch[1], I am not a fan of the current approach.
Kevin
[1] http://marc.info/?l=linux-omap&m=128293652216542&w=2
NOTE: This post may not have made it to linaro-dev since it's moderated,
an I wasn't subscribed when I posted this, but am now.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
@ 2010-08-27 22:08 vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
2010-08-27 9:46 ` Jean Pihet
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A @ 2010-08-27 22:08 UTC (permalink / raw)
To: linux-omap-u79uwXL29TY76Z2rM5mHXA; +Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw
From: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
This patch has instrumentation code for measuring latencies for
various CPUIdle C states for OMAP. Idea here is to capture the
timestamp at various phases of CPU Idle and then compute the sw
latency for various c states. For OMAP, 32k clock is chosen as
reference clock this as is an always on clock. wkup domain memory
(scratchpad memory) is used for storing timestamps. One can see the
worstcase latencies in below sysfs entries (after enabling CONFIG_CPU_IDLE_PROF
in .config). This information can be used to correctly configure cpu idle
latencies for various C states after adding HW latencies for each of
these sw latencies.
/sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
/sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
/sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
Signed-off-by: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
---
arch/arm/mach-omap2/cpuidle34xx.c | 58 ++++++++++++++++--
arch/arm/mach-omap2/pm.h | 5 ++
arch/arm/mach-omap2/sleep34xx.S | 121 +++++++++++++++++++++++++++++++++++++
drivers/cpuidle/Kconfig | 5 ++
drivers/cpuidle/sysfs.c | 16 +++++-
include/linux/cpuidle.h | 3 +
6 files changed, 202 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 3d3d035..398bef8
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -25,6 +25,7 @@
#include <linux/sched.h>
#include <linux/cpuidle.h>
+#include <linux/clk.h>
#include <plat/prcm.h>
#include <plat/irqs.h>
#include <plat/powerdomain.h>
@@ -86,6 +87,11 @@ static struct cpuidle_params cpuidle_params_table[] = {
{1, 10000, 30000, 300000},
};
+#ifdef CONFIG_CPU_IDLE_PROF
+static struct clk *clk_32k;
+#define CONVERT_32K_USEC(lat) (lat * (USEC_PER_SEC/clk_get_rate(clk_32k)))
+#endif
+
static int omap3_idle_bm_check(void)
{
if (!omap3_can_sleep())
@@ -115,21 +121,28 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
* Called from the CPUidle framework to program the device to the
* specified target state selected by the governor.
*/
+
static int omap3_enter_idle(struct cpuidle_device *dev,
struct cpuidle_state *state)
{
struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
struct timespec ts_preidle, ts_postidle, ts_idle;
u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
+#ifdef CONFIG_CPU_IDLE_PROF
+ int idle_time, latency;
+ long sleep_time, wkup_time, total_sleep_time;
+ long preidle_time, postidle_time;
+#endif
current_cx_state = *cx;
- /* Used to keep track of the total time in idle */
- getnstimeofday(&ts_preidle);
-
local_irq_disable();
local_fiq_disable();
-
+ /* Used to keep track of the total time in idle */
+ getnstimeofday(&ts_preidle);
+#ifdef CONFIG_CPU_IDLE_PROF
+ preidle_time = omap3_sram_get_32k_tick();
+#endif
pwrdm_set_next_pwrst(mpu_pd, mpu_state);
pwrdm_set_next_pwrst(core_pd, core_state);
@@ -153,9 +166,39 @@ return_sleep_time:
getnstimeofday(&ts_postidle);
ts_idle = timespec_sub(ts_postidle, ts_preidle);
+#ifdef CONFIG_CPU_IDLE_PROF
+ postidle_time = omap3_sram_get_32k_tick();
+#endif
local_irq_enable();
local_fiq_enable();
+#ifdef CONFIG_CPU_IDLE_PROF
+ sleep_time = omap3_sram_get_sleep_time();
+ wkup_time = omap3_sram_get_wkup_time();
+
+ /* take care of overflow */
+ if (postidle_time < preidle_time)
+ postidle_time += (u32) 0xffffffff;
+ if (wkup_time < sleep_time)
+ wkup_time += (u32) 0xffffffff;
+
+ idle_time = postidle_time - preidle_time;
+ total_sleep_time = wkup_time - sleep_time;
+ latency = idle_time - total_sleep_time;
+ sleep_time = omap3_sram_get_sleep_time();
+ wkup_time = omap3_sram_get_wkup_time();
+
+ /* calculate average latency after ignoring sprious ones */
+ if ((total_sleep_time > 0) && (latency > state->actual_latency)
+ && (latency >= 0)) {
+ state->actual_latency = CONVERT_32K_USEC(latency);
+ latency = (sleep_time - preidle_time);
+ state->sleep_latency = CONVERT_32K_USEC(latency);
+ latency = postidle_time - wkup_time;
+ state->wkup_latency = CONVERT_32K_USEC(latency);
+ }
+#endif
+
return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
}
@@ -423,7 +466,9 @@ int __init omap3_idle_init(void)
struct omap3_processor_cx *cx;
struct cpuidle_state *state;
struct cpuidle_device *dev;
-
+#ifdef CONFIG_CPU_IDLE_PROF
+ static struct device dummy_device;
+#endif
mpu_pd = pwrdm_lookup("mpu_pwrdm");
core_pd = pwrdm_lookup("core_pwrdm");
@@ -456,6 +501,9 @@ int __init omap3_idle_init(void)
omap3_cpuidle_update_states();
+#ifdef CONFIG_CPU_IDLE_PROF
+ clk_32k = clk_get(&dummy_device, "wkup_32k_fck");
+#endif
if (cpuidle_register_device(dev)) {
printk(KERN_ERR "%s: CPUidle register device failed\n",
__func__);
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 3de6ece..e62e87d 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -82,4 +82,9 @@ extern unsigned int save_secure_ram_context_sz;
extern unsigned int omap24xx_cpu_suspend_sz;
extern unsigned int omap34xx_cpu_suspend_sz;
+#ifdef CONFIG_CPU_IDLE_PROF
+extern u32 omap3_sram_get_wkup_time();
+extern u32 omap3_sram_get_sleep_time();
+extern u32 omap3_sram_get_32k_tick();
+#endif
#endif
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index d522cd7..8dec5ef 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -59,6 +59,20 @@
#define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
#define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
+#define TIMER_32K_SYNC_P 0x48320010
+#define TIMER_32K_SYNC OMAP2_L4_IO_ADDRESS(TIMER_32K_SYNC_P)
+
+#define SCRATCHPAD_SLEEP_TIME_OFFSET 0x9f8
+#define SCRATCHPAD_WKUP_TIME_OFFSET 0x9fc
+#define SCRATCHPAD_SLEEP_TIME OMAP343X_CTRL_REGADDR(SCRATCHPAD_SLEEP_TIME_OFFSET)
+#define SCRATCHPAD_WKUP_TIME OMAP343X_CTRL_REGADDR(SCRATCHPAD_WKUP_TIME_OFFSET)
+#define SCRATCHPAD_WKUP_TIME_P OMAP343X_CTRL_BASE + SCRATCHPAD_WKUP_TIME_OFFSET
+
+#define CM_ICLKEN_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_ICLKEN)
+#define CM_ICLKEN_WKUP_P OMAP3430_CM_BASE + WKUP_MOD + CM_ICLKEN
+#define CM_IDLEST_WKUP OMAP34XX_CM_REGADDR(WKUP_MOD, CM_IDLEST)
+#define CM_IDLEST_WKUP_P OMAP3430_CM_BASE + WKUP_MOD + CM_IDLEST
+
.text
/* Function to aquire the semaphore in scratchpad */
ENTRY(lock_scratchpad_sem)
@@ -183,7 +197,31 @@ api_params:
.word 0x4, 0x0, 0x0, 0x1, 0x1
ENTRY(save_secure_ram_context_sz)
.word . - save_secure_ram_context
+#ifdef CONFIG_CPU_IDLE_PROF
+ENTRY(omap3_sram_get_wkup_time)
+ stmfd sp!, {lr} @ save registers on stack
+ ldr r0, wkup_time
+ ldr r0, [r0]
+ ldmfd sp!, {pc} @ restore regs and return
+ENTRY(omap3_sram_get_wkup_time_sz)
+ .word . - omap3_sram_get_wkup_time
+
+ENTRY(omap3_sram_get_sleep_time)
+ stmfd sp!, {lr} @ save registers on stack
+ ldr r0, sleep_time
+ ldr r0, [r0]
+ ldmfd sp!, {pc} @ restore regs and return
+ENTRY(omap3_sram_get_sleep_time_sz)
+ .word . - omap3_sram_get_sleep_time
+ENTRY(omap3_sram_get_32k_tick)
+ stmfd sp!, {lr} @ save registers on stack
+ ldr r0, sync_32k_timer
+ ldr r0, [r0]
+ ldmfd sp!, {pc} @ restore regs and return
+ENTRY(omap3_sram_get_32k_tick_sz)
+ .word . - omap3_sram_get_32k_tick
+#endif
/*
* Forces OMAP into idle state
*
@@ -207,6 +245,13 @@ loop:
cmp r1, #0x0
/* If context save is required, do that and execute wfi */
bne save_context_wfi
+
+#ifdef CONFIG_CPU_IDLE_PROF
+ ldr r4, sync_32k_timer
+ ldr r5, [r4]
+ ldr r6, sleep_time
+ str r5, [r6]
+#endif
/* Data memory barrier and Data sync barrier */
mov r1, #0
mcr p15, 0, r1, c7, c10, 4
@@ -224,8 +269,25 @@ loop:
nop
nop
nop
+#ifdef CONFIG_CPU_IDLE_PROF
+ ldr r4, iclken_wkup
+ ldr r5, [r4]
+ orr r5, r5, #0x4
+ str r5, [r4]
+ ldr r4, idlest_wkup
+wait_idlest:
+ ldr r5, [r4]
+ and r5, r5, #0x4
+ cmp r5, #0x0
+ bne wait_idlest
+ ldr r4, sync_32k_timer
+ ldr r5, [r4]
+ ldr r6, wkup_time
+ str r5, [r6]
+#endif
bl wait_sdrc_ok
+
ldmfd sp!, {r0-r12, pc} @ restore regs and return
restore_es3:
/*b restore_es3*/ @ Enable to debug restore code
@@ -247,6 +309,23 @@ copy_to_sram:
blx r1
restore:
/* b restore*/ @ Enable to debug restore code
+#ifdef CONFIG_CPU_IDLE_PROF
+ ldr r4, iclken_wkup_p
+ ldr r5, [r4]
+ orr r5, r5, #0x4
+ str r5, [r4]
+ ldr r4, idlest_wkup_p
+wait_idlest1:
+ ldr r5, [r4]
+ and r5, r5, #0x4
+ cmp r5, #0x0
+ bne wait_idlest1
+ ldr r4, sync_32k_timer_p
+ ldr r5, [r4]
+ ldr r6, wkup_time_p
+ str r5, [r6]
+#endif
+
/* Check what was the reason for mpu reset and store the reason in r9*/
/* 1 - Only L1 and logic lost */
/* 2 - Only L2 lost - In this case, we wont be here */
@@ -587,6 +666,12 @@ finished:
mcr p15, 2, r10, c0, c0, 0
isb
skip_l2_inval:
+#ifdef CONFIG_CPU_IDLE_PROF
+ ldr r4, sync_32k_timer
+ ldr r5, [r4]
+ ldr r6, sleep_time
+ str r5, [r6]
+#endif
/* Data memory barrier and Data sync barrier */
mov r1, #0
mcr p15, 0, r1, c7, c10, 4
@@ -603,6 +688,22 @@ skip_l2_inval:
nop
nop
nop
+#ifdef CONFIG_CPU_IDLE_PROF
+ ldr r4, iclken_wkup
+ ldr r5, [r4]
+ orr r5, r5, #0x4
+ str r5, [r4]
+ ldr r4, idlest_wkup
+wait_idlest2:
+ ldr r5, [r4]
+ and r5, r5, #0x4
+ cmp r5, #0x0
+ bne wait_idlest2
+ ldr r4, sync_32k_timer
+ ldr r5, [r4]
+ ldr r6, wkup_time
+ str r5, [r6]
+#endif
bl wait_sdrc_ok
/* restore regs and return */
ldmfd sp!, {r0-r12, pc}
@@ -668,5 +769,25 @@ cache_pred_disable_mask:
.word 0xFFFFE7FB
control_stat:
.word CONTROL_STAT
+#ifdef CONFIG_CPU_IDLE_PROF
+sync_32k_timer:
+ .word TIMER_32K_SYNC
+sync_32k_timer_p:
+ .word TIMER_32K_SYNC_P
+sleep_time:
+ .word SCRATCHPAD_SLEEP_TIME
+wkup_time:
+ .word SCRATCHPAD_WKUP_TIME
+wkup_time_p:
+ .word SCRATCHPAD_WKUP_TIME_P
+iclken_wkup:
+ .word CM_ICLKEN_WKUP
+iclken_wkup_p:
+ .word CM_ICLKEN_WKUP_P
+idlest_wkup:
+ .word CM_IDLEST_WKUP
+idlest_wkup_p:
+ .word CM_IDLEST_WKUP_P
+#endif
ENTRY(omap34xx_cpu_suspend_sz)
.word . - omap34xx_cpu_suspend
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index 7dbc4a8..147456d 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -18,3 +18,8 @@ config CPU_IDLE_GOV_MENU
bool
depends on CPU_IDLE && NO_HZ
default y
+
+config CPU_IDLE_PROF
+ bool
+ depends on CPU_IDLE
+ default n
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 0310ffa..a3e9db1 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -249,6 +249,11 @@ define_show_state_ull_function(usage)
define_show_state_ull_function(time)
define_show_state_str_function(name)
define_show_state_str_function(desc)
+#ifdef CONFIG_CPU_IDLE_PROF
+define_show_state_function(actual_latency)
+define_show_state_function(sleep_latency)
+define_show_state_function(wkup_latency)
+#endif
define_one_state_ro(name, show_state_name);
define_one_state_ro(desc, show_state_desc);
@@ -256,7 +261,11 @@ define_one_state_ro(latency, show_state_exit_latency);
define_one_state_ro(power, show_state_power_usage);
define_one_state_ro(usage, show_state_usage);
define_one_state_ro(time, show_state_time);
-
+#ifdef CONFIG_CPU_IDLE_PROF
+define_one_state_ro(actual_latency, show_state_actual_latency);
+define_one_state_ro(sleep_latency, show_state_sleep_latency);
+define_one_state_ro(wkup_latency, show_state_wkup_latency);
+#endif
static struct attribute *cpuidle_state_default_attrs[] = {
&attr_name.attr,
&attr_desc.attr,
@@ -264,6 +273,11 @@ static struct attribute *cpuidle_state_default_attrs[] = {
&attr_power.attr,
&attr_usage.attr,
&attr_time.attr,
+#ifdef CONFIG_CPU_IDLE_PROF
+ &attr_actual_latency.attr,
+ &attr_sleep_latency.attr,
+ &attr_wkup_latency.attr,
+#endif
NULL
};
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 55215cc..6474f6a 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -43,6 +43,9 @@ struct cpuidle_state {
int (*enter) (struct cpuidle_device *dev,
struct cpuidle_state *state);
+#ifdef CONFIG_CPU_IDLE_PROF
+ u32 actual_latency, sleep_latency, wkup_latency;
+#endif
};
/* Idle State Flags */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-27 19:15 ` Kevin Hilman
@ 2010-08-30 12:59 ` Sripathy, Vishwanath
2010-08-31 4:22 ` Silesh C V
[not found] ` <87bp8nn9yx.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
1 sibling, 1 reply; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-08-30 12:59 UTC (permalink / raw)
To: Kevin Hilman, vishwanath.sripathy@linaro.org
Cc: linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
Kevin,
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Kevin Hilman
> Sent: Saturday, August 28, 2010 12:45 AM
> To: vishwanath.sripathy@linaro.org
> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> vishwanath.sripathy@linaro.org writes:
>
> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >
> > This patch has instrumentation code for measuring latencies for
> > various CPUIdle C states for OMAP. Idea here is to capture the
> > timestamp at various phases of CPU Idle and then compute the sw
> > latency for various c states. For OMAP, 32k clock is chosen as
> > reference clock this as is an always on clock. wkup domain memory
> > (scratchpad memory) is used for storing timestamps. One can see the
> > worstcase latencies in below sysfs entries (after enabling
> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
> > correctly configure cpu idle latencies for various C states after
> > adding HW latencies for each of these sw latencies.
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> >
> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> > Cc: linaro-dev@lists.linaro.org
>
> While I have many problems with the implementation details, I won't go
> into them because in general this is the wrong direction for kernel
> instrumentation.
>
> This approach adds quite a bit overhead to the idle path itself. With
> all the reads/writes from/to the scratchpad(?) and all the multiplications
> and divides in every idle path, as well as the wait-for-idlest in both
> the sleep and resume paths. The additional overhead added is non trivial.
>
> Basically, I'd like get away from custom instrumentation and measurement
> coded inside the kernel itself. This kind of code never stops growing
> and morphing into ugliness, and rarely scales well when new SoCs are
> added.
>
> With ftrace/perf, we can add tracepoints at specific points and use
> external tools to extract and analyze the delays, latencys etc.
>
> The point is to keep the minimum possible in the kernel: just the
> tracepoints we're interested in. The rest (calculations, averages,
> analysis, etc.) does not need to be in the kernel and can be done easier
> and with more powerful tools outside the kernel.
The challenge here is that we need to take time stamp at the fag end of CPU Idle which means we have no access to DDR, MMU/Caches are disabled etc (on OMAP3). So I am not sure if we will be able to use ftrace/perf kind of tools here. If we choose to exclude assembly code part for measurement, then we will be omitting major contributor to CPU Idle latency namely ARM context save/restoration part.
Also these calculations are done only when we enable CPUIDLE profiling feature.
In the normal production system, these will not come into picture at all. So I am not sure latencies involved in these calculations are still an issue when we are just doing profiling.
Regards
Vishwa
>
> Kevin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-30 12:59 ` Sripathy, Vishwanath
@ 2010-08-31 4:22 ` Silesh C V
2010-08-31 4:58 ` Sripathy, Vishwanath
0 siblings, 1 reply; 23+ messages in thread
From: Silesh C V @ 2010-08-31 4:22 UTC (permalink / raw)
To: Sripathy, Vishwanath
Cc: Kevin Hilman, vishwanath.sripathy@linaro.org,
linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
Hi Vishwa,
On Mon, Aug 30, 2010 at 6:29 PM, Sripathy, Vishwanath
<vishwanath.bs@ti.com> wrote:
> Kevin,
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Kevin Hilman
>> Sent: Saturday, August 28, 2010 12:45 AM
>> To: vishwanath.sripathy@linaro.org
>> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
>> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>>
>> vishwanath.sripathy@linaro.org writes:
>>
>> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
>> >
>> > This patch has instrumentation code for measuring latencies for
>> > various CPUIdle C states for OMAP. Idea here is to capture the
>> > timestamp at various phases of CPU Idle and then compute the sw
>> > latency for various c states. For OMAP, 32k clock is chosen as
>> > reference clock this as is an always on clock. wkup domain memory
>> > (scratchpad memory) is used for storing timestamps. One can see the
>> > worstcase latencies in below sysfs entries (after enabling
>> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
>> > correctly configure cpu idle latencies for various C states after
>> > adding HW latencies for each of these sw latencies.
>> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
>> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
>> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>> >
>> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>> >
>> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
>> > Cc: linaro-dev@lists.linaro.org
>>
>> While I have many problems with the implementation details, I won't go
>> into them because in general this is the wrong direction for kernel
>> instrumentation.
>>
>> This approach adds quite a bit overhead to the idle path itself. With
>> all the reads/writes from/to the scratchpad(?) and all the multiplications
>> and divides in every idle path, as well as the wait-for-idlest in both
>> the sleep and resume paths. The additional overhead added is non trivial.
>>
>> Basically, I'd like get away from custom instrumentation and measurement
>> coded inside the kernel itself. This kind of code never stops growing
>> and morphing into ugliness, and rarely scales well when new SoCs are
>> added.
>>
>> With ftrace/perf, we can add tracepoints at specific points and use
>> external tools to extract and analyze the delays, latencys etc.
>>
>> The point is to keep the minimum possible in the kernel: just the
>> tracepoints we're interested in. The rest (calculations, averages,
>> analysis, etc.) does not need to be in the kernel and can be done easier
>> and with more powerful tools outside the kernel.
> The challenge here is that we need to take time stamp at the fag end of CPU Idle which means we have no access to DDR, MMU/Caches are disabled etc (on OMAP3). So I am not sure if we will be able to use ftrace/perf kind of tools here. If we choose to exclude assembly code part for measurement, then we will be omitting major contributor to CPU Idle latency namely ARM context save/restoration part.
>
> Also these calculations are done only when we enable CPUIDLE profiling feature.
> In the normal production system, these will not come into picture at all. So I am not sure latencies involved in these calculations are still an issue >when we are just doing profiling.
There are two other issues when we use 32k timer for latency measurement.
<snip>
+
+ /* take care of overflow */
+ if (postidle_time < preidle_time)
+ postidle_time += (u32) 0xffffffff;
+ if (wkup_time < sleep_time)
+ wkup_time += (u32) 0xffffffff;
+
<snip>
1.We are checking postidle_time < preidle_time to find out whether
there had been an
over flow or not. There can be situations in which the timer
overflows and still we have
a greater postidle_time.
2. We are doing the correction for one overflow. What happens if the
timer overflows for
a second or third time. Can we keep track of the number of
overflows and then do the
correction accordingly?
Regards,
Silesh
>
> Regards
> Vishwa
>>
>> Kevin
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-31 4:22 ` Silesh C V
@ 2010-08-31 4:58 ` Sripathy, Vishwanath
2010-08-31 6:57 ` Silesh C V
0 siblings, 1 reply; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-08-31 4:58 UTC (permalink / raw)
To: Silesh C V
Cc: Kevin Hilman, vishwanath.sripathy@linaro.org,
linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
> -----Original Message-----
> From: Silesh C V [mailto:saileshcv@gmail.com]
> Sent: Tuesday, August 31, 2010 9:53 AM
> To: Sripathy, Vishwanath
> Cc: Kevin Hilman; vishwanath.sripathy@linaro.org; linux-omap@vger.kernel.org;
> linaro-dev@lists.linaro.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> Hi Vishwa,
>
> On Mon, Aug 30, 2010 at 6:29 PM, Sripathy, Vishwanath
> <vishwanath.bs@ti.com> wrote:
> > Kevin,
> >
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> owner@vger.kernel.org] On Behalf Of Kevin Hilman
> >> Sent: Saturday, August 28, 2010 12:45 AM
> >> To: vishwanath.sripathy@linaro.org
> >> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> >> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
> >>
> >> vishwanath.sripathy@linaro.org writes:
> >>
> >> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >> >
> >> > This patch has instrumentation code for measuring latencies for
> >> > various CPUIdle C states for OMAP. Idea here is to capture the
> >> > timestamp at various phases of CPU Idle and then compute the sw
> >> > latency for various c states. For OMAP, 32k clock is chosen as
> >> > reference clock this as is an always on clock. wkup domain memory
> >> > (scratchpad memory) is used for storing timestamps. One can see the
> >> > worstcase latencies in below sysfs entries (after enabling
> >> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
> >> > correctly configure cpu idle latencies for various C states after
> >> > adding HW latencies for each of these sw latencies.
> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> >> >
> >> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> >> >
> >> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >> > Cc: linaro-dev@lists.linaro.org
> >>
> >> While I have many problems with the implementation details, I won't go
> >> into them because in general this is the wrong direction for kernel
> >> instrumentation.
> >>
> >> This approach adds quite a bit overhead to the idle path itself. With
> >> all the reads/writes from/to the scratchpad(?) and all the multiplications
> >> and divides in every idle path, as well as the wait-for-idlest in both
> >> the sleep and resume paths. The additional overhead added is non trivial.
> >>
> >> Basically, I'd like get away from custom instrumentation and measurement
> >> coded inside the kernel itself. This kind of code never stops growing
> >> and morphing into ugliness, and rarely scales well when new SoCs are
> >> added.
> >>
> >> With ftrace/perf, we can add tracepoints at specific points and use
> >> external tools to extract and analyze the delays, latencys etc.
> >>
> >> The point is to keep the minimum possible in the kernel: just the
> >> tracepoints we're interested in. The rest (calculations, averages,
> >> analysis, etc.) does not need to be in the kernel and can be done easier
> >> and with more powerful tools outside the kernel.
> > The challenge here is that we need to take time stamp at the fag end of CPU Idle
> which means we have no access to DDR, MMU/Caches are disabled etc (on OMAP3).
> So I am not sure if we will be able to use ftrace/perf kind of tools here. If we choose
> to exclude assembly code part for measurement, then we will be omitting major
> contributor to CPU Idle latency namely ARM context save/restoration part.
> >
> > Also these calculations are done only when we enable CPUIDLE profiling feature.
> > In the normal production system, these will not come into picture at all. So I am
> not sure latencies involved in these calculations are still an issue >when we are just
> doing profiling.
>
>
> There are two other issues when we use 32k timer for latency measurement.
>
> <snip>
> +
> + /* take care of overflow */
> + if (postidle_time < preidle_time)
> + postidle_time += (u32) 0xffffffff;
> + if (wkup_time < sleep_time)
> + wkup_time += (u32) 0xffffffff;
> +
> <snip>
>
> 1.We are checking postidle_time < preidle_time to find out whether
> there had been an
> over flow or not. There can be situations in which the timer
> overflows and still we have
> a greater postidle_time.
>
> 2. We are doing the correction for one overflow. What happens if the
> timer overflows for
> a second or third time. Can we keep track of the number of
> overflows and then do the
> correction accordingly?
Unfortunately, there is no way to check if overflow happens more than once in 32k timer and as you said, theoretically it's possible that if timer overflows more than once, these calculation will wrong. Having said that, do you really see any usecase where system will idle for more than 37 hours in single cpuidle execution to cause timer overflow?
Vishwa
>
> Regards,
> Silesh
>
> >
> > Regards
> > Vishwa
> >>
> >> Kevin
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-31 4:58 ` Sripathy, Vishwanath
@ 2010-08-31 6:57 ` Silesh C V
2010-08-31 9:09 ` Sripathy, Vishwanath
0 siblings, 1 reply; 23+ messages in thread
From: Silesh C V @ 2010-08-31 6:57 UTC (permalink / raw)
To: Sripathy, Vishwanath
Cc: Kevin Hilman, vishwanath.sripathy@linaro.org,
linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
On Tue, Aug 31, 2010 at 10:28 AM, Sripathy, Vishwanath
<vishwanath.bs@ti.com> wrote:
>
>
>> -----Original Message-----
>> From: Silesh C V [mailto:saileshcv@gmail.com]
>> Sent: Tuesday, August 31, 2010 9:53 AM
>> To: Sripathy, Vishwanath
>> Cc: Kevin Hilman; vishwanath.sripathy@linaro.org; linux-omap@vger.kernel.org;
>> linaro-dev@lists.linaro.org
>> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>>
>> Hi Vishwa,
>>
>> On Mon, Aug 30, 2010 at 6:29 PM, Sripathy, Vishwanath
>> <vishwanath.bs@ti.com> wrote:
>> > Kevin,
>> >
>> >> -----Original Message-----
>> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> >> owner@vger.kernel.org] On Behalf Of Kevin Hilman
>> >> Sent: Saturday, August 28, 2010 12:45 AM
>> >> To: vishwanath.sripathy@linaro.org
>> >> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
>> >> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>> >>
>> >> vishwanath.sripathy@linaro.org writes:
>> >>
>> >> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
>> >> >
>> >> > This patch has instrumentation code for measuring latencies for
>> >> > various CPUIdle C states for OMAP. Idea here is to capture the
>> >> > timestamp at various phases of CPU Idle and then compute the sw
>> >> > latency for various c states. For OMAP, 32k clock is chosen as
>> >> > reference clock this as is an always on clock. wkup domain memory
>> >> > (scratchpad memory) is used for storing timestamps. One can see the
>> >> > worstcase latencies in below sysfs entries (after enabling
>> >> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
>> >> > correctly configure cpu idle latencies for various C states after
>> >> > adding HW latencies for each of these sw latencies.
>> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
>> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
>> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>> >> >
>> >> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>> >> >
>> >> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
>> >> > Cc: linaro-dev@lists.linaro.org
>> >>
>> >> While I have many problems with the implementation details, I won't go
>> >> into them because in general this is the wrong direction for kernel
>> >> instrumentation.
>> >>
>> >> This approach adds quite a bit overhead to the idle path itself. With
>> >> all the reads/writes from/to the scratchpad(?) and all the multiplications
>> >> and divides in every idle path, as well as the wait-for-idlest in both
>> >> the sleep and resume paths. The additional overhead added is non trivial.
>> >>
>> >> Basically, I'd like get away from custom instrumentation and measurement
>> >> coded inside the kernel itself. This kind of code never stops growing
>> >> and morphing into ugliness, and rarely scales well when new SoCs are
>> >> added.
>> >>
>> >> With ftrace/perf, we can add tracepoints at specific points and use
>> >> external tools to extract and analyze the delays, latencys etc.
>> >>
>> >> The point is to keep the minimum possible in the kernel: just the
>> >> tracepoints we're interested in. The rest (calculations, averages,
>> >> analysis, etc.) does not need to be in the kernel and can be done easier
>> >> and with more powerful tools outside the kernel.
>> > The challenge here is that we need to take time stamp at the fag end of CPU Idle
>> which means we have no access to DDR, MMU/Caches are disabled etc (on OMAP3).
>> So I am not sure if we will be able to use ftrace/perf kind of tools here. If we choose
>> to exclude assembly code part for measurement, then we will be omitting major
>> contributor to CPU Idle latency namely ARM context save/restoration part.
>> >
>> > Also these calculations are done only when we enable CPUIDLE profiling feature.
>> > In the normal production system, these will not come into picture at all. So I am
>> not sure latencies involved in these calculations are still an issue >when we are just
>> doing profiling.
>>
>>
>> There are two other issues when we use 32k timer for latency measurement.
>>
>> <snip>
>> +
>> + /* take care of overflow */
>> + if (postidle_time < preidle_time)
>> + postidle_time += (u32) 0xffffffff;
>> + if (wkup_time < sleep_time)
>> + wkup_time += (u32) 0xffffffff;
>> +
>> <snip>
>>
>> 1.We are checking postidle_time < preidle_time to find out whether
>> there had been an
>> over flow or not. There can be situations in which the timer
>> overflows and still we have
>> a greater postidle_time.
>>
>> 2. We are doing the correction for one overflow. What happens if the
>> timer overflows for
>> a second or third time. Can we keep track of the number of
>> overflows and then do the
>> correction accordingly?
>
> Unfortunately, there is no way to check if overflow happens more than once in 32k timer and as you said, theoretically it's possible >that if timer overflows more than once, these calculation will wrong. Having said that, do you really see any usecase where system >will idle for more than 37 hours in single cpuidle execution to cause timer overflow?
I am not sure. But can we completely write off such a possibility ?
Regards,
Silesh
>
> Vishwa
>>
>> Regards,
>> Silesh
>>
>> >
>> > Regards
>> > Vishwa
>> >>
>> >> Kevin
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-08-31 6:57 ` Silesh C V
@ 2010-08-31 9:09 ` Sripathy, Vishwanath
0 siblings, 0 replies; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-08-31 9:09 UTC (permalink / raw)
To: C V, Silesh
Cc: Kevin Hilman, vishwanath.sripathy@linaro.org,
linux-omap@vger.kernel.org, linaro-dev@lists.linaro.org
> -----Original Message-----
> From: saileshcv@gmail.com [mailto:saileshcv@gmail.com] On Behalf Of C V, Silesh
> Sent: Tuesday, August 31, 2010 12:27 PM
> To: Sripathy, Vishwanath
> Cc: Kevin Hilman; vishwanath.sripathy@linaro.org; linux-omap@vger.kernel.org;
> linaro-dev@lists.linaro.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> On Tue, Aug 31, 2010 at 10:28 AM, Sripathy, Vishwanath
> <vishwanath.bs@ti.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Silesh C V [mailto:saileshcv@gmail.com]
> >> Sent: Tuesday, August 31, 2010 9:53 AM
> >> To: Sripathy, Vishwanath
> >> Cc: Kevin Hilman; vishwanath.sripathy@linaro.org; linux-omap@vger.kernel.org;
> >> linaro-dev@lists.linaro.org
> >> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
> >>
> >> Hi Vishwa,
> >>
> >> On Mon, Aug 30, 2010 at 6:29 PM, Sripathy, Vishwanath
> >> <vishwanath.bs@ti.com> wrote:
> >> > Kevin,
> >> >
> >> >> -----Original Message-----
> >> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> >> owner@vger.kernel.org] On Behalf Of Kevin Hilman
> >> >> Sent: Saturday, August 28, 2010 12:45 AM
> >> >> To: vishwanath.sripathy@linaro.org
> >> >> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> >> >> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
> >> >>
> >> >> vishwanath.sripathy@linaro.org writes:
> >> >>
> >> >> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >> >> >
> >> >> > This patch has instrumentation code for measuring latencies for
> >> >> > various CPUIdle C states for OMAP. Idea here is to capture the
> >> >> > timestamp at various phases of CPU Idle and then compute the sw
> >> >> > latency for various c states. For OMAP, 32k clock is chosen as
> >> >> > reference clock this as is an always on clock. wkup domain memory
> >> >> > (scratchpad memory) is used for storing timestamps. One can see the
> >> >> > worstcase latencies in below sysfs entries (after enabling
> >> >> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
> >> >> > correctly configure cpu idle latencies for various C states after
> >> >> > adding HW latencies for each of these sw latencies.
> >> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> >> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> >> >> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> >> >> >
> >> >> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> >> >> >
> >> >> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
> >> >> > Cc: linaro-dev@lists.linaro.org
> >> >>
> >> >> While I have many problems with the implementation details, I won't go
> >> >> into them because in general this is the wrong direction for kernel
> >> >> instrumentation.
> >> >>
> >> >> This approach adds quite a bit overhead to the idle path itself. With
> >> >> all the reads/writes from/to the scratchpad(?) and all the multiplications
> >> >> and divides in every idle path, as well as the wait-for-idlest in both
> >> >> the sleep and resume paths. The additional overhead added is non trivial.
> >> >>
> >> >> Basically, I'd like get away from custom instrumentation and measurement
> >> >> coded inside the kernel itself. This kind of code never stops growing
> >> >> and morphing into ugliness, and rarely scales well when new SoCs are
> >> >> added.
> >> >>
> >> >> With ftrace/perf, we can add tracepoints at specific points and use
> >> >> external tools to extract and analyze the delays, latencys etc.
> >> >>
> >> >> The point is to keep the minimum possible in the kernel: just the
> >> >> tracepoints we're interested in. The rest (calculations, averages,
> >> >> analysis, etc.) does not need to be in the kernel and can be done easier
> >> >> and with more powerful tools outside the kernel.
> >> > The challenge here is that we need to take time stamp at the fag end of CPU
> Idle
> >> which means we have no access to DDR, MMU/Caches are disabled etc (on
> OMAP3).
> >> So I am not sure if we will be able to use ftrace/perf kind of tools here. If we
> choose
> >> to exclude assembly code part for measurement, then we will be omitting major
> >> contributor to CPU Idle latency namely ARM context save/restoration part.
> >> >
> >> > Also these calculations are done only when we enable CPUIDLE profiling feature.
> >> > In the normal production system, these will not come into picture at all. So I
> am
> >> not sure latencies involved in these calculations are still an issue >when we are
> just
> >> doing profiling.
> >>
> >>
> >> There are two other issues when we use 32k timer for latency measurement.
> >>
> >> <snip>
> >> +
> >> + /* take care of overflow */
> >> + if (postidle_time < preidle_time)
> >> + postidle_time += (u32) 0xffffffff;
> >> + if (wkup_time < sleep_time)
> >> + wkup_time += (u32) 0xffffffff;
> >> +
> >> <snip>
> >>
> >> 1.We are checking postidle_time < preidle_time to find out whether
> >> there had been an
> >> over flow or not. There can be situations in which the timer
> >> overflows and still we have
> >> a greater postidle_time.
> >>
> >> 2. We are doing the correction for one overflow. What happens if the
> >> timer overflows for
> >> a second or third time. Can we keep track of the number of
> >> overflows and then do the
> >> correction accordingly?
> >
> > Unfortunately, there is no way to check if overflow happens more than once in 32k
> timer and as you said, theoretically it's possible >that if timer overflows more than
> once, these calculation will wrong. Having said that, do you really see any usecase
> where system >will idle for more than 37 hours in single cpuidle execution to cause
> timer overflow?
>
>
> I am not sure. But can we completely write off such a possibility ?
I do not think it's a possibility. Also I believe that this problem is applicable even for system time as it uses 32k clock for maintaining system time.
Vishwa
>
>
> Regards,
> Silesh
>
> >
> > Vishwa
> >>
> >> Regards,
> >> Silesh
> >>
> >> >
> >> > Regards
> >> > Vishwa
> >> >>
> >> >> Kevin
> >> >>
> >> >> --
> >> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> >> the body of a message to majordomo@vger.kernel.org
> >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> > the body of a message to majordomo@vger.kernel.org
> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <87bp8nn9yx.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
@ 2010-09-02 7:56 ` Amit Kucheria
[not found] ` <20100902075605.GB2962-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
2010-09-02 17:55 ` Kevin Hilman
0 siblings, 2 replies; 23+ messages in thread
From: Amit Kucheria @ 2010-09-02 7:56 UTC (permalink / raw)
To: Kevin Hilman
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-omap-u79uwXL29TY76Z2rM5mHXA
On 10 Aug 27, Kevin Hilman wrote:
> vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org writes:
>
> > From: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >
> > This patch has instrumentation code for measuring latencies for
> > various CPUIdle C states for OMAP. Idea here is to capture the
> > timestamp at various phases of CPU Idle and then compute the sw
> > latency for various c states. For OMAP, 32k clock is chosen as
> > reference clock this as is an always on clock. wkup domain memory
> > (scratchpad memory) is used for storing timestamps. One can see the
> > worstcase latencies in below sysfs entries (after enabling
> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
> > correctly configure cpu idle latencies for various C states after
> > adding HW latencies for each of these sw latencies.
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> >
> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
>
> While I have many problems with the implementation details, I won't go
> into them because in general this is the wrong direction for kernel
> instrumentation.
>
> This approach adds quite a bit overhead to the idle path itself. With
> all the reads/writes from/to the scratchpad(?) and all the multiplications
> and divides in every idle path, as well as the wait-for-idlest in both
> the sleep and resume paths. The additional overhead added is non trivial.
>
> Basically, I'd like get away from custom instrumentation and measurement
> coded inside the kernel itself. This kind of code never stops growing
> and morphing into ugliness, and rarely scales well when new SoCs are
> added.
>
> With ftrace/perf, we can add tracepoints at specific points and use
> external tools to extract and analyze the delays, latencys etc.
>
> The point is to keep the minimum possible in the kernel: just the
> tracepoints we're interested in. The rest (calculations, averages,
> analysis, etc.) does not need to be in the kernel and can be done easier
> and with more powerful tools outside the kernel.
Kevin,
I agree. We discussed this a little in our weekly meeting. Vishwa's main
concern was the lack of ability to instrument the last bit of SRAM code.
I have a feeling that even with caches on when we enter this code, we won't
see too much variance in the latency to execute this bit of code. Vishwa is
going to confirm that now. If that hypothesis is true, we can indeed move to
using tracepoints in the idle path and use external tools to track latency.
Even if it isn't true, the rest of the idle path could still contain tracepoints.
Regards,
Amit
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <20100902075605.GB2962-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
@ 2010-09-02 8:11 ` Shilimkar, Santosh
[not found] ` <EAF47CD23C76F840A9E7FCE10091EFAB02CCED1488-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
0 siblings, 1 reply; 23+ messages in thread
From: Shilimkar, Santosh @ 2010-09-02 8:11 UTC (permalink / raw)
To: Amit Kucheria, Kevin Hilman
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> -----Original Message-----
> From: linaro-dev-bounces-cunTk1MwBs8s++Sfvej+rw@public.gmane.org [mailto:linaro-dev-
> bounces-cunTk1MwBs8s++Sfvej+rw@public.gmane.org] On Behalf Of Amit Kucheria
> Sent: Thursday, September 02, 2010 1:26 PM
> To: Kevin Hilman
> Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> On 10 Aug 27, Kevin Hilman wrote:
> > vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org writes:
> >
> > > From: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > >
> > > This patch has instrumentation code for measuring latencies for
> > > various CPUIdle C states for OMAP. Idea here is to capture the
> > > timestamp at various phases of CPU Idle and then compute the sw
> > > latency for various c states. For OMAP, 32k clock is chosen as
> > > reference clock this as is an always on clock. wkup domain memory
> > > (scratchpad memory) is used for storing timestamps. One can see the
> > > worstcase latencies in below sysfs entries (after enabling
> > > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
> > > correctly configure cpu idle latencies for various C states after
> > > adding HW latencies for each of these sw latencies.
> > > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
> > > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
> > > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
> > >
> > > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
> > >
> > > Signed-off-by: Vishwanath BS <vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> >
> > While I have many problems with the implementation details, I won't go
> > into them because in general this is the wrong direction for kernel
> > instrumentation.
> >
> > This approach adds quite a bit overhead to the idle path itself. With
> > all the reads/writes from/to the scratchpad(?) and all the
> multiplications
> > and divides in every idle path, as well as the wait-for-idlest in both
> > the sleep and resume paths. The additional overhead added is non
> trivial.
> >
> > Basically, I'd like get away from custom instrumentation and measurement
> > coded inside the kernel itself. This kind of code never stops growing
> > and morphing into ugliness, and rarely scales well when new SoCs are
> > added.
> >
> > With ftrace/perf, we can add tracepoints at specific points and use
> > external tools to extract and analyze the delays, latencys etc.
> >
> > The point is to keep the minimum possible in the kernel: just the
> > tracepoints we're interested in. The rest (calculations, averages,
> > analysis, etc.) does not need to be in the kernel and can be done easier
> > and with more powerful tools outside the kernel.
>
> Kevin,
>
> I agree. We discussed this a little in our weekly meeting. Vishwa's main
> concern was the lack of ability to instrument the last bit of SRAM code.
>
> I have a feeling that even with caches on when we enter this code, we
> won't
> see too much variance in the latency to execute this bit of code. Vishwa
> is
> going to confirm that now. If that hypothesis is true, we can indeed move
> to
> using tracepoints in the idle path and use external tools to track latency.
There will be difference with and without caches but the delta latency will be constant with caches and without caches. Another important point is
he lowest level code should be just profiled once and for worst CPU/BUS clock speed.
>
> Even if it isn't true, the rest of the idle path could still contain
> tracepoints.
>
I also think this would be better approach considering a generic solution.
Regards,
Santosh
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <EAF47CD23C76F840A9E7FCE10091EFAB02CCED1488-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2010-09-02 9:08 ` Jean Pihet
2010-09-06 11:15 ` Sripathy, Vishwanath
0 siblings, 1 reply; 23+ messages in thread
From: Jean Pihet @ 2010-09-02 9:08 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
Hi Amit, Santosh,
On Thu, Sep 2, 2010 at 10:11 AM, Shilimkar, Santosh
<santosh.shilimkar-l0cyMroinI0@public.gmane.org> wrote:
...
>> > The point is to keep the minimum possible in the kernel: just the
>> > tracepoints we're interested in. The rest (calculations, averages,
>> > analysis, etc.) does not need to be in the kernel and can be done easier
>> > and with more powerful tools outside the kernel.
>>
>> Kevin,
>>
>> I agree. We discussed this a little in our weekly meeting. Vishwa's main
>> concern was the lack of ability to instrument the last bit of SRAM code.
>>
>> I have a feeling that even with caches on when we enter this code, we
>> won't
>> see too much variance in the latency to execute this bit of code. Vishwa
>> is
>> going to confirm that now. If that hypothesis is true, we can indeed move
>> to
>> using tracepoints in the idle path and use external tools to track latency.
I agree. Can you confirm this asap?
I am looking at the instrumentation tracepoints now. I think it would
be ideal to provide multiple tracepoints in both sleep and wake-up
paths.
> There will be difference with and without caches but the delta latency will be constant with caches and without caches. Another important point is
> he lowest level code should be just profiled once and for worst CPU/BUS clock speed.
Ok.
>> Even if it isn't true, the rest of the idle path could still contain
>> tracepoints.
I am looking at the best way to introduce tracepoints in the low level
code, in case it is needed.
> I also think this would be better approach considering a generic solution.
Good!
>
> Regards,
> Santosh
>
Jean
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-09-02 7:56 ` Amit Kucheria
[not found] ` <20100902075605.GB2962-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
@ 2010-09-02 17:55 ` Kevin Hilman
1 sibling, 0 replies; 23+ messages in thread
From: Kevin Hilman @ 2010-09-02 17:55 UTC (permalink / raw)
To: vishwanath.sripathy; +Cc: linux-omap, linaro-dev
Amit Kucheria <amit.kucheria@linaro.org> writes:
> On 10 Aug 27, Kevin Hilman wrote:
>> vishwanath.sripathy@linaro.org writes:
>>
>> > From: Vishwanath BS <vishwanath.sripathy@linaro.org>
>> >
>> > This patch has instrumentation code for measuring latencies for
>> > various CPUIdle C states for OMAP. Idea here is to capture the
>> > timestamp at various phases of CPU Idle and then compute the sw
>> > latency for various c states. For OMAP, 32k clock is chosen as
>> > reference clock this as is an always on clock. wkup domain memory
>> > (scratchpad memory) is used for storing timestamps. One can see the
>> > worstcase latencies in below sysfs entries (after enabling
>> > CONFIG_CPU_IDLE_PROF in .config). This information can be used to
>> > correctly configure cpu idle latencies for various C states after
>> > adding HW latencies for each of these sw latencies.
>> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/actual_latency
>> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/sleep_latency
>> > /sys/devices/system/cpu/cpu0/cpuidle/state<n>/wkup_latency
>> >
>> > THis patch is tested on OMAP ZOOM3 using kevin's pm branch.
>> >
>> > Signed-off-by: Vishwanath BS <vishwanath.sripathy@linaro.org>
>> > Cc: linaro-dev@lists.linaro.org
>>
>> While I have many problems with the implementation details, I won't go
>> into them because in general this is the wrong direction for kernel
>> instrumentation.
>>
>> This approach adds quite a bit overhead to the idle path itself. With
>> all the reads/writes from/to the scratchpad(?) and all the multiplications
>> and divides in every idle path, as well as the wait-for-idlest in both
>> the sleep and resume paths. The additional overhead added is non trivial.
>>
>> Basically, I'd like get away from custom instrumentation and measurement
>> coded inside the kernel itself. This kind of code never stops growing
>> and morphing into ugliness, and rarely scales well when new SoCs are
>> added.
>>
>> With ftrace/perf, we can add tracepoints at specific points and use
>> external tools to extract and analyze the delays, latencys etc.
>>
>> The point is to keep the minimum possible in the kernel: just the
>> tracepoints we're interested in. The rest (calculations, averages,
>> analysis, etc.) does not need to be in the kernel and can be done easier
>> and with more powerful tools outside the kernel.
>
> Kevin,
>
> I agree. We discussed this a little in our weekly meeting. Vishwa's main
> concern was the lack of ability to instrument the last bit of SRAM code.
>
> I have a feeling that even with caches on when we enter this code, we won't
> see too much variance in the latency to execute this bit of code. Vishwa is
> going to confirm that now. If that hypothesis is true, we can indeed move to
> using tracepoints in the idle path and use external tools to track latency.
>
> Even if it isn't true, the rest of the idle path could still contain tracepoints.
Yes.
As Santosh pointed out, that low-level code be a fixed latency and can
likely be profiled once.
That being said, it should still be possible to trace the low-level code.
Jean Pihet and I discussed implementing a "fake" tracepoint in SRAM
during that part of the execution which could then be copied out later.
This would minimize the custom tracing code and allow us to continue to
use userspace tools to analyze the entire path.
Jean is investigating this now.
Kevin
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-09-02 9:08 ` Jean Pihet
@ 2010-09-06 11:15 ` Sripathy, Vishwanath
[not found] ` <FCCFB4CDC6E5564B9182F639FC356087031162CEB7-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
0 siblings, 1 reply; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-09-06 11:15 UTC (permalink / raw)
To: Jean Pihet, Shilimkar, Santosh
Cc: Amit Kucheria, Kevin Hilman, linaro-dev@lists.linaro.org,
linux-omap@vger.kernel.org
Jean,
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Jean Pihet
> Sent: Thursday, September 02, 2010 2:39 PM
> To: Shilimkar, Santosh
> Cc: Amit Kucheria; Kevin Hilman; linaro-dev@lists.linaro.org; linux-
> omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> Hi Amit, Santosh,
>
> On Thu, Sep 2, 2010 at 10:11 AM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
> ...
> >> > The point is to keep the minimum possible in the kernel: just the
> >> > tracepoints we're interested in. The rest (calculations, averages,
> >> > analysis, etc.) does not need to be in the kernel and can be done easier
> >> > and with more powerful tools outside the kernel.
> >>
> >> Kevin,
> >>
> >> I agree. We discussed this a little in our weekly meeting. Vishwa's main
> >> concern was the lack of ability to instrument the last bit of SRAM code.
> >>
> >> I have a feeling that even with caches on when we enter this code, we
> >> won't
> >> see too much variance in the latency to execute this bit of code. Vishwa
> >> is
> >> going to confirm that now. If that hypothesis is true, we can indeed move
> >> to
> >> using tracepoints in the idle path and use external tools to track latency.
> I agree. Can you confirm this asap?
I did some profiling of assembly code on OMAP3630 board (ZOOM3). In worst case it takes around 3.28ms and best case around 2.93ms for mpu off mode. For MPU INACTIVE/RET, it is less than 30us.
However as Kevin mentioned in other email, it would be better to find out a way to trace inside assembly code as well.
Regards
Vishwa
> I am looking at the instrumentation tracepoints now. I think it would
> be ideal to provide multiple tracepoints in both sleep and wake-up
> paths.
>
> > There will be difference with and without caches but the delta latency will be
> constant with caches and without caches. Another important point is
> > he lowest level code should be just profiled once and for worst CPU/BUS clock
> speed.
> Ok.
>
> >> Even if it isn't true, the rest of the idle path could still contain
> >> tracepoints.
> I am looking at the best way to introduce tracepoints in the low level
> code, in case it is needed.
>
> > I also think this would be better approach considering a generic solution.
> Good!
>
> >
> > Regards,
> > Santosh
> >
>
> Jean
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
[not found] ` <FCCFB4CDC6E5564B9182F639FC356087031162CEB7-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2010-09-06 16:22 ` Jean Pihet
2010-09-07 6:19 ` Sripathy, Vishwanath
0 siblings, 1 reply; 23+ messages in thread
From: Jean Pihet @ 2010-09-06 16:22 UTC (permalink / raw)
To: Sripathy, Vishwanath
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hi Vishwa,
On Mon, Sep 6, 2010 at 1:15 PM, Sripathy, Vishwanath
<vishwanath.bs-l0cyMroinI0@public.gmane.org> wrote:
> I did some profiling of assembly code on OMAP3630 board (ZOOM3). In worst case it takes around 3.28ms and best case around 2.93ms for mpu off mode.
Can you give a bit more details? Which measurement has been taken (ASM
only, sleep, wake-up ...?) and what are the significant figures?
>For MPU INACTIVE/RET, it is less than 30us.
Mmh that is the resolution of the 32kHz timer, so I guess you get
either 0 or 1 timer cycle depending when you start the measurement.
IMO the 32kHz timer is too slow to measure those fast events.
> However as Kevin mentioned in other email, it would be better to find out a way to trace inside assembly code as well.
OK that could be done but first I would like to make sure such a
complication is needed.
>
> Regards
> Vishwa
Jean
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
2010-09-06 16:22 ` Jean Pihet
@ 2010-09-07 6:19 ` Sripathy, Vishwanath
0 siblings, 0 replies; 23+ messages in thread
From: Sripathy, Vishwanath @ 2010-09-07 6:19 UTC (permalink / raw)
To: Jean Pihet
Cc: Shilimkar, Santosh, Amit Kucheria, Kevin Hilman,
linaro-dev@lists.linaro.org, linux-omap@vger.kernel.org
Hi Jean,
> -----Original Message-----
> From: Jean Pihet [mailto:jean.pihet@newoldbits.com]
> Sent: Monday, September 06, 2010 9:53 PM
> To: Sripathy, Vishwanath
> Cc: Shilimkar, Santosh; Amit Kucheria; Kevin Hilman; linaro-dev@lists.linaro.org;
> linux-omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP CPUIDLE: CPU Idle latency measurement
>
> Hi Vishwa,
>
> On Mon, Sep 6, 2010 at 1:15 PM, Sripathy, Vishwanath
> <vishwanath.bs@ti.com> wrote:
> > I did some profiling of assembly code on OMAP3630 board (ZOOM3). In worst case
> it takes around 3.28ms and best case around 2.93ms for mpu off mode.
> Can you give a bit more details? Which measurement has been taken (ASM
> only, sleep, wake-up ...?) and what are the significant figures?
Measurement has been done for save (as part of sleep sequence) and restore routine (part of wake up sequence) in assembly code. The above number indicates total time spent in save and restore of ARM context.
>
> >For MPU INACTIVE/RET, it is less than 30us.
> Mmh that is the resolution of the 32kHz timer, so I guess you get
> either 0 or 1 timer cycle depending when you start the measurement.
> IMO the 32kHz timer is too slow to measure those fast events.
Yes I agree. When we use trace events, I believe it would be more accurate as it is based on ARM perf counters.
Vishwa
>
> > However as Kevin mentioned in other email, it would be better to find out a way to
> trace inside assembly code as well.
> OK that could be done but first I would like to make sure such a
> complication is needed.
>
> >
> > Regards
> > Vishwa
>
> Jean
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2010-09-07 6:19 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-27 22:08 [PATCH] OMAP CPUIDLE: CPU Idle latency measurement vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A
2010-08-27 9:46 ` Jean Pihet
2010-08-27 10:00 ` Sripathy, Vishwanath
2010-08-27 9:58 ` Silesh C V
2010-08-27 10:00 ` Sripathy, Vishwanath
2010-08-27 10:25 ` Cousson, Benoit
2010-08-27 13:04 ` Jean Pihet
[not found] ` <4C7792AD.80804-l0cyMroinI0@public.gmane.org>
2010-08-27 13:41 ` Shilimkar, Santosh
2010-08-27 19:15 ` Kevin Hilman
2010-08-30 12:59 ` Sripathy, Vishwanath
2010-08-31 4:22 ` Silesh C V
2010-08-31 4:58 ` Sripathy, Vishwanath
2010-08-31 6:57 ` Silesh C V
2010-08-31 9:09 ` Sripathy, Vishwanath
[not found] ` <87bp8nn9yx.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
2010-09-02 7:56 ` Amit Kucheria
[not found] ` <20100902075605.GB2962-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
2010-09-02 8:11 ` Shilimkar, Santosh
[not found] ` <EAF47CD23C76F840A9E7FCE10091EFAB02CCED1488-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2010-09-02 9:08 ` Jean Pihet
2010-09-06 11:15 ` Sripathy, Vishwanath
[not found] ` <FCCFB4CDC6E5564B9182F639FC356087031162CEB7-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2010-09-06 16:22 ` Jean Pihet
2010-09-07 6:19 ` Sripathy, Vishwanath
2010-09-02 17:55 ` Kevin Hilman
[not found] ` <1282946913-26659-1-git-send-email-vishwanath.sripathy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2010-08-27 20:28 ` Amit Kucheria
[not found] ` <20100827202842.GF2352-HeifvKp/9wrwg6x6O6sdfg@public.gmane.org>
2010-08-27 21:59 ` Kevin Hilman
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).