linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/6]  Multiple intermediate states for genpd
@ 2015-10-20 13:26 ahaslam
  2015-10-20 13:26 ` [PATCH v10 1/6] PM / Domains: prepare for multiple states ahaslam
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Axel Haslam

From: Axel Haslam <ahaslam+renesas@baylibre.com>

Some architectures may have intermediate power levels between on and off.

This patch set adds the ability to declare multiple states for a given
generic power domain, the idea is that the deepest state will be entered
which does not violate any of the device or sub-domain latency constraints.

Changes since v9
*rebased on linux-next

Changes since v8
* rebased to linux-pm next

Changes since v7:
* rebase to 4.3-rc5

* add genpd_init_simple (Lina's suggestion) for platforms that don't have
multiple states and don't declare initial latencies. A default OFF
state with initial 0 latencies will be used in this case.

* Append Mark's patch to add "states" and "timings" to the genpd
debugfs

Changes since v6:
* change int to unsigned int were appropriate.

* spelling mistakes, and fix commit message for removal of latencies.

Changes since v5:
* rebase to 4.1-rc1

* Pass state array as an init argument on pm_genpd_init

* declare a default OFF state with no latencies, that will be used if a
null state argument is given.

* set the deepest state when using sync_poweroff.

* create and use name allocation function in the debug area
instead of inline.

Changes since v4:
* move to power_on/off callbacks out of the state array Platforms can
check the state_idx to know what state the power on/off corresponds to.

* convert states to pointer,
Dynamically allocate the states array to save memory on platforms
with several power domains.

* rename target_state to state_idx and remove init_state.

Changes since v3:
* remove old power on/off function at the end of the
series so that compilation will not break in between.

Changes since v2:
* remove state argument and macros from save/restore callbacks.

* added init_state for platforms to pass the initial state when the genpd
is initially off.

* convert current genpd users for the structure changes.

Changes since v1:
* split the changes so that the actual logic that selects the target state
is a separate patch.

* move the cached logic out of the state function and add
it back to default_power_down_ok.

* rename default_power_down_ok_state to power_down_ok_for_state

Axel Haslam (5):
  PM / Domains: prepare for multiple states
  PM / Domains: core changes for multiple states
  PM / Domains: make governor select deepest state
  ARM: imx6: pm: declare pm domain latency on power_state struct.
  PM / Domains: remove old power on/off latencies.

Marc Titinger (1):
  PM / Domains: add debugfs 'states' and 'timings' seq files

 arch/arm/mach-exynos/pm_domains.c     |   2 +-
 arch/arm/mach-imx/gpc.c               |  18 ++-
 arch/arm/mach-s3c64xx/pm.c            |   4 +-
 arch/arm/mach-shmobile/pm-rmobile.c   |   2 +-
 arch/arm/mach-ux500/pm_domains.c      |   2 +-
 arch/arm/mach-zx/zx296702-pm-domain.c |   2 +-
 drivers/base/power/domain.c           | 258 ++++++++++++++++++++++++++++++++--
 drivers/base/power/domain_governor.c  |  70 +++++----
 drivers/clk/qcom/gdsc.c               |   2 +-
 drivers/clk/shmobile/clk-mstp.c       |   2 +-
 drivers/soc/dove/pmu.c                |   2 +-
 drivers/soc/mediatek/mtk-scpsys.c     |   2 +-
 drivers/soc/rockchip/pm_domains.c     |   2 +-
 include/linux/pm_domain.h             |  33 ++++-
 14 files changed, 346 insertions(+), 55 deletions(-)

-- 
2.4.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v10 1/6] PM / Domains: prepare for multiple states
  2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
@ 2015-10-20 13:26 ` ahaslam
  2015-10-27 14:33   ` Ulf Hansson
  2015-10-20 13:26 ` [PATCH v10 2/6] PM / Domains: core changes " ahaslam
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Axel Haslam

From: Axel Haslam <ahaslam+renesas@baylibre.com>

prepare generic power domain init function parameters to accept a pointer
to the states structure that describes the possible states that a power
domain can enter.

Also, as most platforms are not initializing states or latencies, add
pm_genpd_init_simple that allows platfroms to use a default single OFF
state with initial latencies set to 0.

There is no functional change, as support for multiple domains will be
added in subsequent patches.

Suggested-by: Lina Iyer <lina.iyer@linaro.org>
Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
---
 arch/arm/mach-exynos/pm_domains.c     |  2 +-
 arch/arm/mach-imx/gpc.c               |  2 +-
 arch/arm/mach-s3c64xx/pm.c            |  4 ++--
 arch/arm/mach-shmobile/pm-rmobile.c   |  2 +-
 arch/arm/mach-ux500/pm_domains.c      |  2 +-
 arch/arm/mach-zx/zx296702-pm-domain.c |  2 +-
 drivers/base/power/domain.c           | 19 ++++++++++++++++++-
 drivers/clk/qcom/gdsc.c               |  2 +-
 drivers/clk/shmobile/clk-mstp.c       |  2 +-
 drivers/soc/dove/pmu.c                |  2 +-
 drivers/soc/mediatek/mtk-scpsys.c     |  2 +-
 drivers/soc/rockchip/pm_domains.c     |  2 +-
 include/linux/pm_domain.h             | 27 ++++++++++++++++++++++++++-
 13 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index 7c21760..dc36723 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -187,7 +187,7 @@ static __init int exynos4_pm_init_power_domain(void)
 no_clk:
 		on = __raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN;
 
-		pm_genpd_init(&pd->pd, NULL, !on);
+		pm_genpd_init_simple(&pd->pd, NULL, !on);
 		of_genpd_add_provider_simple(np, &pd->pd);
 	}
 
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 10bf715..aefe92d 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -422,7 +422,7 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
 	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
 		return 0;
 
-	pm_genpd_init(&imx6q_pu_domain.base, NULL, false);
+	pm_genpd_init_simple(&imx6q_pu_domain.base, NULL, false);
 	return of_genpd_add_provider_onecell(dev->of_node,
 					     &imx_gpc_onecell_data);
 
diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c
index 75b14e7..d68a45c 100644
--- a/arch/arm/mach-s3c64xx/pm.c
+++ b/arch/arm/mach-s3c64xx/pm.c
@@ -316,11 +316,11 @@ int __init s3c64xx_pm_init(void)
 	s3c_pm_init();
 
 	for (i = 0; i < ARRAY_SIZE(s3c64xx_always_on_pm_domains); i++)
-		pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
+		pm_genpd_init_simple(&s3c64xx_always_on_pm_domains[i]->pd,
 			      &pm_domain_always_on_gov, false);
 
 	for (i = 0; i < ARRAY_SIZE(s3c64xx_pm_domains); i++)
-		pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
+		pm_genpd_init_simple(&s3c64xx_pm_domains[i]->pd, NULL, false);
 
 #ifdef CONFIG_S3C_DEV_FB
 	if (dev_get_platdata(&s3c_device_fb.dev))
diff --git a/arch/arm/mach-shmobile/pm-rmobile.c b/arch/arm/mach-shmobile/pm-rmobile.c
index 46d0a1d..906a2eb 100644
--- a/arch/arm/mach-shmobile/pm-rmobile.c
+++ b/arch/arm/mach-shmobile/pm-rmobile.c
@@ -131,7 +131,7 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd)
 	struct dev_power_governor *gov = rmobile_pd->gov;
 
 	genpd->flags = GENPD_FLAG_PM_CLK;
-	pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
+	pm_genpd_init_simple(genpd, gov ? : &simple_qos_governor, false);
 	genpd->dev_ops.active_wakeup	= rmobile_pd_active_wakeup;
 	genpd->power_off		= rmobile_pd_power_down;
 	genpd->power_on			= rmobile_pd_power_up;
diff --git a/arch/arm/mach-ux500/pm_domains.c b/arch/arm/mach-ux500/pm_domains.c
index 4d71c90..208e784 100644
--- a/arch/arm/mach-ux500/pm_domains.c
+++ b/arch/arm/mach-ux500/pm_domains.c
@@ -72,7 +72,7 @@ int __init ux500_pm_domains_init(void)
 	genpd_data->num_domains = ARRAY_SIZE(ux500_pm_domains);
 
 	for (i = 0; i < ARRAY_SIZE(ux500_pm_domains); ++i)
-		pm_genpd_init(ux500_pm_domains[i], NULL, false);
+		pm_genpd_init_simple(ux500_pm_domains[i], NULL, false);
 
 	of_genpd_add_provider_onecell(np, genpd_data);
 	return 0;
diff --git a/arch/arm/mach-zx/zx296702-pm-domain.c b/arch/arm/mach-zx/zx296702-pm-domain.c
index e08574d..0ec8f0c 100644
--- a/arch/arm/mach-zx/zx296702-pm-domain.c
+++ b/arch/arm/mach-zx/zx296702-pm-domain.c
@@ -175,7 +175,7 @@ static int zx296702_pd_probe(struct platform_device *pdev)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(zx296702_pm_domains); ++i)
-		pm_genpd_init(zx296702_pm_domains[i], NULL, false);
+		pm_genpd_init_simple(zx296702_pm_domains[i], NULL, false);
 
 	of_genpd_add_provider_onecell(pdev->dev.of_node, genpd_data);
 	return 0;
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index b0b5c43..e0b5229 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1447,10 +1447,14 @@ static int pm_genpd_default_restore_state(struct device *dev)
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.
  * @gov: PM domain governor to associate with the domain (may be NULL).
+ * @states: Array of possible low power states when powering off.
+ * @state_count: Number of low power states.
  * @is_off: Initial value of the domain's power_is_off field.
  */
 void pm_genpd_init(struct generic_pm_domain *genpd,
-		   struct dev_power_governor *gov, bool is_off)
+		   struct dev_power_governor *gov,
+		   const struct genpd_power_state *states,
+		   unsigned int state_count, bool is_off)
 {
 	if (IS_ERR_OR_NULL(genpd))
 		return;
@@ -1502,6 +1506,19 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 }
 EXPORT_SYMBOL_GPL(pm_genpd_init);
 
+/**
+ * pm_genpd_init_simple - Initialize a generic I/O PM domain object.
+ * @genpd: PM domain object to initialize.
+ * @gov: PM domain governor to associate with the domain (may be NULL).
+ * @is_off: Initial value of the domain's power_is_off field.
+ */
+void pm_genpd_init_simple(struct generic_pm_domain *genpd,
+		   struct dev_power_governor *gov, bool is_off)
+{
+	pm_genpd_init(genpd, gov, NULL, 0, is_off);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_init_simple);
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index da9fad8..b709bf3 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -196,7 +196,7 @@ static int gdsc_init(struct gdsc *sc)
 
 	sc->pd.power_off = gdsc_disable;
 	sc->pd.power_on = gdsc_enable;
-	pm_genpd_init(&sc->pd, NULL, !on);
+	pm_genpd_init_simple(&sc->pd, NULL, !on);
 
 	return 0;
 }
diff --git a/drivers/clk/shmobile/clk-mstp.c b/drivers/clk/shmobile/clk-mstp.c
index 3b09716..1f18474 100644
--- a/drivers/clk/shmobile/clk-mstp.c
+++ b/drivers/clk/shmobile/clk-mstp.c
@@ -320,7 +320,7 @@ void __init cpg_mstp_add_clk_domain(struct device_node *np)
 	pd->name = np->name;
 
 	pd->flags = GENPD_FLAG_PM_CLK;
-	pm_genpd_init(pd, &simple_qos_governor, false);
+	pm_genpd_init_simple(pd, &simple_qos_governor, false);
 	pd->attach_dev = cpg_mstp_attach_dev;
 	pd->detach_dev = cpg_mstp_detach_dev;
 
diff --git a/drivers/soc/dove/pmu.c b/drivers/soc/dove/pmu.c
index abd0879..42adc16 100644
--- a/drivers/soc/dove/pmu.c
+++ b/drivers/soc/dove/pmu.c
@@ -215,7 +215,7 @@ static void __pmu_domain_register(struct pmu_domain *domain,
 	domain->base.power_off = pmu_domain_power_off;
 	domain->base.power_on = pmu_domain_power_on;
 
-	pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask));
+	pm_genpd_init_simple(&domain->base, NULL, !(val & domain->pwr_mask));
 
 	if (np)
 		of_genpd_add_provider_simple(np, &domain->base);
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index 164a7d8..adf364f 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -443,7 +443,7 @@ static int __init scpsys_probe(struct platform_device *pdev)
 		 */
 		genpd->power_on(genpd);
 
-		pm_genpd_init(genpd, NULL, false);
+		pm_genpd_init_simple(genpd, NULL, false);
 	}
 
 	/*
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 534c589..617e219 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -299,7 +299,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	pd->genpd.attach_dev = rockchip_pd_attach_dev;
 	pd->genpd.detach_dev = rockchip_pd_detach_dev;
 	pd->genpd.flags = GENPD_FLAG_PM_CLK;
-	pm_genpd_init(&pd->genpd, NULL, false);
+	pm_genpd_init_simple(&pd->genpd, NULL, false);
 
 	pmu->genpd_data.domains[id] = &pd->genpd;
 	return 0;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f4dd810..f37faec 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -37,6 +37,17 @@ struct gpd_dev_ops {
 	bool (*active_wakeup)(struct device *dev);
 };
 
+struct gpd_cpuidle_data {
+	unsigned int saved_exit_latency;
+	struct cpuidle_state *idle_state;
+};
+
+struct genpd_power_state {
+	char *name;
+	s64 power_off_latency_ns;
+	s64 power_on_latency_ns;
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */
@@ -124,7 +135,14 @@ extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
+			  struct dev_power_governor *gov,
+			  const struct genpd_power_state *states,
+			  unsigned int state_count, bool is_off);
+extern void pm_genpd_init_simple(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
+extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
+extern int pm_genpd_name_poweron(const char *domain_name);
+extern void pm_genpd_poweroff_unused(void);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -160,7 +178,14 @@ static inline int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 	return -ENOSYS;
 }
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
-				 struct dev_power_governor *gov, bool is_off)
+				 struct dev_power_governor *gov,
+				 const struct genpd_power_state *states,
+				 unsigned int state_count, bool is_off)
+{
+}
+static inline void pm_genpd_init_simple(struct generic_pm_domain *genpd,
+					struct dev_power_governor *gov,
+					bool is_off)
 {
 }
 #endif
-- 
2.4.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 2/6] PM / Domains: core changes for multiple states
  2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
  2015-10-20 13:26 ` [PATCH v10 1/6] PM / Domains: prepare for multiple states ahaslam
@ 2015-10-20 13:26 ` ahaslam
       [not found]   ` <CAN2waFuQ+72zstgdD7GbxOr=d1T1wkqRvRePfL8KJbTceEGdzA@mail.gmail.com>
  2015-10-20 13:26 ` [PATCH v10 3/6] PM / Domains: make governor select deepest state ahaslam
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Axel Haslam

From: Axel Haslam <ahaslam+renesas@baylibre.com>

Add the core changes to be able to declare multiple states.
When trying to set a power domain to off, genpd will be able to choose
from an array of states declared by the platform. The power on and off
latencies are now tied to a state.

States should be declared in ascending order from shallowest to deepest,
deepest meaning the state which takes longer to enter and exit.

the power_off and power_on function can use the 'state_idx' field of the
generic_pm_domain structure, to distinguish between the different states
and act accordingly.

Example:

static int pd1_power_on(struct generic_pm_domain *domain)
{
	/* domain->state_idx = state the domain is coming from */
}

static int pd1_power_off(struct generic_pm_domain *domain)
{
	/* domain->state_idx = desired powered off state */
}

const struct genpd_power_state pd_states[] = {
	{
		.name = "RET",
		.power_on_latency_ns = ON_LATENCY_FAST,
		.power_off_latency_ns = OFF_LATENCY_FAST,
	},
	{
		.name = "DEEP_RET",
		.power_on_latency_ns = ON_LATENCY_MED,
		.power_off_latency_ns = OFF_LATENCY_MED,
	},
	{
		.name = "OFF",
		.power_on_latency_ns = ON_LATENCY_SLOW,
		.power_off_latency_ns = OFF_LATENCY_SLOW,
	}
};

struct generic_pm_domain pd1 = {
	.name = "PD1",
	.power_on = pd1_power_on,
	.power_off = pd1_power_off,
	[...]
};

int xxx_init_pm_domain(){

	pm_genpd_init(struct generic_pm_domain,
		pd1, pd_states, ARRAY_SIZE(pd_states), true);

}

Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
---
 drivers/base/power/domain.c          | 141 +++++++++++++++++++++++++++++++++--
 drivers/base/power/domain_governor.c |  13 +++-
 include/linux/pm_domain.h            |   4 +
 3 files changed, 149 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e0b5229..2ffee65 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -50,6 +50,12 @@
 	__retval;								\
 })
 
+#define GENPD_MAX_NAME_SIZE 20
+
+static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
+				       const struct genpd_power_state *st,
+				       unsigned int st_count);
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
@@ -124,6 +130,7 @@ static void genpd_sd_counter_inc(struct generic_pm_domain *genpd)
 
 static int genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 {
+	unsigned int state_idx = genpd->state_idx;
 	ktime_t time_start;
 	s64 elapsed_ns;
 	int ret;
@@ -140,10 +147,10 @@ static int genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 		return ret;
 
 	elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
-	if (elapsed_ns <= genpd->power_on_latency_ns)
+	if (elapsed_ns <= genpd->states[state_idx].power_on_latency_ns)
 		return ret;
 
-	genpd->power_on_latency_ns = elapsed_ns;
+	genpd->states[state_idx].power_on_latency_ns = elapsed_ns;
 	genpd->max_off_time_changed = true;
 	pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n",
 		 genpd->name, "on", elapsed_ns);
@@ -153,6 +160,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 
 static int genpd_power_off(struct generic_pm_domain *genpd, bool timed)
 {
+	unsigned int state_idx = genpd->state_idx;
 	ktime_t time_start;
 	s64 elapsed_ns;
 	int ret;
@@ -169,10 +177,10 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool timed)
 		return ret;
 
 	elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
-	if (elapsed_ns <= genpd->power_off_latency_ns)
+	if (elapsed_ns <= genpd->states[state_idx].power_off_latency_ns)
 		return ret;
 
-	genpd->power_off_latency_ns = elapsed_ns;
+	genpd->states[state_idx].power_off_latency_ns = elapsed_ns;
 	genpd->max_off_time_changed = true;
 	pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n",
 		 genpd->name, "off", elapsed_ns);
@@ -576,6 +584,8 @@ static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd,
 	    || atomic_read(&genpd->sd_count) > 0)
 		return;
 
+	/* Choose the deepest state when suspending */
+	genpd->state_idx = genpd->state_count - 1;
 	genpd_power_off(genpd, timed);
 
 	genpd->status = GPD_STATE_POWER_OFF;
@@ -1198,6 +1208,61 @@ static void genpd_free_dev_data(struct device *dev,
 	dev_pm_put_subsys_data(dev);
 }
 
+static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
+				   const struct genpd_power_state *st,
+				   unsigned int st_count)
+{
+	int ret = 0;
+	unsigned int i;
+
+	if (IS_ERR_OR_NULL(genpd)) {
+		ret = -EINVAL;
+		goto err;
+	}
+
+	if (!st || (st_count < 1)) {
+		ret = -EINVAL;
+		goto err;
+	}
+
+	/* Allocate the local memory to keep the states for this genpd */
+	genpd->states = kcalloc(st_count, sizeof(*st), GFP_KERNEL);
+	if (!genpd->states) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	for (i = 0; i < st_count; i++) {
+		genpd->states[i].power_on_latency_ns =
+			st[i].power_on_latency_ns;
+		genpd->states[i].power_off_latency_ns =
+			st[i].power_off_latency_ns;
+	}
+
+	/*
+	 * Copy the latency values To keep compatibility with
+	 * platforms that are not converted to use the multiple states.
+	 * This will be removed once all platforms are converted to use
+	 * multiple states. note that non converted platforms will use the
+	 * default single off state.
+	 */
+	if (genpd->power_on_latency_ns != 0)
+		genpd->states[0].power_on_latency_ns =
+				genpd->power_on_latency_ns;
+
+	if (genpd->power_off_latency_ns != 0)
+		genpd->states[0].power_off_latency_ns =
+				genpd->power_off_latency_ns;
+
+	genpd->state_count = st_count;
+
+	/* to save memory, Name allocation will happen if debug is enabled */
+	pm_genpd_alloc_states_names(genpd, st, st_count);
+
+err:
+	return ret;
+}
+
 /**
  * __pm_genpd_add_device - Add a device to an I/O PM domain.
  * @genpd: PM domain to add the device to.
@@ -1456,9 +1521,23 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 		   const struct genpd_power_state *states,
 		   unsigned int state_count, bool is_off)
 {
+	int ret;
+
 	if (IS_ERR_OR_NULL(genpd))
 		return;
 
+	/* State data should be provided */
+	if (!states || (state_count < 1)) {
+		pr_err("Invalid state data\n");
+		return;
+	}
+
+	ret = genpd_alloc_states_data(genpd, states, state_count);
+	if (ret) {
+		pr_err("Failed to allocate states for %s\n", genpd->name);
+		return;
+	}
+
 	INIT_LIST_HEAD(&genpd->master_links);
 	INIT_LIST_HEAD(&genpd->slave_links);
 	INIT_LIST_HEAD(&genpd->dev_list);
@@ -1470,6 +1549,8 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->device_count = 0;
 	genpd->max_off_time_ns = -1;
 	genpd->max_off_time_changed = true;
+	/* Assume the deepest state on init*/
+	genpd->state_idx = genpd->state_count - 1;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
 	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
 	genpd->domain.ops.prepare = pm_genpd_prepare;
@@ -1515,7 +1596,16 @@ EXPORT_SYMBOL_GPL(pm_genpd_init);
 void pm_genpd_init_simple(struct generic_pm_domain *genpd,
 		   struct dev_power_governor *gov, bool is_off)
 {
-	pm_genpd_init(genpd, gov, NULL, 0, is_off);
+	static const struct genpd_power_state genpd_default_idle_states[] = {
+		{
+			.name = "OFF",
+			.power_off_latency_ns = 0,
+			.power_on_latency_ns = 0,
+		},
+	};
+
+	pm_genpd_init(genpd, gov, genpd_default_idle_states,
+			ARRAY_SIZE(genpd_default_idle_states), is_off);
 }
 EXPORT_SYMBOL_GPL(pm_genpd_init_simple);
 
@@ -1832,6 +1922,33 @@ EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 #include <linux/kobject.h>
 static struct dentry *pm_genpd_debugfs_dir;
 
+static int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
+				       const struct genpd_power_state *st,
+				       unsigned int st_count)
+{
+	unsigned int i;
+
+	if (IS_ERR_OR_NULL(genpd))
+		return -EINVAL;
+
+	if (genpd->state_count != st_count) {
+		pr_err("Invalid allocated state count\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < st_count; i++) {
+		genpd->states[i].name = kstrndup(st[i].name,
+				GENPD_MAX_NAME_SIZE, GFP_KERNEL);
+		if (!genpd->states[i].name) {
+			pr_err("%s Failed to allocate state %d name.\n",
+				genpd->name, i);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * TODO: This function is a slightly modified version of rtpm_status_show
  * from sysfs.c, so generalize it.
@@ -1865,6 +1982,7 @@ static int pm_genpd_summary_one(struct seq_file *s,
 		[GPD_STATE_ACTIVE] = "on",
 		[GPD_STATE_POWER_OFF] = "off"
 	};
+	unsigned int state_idx = genpd->state_idx;
 	struct pm_domain_data *pm_data;
 	const char *kobj_path;
 	struct gpd_link *link;
@@ -1876,7 +1994,11 @@ static int pm_genpd_summary_one(struct seq_file *s,
 
 	if (WARN_ON(genpd->status >= ARRAY_SIZE(status_lookup)))
 		goto exit;
-	seq_printf(s, "%-30s  %-15s ", genpd->name, status_lookup[genpd->status]);
+
+	seq_printf(s, "%-30s  %-15s  ", genpd->name,
+		   (genpd->status == GPD_STATE_POWER_OFF) ?
+		   genpd->states[state_idx].name :
+		   status_lookup[genpd->status]);
 
 	/*
 	 * Modifications on the list require holding locks on both
@@ -1964,4 +2086,11 @@ static void __exit pm_genpd_debug_exit(void)
 	debugfs_remove_recursive(pm_genpd_debugfs_dir);
 }
 __exitcall(pm_genpd_debug_exit);
+#else
+static inline int pm_genpd_alloc_states_names(struct generic_pm_domain *genpd,
+					const struct genpd_power_state *st,
+					unsigned int st_count)
+{
+	return 0;
+}
 #endif /* CONFIG_PM_ADVANCED_DEBUG */
diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index 85e17ba..bf3ac68 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -127,8 +127,12 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
 		return genpd->cached_power_down_ok;
 	}
 
-	off_on_time_ns = genpd->power_off_latency_ns +
-				genpd->power_on_latency_ns;
+	/*
+	 * Use the only available state, until multiple state support is added
+	 * to the governor.
+	 */
+	off_on_time_ns = genpd->states[0].power_off_latency_ns +
+				genpd->states[0].power_on_latency_ns;
 
 	min_off_time_ns = -1;
 	/*
@@ -205,8 +209,11 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
 	 * The difference between the computed minimum subdomain or device off
 	 * time and the time needed to turn the domain on is the maximum
 	 * theoretical time this domain can spend in the "off" state.
+	 * Use the only available state, until multiple state support is added
+	 * to the governor.
 	 */
-	genpd->max_off_time_ns = min_off_time_ns - genpd->power_on_latency_ns;
+	genpd->max_off_time_ns = min_off_time_ns -
+		genpd->states[0].power_on_latency_ns;
 	return true;
 }
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f37faec..3129f85 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -77,6 +77,10 @@ struct generic_pm_domain {
 	void (*detach_dev)(struct generic_pm_domain *domain,
 			   struct device *dev);
 	unsigned int flags;		/* Bit field of configs for genpd */
+	struct genpd_power_state *states;
+	unsigned int state_count; /* number of states */
+	unsigned int state_idx; /* state that genpd will go to when off */
+
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
-- 
2.4.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 3/6] PM / Domains: make governor select deepest state
  2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
  2015-10-20 13:26 ` [PATCH v10 1/6] PM / Domains: prepare for multiple states ahaslam
  2015-10-20 13:26 ` [PATCH v10 2/6] PM / Domains: core changes " ahaslam
@ 2015-10-20 13:26 ` ahaslam
  2015-11-10  9:42   ` Zhaoyang Huang
  2015-10-20 13:26 ` [PATCH v10 4/6] ARM: imx6: pm: declare pm domain latency on power_state struct ahaslam
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Axel Haslam

From: Axel Haslam <ahaslam+renesas@baylibre.com>

Now that the structures of genpd can support multiple state definitions,
add the logic in the governor to select the deepest possible state when
powering down.

For this, create the new function power_down_ok_for_state which will test
if a particular state will not violate the devices and sub-domains
constraints.

default_power_down_ok is modified to try each state starting from the
deepest until a valid state is found or there are no more states to test.

the resulting state will be valid until there are latency or constraint
changes, thus, we can avoid looping every power_down, and use the cached
results instead.

Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
---
 drivers/base/power/domain_governor.c | 75 +++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index bf3ac68..aab2b32 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -100,7 +100,8 @@ static bool default_stop_ok(struct device *dev)
  *
  * This routine must be executed under the PM domain's lock.
  */
-static bool default_power_down_ok(struct dev_pm_domain *pd)
+static bool power_down_ok_for_state(struct dev_pm_domain *pd,
+				     unsigned int state)
 {
 	struct generic_pm_domain *genpd = pd_to_genpd(pd);
 	struct gpd_link *link;
@@ -108,31 +109,9 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
 	s64 min_off_time_ns;
 	s64 off_on_time_ns;
 
-	if (genpd->max_off_time_changed) {
-		struct gpd_link *link;
+	off_on_time_ns = genpd->states[state].power_off_latency_ns +
+		genpd->states[state].power_on_latency_ns;
 
-		/*
-		 * We have to invalidate the cached results for the masters, so
-		 * use the observation that default_power_down_ok() is not
-		 * going to be called for any master until this instance
-		 * returns.
-		 */
-		list_for_each_entry(link, &genpd->slave_links, slave_node)
-			link->master->max_off_time_changed = true;
-
-		genpd->max_off_time_changed = false;
-		genpd->cached_power_down_ok = false;
-		genpd->max_off_time_ns = -1;
-	} else {
-		return genpd->cached_power_down_ok;
-	}
-
-	/*
-	 * Use the only available state, until multiple state support is added
-	 * to the governor.
-	 */
-	off_on_time_ns = genpd->states[0].power_off_latency_ns +
-				genpd->states[0].power_on_latency_ns;
 
 	min_off_time_ns = -1;
 	/*
@@ -195,8 +174,6 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
 			min_off_time_ns = constraint_ns;
 	}
 
-	genpd->cached_power_down_ok = true;
-
 	/*
 	 * If the computed minimum device off time is negative, there are no
 	 * latency constraints, so the domain can spend arbitrary time in the
@@ -209,14 +186,52 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
 	 * The difference between the computed minimum subdomain or device off
 	 * time and the time needed to turn the domain on is the maximum
 	 * theoretical time this domain can spend in the "off" state.
-	 * Use the only available state, until multiple state support is added
-	 * to the governor.
 	 */
 	genpd->max_off_time_ns = min_off_time_ns -
-		genpd->states[0].power_on_latency_ns;
+			genpd->states[state].power_on_latency_ns;
 	return true;
 }
 
+static bool default_power_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	unsigned int last_state_idx = genpd->state_count - 1;
+	struct gpd_link *link;
+	bool retval = false;
+	unsigned int i;
+
+	/*
+	 * if there was no change on max_off_time, we can return the
+	 * cached value and we dont need to find a new target_state
+	 */
+	if (!genpd->max_off_time_changed)
+		return genpd->cached_power_down_ok;
+
+	/*
+	 * We have to invalidate the cached results for the masters, so
+	 * use the observation that default_power_down_ok() is not
+	 * going to be called for any master until this instance
+	 * returns.
+	 */
+	list_for_each_entry(link, &genpd->slave_links, slave_node)
+		link->master->max_off_time_changed = true;
+
+	genpd->max_off_time_ns = -1;
+	genpd->max_off_time_changed = false;
+
+	/* find a state to power down to, starting from the deepest */
+	for (i = 0; i < genpd->state_count; i++) {
+		if (power_down_ok_for_state(pd, last_state_idx - i)) {
+			genpd->state_idx = last_state_idx - i;
+			retval = true;
+			break;
+		}
+	}
+
+	genpd->cached_power_down_ok = retval;
+	return retval;
+}
+
 static bool always_on_power_down_ok(struct dev_pm_domain *domain)
 {
 	return false;
-- 
2.4.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 4/6] ARM: imx6: pm: declare pm domain latency on power_state struct.
  2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
                   ` (2 preceding siblings ...)
  2015-10-20 13:26 ` [PATCH v10 3/6] PM / Domains: make governor select deepest state ahaslam
@ 2015-10-20 13:26 ` ahaslam
  2015-10-20 13:26 ` [PATCH v10 5/6] PM / Domains: remove old power on/off latencies ahaslam
  2015-10-20 13:26 ` [PATCH v10 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files ahaslam
  5 siblings, 0 replies; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Axel Haslam

From: Axel Haslam <ahaslam+renesas@baylibre.com>

The generic_pm_domain structure uses an array of latencies to be able to
declare multiple intermediate states.

Declare a single "OFF" state with the default latencies So that the
power_off_latency_ns and power_on_latency_ns fields of generic_pm_domain
structure can be eventualy removed.

Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
---
 arch/arm/mach-imx/gpc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index aefe92d..7b839b3 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -45,9 +45,11 @@
 
 struct pu_domain {
 	struct generic_pm_domain base;
+	struct genpd_power_state *states;
 	struct regulator *reg;
 	struct clk *clk[GPC_CLK_MAX];
 	int num_clks;
+	unsigned int num_states;
 };
 
 static void __iomem *gpc_base;
@@ -372,14 +374,22 @@ static struct generic_pm_domain imx6q_arm_domain = {
 	.name = "ARM",
 };
 
+static struct genpd_power_state imx6q_arm_domain_states[] = {
+	{
+		.name = "OFF",
+		.power_off_latency_ns = 25000,
+		.power_on_latency_ns = 2000000,
+	},
+};
+
 static struct pu_domain imx6q_pu_domain = {
 	.base = {
 		.name = "PU",
 		.power_off = imx6q_pm_pu_power_off,
 		.power_on = imx6q_pm_pu_power_on,
-		.power_off_latency_ns = 25000,
-		.power_on_latency_ns = 2000000,
 	},
+	.states = imx6q_arm_domain_states,
+	.num_states = ARRAY_SIZE(imx6q_arm_domain_states),
 };
 
 static struct generic_pm_domain imx6sl_display_domain = {
@@ -422,7 +432,9 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
 	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
 		return 0;
 
-	pm_genpd_init_simple(&imx6q_pu_domain.base, NULL, false);
+	pm_genpd_init(&imx6q_pu_domain.base, NULL, imx6q_pu_domain.states,
+			imx6q_pu_domain.num_states, false);
+
 	return of_genpd_add_provider_onecell(dev->of_node,
 					     &imx_gpc_onecell_data);
 
-- 
2.4.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 5/6] PM / Domains: remove old power on/off latencies.
  2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
                   ` (3 preceding siblings ...)
  2015-10-20 13:26 ` [PATCH v10 4/6] ARM: imx6: pm: declare pm domain latency on power_state struct ahaslam
@ 2015-10-20 13:26 ` ahaslam
  2015-10-20 13:26 ` [PATCH v10 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files ahaslam
  5 siblings, 0 replies; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Axel Haslam

From: Axel Haslam <ahaslam+renesas@baylibre.com>

Now that all known users have been converted to use state latencies,
we can remove the latency field in the generic_pm_domain structure.

Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
---
 drivers/base/power/domain.c | 15 ---------------
 include/linux/pm_domain.h   |  2 --
 2 files changed, 17 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 2ffee65..dbab367 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1239,21 +1239,6 @@ static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
 			st[i].power_off_latency_ns;
 	}
 
-	/*
-	 * Copy the latency values To keep compatibility with
-	 * platforms that are not converted to use the multiple states.
-	 * This will be removed once all platforms are converted to use
-	 * multiple states. note that non converted platforms will use the
-	 * default single off state.
-	 */
-	if (genpd->power_on_latency_ns != 0)
-		genpd->states[0].power_on_latency_ns =
-				genpd->power_on_latency_ns;
-
-	if (genpd->power_off_latency_ns != 0)
-		genpd->states[0].power_off_latency_ns =
-				genpd->power_off_latency_ns;
-
 	genpd->state_count = st_count;
 
 	/* to save memory, Name allocation will happen if debug is enabled */
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 3129f85..ff576e4 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -65,9 +65,7 @@ struct generic_pm_domain {
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */
 	bool suspend_power_off;	/* Power status before system suspend */
 	int (*power_off)(struct generic_pm_domain *domain);
-	s64 power_off_latency_ns;
 	int (*power_on)(struct generic_pm_domain *domain);
-	s64 power_on_latency_ns;
 	struct gpd_dev_ops dev_ops;
 	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
 	bool max_off_time_changed;
-- 
2.4.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files
  2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
                   ` (4 preceding siblings ...)
  2015-10-20 13:26 ` [PATCH v10 5/6] PM / Domains: remove old power on/off latencies ahaslam
@ 2015-10-20 13:26 ` ahaslam
  5 siblings, 0 replies; 12+ messages in thread
From: ahaslam @ 2015-10-20 13:26 UTC (permalink / raw)
  To: khilman, ulf.hansson, lina.iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel
  Cc: bcousson, mturquette, Marc Titinger

From: Marc Titinger <mtitinger@baylibre.com>

This purpose of these debug seq-files is to help investigate
generic power domain state transitions, based on device constraints.
requires the "multiple states" patches from Axel Haslam.

also rename 'summary' from 'pm_genpd_summary'

sample output for 'states'
==========================

  Domain             State name        Eval = 2200nter + Exit = Min_off_on (ns)
-----------------------------------------------------------------------
a53_pd               cluster-sleep-0      1500000+800000=2300000
a57_pd               d1-retention         1000000+800000=1800000
a57_pd               cluster-sleep-0      1500000+800000=2300000

sample output for 'timings'
===========================

    Domain Devices, Timings in ns
                       Stop/Start Save/Restore, Effective
----------------------------------------------------  ---
a53_pd
    /cpus/cpu@100        1060  /660    1580  /1940  ,0 (cached stop)
    /cpus/cpu@101        1060  /740    1520  /1600  ,0 (cached stop)
    /cpus/cpu@102        880   /620    1380  /1780  ,0 (cached stop)
    /cpus/cpu@103        1080  /640    1340  /1600  ,0 (cached stop)
a57_pd
    /cpus/cpu@0          1160  /740    3280  /1800  ,0 (cached stop)
    /cpus/cpu@1          780   /1400   1440  /2080  ,0 (cached stop)
    /D1                  600   /540    7140  /6420  ,2199460 (cached stop)

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
 drivers/base/power/domain.c | 115 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 107 insertions(+), 8 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index dbab367..4c5be01 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,21 +2048,120 @@ static const struct file_operations pm_genpd_summary_fops = {
 	.release = single_release,
 };
 
+static int pm_genpd_states_show(struct seq_file *s, void *data)
+{
+	struct generic_pm_domain *genpd;
+
+	seq_puts(s,
+		 "\n  Domain             State name        Enter + Exit = Min_off_on (ns)\n");
+	seq_puts(s,
+		 "-----------------------------------------------------------------------\n");
+
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+
+		int i;
+
+		for (i = 0; i < genpd->state_count; i++) {
+			seq_printf(s, "%-20s %-20s %lld+%lld=%lld\n",
+				   genpd->name,
+				   genpd->states[i].name,
+				   genpd->states[i].power_on_latency_ns,
+				   genpd->states[i].power_off_latency_ns,
+				   genpd->states[i].power_off_latency_ns
+				   + genpd->states[i].power_on_latency_ns);
+		}
+
+	}
+
+	seq_puts(s, "\n");
+
+	return 0;
+}
+
+static int pm_genpd_states_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, pm_genpd_states_show, NULL);
+}
+
+static const struct file_operations pm_genpd_states_fops = {
+	.open = pm_genpd_states_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int pm_genpd_timing_show(struct seq_file *s, void *data)
+{
+	struct generic_pm_domain *genpd;
+
+	seq_puts(s, "\n    Domain Devices, Timings in ns\n");
+	seq_puts(s,
+		 "                       Stop/Start Save/Restore, Effective\n");
+	seq_puts(s,
+		 "----------------------------------------------------  ---\n");
+
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		struct pm_domain_data *pm_data;
+
+		seq_printf(s, "%-30s", genpd->name);
+
+		list_for_each_entry(pm_data, &genpd->dev_list, list_node) {
+			struct gpd_timing_data *td = &to_gpd_data(pm_data)->td;
+
+			if (!pm_data->dev->of_node)
+				continue;
+
+			seq_printf(s,
+				   "\n    %-20s %-6lld/%-6lld %-6lld/%-6lld,%lld %s%s",
+				   pm_data->dev->of_node->full_name,
+				   td->stop_latency_ns, td->start_latency_ns,
+				   td->save_state_latency_ns,
+				   td->restore_state_latency_ns,
+				   td->effective_constraint_ns,
+				   td->cached_stop_ok ? "(cached stop) " : "",
+				   td->constraint_changed ? "(changed)" : "");
+		}
+		seq_puts(s, "\n");
+	}
+	return 0;
+}
+
+static int pm_genpd_timing_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, pm_genpd_timing_show, NULL);
+}
+
+static const struct file_operations pm_genpd_timing_fops = {
+	.open = pm_genpd_timing_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
 static int __init pm_genpd_debug_init(void)
 {
-	struct dentry *d;
+	struct dentry *d = NULL;
 
 	pm_genpd_debugfs_dir = debugfs_create_dir("pm_genpd", NULL);
 
-	if (!pm_genpd_debugfs_dir)
-		return -ENOMEM;
+	if (pm_genpd_debugfs_dir)
+		d = debugfs_create_file("summary", S_IRUGO,
+				pm_genpd_debugfs_dir, NULL,
+				&pm_genpd_summary_fops);
+	if (d)
+		d = debugfs_create_file("states", S_IRUGO,
+				pm_genpd_debugfs_dir, NULL,
+				&pm_genpd_states_fops);
+	if (d)
+		d = debugfs_create_file("timings", S_IRUGO,
+				pm_genpd_debugfs_dir, NULL,
+				&pm_genpd_timing_fops);
+	if (d)
+		return 0;
 
-	d = debugfs_create_file("pm_genpd_summary", S_IRUGO,
-			pm_genpd_debugfs_dir, NULL, &pm_genpd_summary_fops);
-	if (!d)
-		return -ENOMEM;
+	debugfs_remove_recursive(pm_genpd_debugfs_dir /*can be null*/);
 
-	return 0;
+	return -ENOMEM;
 }
 late_initcall(pm_genpd_debug_init);
 
-- 
2.4.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 1/6] PM / Domains: prepare for multiple states
  2015-10-20 13:26 ` [PATCH v10 1/6] PM / Domains: prepare for multiple states ahaslam
@ 2015-10-27 14:33   ` Ulf Hansson
  2015-10-27 17:04     ` Axel Haslam
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2015-10-27 14:33 UTC (permalink / raw)
  To: Axel Haslam
  Cc: Kevin Hilman, Lina Iyer, geert, Krzysztof Kozłowski,
	Rafael J. Wysocki, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Benoit Cousson, Michael Turquette,
	Axel Haslam

On 20 October 2015 at 15:26,  <ahaslam@baylibre.com> wrote:
> From: Axel Haslam <ahaslam+renesas@baylibre.com>
>
> prepare generic power domain init function parameters to accept a pointer
> to the states structure that describes the possible states that a power
> domain can enter.
>
> Also, as most platforms are not initializing states or latencies, add
> pm_genpd_init_simple that allows platfroms to use a default single OFF
> state with initial latencies set to 0.
>
> There is no functional change, as support for multiple domains will be
> added in subsequent patches.
>
> Suggested-by: Lina Iyer <lina.iyer@linaro.org>
> Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>

Hi Axel,

Apologize for the delay in providing feedback.

> ---
>  arch/arm/mach-exynos/pm_domains.c     |  2 +-
>  arch/arm/mach-imx/gpc.c               |  2 +-
>  arch/arm/mach-s3c64xx/pm.c            |  4 ++--
>  arch/arm/mach-shmobile/pm-rmobile.c   |  2 +-
>  arch/arm/mach-ux500/pm_domains.c      |  2 +-
>  arch/arm/mach-zx/zx296702-pm-domain.c |  2 +-

You don't need to make above changes.

Instead of inventing pm_genpd_init_simple(), I would rather see that
you add a new __pm_genpd_init() API. Let's make it take *only* a
"struct generic_pm_domain *genpd" parameter.
Moreover, to prevent code duplication, adopt the code in
pm_genpd_init() so it can invoke __pm_genpd_init().

Then, piece by piece we can convert users from the legacy
pm_genpd_init() to the __pm_genpd_init().

>  drivers/base/power/domain.c           | 19 ++++++++++++++++++-
>  drivers/clk/qcom/gdsc.c               |  2 +-
>  drivers/clk/shmobile/clk-mstp.c       |  2 +-
>  drivers/soc/dove/pmu.c                |  2 +-
>  drivers/soc/mediatek/mtk-scpsys.c     |  2 +-
>  drivers/soc/rockchip/pm_domains.c     |  2 +-
>  include/linux/pm_domain.h             | 27 ++++++++++++++++++++++++++-
>  13 files changed, 56 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
> index 7c21760..dc36723 100644
> --- a/arch/arm/mach-exynos/pm_domains.c
> +++ b/arch/arm/mach-exynos/pm_domains.c
> @@ -187,7 +187,7 @@ static __init int exynos4_pm_init_power_domain(void)
>  no_clk:
>                 on = __raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN;
>
> -               pm_genpd_init(&pd->pd, NULL, !on);
> +               pm_genpd_init_simple(&pd->pd, NULL, !on);
>                 of_genpd_add_provider_simple(np, &pd->pd);
>         }
>
> diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
> index 10bf715..aefe92d 100644
> --- a/arch/arm/mach-imx/gpc.c
> +++ b/arch/arm/mach-imx/gpc.c
> @@ -422,7 +422,7 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
>         if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
>                 return 0;
>
> -       pm_genpd_init(&imx6q_pu_domain.base, NULL, false);
> +       pm_genpd_init_simple(&imx6q_pu_domain.base, NULL, false);
>         return of_genpd_add_provider_onecell(dev->of_node,
>                                              &imx_gpc_onecell_data);
>
> diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c
> index 75b14e7..d68a45c 100644
> --- a/arch/arm/mach-s3c64xx/pm.c
> +++ b/arch/arm/mach-s3c64xx/pm.c
> @@ -316,11 +316,11 @@ int __init s3c64xx_pm_init(void)
>         s3c_pm_init();
>
>         for (i = 0; i < ARRAY_SIZE(s3c64xx_always_on_pm_domains); i++)
> -               pm_genpd_init(&s3c64xx_always_on_pm_domains[i]->pd,
> +               pm_genpd_init_simple(&s3c64xx_always_on_pm_domains[i]->pd,
>                               &pm_domain_always_on_gov, false);
>
>         for (i = 0; i < ARRAY_SIZE(s3c64xx_pm_domains); i++)
> -               pm_genpd_init(&s3c64xx_pm_domains[i]->pd, NULL, false);
> +               pm_genpd_init_simple(&s3c64xx_pm_domains[i]->pd, NULL, false);
>
>  #ifdef CONFIG_S3C_DEV_FB
>         if (dev_get_platdata(&s3c_device_fb.dev))
> diff --git a/arch/arm/mach-shmobile/pm-rmobile.c b/arch/arm/mach-shmobile/pm-rmobile.c
> index 46d0a1d..906a2eb 100644
> --- a/arch/arm/mach-shmobile/pm-rmobile.c
> +++ b/arch/arm/mach-shmobile/pm-rmobile.c
> @@ -131,7 +131,7 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd)
>         struct dev_power_governor *gov = rmobile_pd->gov;
>
>         genpd->flags = GENPD_FLAG_PM_CLK;
> -       pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
> +       pm_genpd_init_simple(genpd, gov ? : &simple_qos_governor, false);
>         genpd->dev_ops.active_wakeup    = rmobile_pd_active_wakeup;
>         genpd->power_off                = rmobile_pd_power_down;
>         genpd->power_on                 = rmobile_pd_power_up;
> diff --git a/arch/arm/mach-ux500/pm_domains.c b/arch/arm/mach-ux500/pm_domains.c
> index 4d71c90..208e784 100644
> --- a/arch/arm/mach-ux500/pm_domains.c
> +++ b/arch/arm/mach-ux500/pm_domains.c
> @@ -72,7 +72,7 @@ int __init ux500_pm_domains_init(void)
>         genpd_data->num_domains = ARRAY_SIZE(ux500_pm_domains);
>
>         for (i = 0; i < ARRAY_SIZE(ux500_pm_domains); ++i)
> -               pm_genpd_init(ux500_pm_domains[i], NULL, false);
> +               pm_genpd_init_simple(ux500_pm_domains[i], NULL, false);
>
>         of_genpd_add_provider_onecell(np, genpd_data);
>         return 0;
> diff --git a/arch/arm/mach-zx/zx296702-pm-domain.c b/arch/arm/mach-zx/zx296702-pm-domain.c
> index e08574d..0ec8f0c 100644
> --- a/arch/arm/mach-zx/zx296702-pm-domain.c
> +++ b/arch/arm/mach-zx/zx296702-pm-domain.c
> @@ -175,7 +175,7 @@ static int zx296702_pd_probe(struct platform_device *pdev)
>         }
>
>         for (i = 0; i < ARRAY_SIZE(zx296702_pm_domains); ++i)
> -               pm_genpd_init(zx296702_pm_domains[i], NULL, false);
> +               pm_genpd_init_simple(zx296702_pm_domains[i], NULL, false);
>
>         of_genpd_add_provider_onecell(pdev->dev.of_node, genpd_data);
>         return 0;
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b0b5c43..e0b5229 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1447,10 +1447,14 @@ static int pm_genpd_default_restore_state(struct device *dev)
>   * pm_genpd_init - Initialize a generic I/O PM domain object.
>   * @genpd: PM domain object to initialize.
>   * @gov: PM domain governor to associate with the domain (may be NULL).
> + * @states: Array of possible low power states when powering off.
> + * @state_count: Number of low power states.
>   * @is_off: Initial value of the domain's power_is_off field.
>   */
>  void pm_genpd_init(struct generic_pm_domain *genpd,
> -                  struct dev_power_governor *gov, bool is_off)
> +                  struct dev_power_governor *gov,
> +                  const struct genpd_power_state *states,
> +                  unsigned int state_count, bool is_off)
>  {
>         if (IS_ERR_OR_NULL(genpd))
>                 return;
> @@ -1502,6 +1506,19 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  }
>  EXPORT_SYMBOL_GPL(pm_genpd_init);
>
> +/**
> + * pm_genpd_init_simple - Initialize a generic I/O PM domain object.
> + * @genpd: PM domain object to initialize.
> + * @gov: PM domain governor to associate with the domain (may be NULL).
> + * @is_off: Initial value of the domain's power_is_off field.
> + */
> +void pm_genpd_init_simple(struct generic_pm_domain *genpd,
> +                  struct dev_power_governor *gov, bool is_off)
> +{
> +       pm_genpd_init(genpd, gov, NULL, 0, is_off);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_init_simple);
> +
>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>  /*
>   * Device Tree based PM domain providers.
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index da9fad8..b709bf3 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -196,7 +196,7 @@ static int gdsc_init(struct gdsc *sc)
>
>         sc->pd.power_off = gdsc_disable;
>         sc->pd.power_on = gdsc_enable;
> -       pm_genpd_init(&sc->pd, NULL, !on);
> +       pm_genpd_init_simple(&sc->pd, NULL, !on);
>
>         return 0;
>  }
> diff --git a/drivers/clk/shmobile/clk-mstp.c b/drivers/clk/shmobile/clk-mstp.c
> index 3b09716..1f18474 100644
> --- a/drivers/clk/shmobile/clk-mstp.c
> +++ b/drivers/clk/shmobile/clk-mstp.c
> @@ -320,7 +320,7 @@ void __init cpg_mstp_add_clk_domain(struct device_node *np)
>         pd->name = np->name;
>
>         pd->flags = GENPD_FLAG_PM_CLK;
> -       pm_genpd_init(pd, &simple_qos_governor, false);
> +       pm_genpd_init_simple(pd, &simple_qos_governor, false);
>         pd->attach_dev = cpg_mstp_attach_dev;
>         pd->detach_dev = cpg_mstp_detach_dev;
>
> diff --git a/drivers/soc/dove/pmu.c b/drivers/soc/dove/pmu.c
> index abd0879..42adc16 100644
> --- a/drivers/soc/dove/pmu.c
> +++ b/drivers/soc/dove/pmu.c
> @@ -215,7 +215,7 @@ static void __pmu_domain_register(struct pmu_domain *domain,
>         domain->base.power_off = pmu_domain_power_off;
>         domain->base.power_on = pmu_domain_power_on;
>
> -       pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask));
> +       pm_genpd_init_simple(&domain->base, NULL, !(val & domain->pwr_mask));
>
>         if (np)
>                 of_genpd_add_provider_simple(np, &domain->base);
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index 164a7d8..adf364f 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -443,7 +443,7 @@ static int __init scpsys_probe(struct platform_device *pdev)
>                  */
>                 genpd->power_on(genpd);
>
> -               pm_genpd_init(genpd, NULL, false);
> +               pm_genpd_init_simple(genpd, NULL, false);
>         }
>
>         /*
> diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
> index 534c589..617e219 100644
> --- a/drivers/soc/rockchip/pm_domains.c
> +++ b/drivers/soc/rockchip/pm_domains.c
> @@ -299,7 +299,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>         pd->genpd.attach_dev = rockchip_pd_attach_dev;
>         pd->genpd.detach_dev = rockchip_pd_detach_dev;
>         pd->genpd.flags = GENPD_FLAG_PM_CLK;
> -       pm_genpd_init(&pd->genpd, NULL, false);
> +       pm_genpd_init_simple(&pd->genpd, NULL, false);
>
>         pmu->genpd_data.domains[id] = &pd->genpd;
>         return 0;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index f4dd810..f37faec 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -37,6 +37,17 @@ struct gpd_dev_ops {
>         bool (*active_wakeup)(struct device *dev);
>  };
>
> +struct gpd_cpuidle_data {
> +       unsigned int saved_exit_latency;
> +       struct cpuidle_state *idle_state;
> +};

This is probably some leftover from an old version, right?

> +
> +struct genpd_power_state {
> +       char *name;

I am wondering a bit about the char *name. Do we really need it and if
so, what's do you intend to use it for?

> +       s64 power_off_latency_ns;
> +       s64 power_on_latency_ns;
> +};
> +
>  struct generic_pm_domain {
>         struct dev_pm_domain domain;    /* PM domain operations */
>         struct list_head gpd_list_node; /* Node in the global PM domains list */
> @@ -124,7 +135,14 @@ extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
>  extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
>                                      struct generic_pm_domain *target);
>  extern void pm_genpd_init(struct generic_pm_domain *genpd,
> +                         struct dev_power_governor *gov,
> +                         const struct genpd_power_state *states,
> +                         unsigned int state_count, bool is_off);
> +extern void pm_genpd_init_simple(struct generic_pm_domain *genpd,
>                           struct dev_power_governor *gov, bool is_off);
> +extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
> +extern int pm_genpd_name_poweron(const char *domain_name);
> +extern void pm_genpd_poweroff_unused(void);
>
>  extern struct dev_power_governor simple_qos_governor;
>  extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -160,7 +178,14 @@ static inline int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
>         return -ENOSYS;
>  }
>  static inline void pm_genpd_init(struct generic_pm_domain *genpd,
> -                                struct dev_power_governor *gov, bool is_off)
> +                                struct dev_power_governor *gov,
> +                                const struct genpd_power_state *states,
> +                                unsigned int state_count, bool is_off)
> +{
> +}
> +static inline void pm_genpd_init_simple(struct generic_pm_domain *genpd,
> +                                       struct dev_power_governor *gov,
> +                                       bool is_off)
>  {
>  }
>  #endif
> --
> 2.4.5
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 1/6] PM / Domains: prepare for multiple states
  2015-10-27 14:33   ` Ulf Hansson
@ 2015-10-27 17:04     ` Axel Haslam
  0 siblings, 0 replies; 12+ messages in thread
From: Axel Haslam @ 2015-10-27 17:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Kevin Hilman, Lina Iyer, geert, Krzysztof Kozłowski,
	Rafael J. Wysocki, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Benoit Cousson, Michael Turquette,
	Axel Haslam

Hi Ulf,

>
> Hi Axel,
>
> Apologize for the delay in providing feedback.
>

Thanks for the review!

>> ---
>>  arch/arm/mach-exynos/pm_domains.c     |  2 +-
>>  arch/arm/mach-imx/gpc.c               |  2 +-
>>  arch/arm/mach-s3c64xx/pm.c            |  4 ++--
>>  arch/arm/mach-shmobile/pm-rmobile.c   |  2 +-
>>  arch/arm/mach-ux500/pm_domains.c      |  2 +-
>>  arch/arm/mach-zx/zx296702-pm-domain.c |  2 +-
>
> You don't need to make above changes.
>
> Instead of inventing pm_genpd_init_simple(), I would rather see that
> you add a new __pm_genpd_init() API. Let's make it take *only* a
> "struct generic_pm_domain *genpd" parameter.
> Moreover, to prevent code duplication, adopt the code in
> pm_genpd_init() so it can invoke __pm_genpd_init().
>
> Then, piece by piece we can convert users from the legacy
> pm_genpd_init() to the __pm_genpd_init().

Ok, ill do that insted.

>>
>> +struct gpd_cpuidle_data {
>> +       unsigned int saved_exit_latency;
>> +       struct cpuidle_state *idle_state;
>> +};
>
> This is probably some leftover from an old version, right?

yes! this slipped by when i re-based to next, i should have
paid more attention. ill remove it.

>
>> +
>> +struct genpd_power_state {
>> +       char *name;
>
> I am wondering a bit about the char *name. Do we really need it and if
> so, what's do you intend to use it for?
>
>

The *name is allocated and used only when CONFIG_PM_ADVANCED_DEBUG
is enabled, its used to print the status on debugfs, when genpd_summary is read.

for example:

#cat /d/pm_genpd/summary
domain                          status          slaves
    /device                                             runtime status
----------------------------------------------------------------------
b3                              OFF
b2                              OFF
b1                              OFF              b2, b3
a3                              OFF
    /devices/platform/D2                                suspended
a4                              RET
    /devices/platform/D1                                suspended
a2                              RET              a4
a1                              RET              a2, a3
cpg_clocks                      on



Regards
Axel.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 3/6] PM / Domains: make governor select deepest state
  2015-10-20 13:26 ` [PATCH v10 3/6] PM / Domains: make governor select deepest state ahaslam
@ 2015-11-10  9:42   ` Zhaoyang Huang
  2015-11-10 13:58     ` Axel Haslam
  0 siblings, 1 reply; 12+ messages in thread
From: Zhaoyang Huang @ 2015-11-10  9:42 UTC (permalink / raw)
  To: ahaslam
  Cc: Kevin Hilman, Ulf Hansson, Lina Iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel, bcousson, mturquette, Axel Haslam

On 20 October 2015 at 21:26,  <ahaslam@baylibre.com> wrote:
> From: Axel Haslam <ahaslam+renesas@baylibre.com>
>
> Now that the structures of genpd can support multiple state definitions,
> add the logic in the governor to select the deepest possible state when
> powering down.
>
> For this, create the new function power_down_ok_for_state which will test
> if a particular state will not violate the devices and sub-domains
> constraints.
>
> default_power_down_ok is modified to try each state starting from the
> deepest until a valid state is found or there are no more states to test.
>
> the resulting state will be valid until there are latency or constraint
> changes, thus, we can avoid looping every power_down, and use the cached
> results instead.
>
> Signed-off-by: Axel Haslam <ahaslam+renesas@baylibre.com>
> ---
>  drivers/base/power/domain_governor.c | 75 +++++++++++++++++++++---------------
>  1 file changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index bf3ac68..aab2b32 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -100,7 +100,8 @@ static bool default_stop_ok(struct device *dev)
>   *
>   * This routine must be executed under the PM domain's lock.
>   */
> -static bool default_power_down_ok(struct dev_pm_domain *pd)
> +static bool power_down_ok_for_state(struct dev_pm_domain *pd,
> +                                    unsigned int state)
>  {
>         struct generic_pm_domain *genpd = pd_to_genpd(pd);
>         struct gpd_link *link;
> @@ -108,31 +109,9 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
>         s64 min_off_time_ns;
>         s64 off_on_time_ns;
>
> -       if (genpd->max_off_time_changed) {
> -               struct gpd_link *link;
> +       off_on_time_ns = genpd->states[state].power_off_latency_ns +
> +               genpd->states[state].power_on_latency_ns;
>
> -               /*
> -                * We have to invalidate the cached results for the masters, so
> -                * use the observation that default_power_down_ok() is not
> -                * going to be called for any master until this instance
> -                * returns.
> -                */
> -               list_for_each_entry(link, &genpd->slave_links, slave_node)
> -                       link->master->max_off_time_changed = true;
> -
> -               genpd->max_off_time_changed = false;
> -               genpd->cached_power_down_ok = false;
> -               genpd->max_off_time_ns = -1;
> -       } else {
> -               return genpd->cached_power_down_ok;
> -       }
> -
> -       /*
> -        * Use the only available state, until multiple state support is added
> -        * to the governor.
> -        */
> -       off_on_time_ns = genpd->states[0].power_off_latency_ns +
> -                               genpd->states[0].power_on_latency_ns;
>
>         min_off_time_ns = -1;
>         /*
> @@ -195,8 +174,6 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
>                         min_off_time_ns = constraint_ns;
>         }
>
> -       genpd->cached_power_down_ok = true;
> -
>         /*
>          * If the computed minimum device off time is negative, there are no
>          * latency constraints, so the domain can spend arbitrary time in the
> @@ -209,14 +186,52 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
>          * The difference between the computed minimum subdomain or device off
>          * time and the time needed to turn the domain on is the maximum
>          * theoretical time this domain can spend in the "off" state.
> -        * Use the only available state, until multiple state support is added
> -        * to the governor.
>          */
>         genpd->max_off_time_ns = min_off_time_ns -
> -               genpd->states[0].power_on_latency_ns;
> +                       genpd->states[state].power_on_latency_ns;
>         return true;
>  }
[question]: Does it mean that the sleep level is just decided by
comparing the pre-configure on_off time with the gpd_timing_data? How
about the next timer event which affect the sleep depth on cpuidle
framework?
>
> +static bool default_power_down_ok(struct dev_pm_domain *pd)
> +{
> +       struct generic_pm_domain *genpd = pd_to_genpd(pd);
> +       unsigned int last_state_idx = genpd->state_count - 1;
> +       struct gpd_link *link;
> +       bool retval = false;
> +       unsigned int i;
> +
> +       /*
> +        * if there was no change on max_off_time, we can return the
> +        * cached value and we dont need to find a new target_state
> +        */
> +       if (!genpd->max_off_time_changed)
> +               return genpd->cached_power_down_ok;
> +
> +       /*
> +        * We have to invalidate the cached results for the masters, so
> +        * use the observation that default_power_down_ok() is not
> +        * going to be called for any master until this instance
> +        * returns.
> +        */
> +       list_for_each_entry(link, &genpd->slave_links, slave_node)
> +               link->master->max_off_time_changed = true;
> +
> +       genpd->max_off_time_ns = -1;
> +       genpd->max_off_time_changed = false;
> +
> +       /* find a state to power down to, starting from the deepest */
> +       for (i = 0; i < genpd->state_count; i++) {
> +               if (power_down_ok_for_state(pd, last_state_idx - i)) {
> +                       genpd->state_idx = last_state_idx - i;
> +                       retval = true;
> +                       break;
> +               }
> +       }
> +
> +       genpd->cached_power_down_ok = retval;
> +       return retval;
> +}
> +
>  static bool always_on_power_down_ok(struct dev_pm_domain *domain)
>  {
>         return false;
[question]How the TICK_NOHZ treated if we substitute cpuidle framework
with this one?
> --
> 2.4.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 2/6] PM / Domains: core changes for multiple states
       [not found]   ` <CAN2waFuQ+72zstgdD7GbxOr=d1T1wkqRvRePfL8KJbTceEGdzA@mail.gmail.com>
@ 2015-11-10 11:02     ` Axel Haslam
  0 siblings, 0 replies; 12+ messages in thread
From: Axel Haslam @ 2015-11-10 11:02 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: Kevin Hilman, Ulf Hansson, Lina Iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel, bcousson, mturquette, Axel Haslam

Hi Zhaoyang,

>     @@ -576,6 +584,8 @@ static void pm_genpd_sync_poweroff(struct
>     generic_pm_domain *genpd,
>                  || atomic_read(&genpd->sd_count) > 0)
>                      return;
>
>     +       /* Choose the deepest state when suspending */
>     +       genpd->state_idx = genpd->state_count - 1;
>
> if there is no governor configured, does it mean that we just try the
> deepest one here?
>

If a governor is not configured, there is no way to choose between the 
states, and the state_idx will not change. so it will remain the one set 
at the genpd init function wich is the same as here: the deepest state. 
(state_count -1). In other words, if there is no governor, there
is only one state: the deepest.

About setting the deepest state in this particular place, My 
understanding is that this code will be called in suspend path,
and in this case genpd_poweroff is not called, hence the genpd governor 
is not called either. Because there are many other things happening 
during suspend/resume, i think that suspend/resume is a slow procedure 
that will most likely not honor the device qos constraints, so i set the 
genpd to its deepest state.

Regards
Axel
>              genpd_power_off(genpd, timed);
>
>              genpd->status = GPD_STATE_POWER_OFF;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 3/6] PM / Domains: make governor select deepest state
  2015-11-10  9:42   ` Zhaoyang Huang
@ 2015-11-10 13:58     ` Axel Haslam
  0 siblings, 0 replies; 12+ messages in thread
From: Axel Haslam @ 2015-11-10 13:58 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: Kevin Hilman, Ulf Hansson, Lina Iyer, geert, k.kozlowski.k, rjw,
	linux-pm, linux-kernel, bcousson, mturquette, Axel Haslam

Hi Zhaoyang,


>> @@ -209,14 +186,52 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)
>>           * The difference between the computed minimum subdomain or device off
>>           * time and the time needed to turn the domain on is the maximum
>>           * theoretical time this domain can spend in the "off" state.
>> -        * Use the only available state, until multiple state support is added
>> -        * to the governor.
>>           */
>>          genpd->max_off_time_ns = min_off_time_ns -
>> -               genpd->states[0].power_on_latency_ns;
>> +                       genpd->states[state].power_on_latency_ns;
>>          return true;
>>   }
> [question]: Does it mean that the sleep level is just decided by
> comparing the pre-configure on_off time with the gpd_timing_data? How
> about the next timer event which affect the sleep depth on cpuidle
> framework?



There are a couple of patches on the list ill try to summarize
what i understand, Lina, Marc please correct me if im wrong!

I did this patches thinking generally about devices in a power domain 
and not so much about a cpu. So these patches are not aimed at replacing 
cpuidle, but were meant for power domains that may have intermediate 
states, for example some logic may be implemented with retention flip-flops.

There is the proposal by Lina [1] to add cpus clusters to powerdomins
and by Marc[2] that tie cluster idle states to the powerdomain handler.
but i think the effort here is to complement cpuidle rather than to
replace it.

regards
Axel

1. https://lwn.net/Articles/653579/
2. https://lwn.net/Articles/658461/

>>
>> +static bool default_power_down_ok(struct dev_pm_domain *pd)
>> +{
>> +       struct generic_pm_domain *genpd = pd_to_genpd(pd);
>> +       unsigned int last_state_idx = genpd->state_count - 1;
>> +       struct gpd_link *link;
>> +       bool retval = false;
>> +       unsigned int i;
>> +
>> +       /*
>> +        * if there was no change on max_off_time, we can return the
>> +        * cached value and we dont need to find a new target_state
>> +        */
>> +       if (!genpd->max_off_time_changed)
>> +               return genpd->cached_power_down_ok;
>> +
>> +       /*
>> +        * We have to invalidate the cached results for the masters, so
>> +        * use the observation that default_power_down_ok() is not
>> +        * going to be called for any master until this instance
>> +        * returns.
>> +        */
>> +       list_for_each_entry(link, &genpd->slave_links, slave_node)
>> +               link->master->max_off_time_changed = true;
>> +
>> +       genpd->max_off_time_ns = -1;
>> +       genpd->max_off_time_changed = false;
>> +
>> +       /* find a state to power down to, starting from the deepest */
>> +       for (i = 0; i < genpd->state_count; i++) {
>> +               if (power_down_ok_for_state(pd, last_state_idx - i)) {
>> +                       genpd->state_idx = last_state_idx - i;
>> +                       retval = true;
>> +                       break;
>> +               }
>> +       }
>> +
>> +       genpd->cached_power_down_ok = retval;
>> +       return retval;
>> +}
>> +
>>   static bool always_on_power_down_ok(struct dev_pm_domain *domain)
>>   {
>>          return false;
> [question]How the TICK_NOHZ treated if we substitute cpuidle framework
> with this one?
>> --
>> 2.4.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-11-10 13:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-20 13:26 [PATCH v10 0/6] Multiple intermediate states for genpd ahaslam
2015-10-20 13:26 ` [PATCH v10 1/6] PM / Domains: prepare for multiple states ahaslam
2015-10-27 14:33   ` Ulf Hansson
2015-10-27 17:04     ` Axel Haslam
2015-10-20 13:26 ` [PATCH v10 2/6] PM / Domains: core changes " ahaslam
     [not found]   ` <CAN2waFuQ+72zstgdD7GbxOr=d1T1wkqRvRePfL8KJbTceEGdzA@mail.gmail.com>
2015-11-10 11:02     ` Axel Haslam
2015-10-20 13:26 ` [PATCH v10 3/6] PM / Domains: make governor select deepest state ahaslam
2015-11-10  9:42   ` Zhaoyang Huang
2015-11-10 13:58     ` Axel Haslam
2015-10-20 13:26 ` [PATCH v10 4/6] ARM: imx6: pm: declare pm domain latency on power_state struct ahaslam
2015-10-20 13:26 ` [PATCH v10 5/6] PM / Domains: remove old power on/off latencies ahaslam
2015-10-20 13:26 ` [PATCH v10 6/6] PM / Domains: add debugfs 'states' and 'timings' seq files ahaslam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).