Re: [PATCH 1/8] ARM: OMAP2+: PM QoS: control the power domains next state from the constraints

2012-12-14 Thread Jean Pihet
Hi Paul,

On Tue, Dec 11, 2012 at 1:05 AM, Paul Walmsley p...@pwsan.com wrote:
 Hi

 On Tue, 18 Sep 2012, Jean Pihet wrote:

 When a PM QoS device latency constraint is requested or removed the
 constraint is stored in the constraints list of the corresponding power
 domain, then the aggregated constraint value is applied by programming
 the next functional power state using pwrdm_set_fpwrst.

 The per-device PM QoS locking requires a spinlock to be used. The reasons
 is that some drivers need to use the per-device PM QoS functionality from
 interrupt context or spinlock protected context, as reported by Djamil.
 An example of such a driver is the OMAP HSI (high-speed synchronous serial
 interface) driver which needs to control the IP block idle state from
 the FIFO empty state, from interrupt context.

 Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
 wake-up latency constraints on MPU, CORE and PER.

 Signed-off-by: Jean Pihet j-pi...@ti.com
 Cc: Djamil Elaidi d-ela...@ti.com

 This patch has been changed here.  Details below, but the major changes
Thanks for reworking this!

 were to fix the locking and to not attempt GFP_ATOMIC allocations.
 Hopefully the reasons why are clear, but if not, feel free to ask.
This is fine.

Note about the locking and the allocations:
The locking had been changed to spinlocks because of the potential
need to call the per-device PM QoS API from interrupt or spinlock
protected context.
Unfortunately the per-device PM QoS cannot be called from such a
context, because (1) it internally uses mutexes and (2) it relies on a
blocking notification mechanism.
However after discussion with Rafael W at ELCE, the PM QoS framework
will be reworked to get rid of the overly complex notification
mechanism [1]. The generic power domain framework will be used instead
to apply the constraints to the power domains at the time the power
domain is set up to transition to a lower power state (e.g. when the
last active device of a power domain is set up to idle automatically).
The implementation on OMAP needs some thinking. This very patch is ok
but the registration mechanism to the PM QoS framework [2] will need
to change.

[1] http://marc.info/?l=linux-pmm=134856516908683w=2
[2] http://marc.info/?l=linux-omapm=134795836304294w=2


 The following patch has only been compile-tested, and a few more minor
 changes will probably be needed.
Reviewed only from my side.

More comments here below.



 - Paul

 From: Jean Pihet jean.pi...@newoldbits.com
 Date: Sun, 9 Dec 2012 18:38:12 -0700
 Subject: [PATCH] ARM: OMAP2+: PM QoS: control the power domains next state
  from the constraints

 When a PM QoS device latency constraint is requested or removed the
 constraint is stored in the constraints list of the corresponding power
 domain, then the aggregated constraint value is applied by programming
 the next functional power state using pwrdm_set_fpwrst.

 The per-device PM QoS locking requires a spinlock to be used. The reasons
 is that some drivers need to use the per-device PM QoS functionality from
 interrupt context or spinlock protected context, as reported by Djamil.
 An example of such a driver is the OMAP HSI (high-speed synchronous serial
 interface) driver which needs to control the IP block idle state from
 the FIFO empty state, from interrupt context.

 Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
 wake-up latency constraints on MPU, CORE and PER.
Has it been tested already?

 Signed-off-by: Jean Pihet j-pi...@ti.com
 Cc: Djamil Elaidi d-ela...@ti.com
 [p...@pwsan.com: updated to apply; renamed *state to *fpwrst and
  define as u8; use existing powerdomain spinlock; pack the struct powerdomain
  variables; removed the GFP_ATOMIC allocation; drop expensive operations
  from the _pwrdm_wakeuplat_update_pwrst() debug; ensure that the entire
  wakeuplat update process takes place under the spinlock; remove
  UNSUP_STATE; remove assumption that fpwrsts start at 0; remove
  unnecessary plist_empty test from update path; standardize 'wakeuplat'
  naming; add kerneldoc]

 XXX conform debug to other powerdomain debugging
There are a few 'XXX' comments left.

 ---
  arch/arm/mach-omap2/powerdomain.c |  208 
 +
  arch/arm/mach-omap2/powerdomain.h |   27 -
  2 files changed, 233 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-omap2/powerdomain.c 
 b/arch/arm/mach-omap2/powerdomain.c
 index 7caceaa..bcba0af 100644
 --- a/arch/arm/mach-omap2/powerdomain.c
 +++ b/arch/arm/mach-omap2/powerdomain.c
 @@ -21,8 +21,10 @@
  #include linux/kernel.h
  #include linux/types.h
  #include linux/list.h
 +#include linux/slab.h
  #include linux/errno.h
  #include linux/string.h
 +#include linux/pm_qos.h
  #include linux/spinlock.h
  #include linux/sched.h
  #include linux/seq_file.h
 @@ -125,6 +127,9 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 for (i = 0; i  PWRDM_FPWRSTS_COUNT; i++)
 

Re: [PATCH 1/8] ARM: OMAP2+: PM QoS: control the power domains next state from the constraints

2012-12-10 Thread Paul Walmsley
Hi

On Tue, 18 Sep 2012, Jean Pihet wrote:

 When a PM QoS device latency constraint is requested or removed the
 constraint is stored in the constraints list of the corresponding power
 domain, then the aggregated constraint value is applied by programming
 the next functional power state using pwrdm_set_fpwrst.
 
 The per-device PM QoS locking requires a spinlock to be used. The reasons
 is that some drivers need to use the per-device PM QoS functionality from
 interrupt context or spinlock protected context, as reported by Djamil.
 An example of such a driver is the OMAP HSI (high-speed synchronous serial
 interface) driver which needs to control the IP block idle state from
 the FIFO empty state, from interrupt context.
 
 Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
 wake-up latency constraints on MPU, CORE and PER.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com
 Cc: Djamil Elaidi d-ela...@ti.com

This patch has been changed here.  Details below, but the major changes 
were to fix the locking and to not attempt GFP_ATOMIC allocations.
Hopefully the reasons why are clear, but if not, feel free to ask.

The following patch has only been compile-tested, and a few more minor 
changes will probably be needed.


- Paul

From: Jean Pihet jean.pi...@newoldbits.com
Date: Sun, 9 Dec 2012 18:38:12 -0700
Subject: [PATCH] ARM: OMAP2+: PM QoS: control the power domains next state
 from the constraints

When a PM QoS device latency constraint is requested or removed the
constraint is stored in the constraints list of the corresponding power
domain, then the aggregated constraint value is applied by programming
the next functional power state using pwrdm_set_fpwrst.

The per-device PM QoS locking requires a spinlock to be used. The reasons
is that some drivers need to use the per-device PM QoS functionality from
interrupt context or spinlock protected context, as reported by Djamil.
An example of such a driver is the OMAP HSI (high-speed synchronous serial
interface) driver which needs to control the IP block idle state from
the FIFO empty state, from interrupt context.

Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
wake-up latency constraints on MPU, CORE and PER.

Signed-off-by: Jean Pihet j-pi...@ti.com
Cc: Djamil Elaidi d-ela...@ti.com
[p...@pwsan.com: updated to apply; renamed *state to *fpwrst and
 define as u8; use existing powerdomain spinlock; pack the struct powerdomain
 variables; removed the GFP_ATOMIC allocation; drop expensive operations
 from the _pwrdm_wakeuplat_update_pwrst() debug; ensure that the entire
 wakeuplat update process takes place under the spinlock; remove
 UNSUP_STATE; remove assumption that fpwrsts start at 0; remove
 unnecessary plist_empty test from update path; standardize 'wakeuplat'
 naming; add kerneldoc]

XXX conform debug to other powerdomain debugging
---
 arch/arm/mach-omap2/powerdomain.c |  208 +
 arch/arm/mach-omap2/powerdomain.h |   27 -
 2 files changed, 233 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c 
b/arch/arm/mach-omap2/powerdomain.c
index 7caceaa..bcba0af 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -21,8 +21,10 @@
 #include linux/kernel.h
 #include linux/types.h
 #include linux/list.h
+#include linux/slab.h
 #include linux/errno.h
 #include linux/string.h
+#include linux/pm_qos.h
 #include linux/spinlock.h
 #include linux/sched.h
 #include linux/seq_file.h
@@ -125,6 +127,9 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
for (i = 0; i  PWRDM_FPWRSTS_COUNT; i++)
pwrdm-fpwrst_counter[i] = 0;
 
+   plist_head_init(pwrdm-wakeuplat_plist_head);
+   pwrdm-wakeuplat_next_fpwrst = PWRDM_FUNC_PWRST_OFF;
+
arch_pwrdm-pwrdm_wait_transition(pwrdm);
pwrdm-fpwrst = pwrdm_read_fpwrst(pwrdm);
pwrdm-fpwrst_counter[pwrdm-fpwrst - PWRDM_FPWRST_OFFSET] = 1;
@@ -765,6 +770,58 @@ static int _pwrdm_set_fpwrst(struct powerdomain *pwrdm,
return ret;
 }
 
+/**
+ * _pwrdm_wakeuplat_update_pwrst - Update power domain power state if needed
+ * @pwrdm: struct powerdomain * to which requesting device belongs to.
+ * @min_latency: the allowed wake-up latency for the given power domain. A
+ *  value of PM_QOS_DEV_LAT_DEFAULT_VALUE means 'no constraint' on the pwrdm.
+ *
+ * Finds the power domain next power state that fulfills the constraint.
+ * Programs a new target state if it is different from current power state.
+ * The power domains get the next power state programmed directly in the
+ * registers.
+ *
+ * Must be called with the powerdomain lock held.
+ *
+ * Returns 0 in case of success, -EINVAL in case of invalid parameters,
+ * or the return value from _pwrdm_set_fpwrst().
+ */
+static int _pwrdm_wakeuplat_update_pwrst(struct powerdomain *pwrdm,
+long min_latency)
+{
+   int ret = 0, new_fpwrst = PWRDM_FUNC_PWRST_ON;
+ 

[PATCH 1/8] ARM: OMAP2+: PM QoS: control the power domains next state from the constraints

2012-09-18 Thread Jean Pihet
When a PM QoS device latency constraint is requested or removed the
constraint is stored in the constraints list of the corresponding power
domain, then the aggregated constraint value is applied by programming
the next functional power state using pwrdm_set_fpwrst.

The per-device PM QoS locking requires a spinlock to be used. The reasons
is that some drivers need to use the per-device PM QoS functionality from
interrupt context or spinlock protected context, as reported by Djamil.
An example of such a driver is the OMAP HSI (high-speed synchronous serial
interface) driver which needs to control the IP block idle state from
the FIFO empty state, from interrupt context.

Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
wake-up latency constraints on MPU, CORE and PER.

Signed-off-by: Jean Pihet j-pi...@ti.com
Cc: Djamil Elaidi d-ela...@ti.com
---
 arch/arm/mach-omap2/powerdomain.c |  212 +
 arch/arm/mach-omap2/powerdomain.h |   18 +++-
 2 files changed, 229 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c 
b/arch/arm/mach-omap2/powerdomain.c
index 37dfabf..3d81ac7 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -17,8 +17,10 @@
 #include linux/kernel.h
 #include linux/types.h
 #include linux/list.h
+#include linux/slab.h
 #include linux/errno.h
 #include linux/string.h
+#include linux/pm_qos.h
 #include linux/ratelimit.h
 #include trace/events/power.h
 
@@ -114,6 +116,12 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
for (i = 0; i  pwrdm-banks; i++)
pwrdm-ret_mem_off_counter[i] = 0;
 
+   /* Initialize the per-device wake-up constraints framework data */
+   spin_lock_init(pwrdm-wkup_lat_plist_lock);
+   plist_head_init(pwrdm-wkup_lat_plist_head);
+   pwrdm-wkup_lat_next_state = PWRDM_FUNC_PWRST_OFF;
+
+   /* Initialize the pwrdm state */
pwrdm_wait_transition(pwrdm);
pwrdm-state = pwrdm_read_fpwrst(pwrdm);
pwrdm-state_counter[pwrdm-state] = 1;
@@ -418,6 +426,60 @@ static int _pwrdm_read_next_fpwrst(struct powerdomain 
*pwrdm)
return _pwrdm_pwrst_to_fpwrst(pwrdm, next_pwrst, next_logic);
 }
 
+/**
+ * _pwrdm_wakeuplat_update_pwrst - Update power domain power state if needed
+ * @pwrdm: struct powerdomain * to which requesting device belongs to.
+ * @min_latency: the allowed wake-up latency for the given power domain. A
+ *  value of PM_QOS_DEV_LAT_DEFAULT_VALUE means 'no constraint' on the pwrdm.
+ *
+ * Finds the power domain next power state that fulfills the constraint.
+ * Programs a new target state if it is different from current power state.
+ * The power domains get the next power state programmed directly in the
+ * registers.
+ *
+ * Must be called with the wkup_lat_plist_lock lock held.
+ *
+ * Returns 0 in case of success, -EINVAL in case of invalid parameters,
+ * or the return value from pwrdm_set_fpwrst.
+ */
+static int _pwrdm_wakeuplat_update_pwrst(struct powerdomain *pwrdm,
+long min_latency)
+{
+   int ret = 0, state, new_state = PWRDM_FUNC_PWRST_ON;
+
+   if (!pwrdm) {
+   WARN(1, powerdomain: %s: invalid parameter(s), __func__);
+   return -EINVAL;
+   }
+
+   /*
+* Find the next supported power state with
+*  wakeup latency = min_latency.
+* Pick the lower state if no constraint on the pwrdm
+*  (min_latency == PM_QOS_DEV_LAT_DEFAULT_VALUE).
+* Skip the states marked as unsupported (UNSUP_STATE).
+* If no power state found, fall back to PWRDM_FUNC_PWRST_ON.
+*/
+   for (state = 0x0; state  PWRDM_MAX_FUNC_PWRSTS; state++) {
+   if ((min_latency == PM_QOS_DEV_LAT_DEFAULT_VALUE) ||
+   ((pwrdm-wakeup_lat[state] != UNSUP_STATE) 
+(pwrdm-wakeup_lat[state] = min_latency))) {
+   new_state = state;
+   break;
+   }
+   }
+
+   pwrdm-wkup_lat_next_state = new_state;
+   ret = pwrdm_set_fpwrst(pwrdm, new_state);
+
+   pr_debug(%s: func pwrst for %s: curr=%d, next=%d, min_latency=%ld, 
new_state=%d\n,
+__func__, pwrdm-name, pwrdm_read_fpwrst(pwrdm),
+pwrdm_read_next_fpwrst(pwrdm), min_latency, new_state);
+
+   return ret;
+}
+
+
 /* Public functions */
 
 /**
@@ -1484,6 +1546,156 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
 }
 
 /**
+ * pwrdm_wakeuplat_update_constraint - Set or update a powerdomain wakeup
+ *  latency constraint and apply it
+ * @pwrdm: struct powerdomain * which the constraint applies to
+ * @cookie: constraint identifier, used for tracking
+ * @min_latency: minimum wakeup latency constraint (in microseconds) for
+ *  the given pwrdm
+ *
+ * Tracks the constraints by @cookie.
+ * Constraint set/update: Adds a new entry to powerdomain's wake-up latency
+ * constraint