[PATCH v4 0/6] PM QoS: implement the OMAP low level constraints management code

[PATCH v4 0/6] PM QoS: implement the OMAP low level constraints management code

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

. Implement the devices wake-up latency constraints using the global
  device PM QoS notification handler which applies the constraints to the
  underlying layer
. Implement the low level code which controls the power domains next power
  states, through the hwmod and pwrdm layers
. Add cpuidle and power domains wake-up latency figures for OMAP3, cf. 
  comments in the code and [1] for the details on where the numbers
  are magically coming from
. Implement the relation between the cpuidle and per-device PM QoS frameworks
  in the OMAP3 specific idle callbacks.
  The chosen C-state shall satisfy the following conditions:
   . the 'valid' field is enabled,
   . it satisfies the enable_off_mode flag,
   . the next state for MPU and CORE power domains is not lower than the
     state programmed by the per-device PM QoS.


ToDo:
1. validate the constraints framework on OMAP4 HW (done on OMAP3)
2. Re-visit the OMAP power domains states initialization procedure. Currently
   the power states that have been changed from the constraints API which were
   applied before the initialization of the power domains are lost
3. Further clean-up the OMAP PM layer, use the generic frameworks instead (OPP,
   PM QoS...)


Based on the pm-qos branch of the linux-pm git tree (3.1.0-rc3), cf. [2].

Tested on OMAP3 Beagleboard (ES2.x) with constraints on MPU, CORE, PER in
RETention and OFF modes.

[1] http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement
[2] git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git


v4:
. split up the patches which remove the omap_pm_ code from the patch set.
  Those patches are to be submitted later, on top of this patch set.
. latency numbers: provide the measurements setup and conditions in the code
  comments, added the link to the details on wiki [1].
. improved kerneldoc
. split big functions into smaller ones, in order to improve the readability

v3: reworked the error return path and improved the kerneldoc

v2: reworked the OMAP specific cpuidle code to demote the initial C-state to
     a valid C-state which fulfills the per-device constraints

v1: initial version


Jean Pihet (6):
  OMAP2+: powerdomain: control power domains next state
  OMAP2+: omap_hwmod: manage the wake-up latency constraints
  OMAP: PM: register to the per-device PM QoS framework
  OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and
    CORE constraints
  OMAP3: update cpuidle latency and threshold figures
  OMAP3: powerdomain data: add wake-up latency figures

 arch/arm/mach-omap2/cpuidle34xx.c            |   99 +++++++----
 arch/arm/mach-omap2/omap_hwmod.c             |   27 +++-
 arch/arm/mach-omap2/pm.c                     |   63 +++++++
 arch/arm/mach-omap2/pm.h                     |   17 ++-
 arch/arm/mach-omap2/powerdomain.c            |  237 ++++++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomain.h            |   35 ++++-
 arch/arm/mach-omap2/powerdomains3xxx_data.c  |   83 +++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 +
 8 files changed, 520 insertions(+), 43 deletions(-)

-- 
1.7.4.1

[PATCH 1/6] OMAP2+: powerdomain: control power domains next state

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

When a PM QoS device latency constraint is requested or removed the
PM QoS layer notifies the underlying layer with the updated aggregated
constraint value. The constraint is stored in the powerdomain constraints
list and then applied to the corresponding power domain.
The power domains get the next power state programmed directly in the
registers via pwrdm_wakeuplat_update_pwrst.

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-pihet@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |  237 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomain.h |   35 +++++-
 2 files changed, 270 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 9af0847..351766d 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 <trace/events/power.h>
 
 #include "cm2xxx_3xxx.h"
@@ -104,6 +106,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-od wake-up constraints data */
+	spin_lock_init(&pwrdm->wkup_lat_plist_lock);
+	plist_head_init(&pwrdm->wkup_lat_plist_head);
+	pwrdm->wkup_lat_next_state = PWRDM_POWER_OFF;
+
+	/* Initialize the pwrdm state */
 	pwrdm_wait_transition(pwrdm);
 	pwrdm->state = pwrdm_read_pwrst(pwrdm);
 	pwrdm->state_counter[pwrdm->state] = 1;
@@ -191,6 +199,158 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
 	return 0;
 }
 
+/**
+ * _pwrdm_wakeuplat_update_list - Set/update/remove a powerdomain wakeup
+ *  latency constraint from the pwrdm's constraint list
+ * @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. The value of PM_QOS_DEV_LAT_DEFAULT_VALUE removes
+ *  the constraint.
+ * @user: pointer to the current list entry
+ * @new_user: allocated list entry, used for insertion of new constraints
+ *  in the list
+ * @free_new_user: set to non-zero if the newly allocated list entry
+ *  is unused and needs to be freed
+ * @free_node: set to non-zero if the current list entry is not in use
+ *  anymore and needs to be freed
+ *
+ * Tracks the constraints by @cookie.
+ * Constraint set/update: Adds a new entry to powerdomain's wake-up latency
+ * constraint list.
+ * If the constraint identifier already exists in the list, the old value is
+ * overwritten.
+ * Constraint removal: Removes the identifier's entry from powerdomain's
+ * wakeup latency constraint list.
+ *
+ * Called with the pwrdm wakeup latency spinlock held.
+ *
+ * Returns 0 upon success, -EINVAL if the constraint is not existing.
+ */
+static inline int _pwrdm_update_wakeuplat_list(
+			struct powerdomain *pwrdm,
+			void *cookie,
+			long min_latency,
+			struct pwrdm_wkup_constraints_entry *user,
+			struct pwrdm_wkup_constraints_entry *new_user,
+			int *free_new_user,
+			int *free_node)
+{
+	struct pwrdm_wkup_constraints_entry *tmp_user;
+
+	/* Check if there already is a constraint for cookie */
+	plist_for_each_entry(tmp_user, &pwrdm->wkup_lat_plist_head, node) {
+		if (tmp_user->cookie == cookie) {
+			user = tmp_user;
+			break;
+		}
+	}
+
+	if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
+		/* If nothing to update, job done */
+		if (user && (user->node.prio == min_latency))
+			return 0;
+
+		if (!user) {
+			/* Add new entry to the list */
+			user = new_user;
+			user->cookie = cookie;
+			*free_new_user = 0;
+		} else {
+			/* Update existing entry */
+			plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
+		}
+
+		plist_node_init(&user->node, min_latency);
+		plist_add(&user->node, &pwrdm->wkup_lat_plist_head);
+	} else {
+		if (user) {
+			/* Remove the constraint from the list */
+			plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
+			*free_node = 1;
+		} else {
+			/* Constraint not existing or list empty, do nothing */
+			return -EINVAL;
+		}
+
+	}
+
+	return 0;
+}
+
+/**
+ * _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.
+ *
+ * Returns 0 in case of success, -EINVAL in case of invalid parameters,
+ * or the return value from omap_set_pwrdm_state.
+ */
+static int _pwrdm_wakeuplat_update_pwrst(struct powerdomain *pwrdm,
+					long min_latency)
+{
+	int ret = 0, new_state;
+
+	if (!pwrdm) {
+		WARN(1, "powerdomain: %s: invalid parameter(s)", __func__);
+		return -EINVAL;
+	}
+
+	/*
+	 * Find the next supported power state with
+	 * wakeup latency < minimum constraint
+	 */
+	for (new_state = 0x0; new_state < PWRDM_MAX_PWRSTS; new_state++) {
+		if (min_latency == PM_QOS_DEV_LAT_DEFAULT_VALUE)
+			break;
+		if ((pwrdm->wakeup_lat[new_state] != UNSUP_STATE) &&
+		    (pwrdm->wakeup_lat[new_state] <= min_latency))
+			break;
+	}
+
+	switch (new_state) {
+	case PWRDM_FUNC_PWRST_OFF:
+		new_state = PWRDM_POWER_OFF;
+		break;
+	case PWRDM_FUNC_PWRST_OSWR:
+		pwrdm_set_logic_retst(pwrdm, PWRDM_POWER_OFF);
+		new_state = PWRDM_POWER_RET;
+		break;
+	case PWRDM_FUNC_PWRST_CSWR:
+		pwrdm_set_logic_retst(pwrdm, PWRDM_POWER_RET);
+		new_state = PWRDM_POWER_RET;
+		break;
+	case PWRDM_FUNC_PWRST_INACTIVE:
+		new_state = PWRDM_POWER_INACTIVE;
+		break;
+	case PWRDM_FUNC_PWRST_ON:
+		new_state = PWRDM_POWER_ON;
+		break;
+	default:
+		pr_warn("powerdomain: requested latency constraint not "
+			"supported %s set to ON state\n", pwrdm->name);
+		new_state = PWRDM_POWER_ON;
+		break;
+	}
+
+	pwrdm->wkup_lat_next_state = new_state;
+	if (pwrdm_read_next_pwrst(pwrdm) != new_state)
+		ret = omap_set_pwrdm_state(pwrdm, new_state);
+
+	pr_debug("powerdomain: %s pwrst: curr=%d, prev=%d next=%d "
+		 "min_latency=%ld, set_state=%d\n", pwrdm->name,
+		 pwrdm_read_pwrst(pwrdm), pwrdm_read_prev_pwrst(pwrdm),
+		 pwrdm_read_next_pwrst(pwrdm), min_latency, new_state);
+
+	return ret;
+}
+
 /* Public functions */
 
 /**
@@ -931,6 +1091,83 @@ int pwrdm_post_transition(void)
 }
 
 /**
+ * pwrdm_set_wkup_lat_constraint - Set/update/remove 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. The value of PM_QOS_DEV_LAT_DEFAULT_VALUE removes
+ *  the constraint.
+ *
+ * Tracks the constraints by @cookie.
+ * Constraint set/update: Adds a new entry to powerdomain's wake-up latency
+ * constraint list.
+ * If the constraint identifier already exists in the list, the old value is
+ * overwritten.
+ * Constraint removal: Removes the identifier's entry from powerdomain's
+ * wakeup latency constraint list.
+ *
+ * Applies the aggregated constraint value for the given pwrdm by calling
+ * _pwrdm_wakeuplat_update_pwrst.
+ *
+ * Returns 0 upon success, -ENOMEM in case of memory shortage, -EINVAL in
+ * case of invalid parameters, or the return value from
+ * _pwrdm_wakeuplat_update_pwrst.
+ *
+ * The caller must check the validity of the parameters.
+ */
+int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
+				  long min_latency)
+{
+	struct pwrdm_wkup_constraints_entry *user = NULL, *new_user = NULL;
+	int ret = 0, free_new_user = 0, free_node = 0;
+	long value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
+	unsigned long flags;
+
+	pr_debug("powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n",
+		 __func__, pwrdm->name, cookie, min_latency);
+
+	if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
+		new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
+				   GFP_KERNEL);
+		if (!new_user) {
+			pr_err("%s: FATAL ERROR: kzalloc failed\n", __func__);
+			return -ENOMEM;
+		}
+		free_new_user = 1;
+	}
+
+	spin_lock_irqsave(&pwrdm->wkup_lat_plist_lock, flags);
+
+	/* Manage the constraints list */
+	ret = _pwrdm_update_wakeuplat_list(pwrdm, cookie, min_latency,
+					   user, new_user,
+					   &free_new_user, &free_node);
+
+	/* Find the aggregated constraint value from the list */
+	if (!ret)
+		if (!plist_head_empty(&pwrdm->wkup_lat_plist_head))
+			value = plist_first(&pwrdm->wkup_lat_plist_head)->prio;
+
+	spin_unlock_irqrestore(&pwrdm->wkup_lat_plist_lock, flags);
+
+	if (free_node)
+		kfree(user);
+
+	if (free_new_user)
+		kfree(new_user);
+
+	/* Apply the constraint to the pwrdm */
+	if (!ret) {
+		pr_debug("powerdomain: %s: pwrdm %s, value=%ld\n",
+			 __func__, pwrdm->name, value);
+		ret = _pwrdm_wakeuplat_update_pwrst(pwrdm, value);
+	}
+
+	return ret;
+}
+
+/**
  * pwrdm_get_context_loss_count - get powerdomain's context loss count
  * @pwrdm: struct powerdomain * to wait for
  *
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index d23d979..41630ec 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -19,7 +19,9 @@
 
 #include <linux/types.h>
 #include <linux/list.h>
-
+#include <linux/plist.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
 #include <linux/atomic.h>
 
 #include <plat/cpu.h>
@@ -43,6 +45,16 @@
 #define PWRSTS_RET_ON		(PWRSTS_RET | PWRSTS_ON)
 #define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | PWRSTS_ON)
 
+/* Powerdomain functional power states */
+#define PWRDM_FUNC_PWRST_OFF		0x0
+#define PWRDM_FUNC_PWRST_OSWR		0x1
+#define PWRDM_FUNC_PWRST_CSWR		0x2
+#define PWRDM_FUNC_PWRST_INACTIVE	0x3
+#define PWRDM_FUNC_PWRST_ON		0x4
+
+#define PWRDM_MAX_FUNC_PWRSTS	5
+
+#define UNSUP_STATE		-1
 
 /* Powerdomain flags */
 #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware save-and-restore support */
@@ -93,7 +105,13 @@ struct powerdomain;
  * @state_counter:
  * @timer:
  * @state_timer:
- *
+ * @wakeup_lat: wakeup latencies (in us) for possible powerdomain power states
+ * Note about the wakeup latencies ordering: the values must be sorted
+ *  in decremental order
+ * @wkup_lat_plist_head: pwrdm wake-up latency constraints list
+ * @wkup_lat_plist_lock: spinlock that protects the constraints lists
+ *  domains states
+ * @wkup_lat_next_state: next pwrdm state, calculated from the constraints list
  * @prcm_partition possible values are defined in mach-omap2/prcm44xx.h.
  */
 struct powerdomain {
@@ -118,6 +136,16 @@ struct powerdomain {
 	s64 timer;
 	s64 state_timer[PWRDM_MAX_PWRSTS];
 #endif
+	const s32 wakeup_lat[PWRDM_MAX_FUNC_PWRSTS];
+	struct plist_head wkup_lat_plist_head;
+	spinlock_t wkup_lat_plist_lock;
+	int wkup_lat_next_state;
+};
+
+/* Linked list for the wake-up latency constraints */
+struct pwrdm_wkup_constraints_entry {
+	void			*cookie;
+	struct plist_node	node;
 };
 
 /**
@@ -207,6 +235,9 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
 int pwrdm_pre_transition(void);
 int pwrdm_post_transition(void);
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
+
+int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
+				  long min_latency);
 u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
 
-- 
1.7.4.1

[PATCH 2/6] OMAP2+: omap_hwmod: manage the wake-up latency constraints

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Hwmod is queried from the OMAP_PM layer to manage the power domains
wake-up latency constraints. Hwmod retrieves the correct power domain
and if it exists it calls the corresponding power domain function.

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-pihet@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   27 +++++++++++++++++++++++++-
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 +
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 84cc0bd..023f3e7 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -143,6 +143,7 @@
 #include "powerdomain.h"
 #include <plat/clock.h>
 #include <plat/omap_hwmod.h>
+#include <plat/omap_device.h>
 #include <plat/prcm.h>
 
 #include "cm2xxx_3xxx.h"
@@ -2619,10 +2620,34 @@ ohsps_unlock:
 }
 
 /**
+ * omap_hwmod_set_wkup_constraint- set/release a wake-up latency constraint
+ *
+ * @oh: struct omap_hwmod* to which the target device belongs to.
+ * @cookie: identifier of the constraints list for @oh.
+ * @min_latency: the minimum allowed wake-up latency for @oh.
+ *
+ * Returns 0 upon success, -EINVAL in case of invalid parameters, or
+ * the return value from pwrdm_set_wkup_lat_constraint.
+ */
+int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh,
+				       void *cookie, long min_latency)
+{
+	struct powerdomain *pwrdm = omap_hwmod_get_pwrdm(oh);
+
+	if (!pwrdm) {
+		pr_err("%s: Error: could not find powerdomain "
+		       "for %s\n", __func__, oh->name);
+		return -EINVAL;
+	}
+
+	return pwrdm_set_wkup_lat_constraint(pwrdm, cookie, min_latency);
+}
+
+/**
  * omap_hwmod_get_context_loss_count - get lost context count
  * @oh: struct omap_hwmod *
  *
- * Query the powerdomain of of @oh to get the context loss
+ * Query the powerdomain of @oh to get the context loss
  * count for this device.
  *
  * Returns the context loss count of the powerdomain assocated with @oh
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 0e329ca..75e0e7a 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -603,6 +603,8 @@ int omap_hwmod_for_each_by_class(const char *classname,
 				 void *user);
 
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
+int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh, void *cookie,
+				       long min_latency);
 u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
-- 
1.7.4.1

[PATCH 3/6] OMAP: PM: register to the per-device PM QoS framework

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Implement the devices wake-up latency constraints using the global
device PM QoS notification handler which applies the constraints to the
underlying layer by calling the corresponding function at hwmod level.

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-pihet@ti.com>
---
 arch/arm/mach-omap2/pm.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 3feb359..58b4b76 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -11,13 +11,16 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/notifier.h>
 #include <linux/io.h>
 #include <linux/err.h>
 #include <linux/opp.h>
+#include <linux/pm_qos.h>
 
 #include <plat/omap-pm.h>
 #include <plat/omap_device.h>
 #include <plat/common.h>
+#include <plat/omap_hwmod.h>
 
 #include "voltage.h"
 #include "powerdomain.h"
@@ -242,11 +245,71 @@ static void __init omap4_init_voltages(void)
 	omap2_set_init_voltage("iva", "dpll_iva_m5x2_ck", iva_dev);
 }
 
+/* Interface to the per-device PM QoS framework */
+static int omap2_dev_pm_qos_handler(struct notifier_block *nb,
+				    unsigned long new_value,
+				    void *req)
+{
+	struct omap_device *od;
+	struct omap_hwmod *oh;
+	struct platform_device *pdev;
+	struct dev_pm_qos_request *dev_pm_qos_req = req;
+
+	pr_debug("OMAP PM CONSTRAINTS: req@0x%p, new_value=%lu\n",
+		 req, new_value);
+
+	/* Look for the platform device for the constraint target device */
+	pdev = to_platform_device(dev_pm_qos_req->dev);
+
+	/* Try to catch non platform devices */
+	if (pdev->name == NULL) {
+		pr_err("%s: Error: platform device for device %s not valid\n",
+		       __func__, dev_name(dev_pm_qos_req->dev));
+		return -EINVAL;
+	}
+
+	/* Find the associated omap_device for dev */
+	od = container_of(pdev, struct omap_device, pdev);
+	if (od->hwmods_cnt != 1) {
+		pr_err("%s: Error: No unique hwmod for device %s\n",
+		       __func__, dev_name(dev_pm_qos_req->dev));
+		return -EINVAL;
+	}
+
+	/* Find the primary omap_hwmod for dev */
+	oh = od->hwmods[0];
+
+	pr_debug("OMAP PM CONSTRAINTS: req@0x%p, dev=0x%p, new_value=%lu\n",
+		 req, dev_pm_qos_req->dev, new_value);
+
+	/* Apply the constraint */
+	return omap_hwmod_set_wkup_lat_constraint(oh, dev_pm_qos_req,
+						  new_value);
+}
+
+static struct notifier_block omap2_dev_pm_qos_notifier = {
+	.notifier_call	= omap2_dev_pm_qos_handler,
+};
+
+static int __init omap2_dev_pm_qos_init(void)
+{
+	int ret;
+
+	ret = dev_pm_qos_add_global_notifier(&omap2_dev_pm_qos_notifier);
+	if (ret)
+		WARN(1, KERN_ERR "Cannot add global notifier for dev PM QoS\n");
+
+	return ret;
+}
+
 static int __init omap2_common_pm_init(void)
 {
 	omap2_init_processor_devices();
 	omap_pm_if_init();
 
+	/* Register to the per-device PM QoS framework */
+	omap2_dev_pm_qos_init();
+
 	return 0;
 }
 postcore_initcall(omap2_common_pm_init);
-- 
1.7.4.1

[PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

The MPU latency figures for cpuidle include the MPU itself and also
the peripherals needed for the MPU to execute instructions (e.g.
main memory, caches, IRQ controller, MMU etc). On OMAP3 those
peripherals belong to the MPU and CORE power domains and so the
cpuidle C-states are a combination of MPU and CORE states.

This patch implements the relation between the cpuidle and per-
device PM QoS frameworks in the OMAP3 specific idle callbacks.

The chosen C-state shall satisfy the following conditions:
 . the 'valid' field is enabled,
 . it satisfies the enable_off_mode flag,
 . the next state for MPU and CORE power domains is not lower than the
   next state calculated by the per-device PM QoS.

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

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   49 ++++++++++++++++++++++---------------
 arch/arm/mach-omap2/pm.h          |   17 +++++++++++-
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 4bf6e6e..1b8e0da 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -37,7 +37,7 @@
 #ifdef CONFIG_CPU_IDLE
 
 /*
- * The latencies/thresholds for various C states have
+ * The MPU latencies/thresholds for various C states have
  * to be configured from the respective board files.
  * These are some default values (which might not provide
  * the best power savings) used on boards which do not
@@ -72,14 +72,14 @@ struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
 struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
 
 static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
+			       struct clockdomain *clkdm)
 {
 	clkdm_allow_idle(clkdm);
 	return 0;
 }
 
 static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
+			      struct clockdomain *clkdm)
 {
 	clkdm_deny_idle(clkdm);
 	return 0;
@@ -92,9 +92,13 @@ 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.
+ *
+ * Note: this function does not check for any pending activity or dependency
+ * between power domains states, so the caller shall check the parameters
+ * correctness.
  */
 static int omap3_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_state *state)
+			    struct cpuidle_state *state)
 {
 	struct omap3_idle_statedata *cx = cpuidle_get_statedata(state);
 	struct timespec ts_preidle, ts_postidle, ts_idle;
@@ -146,8 +150,11 @@ return_sleep_time:
  * Else, this function searches for a lower c-state which is still
  * valid.
  *
- * A state is valid if the 'valid' field is enabled and
- * if it satisfies the enable_off_mode condition.
+ * A state is valid if:
+ * . the 'valid' field is enabled,
+ * . it satisfies the enable_off_mode flag,
+ * . the next state for MPU and CORE power domains is not lower than the
+ *   state programmed by the per-device PM QoS.
  */
 static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 					      struct cpuidle_state *curr)
@@ -156,6 +163,8 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
 	u32 mpu_deepest_state = PWRDM_POWER_RET;
 	u32 core_deepest_state = PWRDM_POWER_RET;
+	u32 mpu_pm_qos_next_state = mpu_pd->wkup_lat_next_state;
+	u32 core_pm_qos_next_state = core_pd->wkup_lat_next_state;
 
 	if (enable_off_mode) {
 		mpu_deepest_state = PWRDM_POWER_OFF;
@@ -171,7 +180,9 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 	/* Check if current state is valid */
 	if ((cx->valid) &&
 	    (cx->mpu_state >= mpu_deepest_state) &&
-	    (cx->core_state >= core_deepest_state)) {
+	    (cx->core_state >= core_deepest_state) &&
+	    (cx->mpu_state >= mpu_pm_qos_next_state) &&
+	    (cx->core_state >= core_pm_qos_next_state)) {
 		return curr;
 	} else {
 		int idx = OMAP3_NUM_STATES - 1;
@@ -196,7 +207,9 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 			cx = cpuidle_get_statedata(&dev->states[idx]);
 			if ((cx->valid) &&
 			    (cx->mpu_state >= mpu_deepest_state) &&
-			    (cx->core_state >= core_deepest_state)) {
+			    (cx->core_state >= core_deepest_state) &&
+			    (cx->mpu_state >= mpu_pm_qos_next_state) &&
+			    (cx->core_state >= core_pm_qos_next_state)) {
 				next = &dev->states[idx];
 				break;
 			}
@@ -215,8 +228,12 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
  * @dev: cpuidle device
  * @state: The target state to be programmed
  *
- * This function checks for any pending activity and then programs
- * the device to the specified or a safer state.
+ * Called from the CPUidle framework to program the device to the
+ * specified target state selected by the governor.
+ *
+ * This function checks for any pending activity or dependency between
+ * power domains states and then programs the device to the specified
+ * or a safer state.
  */
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 			       struct cpuidle_state *state)
@@ -241,19 +258,13 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 		goto select_state;
 	}
 
-	/*
-	 * FIXME: we currently manage device-specific idle states
-	 *        for PER and CORE in combination with CPU-specific
-	 *        idle states.  This is wrong, and device-specific
-	 *        idle management needs to be separated out into
-	 *        its own code.
-	 */
+	new_state = next_valid_state(dev, state);
 
 	/*
 	 * Prevent PER off if CORE is not in retention or off as this
 	 * would disable PER wakeups completely.
 	 */
-	cx = cpuidle_get_statedata(state);
+	cx = cpuidle_get_statedata(new_state);
 	core_next_state = cx->core_state;
 	per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
 	if ((per_next_state == PWRDM_POWER_OFF) &&
@@ -264,8 +275,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	if (per_next_state != per_saved_state)
 		pwrdm_set_next_pwrst(per_pd, per_next_state);
 
-	new_state = next_valid_state(dev, state);
-
 select_state:
 	dev->last_state = new_state;
 	ret = omap3_enter_idle(dev, new_state);
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 4e166ad..aca3b6c 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -43,9 +43,22 @@ static inline int omap4_opp_init(void)
  * omap3_pm_init_cpuidle
  */
 struct cpuidle_params {
-	u32 exit_latency;	/* exit_latency = sleep + wake-up latencies */
+	/*
+	 * exit_latency = sleep + wake-up latencies of the MPU,
+	 * which include the MPU itself and the peripherals needed
+	 * for the MPU to execute instructions (e.g. main memory,
+	 * caches, IRQ controller, MMU etc). Some of those peripherals
+	 * can belong to other power domains than the MPU subsystem and so
+	 * the corresponding latencies must be included in this figure.
+	 */
+	u32 exit_latency;
+	/*
+	 * target_residency: required amount of time in the C state
+	 * to break even on energy cost
+	 */
 	u32 target_residency;
-	u8 valid;		/* validates the C-state */
+	/* validates the C-state on the given board */
+	u8 valid;
 };
 
 #if defined(CONFIG_PM) && defined(CONFIG_CPU_IDLE)
-- 
1.7.4.1

[PATCH 5/6] OMAP3: update cpuidle latency and threshold figures

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Update the data from the measurements performed at HW and SW levels.

Cf. http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement
for a detailed explanation on where are the numbers coming from.

ToDo:
- Measure the wake-up latencies for all power domains for OMAP3
- Correct some numbers when sys_clkreq and sys_offmode are supported

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   52 +++++++++++++++++++++++-------------
 1 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1b8e0da..336a4ec 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -37,27 +37,41 @@
 #ifdef CONFIG_CPU_IDLE
 
 /*
- * The MPU latencies/thresholds for various C states have
- * to be configured from the respective board files.
- * These are some default values (which might not provide
- * the best power savings) used on boards which do not
- * pass these details from the board file.
+ * The MPU latency and threshold values for the C-states are the worst case
+ * values from the HW and SW, as described in details at
+ * http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement#cpuidle_results
+ *
+ * Measurements conditions and remarks:
+ *  . the measurements have been performed at OPP50
+ *  . the sys_offmode signal is not supported and so not used for the
+ *    measurements. Instead the latency and threshold values for C9 are
+ *    corrected with the value for Triton 2, which is 11.5ms
+ *  . the sys_clkreq signal is not used and so a correction is needed - TBD
+ *  . the sys_clkoff signal is supported, this value need to be corrected with
+ *    the correct value of SYSCLK on/off timings (1ms for sysclk on, 2.5ms
+ *    for sysclk off)
+ *  . in order to force the cpuidle algorithm to chose the power efficient
+ *    C-states (C1, C3, C5, C7) in preference, the other C-states have a
+ *    threshold value equal to the next power efficient C-state
+ * 
+ * The latency and threshold values can be overriden by data from the board
+ * files, using omap3_pm_init_cpuidle.
  */
 static struct cpuidle_params cpuidle_params_table[] = {
-	/* C1 */
-	{2 + 2, 5, 1},
-	/* C2 */
-	{10 + 10, 30, 1},
-	/* C3 */
-	{50 + 50, 300, 1},
-	/* C4 */
-	{1500 + 1800, 4000, 1},
-	/* C5 */
-	{2500 + 7500, 12000, 1},
-	/* C6 */
-	{3000 + 8500, 15000, 1},
-	/* C7 */
-	{10000 + 30000, 300000, 1},
+	/* C1 . MPU WFI + Core active */
+	{73 + 78, 152, 1},
+	/* C2 . MPU WFI + Core inactive */
+	{165 + 88, 345, 1},
+	/* C3 . MPU CSWR + Core inactive */
+	{163 + 182, 345, 1},
+	/* C4 . MPU OFF + Core inactive */
+	{2852 + 605, 150000, 1},
+	/* C5 . MPU RET + Core RET */
+	{800 + 366, 2120, 1},
+	/* C6 . MPU OFF + Core RET */
+	{4080 + 801, 215000, 1},
+	/* C7 . MPU OFF + Core OFF */
+	{4300 + 13000, 215000, 1},
 };
 #define OMAP3_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
 
-- 
1.7.4.1

[PATCH 6/6] OMAP3: powerdomain data: add wake-up latency figures

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Figures are added to the power domains structs for RET and OFF modes.

Note: the latency figures for MPU, PER, CORE, NEON have been obtained
from actual measurements.
The latency figures for the other power domains are preliminary and
shall be added.

Cf. http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement
for a detailed explanation on where are the numbers coming from.

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

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/powerdomains3xxx_data.c |   83 +++++++++++++++++++++++++++
 1 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c b/arch/arm/mach-omap2/powerdomains3xxx_data.c
index 469a920..4b2b044 100644
--- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
@@ -31,6 +31,19 @@
 
 /*
  * Powerdomains
+ *
+ * The wakeup_lat values are derived from HW and SW measurements on
+ * the actual target. For more details cf.
+ * http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement#Results_for_individual_power_domains
+ *
+ * Note: the latency figures for MPU, PER, CORE, NEON have been obtained
+ * from actual measurements.
+ * The latency figures for the other power domains are preliminary and
+ * shall be added.
+ *
+ * Note: only the SW restore timing values are taken into account.
+ * The HW impact of the sys_clkreq and sys_offmode signals is not taken
+ * into account - TDB
  */
 
 static struct powerdomain iva2_pwrdm = {
@@ -52,6 +65,13 @@ static struct powerdomain iva2_pwrdm = {
 		[2] = PWRSTS_OFF_ON,
 		[3] = PWRSTS_ON,
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 1100,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 350,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain mpu_3xxx_pwrdm = {
@@ -68,6 +88,13 @@ static struct powerdomain mpu_3xxx_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRSTS_OFF_ON,
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 1830,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 121,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 /*
@@ -98,6 +125,13 @@ static struct powerdomain core_3xxx_pre_es3_1_pwrdm = {
 		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
 		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 3082,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 153,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain core_3xxx_es3_1_pwrdm = {
@@ -121,6 +155,13 @@ static struct powerdomain core_3xxx_es3_1_pwrdm = {
 		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
 		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 3082,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 153,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain dss_pwrdm = {
@@ -136,6 +177,13 @@ static struct powerdomain dss_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRSTS_ON,  /* MEMONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 70,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 20,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 /*
@@ -157,6 +205,13 @@ static struct powerdomain sgx_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRSTS_ON,  /* MEMONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 1000,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain cam_pwrdm = {
@@ -172,6 +227,13 @@ static struct powerdomain cam_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRSTS_ON,  /* MEMONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 850,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 35,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain per_pwrdm = {
@@ -187,6 +249,13 @@ static struct powerdomain per_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRSTS_ON,  /* MEMONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 671,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 31,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain emu_pwrdm = {
@@ -201,6 +270,13 @@ static struct powerdomain neon_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.pwrsts		  = PWRSTS_OFF_RET_ON,
 	.pwrsts_logic_ret = PWRSTS_RET,
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 0,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 0,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain usbhost_pwrdm = {
@@ -223,6 +299,13 @@ static struct powerdomain usbhost_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRSTS_ON,  /* MEMONSTATE */
 	},
+	.wakeup_lat = {
+		[PWRDM_FUNC_PWRST_OFF] = 800,
+		[PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_CSWR] = 150,
+		[PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+		[PWRDM_FUNC_PWRST_ON] = 0,
+	},
 };
 
 static struct powerdomain dpll1_pwrdm = {
-- 
1.7.4.1

Re: [PATCH 1/6] OMAP2+: powerdomain: control power domains next state

[Kevin Hilman]

jean.pihet@newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> When a PM QoS device latency constraint is requested or removed the
> PM QoS layer notifies the underlying layer with the updated aggregated
> constraint value. The constraint is stored in the powerdomain constraints
> list and then applied to the corresponding power domain.
> The power domains get the next power state programmed directly in the
> registers via pwrdm_wakeuplat_update_pwrst.
>
> 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-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/powerdomain.c |  237 +++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-omap2/powerdomain.h |   35 +++++-
>  2 files changed, 270 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 9af0847..351766d 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 <trace/events/power.h>
>  
>  #include "cm2xxx_3xxx.h"
> @@ -104,6 +106,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-od wake-up constraints data */

This comment needs an update (they are not per-od, but per pwrdm), or
could probably be removed, since it doesn't add any value over the code.

> +	spin_lock_init(&pwrdm->wkup_lat_plist_lock);
> +	plist_head_init(&pwrdm->wkup_lat_plist_head);
> +	pwrdm->wkup_lat_next_state = PWRDM_POWER_OFF;
> +
> +	/* Initialize the pwrdm state */
>  	pwrdm_wait_transition(pwrdm);
>  	pwrdm->state = pwrdm_read_pwrst(pwrdm);
>  	pwrdm->state_counter[pwrdm->state] = 1;
> @@ -191,6 +199,158 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
>  	return 0;
>  }
>  
> +/**
> + * _pwrdm_wakeuplat_update_list - Set/update/remove a powerdomain wakeup
> + *  latency constraint from the pwrdm's constraint list
> + * @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. The value of PM_QOS_DEV_LAT_DEFAULT_VALUE removes
> + *  the constraint.
> + * @user: pointer to the current list entry
> + * @new_user: allocated list entry, used for insertion of new constraints
> + *  in the list
> + * @free_new_user: set to non-zero if the newly allocated list entry
> + *  is unused and needs to be freed
> + * @free_node: set to non-zero if the current list entry is not in use
> + *  anymore and needs to be freed
> + *
> + * Tracks the constraints by @cookie.
> + * Constraint set/update: Adds a new entry to powerdomain's wake-up latency
> + * constraint list.
> + * If the constraint identifier already exists in the list, the old value is
> + * overwritten.
> + * Constraint removal: Removes the identifier's entry from powerdomain's
> + * wakeup latency constraint list.
> + *
> + * Called with the pwrdm wakeup latency spinlock held.
> + *
> + * Returns 0 upon success, -EINVAL if the constraint is not existing.
> + */
> +static inline int _pwrdm_update_wakeuplat_list(
> +			struct powerdomain *pwrdm,
> +			void *cookie,
> +			long min_latency,
> +			struct pwrdm_wkup_constraints_entry *user,
> +			struct pwrdm_wkup_constraints_entry *new_user,
> +			int *free_new_user,
> +			int *free_node)
> +{
> +	struct pwrdm_wkup_constraints_entry *tmp_user;
> +
> +	/* Check if there already is a constraint for cookie */
> +	plist_for_each_entry(tmp_user, &pwrdm->wkup_lat_plist_head, node) {
> +		if (tmp_user->cookie == cookie) {
> +			user = tmp_user;
> +			break;
> +		}
> +	}
> +
> +	if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
> +		/* If nothing to update, job done */
> +		if (user && (user->node.prio == min_latency))
> +			return 0;
> +
> +		if (!user) {
> +			/* Add new entry to the list */
> +			user = new_user;
> +			user->cookie = cookie;
> +			*free_new_user = 0;
> +		} else {
> +			/* Update existing entry */
> +			plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
> +		}
> +
> +		plist_node_init(&user->node, min_latency);
> +		plist_add(&user->node, &pwrdm->wkup_lat_plist_head);
> +	} else {
> +		if (user) {
> +			/* Remove the constraint from the list */
> +			plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
> +			*free_node = 1;
> +		} else {
> +			/* Constraint not existing or list empty, do nothing */
> +			return -EINVAL;
> +		}
> +
> +	}
> +
> +	return 0;
> +}

[...]

>  /**
> + * pwrdm_set_wkup_lat_constraint - Set/update/remove 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. The value of PM_QOS_DEV_LAT_DEFAULT_VALUE removes
> + *  the constraint.
> + *
> + * Tracks the constraints by @cookie.
> + * Constraint set/update: Adds a new entry to powerdomain's wake-up latency
> + * constraint list.
> + * If the constraint identifier already exists in the list, the old value is
> + * overwritten.
> + * Constraint removal: Removes the identifier's entry from powerdomain's
> + * wakeup latency constraint list.
> + *
> + * Applies the aggregated constraint value for the given pwrdm by calling
> + * _pwrdm_wakeuplat_update_pwrst.
> + *
> + * Returns 0 upon success, -ENOMEM in case of memory shortage, -EINVAL in
> + * case of invalid parameters, or the return value from
> + * _pwrdm_wakeuplat_update_pwrst.
> + *
> + * The caller must check the validity of the parameters.
> + */
> +int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
> +				  long min_latency)
> +{
> +	struct pwrdm_wkup_constraints_entry *user = NULL, *new_user = NULL;
> +	int ret = 0, free_new_user = 0, free_node = 0;
> +	long value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
> +	unsigned long flags;
> +
> +	pr_debug("powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n",
> +		 __func__, pwrdm->name, cookie, min_latency);
> +
> +	if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
> +		new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
> +				   GFP_KERNEL);
> +		if (!new_user) {
> +			pr_err("%s: FATAL ERROR: kzalloc failed\n", __func__);
> +			return -ENOMEM;
> +		}
> +		free_new_user = 1;
> +	}
> +
> +	spin_lock_irqsave(&pwrdm->wkup_lat_plist_lock, flags);
> +
> +	/* Manage the constraints list */
> +	ret = _pwrdm_update_wakeuplat_list(pwrdm, cookie, min_latency,
> +					   user, new_user,
> +					   &free_new_user, &free_node);
> +
> +	/* Find the aggregated constraint value from the list */
> +	if (!ret)
> +		if (!plist_head_empty(&pwrdm->wkup_lat_plist_head))
> +			value = plist_first(&pwrdm->wkup_lat_plist_head)->prio;
> +
> +	spin_unlock_irqrestore(&pwrdm->wkup_lat_plist_lock, flags);
> +
> +	if (free_node)
> +		kfree(user);
> +
> +	if (free_new_user)
> +		kfree(new_user);

The alloc/free of these buffers is not terribly obvious when reading.  I
think the code/changelog needs some comments describing the logic
behind how/when these nodes are allocated and freed.

> +	/* Apply the constraint to the pwrdm */
> +	if (!ret) {
> +		pr_debug("powerdomain: %s: pwrdm %s, value=%ld\n",
> +			 __func__, pwrdm->name, value);
> +		ret = _pwrdm_wakeuplat_update_pwrst(pwrdm, value);
> +	}
> +
> +	return ret;
> +}
> +

Kevin

Re: [PATCH 3/6] OMAP: PM: register to the per-device PM QoS framework

[Kevin Hilman]

jean.pihet@newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> Implement the devices wake-up latency constraints using the global
> device PM QoS notification handler which applies the constraints to the
> underlying layer by calling the corresponding function at hwmod level.
>
> 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-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/pm.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 63 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index 3feb359..58b4b76 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -11,13 +11,16 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/notifier.h>
>  #include <linux/io.h>
>  #include <linux/err.h>
>  #include <linux/opp.h>
> +#include <linux/pm_qos.h>
>  
>  #include <plat/omap-pm.h>
>  #include <plat/omap_device.h>
>  #include <plat/common.h>
> +#include <plat/omap_hwmod.h>
>  
>  #include "voltage.h"
>  #include "powerdomain.h"
> @@ -242,11 +245,71 @@ static void __init omap4_init_voltages(void)
>  	omap2_set_init_voltage("iva", "dpll_iva_m5x2_ck", iva_dev);
>  }
>  
> +/* Interface to the per-device PM QoS framework */
> +static int omap2_dev_pm_qos_handler(struct notifier_block *nb,
> +				    unsigned long new_value,
> +				    void *req)
> +{
> +	struct omap_device *od;
> +	struct omap_hwmod *oh;
> +	struct platform_device *pdev;
> +	struct dev_pm_qos_request *dev_pm_qos_req = req;
> +
> +	pr_debug("OMAP PM CONSTRAINTS: req@0x%p, new_value=%lu\n",

s/CONSTRAINTS/constraints/
another one below.

> +		 req, new_value);
> +
> +	/* Look for the platform device for the constraint target device */
> +	pdev = to_platform_device(dev_pm_qos_req->dev);
> +
> +	/* Try to catch non platform devices */

why?

> +	if (pdev->name == NULL) {
> +		pr_err("%s: Error: platform device for device %s not valid\n",
> +		       __func__, dev_name(dev_pm_qos_req->dev));
> +		return -EINVAL;
> +	}
> +
> +	/* Find the associated omap_device for dev */
> +	od = container_of(pdev, struct omap_device, pdev);

What about devices that are valid platform_devices, but not omap_devices?

> +	if (od->hwmods_cnt != 1) {
> +		pr_err("%s: Error: No unique hwmod for device %s\n",
> +		       __func__, dev_name(dev_pm_qos_req->dev));
> +		return -EINVAL;
> +	}
> +
> +	/* Find the primary omap_hwmod for dev */
> +	oh = od->hwmods[0];
> +
> +	pr_debug("OMAP PM CONSTRAINTS: req@0x%p, dev=0x%p, new_value=%lu\n",
> +		 req, dev_pm_qos_req->dev, new_value);
> +
> +	/* Apply the constraint */
> +	return omap_hwmod_set_wkup_lat_constraint(oh, dev_pm_qos_req,
> +						  new_value);
> +}
> +
> +static struct notifier_block omap2_dev_pm_qos_notifier = {
> +	.notifier_call	= omap2_dev_pm_qos_handler,
> +};
> +
> +static int __init omap2_dev_pm_qos_init(void)
> +{
> +	int ret;
> +
> +	ret = dev_pm_qos_add_global_notifier(&omap2_dev_pm_qos_notifier);
> +	if (ret)
> +		WARN(1, KERN_ERR "Cannot add global notifier for dev PM QoS\n");

minor: could use WARN_ON()

> +	return ret;
> +}
> +
>  static int __init omap2_common_pm_init(void)
>  {
>  	omap2_init_processor_devices();
>  	omap_pm_if_init();
>  
> +	/* Register to the per-device PM QoS framework */
> +	omap2_dev_pm_qos_init();
> +
>  	return 0;
>  }
>  postcore_initcall(omap2_common_pm_init);

Kevin

Re: [PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[Kevin Hilman]

jean.pihet@newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> The MPU latency figures for cpuidle include the MPU itself and also
> the peripherals needed for the MPU to execute instructions (e.g.
> main memory, caches, IRQ controller, MMU etc). On OMAP3 those
> peripherals belong to the MPU and CORE power domains and so the
> cpuidle C-states are a combination of MPU and CORE states.
>
> This patch implements the relation between the cpuidle and per-
> device PM QoS frameworks in the OMAP3 specific idle callbacks.
>
> The chosen C-state shall satisfy the following conditions:
>  . the 'valid' field is enabled,
>  . it satisfies the enable_off_mode flag,

Not directly related to this patch, but is there any reason to keep the
'enable_off_mode' flag after this series?

Kevin

Re: [PATCH 1/6] OMAP2+: powerdomain: control power domains next state

[Jean Pihet]

Kevin,

On Thu, Nov 17, 2011 at 10:16 PM, Kevin Hilman <khilman@ti.com> wrote:
> jean.pihet@newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> When a PM QoS device latency constraint is requested or removed the
>> PM QoS layer notifies the underlying layer with the updated aggregated
>> constraint value. The constraint is stored in the powerdomain constraints
>> list and then applied to the corresponding power domain.
>> The power domains get the next power state programmed directly in the
>> registers via pwrdm_wakeuplat_update_pwrst.
>>
>> 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-pihet@ti.com>
>> ---
...

>> @@ -104,6 +106,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-od wake-up constraints data */
>
> This comment needs an update (they are not per-od, but per pwrdm), or
> could probably be removed, since it doesn't add any value over the code.
Ok to remove it

>
>> +     spin_lock_init(&pwrdm->wkup_lat_plist_lock);
>> +     plist_head_init(&pwrdm->wkup_lat_plist_head);
>> +     pwrdm->wkup_lat_next_state = PWRDM_POWER_OFF;
>> +
>> +     /* Initialize the pwrdm state */
>>       pwrdm_wait_transition(pwrdm);
>>       pwrdm->state = pwrdm_read_pwrst(pwrdm);
>>       pwrdm->state_counter[pwrdm->state] = 1;
...

>> +int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
>> +                               long min_latency)
>> +{
>> +     struct pwrdm_wkup_constraints_entry *user = NULL, *new_user = NULL;
>> +     int ret = 0, free_new_user = 0, free_node = 0;
>> +     long value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
>> +     unsigned long flags;
>> +
>> +     pr_debug("powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n",
>> +              __func__, pwrdm->name, cookie, min_latency);
>> +
>> +     if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
>> +             new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
>> +                                GFP_KERNEL);
>> +             if (!new_user) {
>> +                     pr_err("%s: FATAL ERROR: kzalloc failed\n", __func__);
>> +                     return -ENOMEM;
>> +             }
>> +             free_new_user = 1;
>> +     }
>> +
>> +     spin_lock_irqsave(&pwrdm->wkup_lat_plist_lock, flags);
>> +
>> +     /* Manage the constraints list */
>> +     ret = _pwrdm_update_wakeuplat_list(pwrdm, cookie, min_latency,
>> +                                        user, new_user,
>> +                                        &free_new_user, &free_node);
>> +
>> +     /* Find the aggregated constraint value from the list */
>> +     if (!ret)
>> +             if (!plist_head_empty(&pwrdm->wkup_lat_plist_head))
>> +                     value = plist_first(&pwrdm->wkup_lat_plist_head)->prio;
>> +
>> +     spin_unlock_irqrestore(&pwrdm->wkup_lat_plist_lock, flags);
>> +
>> +     if (free_node)
>> +             kfree(user);
>> +
>> +     if (free_new_user)
>> +             kfree(new_user);
>
> The alloc/free of these buffers is not terribly obvious when reading.  I
Agreed.

> think the code/changelog needs some comments describing the logic
> behind how/when these nodes are allocated and freed.
Ok I will add it.

...

>
> Kevin
>

Thanks,
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

Re: [PATCH 3/6] OMAP: PM: register to the per-device PM QoS framework

[Jean Pihet]

On Thu, Nov 17, 2011 at 10:24 PM, Kevin Hilman <khilman@ti.com> wrote:
> jean.pihet@newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Implement the devices wake-up latency constraints using the global
>> device PM QoS notification handler which applies the constraints to the
>> underlying layer by calling the corresponding function at hwmod level.
>>
>> 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-pihet@ti.com>
...

>> +/* Interface to the per-device PM QoS framework */
>> +static int omap2_dev_pm_qos_handler(struct notifier_block *nb,
>> +                                 unsigned long new_value,
>> +                                 void *req)
>> +{
>> +     struct omap_device *od;
>> +     struct omap_hwmod *oh;
>> +     struct platform_device *pdev;
>> +     struct dev_pm_qos_request *dev_pm_qos_req = req;
>> +
>> +     pr_debug("OMAP PM CONSTRAINTS: req@0x%p, new_value=%lu\n",
>
> s/CONSTRAINTS/constraints/
> another one below.
Ok

>
>> +              req, new_value);
>> +
>> +     /* Look for the platform device for the constraint target device */
>> +     pdev = to_platform_device(dev_pm_qos_req->dev);
>> +
>> +     /* Try to catch non platform devices */
>
> why?
The constraints targets are the power domains, which you find by
walking through the chain of structs dev, pdev, omap_device, hwmod and
finally pwrdm.

>
>> +     if (pdev->name == NULL) {
>> +             pr_err("%s: Error: platform device for device %s not valid\n",
>> +                    __func__, dev_name(dev_pm_qos_req->dev));
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* Find the associated omap_device for dev */
>> +     od = container_of(pdev, struct omap_device, pdev);
>
> What about devices that are valid platform_devices, but not omap_devices?
Do you mean that od should be tested for NULL value?

>
>> +     if (od->hwmods_cnt != 1) {
>> +             pr_err("%s: Error: No unique hwmod for device %s\n",
>> +                    __func__, dev_name(dev_pm_qos_req->dev));
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* Find the primary omap_hwmod for dev */
>> +     oh = od->hwmods[0];
>> +
>> +     pr_debug("OMAP PM CONSTRAINTS: req@0x%p, dev=0x%p, new_value=%lu\n",
>> +              req, dev_pm_qos_req->dev, new_value);
>> +
>> +     /* Apply the constraint */
>> +     return omap_hwmod_set_wkup_lat_constraint(oh, dev_pm_qos_req,
>> +                                               new_value);
>> +}
>> +
>> +static struct notifier_block omap2_dev_pm_qos_notifier = {
>> +     .notifier_call  = omap2_dev_pm_qos_handler,
>> +};
>> +
>> +static int __init omap2_dev_pm_qos_init(void)
>> +{
>> +     int ret;
>> +
>> +     ret = dev_pm_qos_add_global_notifier(&omap2_dev_pm_qos_notifier);
>> +     if (ret)
>> +             WARN(1, KERN_ERR "Cannot add global notifier for dev PM QoS\n");
>
> minor: could use WARN_ON()
Ok

>
>> +     return ret;
>> +}
>> +
>>  static int __init omap2_common_pm_init(void)
>>  {
>>       omap2_init_processor_devices();
>>       omap_pm_if_init();
>>
>> +     /* Register to the per-device PM QoS framework */
>> +     omap2_dev_pm_qos_init();
>> +
>>       return 0;
>>  }
>>  postcore_initcall(omap2_common_pm_init);
>
> Kevin
>

Thanks,
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

Re: [PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[Jean Pihet]

On Thu, Nov 17, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
> jean.pihet@newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> The MPU latency figures for cpuidle include the MPU itself and also
>> the peripherals needed for the MPU to execute instructions (e.g.
>> main memory, caches, IRQ controller, MMU etc). On OMAP3 those
>> peripherals belong to the MPU and CORE power domains and so the
>> cpuidle C-states are a combination of MPU and CORE states.
>>
>> This patch implements the relation between the cpuidle and per-
>> device PM QoS frameworks in the OMAP3 specific idle callbacks.
>>
>> The chosen C-state shall satisfy the following conditions:
>>  . the 'valid' field is enabled,
>>  . it satisfies the enable_off_mode flag,
>
> Not directly related to this patch, but is there any reason to keep the
> 'enable_off_mode' flag after this series?
enable_off_mode could be removed completely after this series unless
there is a need to prevent OFF mode for debug reasons.

>
> Kevin
>

Thanks,
Jean

Re: [PATCH 3/6] OMAP: PM: register to the per-device PM QoS framework

[Kevin Hilman]

Jean Pihet <jean.pihet@newoldbits.com> writes:

> On Thu, Nov 17, 2011 at 10:24 PM, Kevin Hilman <khilman@ti.com> wrote:
>> jean.pihet@newoldbits.com writes:
>>
>>> From: Jean Pihet <j-pihet@ti.com>
>>>
>>> Implement the devices wake-up latency constraints using the global
>>> device PM QoS notification handler which applies the constraints to the
>>> underlying layer by calling the corresponding function at hwmod level.
>>>
>>> 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-pihet@ti.com>
> ...
>
>>> +/* Interface to the per-device PM QoS framework */
>>> +static int omap2_dev_pm_qos_handler(struct notifier_block *nb,
>>> +                                 unsigned long new_value,
>>> +                                 void *req)
>>> +{
>>> +     struct omap_device *od;
>>> +     struct omap_hwmod *oh;
>>> +     struct platform_device *pdev;
>>> +     struct dev_pm_qos_request *dev_pm_qos_req = req;
>>> +
>>> +     pr_debug("OMAP PM CONSTRAINTS: req@0x%p, new_value=%lu\n",
>>
>> s/CONSTRAINTS/constraints/
>> another one below.
> Ok
>
>>
>>> +              req, new_value);
>>> +
>>> +     /* Look for the platform device for the constraint target device */
>>> +     pdev = to_platform_device(dev_pm_qos_req->dev);
>>> +
>>> +     /* Try to catch non platform devices */
>>
>> why?
> The constraints targets are the power domains, which you find by
> walking through the chain of structs dev, pdev, omap_device, hwmod and
> finally pwrdm.

OK

>>
>>> +     if (pdev->name == NULL) {
>>> +             pr_err("%s: Error: platform device for device %s not valid\n",
>>> +                    __func__, dev_name(dev_pm_qos_req->dev));
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     /* Find the associated omap_device for dev */
>>> +     od = container_of(pdev, struct omap_device, pdev);
>>
>> What about devices that are valid platform_devices, but not omap_devices?
> Do you mean that od should be tested for NULL value?

First, it should be using the to_omap_device() helper function from
omap_device.h.

Until v3.2, it was based on container_of() so checking for NULL will not
help.  However, as of v3.2, because I decoupled platform_device and
omap_device, checking for a NULL return from to_omap_device() will be a
good enough check.

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

Re: [linux-pm] [PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[Kevin Hilman]

Jean Pihet <jean.pihet@newoldbits.com> writes:

> On Thu, Nov 17, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
>> jean.pihet@newoldbits.com writes:
>>
>>> From: Jean Pihet <j-pihet@ti.com>
>>>
>>> The MPU latency figures for cpuidle include the MPU itself and also
>>> the peripherals needed for the MPU to execute instructions (e.g.
>>> main memory, caches, IRQ controller, MMU etc). On OMAP3 those
>>> peripherals belong to the MPU and CORE power domains and so the
>>> cpuidle C-states are a combination of MPU and CORE states.
>>>
>>> This patch implements the relation between the cpuidle and per-
>>> device PM QoS frameworks in the OMAP3 specific idle callbacks.
>>>
>>> The chosen C-state shall satisfy the following conditions:
>>>  . the 'valid' field is enabled,
>>>  . it satisfies the enable_off_mode flag,
>>
>> Not directly related to this patch, but is there any reason to keep the
>> 'enable_off_mode' flag after this series?
> enable_off_mode could be removed completely after this series unless
> there is a need to prevent OFF mode for debug reasons.

Great.

For debug reasons, we can just as easily set constraints to prevent off
mode, so I would like to see it disappear.

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

[PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

The MPU latency figures for cpuidle include the MPU itself and also
the peripherals needed for the MPU to execute instructions (e.g.
main memory, caches, IRQ controller, MMU etc). On OMAP3 those
peripherals belong to the MPU and CORE power domains and so the
cpuidle C-states are a combination of MPU and CORE states.

This patch implements the relation between the cpuidle and per-
device PM QoS frameworks in the OMAP3 specific idle callbacks.

The chosen C-state shall satisfy the following conditions:
 . the 'valid' field is enabled,
 . it satisfies the enable_off_mode flag,
 . the next state for MPU and CORE power domains is not lower than the
   next state calculated by the per-device PM QoS.

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

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   57 ++++++++++++++++++++----------------
 arch/arm/mach-omap2/pm.h          |   17 ++++++++++-
 2 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1f71ebb..c67835f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -40,7 +40,7 @@
 #ifdef CONFIG_CPU_IDLE
 
 /*
- * The latencies/thresholds for various C states have
+ * The MPU latencies/thresholds for various C states have
  * to be configured from the respective board files.
  * These are some default values (which might not provide
  * the best power savings) used on boards which do not
@@ -75,14 +75,14 @@ struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
 struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
 
 static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
+			       struct clockdomain *clkdm)
 {
 	clkdm_allow_idle(clkdm);
 	return 0;
 }
 
 static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
+			      struct clockdomain *clkdm)
 {
 	clkdm_deny_idle(clkdm);
 	return 0;
@@ -96,10 +96,13 @@ 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.
+ *
+ * Note: this function does not check for any pending activity or dependency
+ * between power domains states, so the caller shall check the parameters
+ * correctness.
  */
 static int omap3_enter_idle(struct cpuidle_device *dev,
-				struct cpuidle_driver *drv,
-				int index)
+			    struct cpuidle_driver *drv, int index)
 {
 	struct omap3_idle_statedata *cx =
 			cpuidle_get_statedata(&dev->states_usage[index]);
@@ -174,18 +177,23 @@ return_sleep_time:
  * to the caller. Else, this function searches for a lower c-state which is
  * still valid (as defined in omap3_power_states[]) and returns its index.
  *
- * A state is valid if the 'valid' field is enabled and
- * if it satisfies the enable_off_mode condition.
+ * A state is valid if:
+ * . the 'valid' field is enabled,
+ * . it satisfies the enable_off_mode flag,
+ * . the next state for MPU and CORE power domains is not lower than the
+ *   state programmed by the per-device PM QoS.
  */
 static int next_valid_state(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-				int index)
+			    struct cpuidle_driver *drv, int index)
 {
 	struct cpuidle_state_usage *curr_usage = &dev->states_usage[index];
 	struct cpuidle_state *curr = &drv->states[index];
 	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr_usage);
 	u32 mpu_deepest_state = PWRDM_POWER_RET;
 	u32 core_deepest_state = PWRDM_POWER_RET;
+	u32 mpu_pm_qos_next_state = mpu_pd->wkup_lat_next_state;
+	u32 core_pm_qos_next_state = core_pd->wkup_lat_next_state;
+
 	int next_index = -1;
 
 	if (enable_off_mode) {
@@ -202,7 +210,9 @@ static int next_valid_state(struct cpuidle_device *dev,
 	/* Check if current state is valid */
 	if ((cx->valid) &&
 	    (cx->mpu_state >= mpu_deepest_state) &&
-	    (cx->core_state >= core_deepest_state)) {
+	    (cx->core_state >= core_deepest_state) &&
+	    (cx->mpu_state >= mpu_pm_qos_next_state) &&
+	    (cx->core_state >= core_pm_qos_next_state)) {
 		return index;
 	} else {
 		int idx = OMAP3_NUM_STATES - 1;
@@ -227,7 +237,9 @@ static int next_valid_state(struct cpuidle_device *dev,
 			cx = cpuidle_get_statedata(&dev->states_usage[idx]);
 			if ((cx->valid) &&
 			    (cx->mpu_state >= mpu_deepest_state) &&
-			    (cx->core_state >= core_deepest_state)) {
+			    (cx->core_state >= core_deepest_state) &&
+			    (cx->mpu_state >= mpu_pm_qos_next_state) &&
+			    (cx->core_state >= core_pm_qos_next_state)) {
 				next_index = idx;
 				break;
 			}
@@ -248,12 +260,15 @@ static int next_valid_state(struct cpuidle_device *dev,
  * @drv: cpuidle driver
  * @index: array index of target state to be programmed
  *
- * This function checks for any pending activity and then programs
- * the device to the specified or a safer state.
+ * Called from the CPUidle framework to program the device to the
+ * specified target state selected by the governor.
+ *
+ * This function checks for any pending activity or dependency between
+ * power domains states and then programs the device to the specified
+ * or a safer state.
  */
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
-				struct cpuidle_driver *drv,
-			       int index)
+			       struct cpuidle_driver *drv, int index)
 {
 	int new_state_idx;
 	u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state;
@@ -275,19 +290,13 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 		goto select_state;
 	}
 
-	/*
-	 * FIXME: we currently manage device-specific idle states
-	 *        for PER and CORE in combination with CPU-specific
-	 *        idle states.  This is wrong, and device-specific
-	 *        idle management needs to be separated out into
-	 *        its own code.
-	 */
+	new_state_idx = next_valid_state(dev, drv, index);
 
 	/*
 	 * Prevent PER off if CORE is not in retention or off as this
 	 * would disable PER wakeups completely.
 	 */
-	cx = cpuidle_get_statedata(&dev->states_usage[index]);
+	cx = cpuidle_get_statedata(&dev->states_usage[new_state_idx]);
 	core_next_state = cx->core_state;
 	per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
 	if ((per_next_state == PWRDM_POWER_OFF) &&
@@ -298,8 +307,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	if (per_next_state != per_saved_state)
 		pwrdm_set_next_pwrst(per_pd, per_next_state);
 
-	new_state_idx = next_valid_state(dev, drv, index);
-
 select_state:
 	ret = omap3_enter_idle(dev, drv, new_state_idx);
 
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index b737b11..ab32465 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -44,9 +44,22 @@ static inline int omap4_opp_init(void)
  * omap3_pm_init_cpuidle
  */
 struct cpuidle_params {
-	u32 exit_latency;	/* exit_latency = sleep + wake-up latencies */
+	/*
+	 * exit_latency = sleep + wake-up latencies of the MPU,
+	 * which include the MPU itself and the peripherals needed
+	 * for the MPU to execute instructions (e.g. main memory,
+	 * caches, IRQ controller, MMU etc). Some of those peripherals
+	 * can belong to other power domains than the MPU subsystem and so
+	 * the corresponding latencies must be included in this figure.
+	 */
+	u32 exit_latency;
+	/*
+	 * target_residency: required amount of time in the C state
+	 * to break even on energy cost
+	 */
 	u32 target_residency;
-	u8 valid;		/* validates the C-state */
+	/* validates the C-state on the given board */
+	u8 valid;
 };
 
 #if defined(CONFIG_PM) && defined(CONFIG_CPU_IDLE)
-- 
1.7.5.4

Re: [linux-pm] [PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[Jean Pihet]

Hi Kevin,

On Wed, Nov 23, 2011 at 8:43 PM, Kevin Hilman <khilman@ti.com> wrote:
> Jean Pihet <jean.pihet@newoldbits.com> writes:
>
>> On Thu, Nov 17, 2011 at 10:29 PM, Kevin Hilman <khilman@ti.com> wrote:
>>> jean.pihet@newoldbits.com writes:
>>>
>>>> From: Jean Pihet <j-pihet@ti.com>
>>>>
>>>> The MPU latency figures for cpuidle include the MPU itself and also
>>>> the peripherals needed for the MPU to execute instructions (e.g.
>>>> main memory, caches, IRQ controller, MMU etc). On OMAP3 those
>>>> peripherals belong to the MPU and CORE power domains and so the
>>>> cpuidle C-states are a combination of MPU and CORE states.
>>>>
>>>> This patch implements the relation between the cpuidle and per-
>>>> device PM QoS frameworks in the OMAP3 specific idle callbacks.
>>>>
>>>> The chosen C-state shall satisfy the following conditions:
>>>>  . the 'valid' field is enabled,
>>>>  . it satisfies the enable_off_mode flag,
>>>
>>> Not directly related to this patch, but is there any reason to keep the
>>> 'enable_off_mode' flag after this series?
>> enable_off_mode could be removed completely after this series unless
>> there is a need to prevent OFF mode for debug reasons.
>
> Great.
>
> For debug reasons, we can just as easily set constraints to prevent off
> mode, so I would like to see it disappear.
I have a WIP patch that removes the enable_off_mode flag and will post
it as soon as this series is in the upstream pipe.

Thanks,
Jean

>
> 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

Re: [PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[Kevin Hilman]

jean.pihet@newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> The MPU latency figures for cpuidle include the MPU itself and also
> the peripherals needed for the MPU to execute instructions (e.g.
> main memory, caches, IRQ controller, MMU etc). On OMAP3 those
> peripherals belong to the MPU and CORE power domains and so the
> cpuidle C-states are a combination of MPU and CORE states.
>
> This patch implements the relation between the cpuidle and per-
> device PM QoS frameworks in the OMAP3 specific idle callbacks.
>
> The chosen C-state shall satisfy the following conditions:
>  . the 'valid' field is enabled,
>  . it satisfies the enable_off_mode flag,
>  . the next state for MPU and CORE power domains is not lower than the
>    next state calculated by the per-device PM QoS.
>
> Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
> on MPU, CORE and PER.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

nit: this patch mixes functional changes and non-functional changes
(whitespace cleanups, alignments etc.)  For ease of review, it's best to
do non-functional cleanups as a separate patch.

Kevin

[PATCH 4/6] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

The MPU latency figures for cpuidle include the MPU itself and also
the peripherals needed for the MPU to execute instructions (e.g.
main memory, caches, IRQ controller, MMU etc). On OMAP3 those
peripherals belong to the MPU and CORE power domains and so the
cpuidle C-states are a combination of MPU and CORE states.

This patch implements the relation between the cpuidle and per-
device PM QoS frameworks in the OMAP3 specific idle callbacks.

The chosen C-state shall satisfy the following conditions:
 . the 'valid' field is enabled,
 . it satisfies the enable_off_mode flag,
 . the next state for MPU and CORE power domains is not lower than the
   next state calculated by the per-device PM QoS.

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

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   57 ++++++++++++++++++++----------------
 arch/arm/mach-omap2/pm.h          |   17 ++++++++++-
 2 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1f71ebb..c67835f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -40,7 +40,7 @@
 #ifdef CONFIG_CPU_IDLE
 
 /*
- * The latencies/thresholds for various C states have
+ * The MPU latencies/thresholds for various C states have
  * to be configured from the respective board files.
  * These are some default values (which might not provide
  * the best power savings) used on boards which do not
@@ -75,14 +75,14 @@ struct omap3_idle_statedata omap3_idle_data[OMAP3_NUM_STATES];
 struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
 
 static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
+			       struct clockdomain *clkdm)
 {
 	clkdm_allow_idle(clkdm);
 	return 0;
 }
 
 static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
+			      struct clockdomain *clkdm)
 {
 	clkdm_deny_idle(clkdm);
 	return 0;
@@ -96,10 +96,13 @@ 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.
+ *
+ * Note: this function does not check for any pending activity or dependency
+ * between power domains states, so the caller shall check the parameters
+ * correctness.
  */
 static int omap3_enter_idle(struct cpuidle_device *dev,
-				struct cpuidle_driver *drv,
-				int index)
+			    struct cpuidle_driver *drv, int index)
 {
 	struct omap3_idle_statedata *cx =
 			cpuidle_get_statedata(&dev->states_usage[index]);
@@ -174,18 +177,23 @@ return_sleep_time:
  * to the caller. Else, this function searches for a lower c-state which is
  * still valid (as defined in omap3_power_states[]) and returns its index.
  *
- * A state is valid if the 'valid' field is enabled and
- * if it satisfies the enable_off_mode condition.
+ * A state is valid if:
+ * . the 'valid' field is enabled,
+ * . it satisfies the enable_off_mode flag,
+ * . the next state for MPU and CORE power domains is not lower than the
+ *   state programmed by the per-device PM QoS.
  */
 static int next_valid_state(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-				int index)
+			    struct cpuidle_driver *drv, int index)
 {
 	struct cpuidle_state_usage *curr_usage = &dev->states_usage[index];
 	struct cpuidle_state *curr = &drv->states[index];
 	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr_usage);
 	u32 mpu_deepest_state = PWRDM_POWER_RET;
 	u32 core_deepest_state = PWRDM_POWER_RET;
+	u32 mpu_pm_qos_next_state = mpu_pd->wkup_lat_next_state;
+	u32 core_pm_qos_next_state = core_pd->wkup_lat_next_state;
+
 	int next_index = -1;
 
 	if (enable_off_mode) {
@@ -202,7 +210,9 @@ static int next_valid_state(struct cpuidle_device *dev,
 	/* Check if current state is valid */
 	if ((cx->valid) &&
 	    (cx->mpu_state >= mpu_deepest_state) &&
-	    (cx->core_state >= core_deepest_state)) {
+	    (cx->core_state >= core_deepest_state) &&
+	    (cx->mpu_state >= mpu_pm_qos_next_state) &&
+	    (cx->core_state >= core_pm_qos_next_state)) {
 		return index;
 	} else {
 		int idx = OMAP3_NUM_STATES - 1;
@@ -227,7 +237,9 @@ static int next_valid_state(struct cpuidle_device *dev,
 			cx = cpuidle_get_statedata(&dev->states_usage[idx]);
 			if ((cx->valid) &&
 			    (cx->mpu_state >= mpu_deepest_state) &&
-			    (cx->core_state >= core_deepest_state)) {
+			    (cx->core_state >= core_deepest_state) &&
+			    (cx->mpu_state >= mpu_pm_qos_next_state) &&
+			    (cx->core_state >= core_pm_qos_next_state)) {
 				next_index = idx;
 				break;
 			}
@@ -248,12 +260,15 @@ static int next_valid_state(struct cpuidle_device *dev,
  * @drv: cpuidle driver
  * @index: array index of target state to be programmed
  *
- * This function checks for any pending activity and then programs
- * the device to the specified or a safer state.
+ * Called from the CPUidle framework to program the device to the
+ * specified target state selected by the governor.
+ *
+ * This function checks for any pending activity or dependency between
+ * power domains states and then programs the device to the specified
+ * or a safer state.
  */
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
-				struct cpuidle_driver *drv,
-			       int index)
+			       struct cpuidle_driver *drv, int index)
 {
 	int new_state_idx;
 	u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state;
@@ -275,19 +290,13 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 		goto select_state;
 	}
 
-	/*
-	 * FIXME: we currently manage device-specific idle states
-	 *        for PER and CORE in combination with CPU-specific
-	 *        idle states.  This is wrong, and device-specific
-	 *        idle management needs to be separated out into
-	 *        its own code.
-	 */
+	new_state_idx = next_valid_state(dev, drv, index);
 
 	/*
 	 * Prevent PER off if CORE is not in retention or off as this
 	 * would disable PER wakeups completely.
 	 */
-	cx = cpuidle_get_statedata(&dev->states_usage[index]);
+	cx = cpuidle_get_statedata(&dev->states_usage[new_state_idx]);
 	core_next_state = cx->core_state;
 	per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
 	if ((per_next_state == PWRDM_POWER_OFF) &&
@@ -298,8 +307,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	if (per_next_state != per_saved_state)
 		pwrdm_set_next_pwrst(per_pd, per_next_state);
 
-	new_state_idx = next_valid_state(dev, drv, index);
-
 select_state:
 	ret = omap3_enter_idle(dev, drv, new_state_idx);
 
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index b737b11..ab32465 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -44,9 +44,22 @@ static inline int omap4_opp_init(void)
  * omap3_pm_init_cpuidle
  */
 struct cpuidle_params {
-	u32 exit_latency;	/* exit_latency = sleep + wake-up latencies */
+	/*
+	 * exit_latency = sleep + wake-up latencies of the MPU,
+	 * which include the MPU itself and the peripherals needed
+	 * for the MPU to execute instructions (e.g. main memory,
+	 * caches, IRQ controller, MMU etc). Some of those peripherals
+	 * can belong to other power domains than the MPU subsystem and so
+	 * the corresponding latencies must be included in this figure.
+	 */
+	u32 exit_latency;
+	/*
+	 * target_residency: required amount of time in the C state
+	 * to break even on energy cost
+	 */
 	u32 target_residency;
-	u8 valid;		/* validates the C-state */
+	/* validates the C-state on the given board */
+	u8 valid;
 };
 
 #if defined(CONFIG_PM) && defined(CONFIG_CPU_IDLE)
-- 
1.7.5.4