diff mbox

sh: hibernation support

Message ID aec7e5c30903102126y53e0ee09r332ac3409f41329c@mail.gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Magnus Damm March 11, 2009, 4:26 a.m. UTC
Hi Francesco,

On Tue, Mar 10, 2009 at 10:19 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> Hi Magnus
> In the clock framework I would propose the attached solution

Thanks for your help! I tested your patch but I need the attached fix.

Can you please include my fix and resubmit inline? Feel free to add an
ack from me.

Thanks!

/ magnus

Comments

Francesco VIRLINZI March 11, 2009, 6:50 a.m. UTC | #1
Hi Magnus.

> Thanks for your help! I tested your patch but I need the attached fix.
>
> Can you please include my fix and resubmit inline? Feel free to add an
> ack from me.
>   
Ok.
While we are 'fixing' the clock framework on resume from hibernation.
I'd like to do some similar thing on suspend/resume from standby.
I was thinking to add some flags/capability able to say to the framework 
what it has to be done in standby.
Something like:

#define CLK_TURNOFF_ON_SUSPEND   ....
#define CLK_REDUCE_ON_SUSPEND ...

In this manner in the single file where the clock tree is described we 
have a clear view of all the scenario
 (normal runtime, suspend, hibernation)...
Moreover this should not impact "medium" power level.
Regards
 Francesco
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Magnus Damm March 11, 2009, 7:29 a.m. UTC | #2
Hi again Francesco,

On Wed, Mar 11, 2009 at 3:50 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
> While we are 'fixing' the clock framework on resume from hibernation.
> I'd like to do some similar thing on suspend/resume from standby.

Good plan! I'd like to have this working for suspend as well.

> I was thinking to add some flags/capability able to say to the framework
> what it has to be done in standby.
> Something like:
>
> #define CLK_TURNOFF_ON_SUSPEND   ....
> #define CLK_REDUCE_ON_SUSPEND ...
>
> In this manner in the single file where the clock tree is described we have
> a clear view of all the scenario
> (normal runtime, suspend, hibernation)...
> Moreover this should not impact "medium" power level.

I guess your goal is to minimize power consumption by turning off
unused clocks or at least set them to a minimal value before
suspending. I'd like to do something like that at least. For the clock
framework i think we should simply minimize the clock when the usage
count drops to zero. And let the drivers disable the clock as long as
it's not needed for wakeup.

If you're talking about frequency control then maybe cpufreq is a good
option. I'm not sure about this yet, it's still on my TODO list.

For the overall system my gut feeling is that it would be useful to
define a set of standby modes, and let the suspend code or cpuidle
enter the best mode possible after checking dependencies.

Right now I have the following modes setup for SuperH Mobile:

#define SUSP_MODE_SLEEP                (SUSP_SH_SLEEP)
#define SUSP_MODE_SLEEP_SF     (SUSP_SH_SLEEP | SUSP_SH_SF)
#define SUSP_MODE_STANDBY_SF   (SUSP_SH_STANDBY | SUSP_SH_SF)

The first is just plain old light sleep using the sleep instruction -
nothing exciting.

The second is a bit better since it puts the system ram in
self-refresh mode but the processor is still sleeping lightly. Before
entering this mode the code needs to make sure that no hardware is
using the system memory, ie we cannot enter this if some hardware is
bus mastering. We can however enter this mode waiting for incoming
data on a SCIF.

The third mode turns off the clock so system ram self-refresh is
needed. We cannot enter this mode if hardware blocks that use the
clocks are active. Or if we need the clock enabled to be able to
wakeup. =)

There is one additional level that turns off the power to the hardware
blocks as well. This mode is close to hibernation from a software
point of view. I guess a good match is to use dev_pm_ops->freeze to
save hardware state before suspending and use thaw to restore state
when resuming. Obviously we can't power off hardware blocks needed for
wakeup.

How is this compared to your architecture(s)?
Are you using cpuidle?
What about cpufreq?
Do you have some profiles setup for run time power management today?

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Francesco VIRLINZI March 11, 2009, 1:20 p.m. UTC | #3
Hi Magnus
>
> Good plan! I'd like to have this working for suspend as well.
Good.
>
> I guess your goal is to minimize power consumption by turning off
> unused clocks or at least set them to a minimal value before
> suspending.
Yes.
>  I'd like to do something like that at least. For the clock
> framework i think we should simply minimize the clock when the usage
> count drops to zero. And let the drivers disable the clock as long as
> it's not needed for wakeup.
Unfortunately it isn't so easy.
If we have one-to-one clock per device a simple counter is enough, but 
if you have shared clocks (i.e. a clock X shared between a two wakeup 
devices than on suspend the first device could asks "clock_X @ xx MHz" 
the second device could asks "clock_X @ yy MHz"  and as platform device  
the final rate of clock X will be based on how the devices were 
registered...
Moreover without a notification of the rate of each clock a wakeup 
device will be broken and it doesn't know it is broken.

> ...
> For the overall system my gut feeling is that it would be useful to
> define a set of standby modes, and let the suspend code or cpuidle
> enter the best mode possible after checking dependencies.
Not so easy for us because we have some A-SMP therefore an "idle linux" 
doesn't mean an idle hardware.
>
> Right now I have the following modes setup for SuperH Mobile:
>
> #define SUSP_MODE_SLEEP                (SUSP_SH_SLEEP)
> #define SUSP_MODE_SLEEP_SF     (SUSP_SH_SLEEP | SUSP_SH_SF)
> #define SUSP_MODE_STANDBY_SF   (SUSP_SH_STANDBY | SUSP_SH_SF)
>
>
> There is one additional level that turns off the power to the hardware
> blocks as well. This mode is close to hibernation from a software
> point of view.
Yes also us we plan something like that.
>  I guess a good match is to use dev_pm_ops->freeze to
> save hardware state before suspending and use thaw to restore state
> when resuming.

>  Obviously we can't power off hardware blocks needed for
> wakeup.
>
> How is this compared to your architecture(s)?
On  SUSP_MODE_SLEEP_SF and SUSP_MODE_STANDBY_SF.
If I understood you are speaking on 'sleep' and 'deep sleep' capability 
of sh4. In this case in the ST40 this difference was removed.
But the behavioral is similar (i.e.: mem in self-refresh, clocks 
disabled/reduced... wakeup devices still able to works).

> Are you using cpuidle?
Not currently and I think it will be not so easy to use for us.
> What about cpufreq?
Supported.
> Do you have some profiles setup for run time power management today?
We have a kernel tool to create "power profile" sits on top of 
cpufreq-clockfm-device model and until possible (currently possible) it 
doesn't touch any device driver; this means the device aren't aware of a 
change in term of power profile... they see change in term of 
clock_rate-device_state.

Regards
 Francesco
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Magnus Damm March 12, 2009, 5:47 a.m. UTC | #4
On Wed, Mar 11, 2009 at 10:20 PM, Francesco VIRLINZI
<francesco.virlinzi@st.com> wrote:
>>  I'd like to do something like that at least. For the clock
>> framework i think we should simply minimize the clock when the usage
>> count drops to zero. And let the drivers disable the clock as long as
>> it's not needed for wakeup.
>
> Unfortunately it isn't so easy.
> If we have one-to-one clock per device a simple counter is enough, but if
> you have shared clocks (i.e. a clock X shared between a two wakeup devices
> than on suspend the first device could asks "clock_X @ xx MHz" the second
> device could asks "clock_X @ yy MHz"  and as platform device  the final rate
> of clock X will be based on how the devices were registered...

I understand the problem with the ordering. I wonder if converting
your shared clock into one parent clock and two children would help?
Each device connects to one child clock, and when requesting a clock
rate this rate gets stored in each child clock. The parent simply
determines the clock by MAX(c1, c2) where c1 and c2 are the child
clock rates.

> Moreover without a notification of the rate of each clock a wakeup device
> will be broken and it doesn't know it is broken.

Yeah, notification is needed.

>> ...
>> For the overall system my gut feeling is that it would be useful to
>> define a set of standby modes, and let the suspend code or cpuidle
>> enter the best mode possible after checking dependencies.
>
> Not so easy for us because we have some A-SMP therefore an "idle linux"
> doesn't mean an idle hardware.

Sure, but someone needs to be in control of power management. Even on
single core systems "idle linux" doesn't mean idle hardware. Say that
some hardware engine is programmed to crunch some data. The cpu is
idle. We cannot enter deep sleep because the hardware engine needs the
clock. But after the hardware engine is done it will issue an
interrupt, and the processor can wake up, do it's thing and go back to
sleep - deep sleep this time since the hardware engine isn't active.

I think the case above is very similar to multi core power management.
If you have time, please explain more about issues you've experienced
related to A-SMP.

In my mind, at least each independent domain needs some central point
(master) of power management. So if some processor would be a slave,
how about notifying the master before sleeping and let the master
handle the power management. This action may be the last part of some
application processor code where the master writes commands to an
mailbox and receives the results and get an interrupt as notification.

In the case above the application processor can sleep after that and
the power management code in the main processor will know this state.
I guess it becomes more difficult if the application processor
receives an interrupt from some other source and wakes up, but the
main processor needs to take the system ram out of self-refresh before
the application processor can start serving the interrupt..

> If I understood you are speaking on 'sleep' and 'deep sleep' capability of
> sh4. In this case in the ST40 this difference was removed.
> But the behavioral is similar (i.e.: mem in self-refresh, clocks
> disabled/reduced... wakeup devices still able to works).

The states that I mentioned are from SuperH Mobile. "sleep" and "deep
sleep" are black and white terms in a very gray world, so I should do
my best to stay away from them in the future. Other SH devices from
Renesas also do different levels of power management. In the end I
think this stuff is pretty standard all over the place, except the
clock dependencies that differ from architecture to architecture and
between processor families.

>> Are you using cpuidle?
>
> Not currently and I think it will be not so easy to use for us.

Because of A-SMP?
I have a cpuidle prototype running, but I need to improve the clock
framework code to track dependencies better before I can integrate it
upstream.

>> What about cpufreq?
>
> Supported.

Still on my TODO-list. The SuperH Mobile devices have quite a few
restrictions on which divider combinations that are allowed for the
system clocks. So if someone changes the processor clock for instance,
many other clocks may need to follow.

>> Do you have some profiles setup for run time power management today?
>
> We have a kernel tool to create "power profile" sits on top of
> cpufreq-clockfm-device model and until possible (currently possible) it
> doesn't touch any device driver; this means the device aren't aware of a
> change in term of power profile... they see change in term of
> clock_rate-device_state.

I see. Some partial system suspend (some devices suspended only) would
be good to have, don't you think?

Thanks for your comments!

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Francesco VIRLINZI March 12, 2009, 8:54 a.m. UTC | #5
Hi Magnus
>
> Yeah, notification is needed.
... Yes unfortunately...
>
>>> Are you using cpuidle?
>> Not currently and I think it will be not so easy to use for us.
>
> Because of A-SMP?
Yes
> I have a cpuidle prototype running, but I need to improve the clock
> framework code to track dependencies better before I can integrate it
> upstream.
Interesting when available I will see it.
>
>>> What about cpufreq?
>> Supported.
>
> Still on my TODO-list. The SuperH Mobile devices have quite a few
> restrictions on which divider combinations that are allowed for the
> system clocks. So if someone changes the processor clock for instance,
> many other clocks may need to follow.
Yes I understand... the famous sh4_clk/sh4_per/module_clk ratios..) ...
We still have some platform with these constraints...
What I can say you is that the timer-tmu.c is already ready for this kind
 of operation and you will have no impact on the system timer or No_HZ 
system.

>
> I see. Some partial system suspend (some devices suspended only) would
> be good to have, don't you think?
Yes I agree. But I prefer a tool to leave the people free to create 
their own 'profile'.

Ciao
 Francesco
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- 0003/arch/sh/kernel/cpu/clock.c
+++ work/arch/sh/kernel/cpu/clock.c	2009-03-11 12:36:44.000000000 +0900
@@ -369,7 +369,7 @@  static int clks_resume(struct sys_device
 }
 
 static struct sysdev_class clks_class = {
-	.kset.kobj.name = "clks",
+	.name = "clks",
 };
 
 static struct sysdev_driver clks_driver = {
@@ -405,10 +405,10 @@  int __init clk_init(void)
 
 static int __init clk_sysdev_init(void)
 {
-
 	sysdev_class_register(&clks_class);
 	sysdev_driver_register(&clks_class, &clks_driver);
 	sysdev_register(&clks_dev);
+
 	return 0;
 }
 subsys_initcall(clk_sysdev_init);