From: hl <hl@rock-chips.com>
To: Chanwoo Choi <cw00.choi@samsung.com>, myungjoo.ham@samsung.com
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"dbasehore@chromium.org" <dbasehore@chromium.org>,
"dianders@chromium.org" <dianders@chromium.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-rockchip@lists.infradead.org"
<linux-rockchip@lists.infradead.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Subject: Re: [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
Date: Thu, 24 Nov 2016 15:34:39 +0800 [thread overview]
Message-ID: <5836980F.6050006@rock-chips.com> (raw)
In-Reply-To: <5836927B.9010205@samsung.com>
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;
};
};
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 <hl@rock-chips.com> 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
>>> Linux-rockchip@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>> --
>> Lin Huang
>>
>
>
>
--
Lin Huang
WARNING: multiple messages have this Message-ID (diff)
From: hl@rock-chips.com (hl)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
Date: Thu, 24 Nov 2016 15:34:39 +0800 [thread overview]
Message-ID: <5836980F.6050006@rock-chips.com> (raw)
In-Reply-To: <5836927B.9010205@samsung.com>
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;
};
};
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 <hl@rock-chips.com> 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
>>> Linux-rockchip at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>> --
>> Lin Huang
>>
>
>
>
--
Lin Huang
next prev parent reply other threads:[~2016-11-24 7:34 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20161124061416epcms1p44a0152bca14312f1229cab835ea0297f@epcms1p4>
2016-11-24 6:14 ` [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support MyungJoo Ham
2016-11-24 6:14 ` MyungJoo Ham
[not found] ` <58368C91.8030502@rock-chips.com>
[not found] ` <58368C91.8030502-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-11-24 7:10 ` Chanwoo Choi
2016-11-24 7:10 ` Chanwoo Choi
2016-11-24 7:10 ` Chanwoo Choi
2016-11-24 7:34 ` hl [this message]
2016-11-24 7:34 ` hl
2016-11-24 8:16 ` Chanwoo Choi
2016-11-24 8:16 ` Chanwoo Choi
2016-11-24 8:34 ` hl
2016-11-24 8:34 ` hl
2016-11-24 9:28 ` Chanwoo Choi
2016-11-24 9:28 ` Chanwoo Choi
2016-11-24 9:54 ` Chanwoo Choi
2016-11-24 9:54 ` Chanwoo Choi
2016-12-17 14:50 ` Chanwoo Choi
2016-12-17 14:50 ` Chanwoo Choi
2016-12-17 15:13 ` Tobias Jakobi
2016-12-17 15:13 ` Tobias Jakobi
2016-12-17 16:39 ` Chanwoo Choi
2016-12-17 16:39 ` Chanwoo Choi
2016-12-17 18:19 ` Tobias Jakobi
2016-12-17 18:19 ` Tobias Jakobi
2016-12-17 22:03 ` Chanwoo Choi
2016-12-17 22:03 ` Chanwoo Choi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5836980F.6050006@rock-chips.com \
--to=hl@rock-chips.com \
--cc=cw00.choi@samsung.com \
--cc=dbasehore@chromium.org \
--cc=dianders@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=myungjoo.ham@samsung.com \
--cc=tjakobi@math.uni-bielefeld.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.