subject:"RE\: \[RFC\/PATCH 2\/2\] OMAP3\: CPUidle\: trigger early idle notification call chain"

Re: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

2010-10-21 Thread Kevin Hilman

Sripathy, Vishwanath vishwanath...@ti.com writes:

 
 During the early part of the CPUidle path (state selection by the
 governer, checking for device activity, etc.) there is no (good)
 reason to have interrupts disabled.
 
 Therefore, enable interrupts early in the CPUidle path and then
 trigger the early idle notifier call chain after device activity
 checks and the next power states have been programmed.  Interrupts are
 then (re)disabled after the final state is selected.
 
 Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
 ---
  arch/arm/mach-omap2/cpuidle34xx.c |   27 --
 -
  1 files changed, 24 insertions(+), 3 deletions(-)
 

 diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-
 omap2/cpuidle34xx.c
 index 0d50b45..8c57360 100644
 --- a/arch/arm/mach-omap2/cpuidle34xx.c
 +++ b/arch/arm/mach-omap2/cpuidle34xx.c
 @@ -30,6 +30,7 @@
  #include plat/powerdomain.h
  #include plat/clockdomain.h
  #include plat/serial.h
 +#include plat/common.h
 
  #include pm.h
  #include control.h
 @@ -128,12 +129,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
  /* Used to keep track of the total time in idle */
  getnstimeofday(ts_preidle);
 
 -local_irq_disable();
 -local_fiq_disable();
 -
  pwrdm_set_next_pwrst(mpu_pd, mpu_state);
  pwrdm_set_next_pwrst(core_pd, core_state);
 
 +omap_early_idle_notifier_start();
 +
 +local_irq_disable();
 +local_fiq_disable();
 +
  if (omap_irq_pending() || need_resched())
  goto return_sleep_time;
 
 @@ -157,6 +160,8 @@ return_sleep_time:
  local_irq_enable();
  local_fiq_enable();
 
 +omap_early_idle_notifier_end();

 It appears that idle notifiers (for restore path) are called after
 getnstimeofday is called which means time spent in idle callbacks are
 not accounted in cpuidle time.  Will it not impact the cpuidle c state
 prediction and latency computation?

You're right.

The current patch includes the pre-idle notifiers, but not the post-idle
notifiers.  We should either include both or exclude both in the timing
but not do one or the other.

I will post and updated version which includes both in the timing
measurement.

But you've hit on something that I'm not too crazy about, and that
Rajendra raised earlier as well, and also why this series is still RFC.

Not only does the CPUidle timing measurement now measure the idle
callbacks, because interrupts are enabled, it will also measure any
other interrupt processing and/or scheduling that might happend because
of the notifiers.

At LPC this year, we'll hopefully be discussing better ways to decouple
CPU idle states and device idle states, and hopefully come up with some
better options here.  Until then, I think this approach is a good
compromise.

Kevin




--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

2010-10-21 Thread Kevin Hilman

[updated version of the patch, fixing issues raised by Vishwa]

From 1661b0f7ad614d221f90eed590040f2cca01c265 Mon Sep 17 00:00:00 2001
From: Kevin Hilman khil...@deeprootsystems.com
Date: Thu, 23 Sep 2010 09:54:41 -0700
Subject: [PATCH] OMAP3: CPUidle: trigger early idle notification call chain

During the early part of the CPUidle path (state selection by the
governer, checking for device activity, etc.) there is no (good)
reason to have interrupts disabled.

Therefore, enable interrupts early in the CPUidle path and then
trigger the early idle notifier call chain after device activity
checks and the next power states have been programmed.  Interrupts are
then (re)disabled after the final state is selected.

Also adjust the timing measurements reported back to the CPUidle core
to ensure that time spent in the idle notifiers is included as part of
the accounted time.

Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
---
 arch/arm/mach-omap2/cpuidle34xx.c |   33 +++--
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
b/arch/arm/mach-omap2/cpuidle34xx.c
index 0d50b45..6c806a8 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -30,6 +30,7 @@
 #include plat/powerdomain.h
 #include plat/clockdomain.h
 #include plat/serial.h
+#include plat/common.h
 
 #include pm.h
 #include control.h
@@ -128,12 +129,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
/* Used to keep track of the total time in idle */
getnstimeofday(ts_preidle);
 
-   local_irq_disable();
-   local_fiq_disable();
-
pwrdm_set_next_pwrst(mpu_pd, mpu_state);
pwrdm_set_next_pwrst(core_pd, core_state);
 
+   omap_early_idle_notifier_start();
+
+   local_irq_disable();
+   local_fiq_disable();
+
if (omap_irq_pending() || need_resched())
goto return_sleep_time;
 
@@ -151,12 +154,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
}
 
 return_sleep_time:
-   getnstimeofday(ts_postidle);
-   ts_idle = timespec_sub(ts_postidle, ts_preidle);
-
local_irq_enable();
local_fiq_enable();
 
+   omap_early_idle_notifier_end();
+
+   getnstimeofday(ts_postidle);
+   ts_idle = timespec_sub(ts_postidle, ts_preidle);
+
return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
 }
 
@@ -459,6 +464,21 @@ struct cpuidle_driver omap3_idle_driver = {
.owner =THIS_MODULE,
 };
 
+static int omap3_idle_prepare(struct cpuidle_device *dev)
+{
+   /*
+* Enable interrupts during the early part of the CPUidle path.
+* They are later (re)disabled when the final state is selected.
+*
+* The primary reason for this is to enable non-atomic idle
+* notifications to drivers that want to coordinate device
+* idle transitions with CPU idle transitions.
+*/
+   local_irq_enable();
+
+   return 0;
+}
+
 /**
  * omap3_idle_init - Init routine for OMAP3 idle
  *
@@ -482,6 +502,7 @@ int __init omap3_idle_init(void)
 
dev = per_cpu(omap3_idle_dev, smp_processor_id());
 
+   dev-prepare = omap3_idle_prepare;
for (i = OMAP3_STATE_C1; i  OMAP3_MAX_STATES; i++) {
cx = omap3_power_states[i];
state = dev-states[count];
-- 
1.7.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

2010-10-20 Thread Sripathy, Vishwanath

Kevin,

 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of Kevin Hilman
 Sent: Thursday, October 21, 2010 5:09 AM
 To: linux-omap@vger.kernel.org
 Cc: linux-arm-ker...@lists.infradead.org
 Subject: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call 
 chain
 
 During the early part of the CPUidle path (state selection by the
 governer, checking for device activity, etc.) there is no (good)
 reason to have interrupts disabled.
 
 Therefore, enable interrupts early in the CPUidle path and then
 trigger the early idle notifier call chain after device activity
 checks and the next power states have been programmed.  Interrupts are
 then (re)disabled after the final state is selected.
 
 Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
 ---
  arch/arm/mach-omap2/cpuidle34xx.c |   27 --
 -
  1 files changed, 24 insertions(+), 3 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-
 omap2/cpuidle34xx.c
 index 0d50b45..8c57360 100644
 --- a/arch/arm/mach-omap2/cpuidle34xx.c
 +++ b/arch/arm/mach-omap2/cpuidle34xx.c
 @@ -30,6 +30,7 @@
  #include plat/powerdomain.h
  #include plat/clockdomain.h
  #include plat/serial.h
 +#include plat/common.h
 
  #include pm.h
  #include control.h
 @@ -128,12 +129,14 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
   /* Used to keep track of the total time in idle */
   getnstimeofday(ts_preidle);
 
 - local_irq_disable();
 - local_fiq_disable();
 -
   pwrdm_set_next_pwrst(mpu_pd, mpu_state);
   pwrdm_set_next_pwrst(core_pd, core_state);
 
 + omap_early_idle_notifier_start();
 +
 + local_irq_disable();
 + local_fiq_disable();
 +
   if (omap_irq_pending() || need_resched())
   goto return_sleep_time;
 
 @@ -157,6 +160,8 @@ return_sleep_time:
   local_irq_enable();
   local_fiq_enable();
 
 + omap_early_idle_notifier_end();
It appears that idle notifiers (for restore path) are called after 
getnstimeofday is called which means time spent in idle callbacks are not 
accounted in cpuidle time. 
Will it not impact the cpuidle c state prediction and latency computation?

Regards
Vishwa

 +
   return ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * USEC_PER_SEC;
  }
 
 @@ -459,6 +464,21 @@ struct cpuidle_driver omap3_idle_driver = {
   .owner =THIS_MODULE,
  };
 
 +static int omap3_idle_prepare(struct cpuidle_device *dev)
 +{
 + /*
 +  * Enable interrupts during the early part of the CPUidle path.
 +  * They are later (re)disabled when the final state is selected.
 +  *
 +  * The primary reason for this is to enable non-atomic idle
 +  * notifications to drivers that want to coordinate device
 +  * idle transitions with CPU idle transitions.
 +  */
 + local_irq_enable();
 +
 + return 0;
 +}
 +
  /**
   * omap3_idle_init - Init routine for OMAP3 idle
   *
 @@ -482,6 +502,7 @@ int __init omap3_idle_init(void)
 
   dev = per_cpu(omap3_idle_dev, smp_processor_id());
 
 + dev-prepare = omap3_idle_prepare;
   for (i = OMAP3_STATE_C1; i  OMAP3_MAX_STATES; i++) {
   cx = omap3_power_states[i];
   state = dev-states[count];
 --
 1.7.2.1
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

Re: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

RE: [RFC/PATCH 2/2] OMAP3: CPUidle: trigger early idle notification call chain

3 matches

Site Navigation

Mail list logo

Footer information