Hi Chanwoo Choi,
On 2016年11月24日 16:16, Chanwoo Choi wrote:
Hi Lin,
On 2016년 11월 24일 16:34, hl wrote:
Hi Chanwoo Choi,
I think the dev_pm_opp_get_suspend_opp() have implement most of
the funtion, all we need is just define the node in dts, like following:
&dmc_opp_table {
opp06 {
opp-suspend;
};
};
Two approaches use the 'opp-suspend' property.
I think that the method to support suspend-opp have to
guarantee following conditions:
- Support the all of devfreq's governors.
As MyungJoo Ham suggestion, i will set the suspend frequency in
devfreq_suspend_device(),
which will ingore governor.
- Devfreq framework have the responsibility to change the
frequency/voltage for suspend-opp. If we uses the
new devfreq_suspend(), each devfreq device don't care
how to support the suspend-opp. Just the developer of each
devfreq device need to add 'opp-suspend' propet to OPP entry in DT file.
Why should support change the voltage in devfreq framework, i think it
shuold be handle in
specific driver, i think the devfreq only handle it can get the right
frequency, then pass it to
specific driver, i think the voltage should handle in the
devfreq->profile->target();
Best Regards,
Chanwoo Choi
so i think my way semm more simple.
On 2016年11月24日 15:10, Chanwoo Choi wrote:
+ Tobias Jakobi,
Hi Lin,
We need to discuss how to support the suspend-opp of devfreq device.
Now, there are two patch thread for suspend-opp of devfreq.
The Lin's approach modify the devfreq_suspend_device() to support suspend-opp.
The Tobias's approach[1] add new devfreq_suspend() and then call it on
dpm_suspend()
when entering the suspend state.
[1] [RFC 0/4] PM / devfreq: draft for OPP suspend impl
- https://patchwork.kernel.org/patch/9443323/
- https://patchwork.kernel.org/patch/9443325/
- https://patchwork.kernel.org/patch/9443329/
- https://patchwork.kernel.org/patch/9443331/
I think we need to discuss it together.
Regards,
Chanwoo Choi
On 2016년 11월 24일 15:45, hl wrote:
Hi MyungJoo Ham,
On 2016年11月24日 14:14, MyungJoo Ham wrote:
On Thu, Nov 24, 2016 at 11:18 AM, hl <[email protected]> wrote:
Hi MyungJoo Ham,
[]
We still need to sync the all status even i call target() in
devfreq_suspend/resume_device
directly, so still need update_devfreq() other setp except
devfreq->governor->get_target_freq(devfreq, &freq);
And i think it better to be governor behaviors, for userspace they may not
want to change
the suspend frequency like other governor, the frequency should decide by
the user, if they
want this function, they should like other governor to rigister a
devfreq_monitor_suspend().
What do you think about my rev6 patch?
If I understand the intention correctly, this is for the stability of
the device due to the behavior or bootloader/SoC-initializer, which
has nothing to do with governors.
Even if users are using userspace, as long as they set the custom
frequencies lower than the default, they have the possibility of
being unstable as ondemand is going to have.
To reuse the update_devfreq() code, you may do something like:
static int _update_freq(struct devfreq *devfreq, bool is_suspending)
{
/* original contents of update_freq with if statement with is_suspending
wrapping get_target_freq */
}
int update_freq(struct devfreq *devfreq)
{
return _update_freq(devfreq, false);
}
There should be other good non-invasive methods that are not governoe-specific
as well.
Thanks for your suggestion, i will update the new version soon.
Cheers,
MyungJoo
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
Lin Huang
--
Lin Huang
