Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Thursday, December 08, 2011, Mark Brown wrote: On Wed, Dec 07, 2011 at 10:44:02PM +0100, Rafael J. Wysocki wrote: On Friday, December 02, 2011, Mark Brown wrote: 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 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Thursday, December 08, 2011, Rafael J. Wysocki wrote: On Thursday, December 08, 2011, Mark Brown wrote: On Wed, Dec 07, 2011 at 10:44:02PM +0100, Rafael J. Wysocki wrote: On Friday, December 02, 2011, Mark Brown wrote: 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? Ah, I've just received it. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Friday, December 02, 2011, Mark Brown wrote:
On Fri, Dec 02, 2011 at 09:10:27PM +0100, Sylwester Nawrocki wrote:
+ /* 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.
Are you going to post an updated patch?
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Wed, Dec 07, 2011 at 10:44:02PM +0100, Rafael J. Wysocki wrote: On Friday, December 02, 2011, Mark Brown wrote: 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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
Hi, W dniu 2 grudnia 2011 01:56 użytkownik Mark Brown broo...@opensource.wolfsonmicro.com napisał: On Fri, Dec 02, 2011 at 09:35:44AM +0900, Kyungmin Park wrote: 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 :) 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 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Fri, Dec 02, 2011 at 07:25:01PM +0100, Tomasz Figa wrote: 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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
Hi Mark,
good to see someone adding a proper power domain support for s3c64xx.
On 12/01/2011 07:48 PM, Mark Brown wrote:
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.
Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
---
...
+
+static int s3c64xx_pd_on(struct generic_pm_domain *domain)
+{
+ struct s3c64xx_pm_domain *pd;
+ u32 val;
+ long retry = 100L;
+
+ 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;
How about adding cpu_relax() in this busy wait loop ?
+ if (!retry) {
+ pr_err(Failed to start domain %s\n, pd-name);
+ return -EBUSY;
+ }
+ }
+
+ return 0;
+}
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Fri, Dec 02, 2011 at 09:10:27PM +0100, Sylwester Nawrocki wrote:
+ /* 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.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [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
On 12/2/11, Mark Brown broo...@opensource.wolfsonmicro.com wrote:
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.
Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
---
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(-)
diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig
index 4d8c489..5c6c22e 100644
--- a/arch/arm/mach-s3c64xx/Kconfig
+++ b/arch/arm/mach-s3c64xx/Kconfig
@@ -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
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c
b/arch/arm/mach-s3c64xx/mach-crag6410.c
index 98d42b3..4ce4a74 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
@@ -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)
diff --git a/arch/arm/mach-s3c64xx/pm.c b/arch/arm/mach-s3c64xx/pm.c
index b375cd5..aa7b5d1 100644
--- a/arch/arm/mach-s3c64xx/pm.c
+++ b/arch/arm/mach-s3c64xx/pm.c
@@ -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 = 100L;
+
+ 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
Re: [PATCH 3/3] ARM: S3C64XX: Implement basic power domain support
On Fri, Dec 02, 2011 at 09:35:44AM +0900, Kyungmin Park wrote: 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 :) -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
