Skip to contentSkip to conversation

PM: Provide an always on power domain governor

PATCH New Mark Brown 7 participants 33 messages Dec 1, 2011 – Dec 8, 2011

Subsystems (9)
lore
Share & tools
lore

Mark Brown

author
#0 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Since systems are likely to have power domains that can't be turned off for various reasons at least temporarily while implementing power domain support provide a default governor which will always refuse to power off the domain, saving platforms having to implement their own.

Since the code is so tiny don't bother with a Kconfig symbol for it.

---
 drivers/base/power/domain_governor.c |   13 +++++++++++++
 include/linux/pm_domain.h            |    2 ++
 2 files changed, 15 insertions(+), 0 deletions(-)
drivers/base/power/domain_governor.c +13 -0
index da78540..51527ee 100644
@@ -141,3 +141,16 @@ struct dev_power_governor simple_qos_governor = {
 	.stop_ok = default_stop_ok,
 	.power_down_ok = default_power_down_ok,
 };
+
+static bool always_on_power_down_ok(struct dev_pm_domain *domain)
+{
+	return false;
+}
+
+/**
+ * pm_genpd_gov_always_on - A governor implementing an always-on policy
+ */
+struct dev_power_governor pm_domain_always_on_gov = {
+	.power_down_ok = always_on_power_down_ok,
+	.stop_ok = default_stop_ok,
+};
include/linux/pm_domain.h +2 -0
index fbb81bc..fda14f1 100644
@@ -139,6 +139,7 @@ extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
 
 extern bool default_stop_ok(struct device *dev);
 
+extern struct dev_power_governor pm_domain_always_on_gov;
 #else
 
 static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
@@ -192,6 +193,7 @@ static inline bool default_stop_ok(struct device *dev)
 {
 	return false;
 }
+#define pm_domain_always_on_gov NULL
 #endif
 
 static inline int pm_genpd_remove_callbacks(struct device *dev)
Signature · 1 line
1.7.7.3
author
#1 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Saves a tiny amount of code.

---
 arch/arm/mach-shmobile/pm-sh7372.c |   43 +----------------------------------
 1 files changed, 2 insertions(+), 41 deletions(-)
arch/arm/mach-shmobile/pm-sh7372.c +2 -41
index adf1765..5f7d575 100644
@@ -162,45 +162,6 @@ static bool pd_active_wakeup(struct device *dev)
 	return active_wakeup ? active_wakeup(dev) : true;
 }
 
-static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
-{
-	return false;
-}
-
-struct dev_power_governor sh7372_always_on_gov = {
-	.power_down_ok = sh7372_power_down_forbidden,
-	.stop_ok = default_stop_ok,
-};
-
-static int sh7372_stop_dev(struct device *dev)
-{
-	int (*stop)(struct device *dev);
-
-	stop = dev_gpd_data(dev)->ops.stop;
-	if (stop) {
-		int ret = stop(dev);
-		if (ret)
-			return ret;
-	}
-	return pm_clk_suspend(dev);
-}
-
-static int sh7372_start_dev(struct device *dev)
-{
-	int (*start)(struct device *dev);
-	int ret;
-
-	ret = pm_clk_resume(dev);
-	if (ret)
-		return ret;
-
-	start = dev_gpd_data(dev)->ops.start;
-	if (start)
-		ret = start(dev);
-
-	return ret;
-}
-
 void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
@@ -246,7 +207,7 @@ struct sh7372_pm_domain sh7372_d4 = {
 
 struct sh7372_pm_domain sh7372_a4r = {
 	.bit_shift = 5,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 	.stay_on = true,
@@ -262,7 +223,7 @@ struct sh7372_pm_domain sh7372_a3ri = {
 
 struct sh7372_pm_domain sh7372_a3sp = {
 	.bit_shift = 11,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
 };
 
Signature · 1 line
1.7.7.3
#2 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Hi,

Quoted 1 line · Mark Brown

Saves a tiny amount of code.

How so?

Quoted 42 lines
---
 arch/arm/mach-shmobile/pm-sh7372.c |   43 +----------------------------------
 1 files changed, 2 insertions(+), 41 deletions(-)
arch/arm/mach-shmobile/pm-sh7372.c +0 -29
index adf1765..5f7d575 100644
@@ -162,45 +162,6 @@ static bool pd_active_wakeup(struct device *dev)
 	return active_wakeup ? active_wakeup(dev) : true;
 }
 
-static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
-{
-	return false;
-}
-
-struct dev_power_governor sh7372_always_on_gov = {
-	.power_down_ok = sh7372_power_down_forbidden,
-	.stop_ok = default_stop_ok,
-};
-
-static int sh7372_stop_dev(struct device *dev)
-{
-	int (*stop)(struct device *dev);
-
-	stop = dev_gpd_data(dev)->ops.stop;
-	if (stop) {
-		int ret = stop(dev);
-		if (ret)
-			return ret;
-	}
-	return pm_clk_suspend(dev);
-}
-
-static int sh7372_start_dev(struct device *dev)
-{
-	int (*start)(struct device *dev);
-	int ret;
-
-	ret = pm_clk_resume(dev);

arch/arm/mach-shmobile/pm-sh7372.c static bool pd_active_wakeup(struct device *dev) · line 203

Quoted 10 lines
arch/arm/mach-shmobile/pm-sh7372.c +0 -10
-	if (ret)
-		return ret;
-
-	start = dev_gpd_data(dev)->ops.start;
-	if (start)
-		ret = start(dev);
-
-	return ret;
-}
-

The above functions are actually necessary, please don't remove them without introducing replacements.

Quoted 13 lines
arch/arm/mach-shmobile/pm-sh7372.c +1 -1
 void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
@@ -246,7 +207,7 @@ struct sh7372_pm_domain sh7372_d4 = {
 
 struct sh7372_pm_domain sh7372_a4r = {
 	.bit_shift = 5,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 	.stay_on = true,

arch/arm/mach-shmobile/pm-sh7372.c struct sh7372_pm_domain sh7372_a3ri = { · line 228

Quoted 7 lines
arch/arm/mach-shmobile/pm-sh7372.c +1 -1
@@ -262,7 +223,7 @@ struct sh7372_pm_domain sh7372_a3ri = {
 
 struct sh7372_pm_domain sh7372_a3sp = {
 	.bit_shift = 11,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
 };

Thanks,
Rafael

author
#3 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Quoted 4 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown

Saves a tiny amount of code.

How so?

It saves source code size when we get the second user, by itself you're right it doesn't achieve anything (except avoiding redundancy once 1/3 gets applied I guess).

#4 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Quoted 9 lines · Mark Brown
Quoted 4 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown

Saves a tiny amount of code.

How so?

It saves source code size when we get the second user, by itself you're right it doesn't achieve anything (except avoiding redundancy once 1/3 gets applied I guess).

But the patch removed useful code without adding any replacements, as far as I could see it.

Thanks,
Rafael

author
#5 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Quoted 8 lines · Rafael J. Wysocki
Quoted 3 lines · Mark Brown

It saves source code size when we get the second user, by itself you're right it doesn't achieve anything (except avoiding redundancy once 1/3 gets applied I guess).

But the patch removed useful code without adding any replacements, as far as I could see it.

The first patch in the series added the replacement as generic code, this patch is deleting the shmobile specific implementation and using the generic code instead.

#6 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Quoted 7 lines · Mark Brown
Quoted 5 lines · Rafael J. Wysocki
Quoted 3 lines · Mark Brown

It saves source code size when we get the second user, by itself you're right it doesn't achieve anything (except avoiding redundancy once 1/3 gets applied I guess).

Quoted 6 lines
Quoted 2 lines

But the patch removed useful code without adding any replacements, as far as I could see it.

The first patch in the series added the replacement as generic code, this patch is deleting the shmobile specific implementation and using the generic code instead.

No, the first patch didn't add any replacements for the functions that the second patch removed. At least I don't see the replacements in the first patch.

Thanks,
Rafael

author
#7 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Quoted 9 lines · Rafael J. Wysocki
Quoted 3 lines · Mark Brown

The first patch in the series added the replacement as generic code, this patch is deleting the shmobile specific implementation and using the generic code instead.

No, the first patch didn't add any replacements for the functions that the second patch removed. At least I don't see the replacements in the first patch.

Oh, right. It'd really help if you were more specific. I'd probably have realised earlier that you are probably talking about the stop_dev and start_dev functions which appear to have got caught up in the mess of a rebase somehow - I didn't intentionally remove them as their absence from the patch description and the lack of any removal of references to them should suggest.

#8 lore raw

PATCH 2/3 ARM: mach-shmobile: Use common always on power domain governor

Quoted 11 lines · Mark Brown
Quoted 9 lines · Rafael J. Wysocki
Quoted 3 lines · Mark Brown

The first patch in the series added the replacement as generic code, this patch is deleting the shmobile specific implementation and using the generic code instead.

No, the first patch didn't add any replacements for the functions that the second patch removed. At least I don't see the replacements in the first patch.

Quoted 6 lines

Oh, right. It'd really help if you were more specific. I'd probably have realised earlier that you are probably talking about the stop_dev and start_dev functions which appear to have got caught up in the mess of a rebase somehow - I didn't intentionally remove them as their absence from the patch description and the lack of any removal of references to them should suggest.

Well, I mentioned these two functions specifically in my first reply to the patch message. :-)

Thanks,
Rafael

author
#9 lore raw

PATCH v2 1/1 ARM: mach-shmobile: Use common always on power domain governor

In reply to Mark Brown

Saves a tiny amount of code.

---
 arch/arm/mach-shmobile/pm-sh7372.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)
arch/arm/mach-shmobile/pm-sh7372.c +2 -12
index adf1765..7d7cdfc 100644
@@ -162,16 +162,6 @@ static bool pd_active_wakeup(struct device *dev)
 	return active_wakeup ? active_wakeup(dev) : true;
 }
 
-static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
-{
-	return false;
-}
-
-struct dev_power_governor sh7372_always_on_gov = {
-	.power_down_ok = sh7372_power_down_forbidden,
-	.stop_ok = default_stop_ok,
-};
-
 static int sh7372_stop_dev(struct device *dev)
 {
 	int (*stop)(struct device *dev);
@@ -246,7 +236,7 @@ struct sh7372_pm_domain sh7372_d4 = {
 
 struct sh7372_pm_domain sh7372_a4r = {
 	.bit_shift = 5,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 	.stay_on = true,
@@ -262,7 +252,7 @@ struct sh7372_pm_domain sh7372_a3ri = {
 
 struct sh7372_pm_domain sh7372_a3sp = {
 	.bit_shift = 11,
-	.gov = &sh7372_always_on_gov,
+	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
 };
 
Signature · 1 line
1.7.7.3
author
#10 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

In reply to Mark Brown

The S3C64xx SoCs contain a set of gateable power domains which can be enabled and disabled at runtime in order to save power. Use the generic power domain code to implement support for these in software, enabling runtime control of most domains:

- ETM (not supported in mainline).
- Domain G: 3D acceleration (no mainline support).
- Domain V: MFC (no mainline support).
- Domain I: JPEG and camera interface (no mainline support).
- Domain P: 2D acceleration, TV encoder and scaler (no mainline support)
- Domain S: Security (no mainline support).
- Domain F: LCD (driver already uses runtime PM), post processing and rotation (no mainline support).

The IROM domain is marked as always enabled as we should arrange for it to be enabled when we suspend which will need a bit more work.

Due to all the conditional device registration that the platform does wrap s3c_pm_init() with s3c64xx_pm_init() which actually puts the device into the power domain after the machines have registered, looking for platform data to tell if the device was registered. Since currently only Cragganmore actually sets up PM that is the only machine updated.

---

Kikjin, this superceeds my previous patch unconditionally disabling some of the domains. I've given it quite a bit of testing and it appears to be working well on my systems though I'm not sure the power saving is really all that much to write home about.

 arch/arm/mach-s3c64xx/Kconfig           |    1 +
 arch/arm/mach-s3c64xx/mach-crag6410.c   |    2 +-
 arch/arm/mach-s3c64xx/pm.c              |  173 ++++++++++++++++++++++++++++++-
 arch/arm/plat-samsung/include/plat/pm.h |    6 +
 4 files changed, 179 insertions(+), 3 deletions(-)
arch/arm/mach-s3c64xx/Kconfig +1 -0
index 4d8c489..5c6c22e 100644
@@ -8,6 +8,7 @@ config PLAT_S3C64XX
 	bool
 	depends on ARCH_S3C64XX
 	select SAMSUNG_WAKEMASK
+	select PM_GENERIC_DOMAINS
 	default y
 	help
 	  Base platform code for any Samsung S3C64XX device
arch/arm/mach-s3c64xx/mach-crag6410.c +1 -1
index 98d42b3..4ce4a74 100644
@@ -743,7 +743,7 @@ static void __init crag6410_machine_init(void)
 
 	regulator_has_full_constraints();
 
-	s3c_pm_init();
+	s3c64xx_pm_init();
 }
 
 MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
arch/arm/mach-s3c64xx/pm.c +171 -2

205 lines not on this page · raw message

arch/arm/plat-samsung/include/plat/pm.h +6 -0
index dcf6870..a6bdee2 100644
@@ -22,6 +22,7 @@ struct sys_device;
 #ifdef CONFIG_PM
 
 extern __init int s3c_pm_init(void);
+extern __init int s3c64xx_pm_init(void);
 
 #else
 
@@ -29,6 +30,11 @@ static inline int s3c_pm_init(void)
 {
 	return 0;
 }
+
+static inline int s3c64xx_pm_init(void)
+{
+	return 0;
+}
 #endif
 
 /* configuration for the IRQ mask over sleep */
Signature · 1 line
1.7.7.3
#11 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Hi Mark,

I'm not sure what's the next step at s3c64xx for generic power domain. Related with exysno4 series, it's helpful to read following threads.
http://68.183.106.108/lists/linux-pm/msg26291.html

"I don't think we should control/gate the clocks with regarding power domain" from Mr. Kim

Thank you,
Kyungmin Park

Quoted 312 lines · Mark Brown

The S3C64xx SoCs contain a set of gateable power domains which can be enabled and disabled at runtime in order to save power. Use the generic power domain code to implement support for these in software, enabling runtime control of most domains:

- ETM (not supported in mainline).
- Domain G: 3D acceleration (no mainline support).
- Domain V: MFC (no mainline support).
- Domain I: JPEG and camera interface (no mainline support).
- Domain P: 2D acceleration, TV encoder and scaler (no mainline support)
- Domain S: Security (no mainline support).
- Domain F: LCD (driver already uses runtime PM), post processing and rotation (no mainline support).

The IROM domain is marked as always enabled as we should arrange for it to be enabled when we suspend which will need a bit more work.

Due to all the conditional device registration that the platform does wrap s3c_pm_init() with s3c64xx_pm_init() which actually puts the device into the power domain after the machines have registered, looking for platform data to tell if the device was registered. Since currently only Cragganmore actually sets up PM that is the only machine updated.

---

Kikjin, this superceeds my previous patch unconditionally disabling some of the domains. I've given it quite a bit of testing and it appears to be working well on my systems though I'm not sure the power saving is really all that much to write home about.

 arch/arm/mach-s3c64xx/Kconfig           |    1 +
 arch/arm/mach-s3c64xx/mach-crag6410.c   |    2 +-
 arch/arm/mach-s3c64xx/pm.c              |  173
++++++++++++++++++++++++++++++-
 arch/arm/plat-samsung/include/plat/pm.h |    6 +
 4 files changed, 179 insertions(+), 3 deletions(-)
arch/arm/mach-s3c64xx/Kconfig +1 -0
index 4d8c489..5c6c22e 100644
@@ -8,6 +8,7 @@ config PLAT_S3C64XX
 	bool
 	depends on ARCH_S3C64XX
 	select SAMSUNG_WAKEMASK
+	select PM_GENERIC_DOMAINS
 	default y
 	help
 	  Base platform code for any Samsung S3C64XX device
arch/arm/mach-s3c64xx/mach-crag6410.c +1 -1
index 98d42b3..4ce4a74 100644
@@ -743,7 +743,7 @@ static void __init crag6410_machine_init(void)
 
 	regulator_has_full_constraints();
 
-	s3c_pm_init();
+	s3c64xx_pm_init();
 }
 
 MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
arch/arm/mach-s3c64xx/pm.c +171 -2
index b375cd5..aa7b5d1 100644
@@ -17,10 +17,12 @@
 #include <linux/serial_core.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
+#include <linux/pm_domain.h>
 
 #include <mach/map.h>
 #include <mach/irqs.h>
 
+#include <plat/devs.h>
 #include <plat/pm.h>
 #include <plat/wakeup-mask.h>
 
@@ -31,6 +33,145 @@
 #include <mach/regs-gpio-memport.h>
 #include <mach/regs-modem.h>
 
+struct s3c64xx_pm_domain {
+	char *const name;
+	u32 ena;
+	u32 pwr_stat;
+	struct generic_pm_domain pd;
+};
+
+static int s3c64xx_pd_off(struct generic_pm_domain *domain)
+{
+	struct s3c64xx_pm_domain *pd;
+	u32 val;
+
+	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
+
+	val = __raw_readl(S3C64XX_NORMAL_CFG);
+	val &= ~(pd->ena);
+	__raw_writel(val, S3C64XX_NORMAL_CFG);
+
+	return 0;
+}
+
+static int s3c64xx_pd_on(struct generic_pm_domain *domain)
+{
+	struct s3c64xx_pm_domain *pd;
+	u32 val;
+	long retry = 1000000L;
+
+	pd = container_of(domain, struct s3c64xx_pm_domain, pd);
+
+	val = __raw_readl(S3C64XX_NORMAL_CFG);
+	val |= pd->ena;
+	__raw_writel(val, S3C64XX_NORMAL_CFG);
+
+	/* Not all domains provide power status readback */
+	if (pd->pwr_stat) {
+		while (retry--)
+			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
+				break;
+		if (!retry) {
+			pr_err("Failed to start domain %s\n", pd->name);
+			return -EBUSY;
+		}
+	}
+
+	return 0;
+}
+
+static struct s3c64xx_pm_domain s3c64xx_pm_irom = {
+	.name = "IROM",
+	.ena = S3C64XX_NORMALCFG_IROM_ON,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_etm = {
+	.name = "ETM",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_ETM_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_ETM,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_s = {
+	.name = "S",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_S_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_S,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_f = {
+	.name = "F",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_F_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_F,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_p = {
+	.name = "P",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_P_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_P,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_i = {
+	.name = "I",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_I_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_I,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_g = {
+	.name = "G",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_G_ON,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain s3c64xx_pm_v = {
+	.name = "V",
+	.ena = S3C64XX_NORMALCFG_DOMAIN_V_ON,
+	.pwr_stat = S3C64XX_BLKPWRSTAT_V,
+	.pd = {
+		.power_off = s3c64xx_pd_off,
+		.power_on = s3c64xx_pd_on,
+	},
+};
+
+static struct s3c64xx_pm_domain *s3c64xx_always_on_pm_domains[] = {
+	&s3c64xx_pm_irom,
+};
+
+static struct s3c64xx_pm_domain *s3c64xx_pm_domains[] = {
+	&s3c64xx_pm_etm,
+	&s3c64xx_pm_g,
+	&s3c64xx_pm_v,
+	&s3c64xx_pm_i,
+	&s3c64xx_pm_p,
+	&s3c64xx_pm_s,
+	&s3c64xx_pm_f,
+};
+
 #ifdef CONFIG_S3C_PM_DEBUG_LED_SMDK
 void s3c_pm_debug_smdkled(u32 set, u32 clear)
 {
@@ -89,6 +230,8 @@ static struct sleep_save misc_save[] = {
 
 	SAVE_ITEM(S3C64XX_SDMA_SEL),
 	SAVE_ITEM(S3C64XX_MODEM_MIFPCON),
+
+	SAVE_ITEM(S3C64XX_NORMAL_CFG),
 };
 
 void s3c_pm_configure_extint(void)
@@ -179,7 +322,26 @@ static void s3c64xx_pm_prepare(void)
 	__raw_writel(__raw_readl(S3C64XX_WAKEUP_STAT), S3C64XX_WAKEUP_STAT);
 }
 
-static int s3c64xx_pm_init(void)
+int __init s3c64xx_pm_init(void)
+{
+	int i;
+
+	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_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);
+
+	if (dev_get_platdata(&s3c_device_fb.dev))
+		pm_genpd_add_device(&s3c64xx_pm_f.pd, &s3c_device_fb.dev);
+
+	return 0;
+}
+
+static __init int s3c64xx_pm_initcall(void)
 {
 	pm_cpu_prep = s3c64xx_pm_prepare;
 	pm_cpu_sleep = s3c64xx_cpu_suspend;
@@ -198,5 +360,12 @@ static int s3c64xx_pm_init(void)
 
 	return 0;
 }
+arch_initcall(s3c64xx_pm_initcall);
+
+static __init int s3c64xx_pm_late_initcall(void)
+{
+	pm_genpd_poweroff_unused();
 
-arch_initcall(s3c64xx_pm_init);
+	return 0;
+}
+late_initcall(s3c64xx_pm_late_initcall);
arch/arm/plat-samsung/include/plat/pm.h +6 -1
index dcf6870..a6bdee2 100644
@@ -22,6 +22,7 @@ struct sys_device;
 #ifdef CONFIG_PM
 
 extern __init int s3c_pm_init(void);
+extern __init int s3c64xx_pm_init(void);
 
 #else
 
@@ -29,6 +30,11 @@ static inline int s3c_pm_init(void)
 {
 	return 0;
 }
+
+static inline int s3c64xx_pm_init(void)
+{
+	return 0;
+}
 #endif
 
 /* configuration for the IRQ mask over sleep */
--

1.7.7.3

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

author
#12 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 6 lines · Kyungmin Park

I'm not sure what's the next step at s3c64xx for generic power domain. Related with exysno4 series, it's helpful to read following threads.
http://68.183.106.108/lists/linux-pm/msg26291.html

"I don't think we should control/gate the clocks with regarding power domain" from Mr. Kim

I tend to agree with him that gating the clocks automatically using runtime PM would be nice as if nothing else it saves code in drivers. I'm not sure how easy it's going to be to transition to doing that in one jump, though - there's a lot of SoCs sharing IP right back to S3C24xx.

Of course it should be fairly easy for most of the domains on s3c64xx right now as there's only one actual driver involved in mainline, we should just be gating almost all of the clocks concerned immediately after we boot (assuming they aren't gated by default, I'd need to check) and never touching them again anyway. It'd only be the framebuffer clocks that'd need some work and for a first pass we could just mark the framebuffer domain as always on and get most of the win. Sometimes lack of driver support is a good thing :)

#13 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Hi,

W dniu 2 grudnia 2011 01:56 u?ytkownik Mark Brown
<[email protected]> napisa?:

Quoted 15 lines · Kyungmin Park
Quoted 7 lines

I'm not sure what's the next step at s3c64xx for generic power domain. Related with exysno4 series, it's helpful to read following threads.
http://68.183.106.108/lists/linux-pm/msg26291.html

"I don't think we should control/gate the clocks with regarding power domain" from Mr. Kim

I tend to agree with him that gating the clocks automatically using runtime PM would be nice as if nothing else it saves code in drivers. I'm not sure how easy it's going to be to transition to doing that in one jump, though - there's a lot of SoCs sharing IP right back to S3C24xx.

Quoted 8 lines

Of course it should be fairly easy for most of the domains on s3c64xx right now as there's only one actual driver involved in mainline, we should just be gating almost all of the clocks concerned immediately after we boot (assuming they aren't gated by default, I'd need to check) and never touching them again anyway. It'd only be the framebuffer clocks that'd need some work and for a first pass we could just mark the framebuffer domain as always on and get most of the win. Sometimes lack of driver support is a good thing :)

Please do not forget that there might be some drivers not yet submited to mainline and mainline should not break them with an assumption that there are no such drivers.

For example, there is an on-going work on an open source OpenGL implementation for the GPU in S3C6410, known as OpenFIMG. Currently it uses a little kernel module for low level hardware management (interrupts, contexts, locking, power management), involving clock gating and runtime power management, but a DRM driver is planned.

Best regards,
Tom

author
#14 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 9 lines · Tomasz Figa

Please do not forget that there might be some drivers not yet submited to mainline and mainline should not break them with an assumption that there are no such drivers.

For example, there is an on-going work on an open source OpenGL implementation for the GPU in S3C6410, known as OpenFIMG. Currently it uses a little kernel module for low level hardware management (interrupts, contexts, locking, power management), involving clock gating and runtime power management, but a DRM driver is planned.

As I said in the commit message for the patch you're following up on I don't think that's a problem as it's pretty straightforward to add the requisite hooks as part of adding the actual drivers. The power domain framework is really easy to use so it's not like it should cause real effort, if you can figure out how to drive a 3D graphics engine adding a device to a power domain shouldn't be too hard.

There will need to be arch/arm changes to add the platform devices and hook up the clocks anyway so it's not like this is creating a need for modifications. Indeed I'd suggest adding the arch/arm stuff as it gets written anyway, even if the actual drivers are still a work in progress. It's one less thing to carry out of tree and it means that updates like this or like the clk API reworks that I'd expect to start appearing soon will happen for free.

#15 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 18 lines · On Friday 02 of December 2011 18:57:43 Mark Brown wrote:
Quoted 9 lines · Tomasz Figa

Please do not forget that there might be some drivers not yet submited to mainline and mainline should not break them with an assumption that there are no such drivers.

For example, there is an on-going work on an open source OpenGL implementation for the GPU in S3C6410, known as OpenFIMG. Currently it uses a little kernel module for low level hardware management (interrupts, contexts, locking, power management), involving clock gating and runtime power management, but a DRM driver is planned.

As I said in the commit message for the patch you're following up on I don't think that's a problem as it's pretty straightforward to add the requisite hooks as part of adding the actual drivers. The power domain framework is really easy to use so it's not like it should cause real effort, if you can figure out how to drive a 3D graphics engine adding a device to a power domain shouldn't be too hard.

Quoted 7 lines

There will need to be arch/arm changes to add the platform devices and hook up the clocks anyway so it's not like this is creating a need for modifications. Indeed I'd suggest adding the arch/arm stuff as it gets written anyway, even if the actual drivers are still a work in progress. It's one less thing to carry out of tree and it means that updates like this or like the clk API reworks that I'd expect to start appearing soon will happen for free.

Yeah, I'm okay with that, I just noted that there is more support for S3C6410 available than just what is present in mainline.

Best regards,
Tom

#16 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

In reply to Mark Brown

Hi Mark,

good to see someone adding a proper power domain support for s3c64xx.

Quoted 25 lines · Mark Brown

The S3C64xx SoCs contain a set of gateable power domains which can be enabled and disabled at runtime in order to save power. Use the generic power domain code to implement support for these in software, enabling runtime control of most domains:

- ETM (not supported in mainline).
- Domain G: 3D acceleration (no mainline support).
- Domain V: MFC (no mainline support).
- Domain I: JPEG and camera interface (no mainline support).
- Domain P: 2D acceleration, TV encoder and scaler (no mainline support)
- Domain S: Security (no mainline support).
- Domain F: LCD (driver already uses runtime PM), post processing and rotation (no mainline support).

The IROM domain is marked as always enabled as we should arrange for it to be enabled when we suspend which will need a bit more work.

Due to all the conditional device registration that the platform does wrap s3c_pm_init() with s3c64xx_pm_init() which actually puts the device into the power domain after the machines have registered, looking for platform data to tell if the device was registered. Since currently only Cragganmore actually sets up PM that is the only machine updated.

---

...

Quoted 8 lines
arch/arm/mach-s3c64xx/pm.c +8 -0
+
+static int s3c64xx_pd_on(struct generic_pm_domain *domain)
+{
+	struct s3c64xx_pm_domain *pd;
+	u32 val;
+	long retry = 1000000L;
+
+	pd = container_of(domain, struct s3c64xx_pm_domain, pd);

arch/arm/mach-s3c64xx/pm.c line 73

Quoted 10 lines
arch/arm/mach-s3c64xx/pm.c +10 -0
+
+	val = __raw_readl(S3C64XX_NORMAL_CFG);
+	val |= pd->ena;
+	__raw_writel(val, S3C64XX_NORMAL_CFG);
+
+	/* Not all domains provide power status readback */
+	if (pd->pwr_stat) {
+		while (retry--)
+			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
+				break;

How about adding cpu_relax() in this busy wait loop ?

arch/arm/mach-s3c64xx/pm.c line 81

Quoted 8 lines
arch/arm/mach-s3c64xx/pm.c +8 -0
+		if (!retry) {
+			pr_err("Failed to start domain %s\n", pd->name);
+			return -EBUSY;
+		}
+	}
+
+	return 0;
+}

--

Regards,
Sylwester

author
#17 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 7 lines · Sylwester Nawrocki

arch/arm/mach-s3c64xx/pm.c line 73

Quoted 5 lines
arch/arm/mach-s3c64xx/pm.c +5 -0
+	/* Not all domains provide power status readback */
+	if (pd->pwr_stat) {
+		while (retry--)
+			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
+				break;

How about adding cpu_relax() in this busy wait loop ?

May as well, yes - I didn't actually measure how long it tends to take to do the spin but it's not going to hurt.

#18 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 10 lines · Mark Brown
Quoted 7 lines · Sylwester Nawrocki

arch/arm/mach-s3c64xx/pm.c line 73

Quoted 5 lines
arch/arm/mach-s3c64xx/pm.c +5 -0
+	/* Not all domains provide power status readback */
+	if (pd->pwr_stat) {
+		while (retry--)
+			if (__raw_readl(S3C64XX_BLK_PWR_STAT) & pd->pwr_stat)
+				break;

How about adding cpu_relax() in this busy wait loop ?

Quoted 2 lines

May as well, yes - I didn't actually measure how long it tends to take to do the spin but it's not going to hurt.

Are you going to post an updated patch?

Rafael

author
#19 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 6 lines · Rafael J. Wysocki
Quoted 2 lines · Mark Brown

May as well, yes - I didn't actually measure how long it tends to take to do the spin but it's not going to hurt.

Are you going to post an updated patch?

I've resent it but my development laptop is currently not internet connected so I'm not sure when it'll actually go out.

#20 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 10 lines · Mark Brown
Quoted 6 lines · Rafael J. Wysocki
Quoted 2 lines · Mark Brown

May as well, yes - I didn't actually measure how long it tends to take to do the spin but it's not going to hurt.

Are you going to post an updated patch?

I've resent it but my development laptop is currently not internet connected so I'm not sure when it'll actually go out.

Hmm. I can't find it in my e-mail archives.

Do you have any pointer to a web archive somewhere, where I can get the patch from?

Rafael

#21 lore raw

PATCH 3/3 ARM: S3C64XX: Implement basic power domain support

Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Mark Brown
Quoted 6 lines · Rafael J. Wysocki
Quoted 2 lines · Mark Brown

May as well, yes - I didn't actually measure how long it tends to take to do the spin but it's not going to hurt.

Are you going to post an updated patch?

Quoted 7 lines
Quoted 2 lines

I've resent it but my development laptop is currently not internet connected so I'm not sure when it'll actually go out.

Hmm. I can't find it in my e-mail archives.

Do you have any pointer to a web archive somewhere, where I can get the patch from?

Ah, I've just received it.

Thanks,
Rafael

#22 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

In reply to Mark Brown
Quoted 8 lines · Mark Brown

Since systems are likely to have power domains that can't be turned off for various reasons at least temporarily while implementing power domain support provide a default governor which will always refuse to power off the domain, saving platforms having to implement their own.

Since the code is so tiny don't bother with a Kconfig symbol for it.

Looks good with the new version of [2/3].

Do you want me to takt it?

Rafael

Quoted 47 lines
---
 drivers/base/power/domain_governor.c |   13 +++++++++++++
 include/linux/pm_domain.h            |    2 ++
 2 files changed, 15 insertions(+), 0 deletions(-)
drivers/base/power/domain_governor.c +13 -0
index da78540..51527ee 100644
@@ -141,3 +141,16 @@ struct dev_power_governor simple_qos_governor = {
 	.stop_ok = default_stop_ok,
 	.power_down_ok = default_power_down_ok,
 };
+
+static bool always_on_power_down_ok(struct dev_pm_domain *domain)
+{
+	return false;
+}
+
+/**
+ * pm_genpd_gov_always_on - A governor implementing an always-on policy
+ */
+struct dev_power_governor pm_domain_always_on_gov = {
+	.power_down_ok = always_on_power_down_ok,
+	.stop_ok = default_stop_ok,
+};
include/linux/pm_domain.h +2 -0
index fbb81bc..fda14f1 100644
@@ -139,6 +139,7 @@ extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
 
 extern bool default_stop_ok(struct device *dev);
 
+extern struct dev_power_governor pm_domain_always_on_gov;
 #else
 
 static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
@@ -192,6 +193,7 @@ static inline bool default_stop_ok(struct device *dev)
 {
 	return false;
 }
+#define pm_domain_always_on_gov NULL
 #endif
 
 static inline int pm_genpd_remove_callbacks(struct device *dev)
author
#23 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 7 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown

Looks good with the new version of [2/3].

Do you want me to takt it?

It might make sense for it to go via the Samsung tree - the third patch depends on it and there's some other stuff going on with the s3c64xx power management - so let's see what Kukjin thinks is best.

#24 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 5 lines · Mark Brown
Quoted 3 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown
Quoted 7 lines
Quoted 3 lines

Looks good with the new version of [2/3].

Do you want me to takt it?

It might make sense for it to go via the Samsung tree - the third patch depends on it and there's some other stuff going on with the s3c64xx power management - so let's see what Kukjin thinks is best.

OTOH, it will conflict with some patches I have in the works.

author
#25 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 7 lines · Rafael J. Wysocki
Quoted 3 lines · Mark Brown

It might make sense for it to go via the Samsung tree - the third patch depends on it and there's some other stuff going on with the s3c64xx power management - so let's see what Kukjin thinks is best.

OTOH, it will conflict with some patches I have in the works.

Ah, OK - in that case I guess it's sensible if you apply it. Ideally it could be on a separate branch so it could be cross-merged with other trees if there's a need later.

#26 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 6 lines · Mark Brown
Quoted 3 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown

It might make sense for it to go via the Samsung tree - the third

patch

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

depends on it and there's some other stuff going on with the s3c64xx power management - so let's see what Kukjin thinks is best.

OTOH, it will conflict with some patches I have in the works.

Ah, OK - in that case I guess it's sensible if you apply it. Ideally it could be on a separate branch so it could be cross-merged with other trees if there's a need later.

Hi Mark and Rafael,

Looks ok to me on this series and would be nice to me if Rafael could create topic branch in his tree and apply this series with my ack for Samsung stuff.

Rafael, please let me know the branch when you create it so that I can merge it into Samsung tree to avoid other conflicts.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#27 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Hello,

Quoted 16 lines · Kukjin Kim
Quoted 6 lines · Mark Brown
Quoted 3 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown

It might make sense for it to go via the Samsung tree - the third

patch

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

depends on it and there's some other stuff going on with the s3c64xx power management - so let's see what Kukjin thinks is best.

OTOH, it will conflict with some patches I have in the works.

Ah, OK - in that case I guess it's sensible if you apply it. Ideally it could be on a separate branch so it could be cross-merged with other trees if there's a need later.

Quoted 8 lines

Hi Mark and Rafael,

Looks ok to me on this series and would be nice to me if Rafael could create topic branch in his tree and apply this series with my ack for Samsung stuff.

Rafael, please let me know the branch when you create it so that I can merge it into Samsung tree to avoid other conflicts.

What about similar patches for S5PV210/S5PC110 series and Exynos4? gen_pd based power domain code for Exynos4 have been rejected in favor of custom platform-device based power domain drivers. It would be much easier to have only one type of the drivers across different Samsung SoCs. This will also help to hide some differences between the series from the device drivers (clock gating, power management).

Best regards

Signature · 2 lines
Marek Szyprowski
Samsung Poland R&D Center
author
#28 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

On Tue, Dec 06, 2011 at 01:04:53PM +0100, Marek Szyprowski wrote:

Please fix your mailer to word wrap at less than 80 columns, I've reflowed your text for legibility.

Quoted 1 line

What about similar patches for S5PV210/S5PC110 series and Exynos4?

I don't really have access to these systems (well, I have a Nexus S I can use but that doesn't boot mainline) and I don't have datasheets so it's hard for me to actively work on them myself.

Quoted 2 lines

gen_pd based power domain code for Exynos4 have been rejected in favor of custom platform-device based power domain drivers. It would be much

Oh, that's very surprising to me - looking at the code it appeared that it just predated gen_pd, I didn't see anything in there that suggested an active decision to avoid using it. What was then thinking there? Is this something we can fix by enhancing gen_pd?

Quoted 3 lines

easier to have only one type of the drivers across different Samsung SoCs. This will also help to hide some differences between the series from the device drivers (clock gating, power management).

When I looked at the code I'd expected the other SoCs to also transition over to gen_pd and then start pulling things out from there.

Looking at what you can do on s3c64xx the power domain code looked very small once you use the gen_pd stuff (which is very nice to use, it really makes the whole thing very easy) so I'm not sure how much win it is immediately. How much win it is long term will depend on where things get implemented - a lot of the stuff I could tink of doing seemed like it might be more of a fit for the opp or PM QoS stuff than for the power domains, with the power domains staying pretty thin and just providing inputs but perhaps that's not a sensible approach.

The main trick is going to be actually putting the devices into the power domains which is entertaining as currently every board directly registers platform devices so it's hard for core code to work out what Linux knows about.

In terms of the drivers I wasn't expecting to see much impact as the power domain and runtime PM APIs provide a pretty good abstraction already - the actual implementation of things is totally transparent to them. The only device actively able to use the power domains on the S3C6410 is the framebuffer and the driver there just worked with the existing runtime PM code.

#29 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

In reply to Kukjin Kim
Quoted 16 lines · Kukjin Kim
Quoted 6 lines · Mark Brown
Quoted 3 lines · Rafael J. Wysocki
Quoted 1 line · Mark Brown

It might make sense for it to go via the Samsung tree - the third

patch

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

depends on it and there's some other stuff going on with the s3c64xx power management - so let's see what Kukjin thinks is best.

OTOH, it will conflict with some patches I have in the works.

Ah, OK - in that case I guess it's sensible if you apply it. Ideally it could be on a separate branch so it could be cross-merged with other trees if there's a need later.

Quoted 8 lines

Hi Mark and Rafael,

Looks ok to me on this series and would be nice to me if Rafael could create topic branch in his tree and apply this series with my ack for Samsung stuff.

Rafael, please let me know the branch when you create it so that I can merge it into Samsung tree to avoid other conflicts.

I need an ack from Magnus or Paul on the shmobile patch. When I get one, I'll put patches [1/3] and [2/3] (new version) into my linux-next branch and then (after a few days spent in linux-next) to the pm-domains branch. I'll let you know when that happens.

Thanks,
Rafael

author
#30 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 9 lines · Rafael J. Wysocki
Quoted 2 lines · Kukjin Kim

Rafael, please let me know the branch when you create it so that I can merge it into Samsung tree to avoid other conflicts.

I need an ack from Magnus or Paul on the shmobile patch. When I get one, I'll put patches [1/3] and [2/3] (new version) into my linux-next branch and then (after a few days spent in linux-next) to the pm-domains branch. I'll let you know when that happens.

Well, I guess you could put patch 1 and 3 in then add 2 if an ack is forthcoming for that - there's no direct dependency.

#31 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 6 lines · Mark Brown
Quoted 4 lines · Rafael J. Wysocki
Quoted 2 lines · Kukjin Kim

Rafael, please let me know the branch when you create it so that I can merge it into Samsung tree to avoid other conflicts.

Quoted 7 lines
Quoted 4 lines

I need an ack from Magnus or Paul on the shmobile patch. When I get one, I'll put patches [1/3] and [2/3] (new version) into my linux-next branch and then (after a few days spent in linux-next) to the pm-domains branch. I'll let you know when that happens.

Well, I guess you could put patch 1 and 3 in then add 2 if an ack is forthcoming for that - there's no direct dependency.

It's been acked already, I'm going to pul all three patches into linux-pm/linux-next later today.

Thanks,
Rafael

author
#32 lore raw

PATCH 1/3 PM: Provide an always on power domain governor

Quoted 7 lines · Rafael J. Wysocki
Quoted 2 lines · Mark Brown

Well, I guess you could put patch 1 and 3 in then add 2 if an ack is forthcoming for that - there's no direct dependency.

It's been acked already, I'm going to pul all three patches into linux-pm/linux-next later today.

Oh, OK - I didn't seem to get copied on any of the acks or at least I didn't see them.