Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
On 4/1/21 9:16 AM, Chanwoo Choi wrote: > On 3/31/21 10:03 PM, andrew-sh.cheng wrote: >> On Wed, 2021-03-31 at 17:35 +0900, Chanwoo Choi wrote: >>> On 3/31/21 5:27 PM, Chanwoo Choi wrote: Hi, On 3/31/21 5:03 PM, andrew-sh.cheng wrote: > On Thu, 2021-03-25 at 17:14 +0900, Chanwoo Choi wrote: >> Hi, >> >> You are missing to add these patches to linux-pm mailing list. >> Need to send them to linu-pm ML. >> >> Also, before received this series, I tried to clean-up these patches >> on testing branch[1]. So that I add my comment with my clean-up case. >> [1] >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgAPh9XRRs$ >> >> >> And 'Saravana Kannan ' is wrong email address. >> Please update the email or drop this email. > > Hi Chanwoo, > > Thank you for the advices. > I will resend patch v9 (add to linux-pm ML), remove this patch, and note > that my patch set base on > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$ > I has not yet test this patch[1] on devfreq-testing-passive-gov branch. So that if possible, I'd like you to test your patches with this patch[1] and then if there is no problem, could you send the next patches with patch[1]? [1]https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=39c80d11a8f42dd63ecea1e0df595a0ceb83b454__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJR2cQqZs$ >>> >>> >>> Sorry for the confusion. I make the devfreq-testing-passive-gov[1] >>> branch based on latest devfreq-next branch. >>> [1] >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$ >>> >>> >>> First of all, if possible, I want to test them[1] with your patches in this >>> series. >>> And then if there are no any problem, please let me know. After confirmed >>> from you, >>> I'll send the patches of devfreq-testing-passive-gov[1] branch. >>> How about that? >>> >> Hi Chanwoo~ >> >> We will use this on Google Chrome project. >> Google Hsin-Yi has test your patch + my patch set v8 [2~8] >> >> make sure cci devfreqs runs with cpufreq. >> suspend resume >> speedometer2 benchmark >> It is okay. >> >> Please send the patches of devfreq-testing-passive-gov[1] branch. >> >> I will send patch v9 base on yours latter. > > Thanks for your test. I'll send the patches today. I'm sorry for delay because when I tested the patches for devfreq parent type on Odroid-xu3, there are some problem related to lazy linking of OPP. So I'm trying to analyze them. Unfortunately, we need to postpone these patches to next linux version. [snip] -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
On 3/31/21 10:03 PM, andrew-sh.cheng wrote: > On Wed, 2021-03-31 at 17:35 +0900, Chanwoo Choi wrote: >> On 3/31/21 5:27 PM, Chanwoo Choi wrote: >>> Hi, >>> >>> On 3/31/21 5:03 PM, andrew-sh.cheng wrote: On Thu, 2021-03-25 at 17:14 +0900, Chanwoo Choi wrote: > Hi, > > You are missing to add these patches to linux-pm mailing list. > Need to send them to linu-pm ML. > > Also, before received this series, I tried to clean-up these patches > on testing branch[1]. So that I add my comment with my clean-up case. > [1] > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgAPh9XRRs$ > > > And 'Saravana Kannan ' is wrong email address. > Please update the email or drop this email. Hi Chanwoo, Thank you for the advices. I will resend patch v9 (add to linux-pm ML), remove this patch, and note that my patch set base on https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$ >>> >>> I has not yet test this patch[1] on devfreq-testing-passive-gov branch. >>> So that if possible, I'd like you to test your patches with this patch[1] >>> and then if there is no problem, could you send the next patches with >>> patch[1]? >>> >>> [1]https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=39c80d11a8f42dd63ecea1e0df595a0ceb83b454__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJR2cQqZs$ >>> >> >> >> Sorry for the confusion. I make the devfreq-testing-passive-gov[1] >> branch based on latest devfreq-next branch. >> [1] >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!yUlsuxrL5PcbF7o6A9DlCfvoA6w8V8VXKjYIybYyiJg3D0HM-lI2xRuxLUV6b3UJ8WFhg_g$ >> >> >> First of all, if possible, I want to test them[1] with your patches in this >> series. >> And then if there are no any problem, please let me know. After confirmed >> from you, >> I'll send the patches of devfreq-testing-passive-gov[1] branch. >> How about that? >> > Hi Chanwoo~ > > We will use this on Google Chrome project. > Google Hsin-Yi has test your patch + my patch set v8 [2~8] > > make sure cci devfreqs runs with cpufreq. > suspend resume > speedometer2 benchmark > It is okay. > > Please send the patches of devfreq-testing-passive-gov[1] branch. > > I will send patch v9 base on yours latter. Thanks for your test. I'll send the patches today. > > >> >>> > > > On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote: >> From: Saravana Kannan >> >> Many CPU architectures have caches that can scale independent of the >> CPUs. Frequency scaling of the caches is necessary to make sure that the >> cache is not a performance bottleneck that leads to poor performance and >> power. The same idea applies for RAM/DDR. >> >> To achieve this, this patch adds support for cpu based scaling to the >> passive governor. This is accomplished by taking the current frequency >> of each CPU frequency domain and then adjust the frequency of the cache >> (or any devfreq device) based on the frequency of the CPUs. It listens >> to CPU frequency transition notifiers to keep itself up to date on the >> current CPU frequency. >> >> To decide the frequency of the device, the governor does one of the >> following: >> * Derives the optimal devfreq device opp from required-opps property of >> the parent cpu opp_table. >> >> * Scales the device frequency in proportion to the CPU frequency. So, if >> the CPUs are running at their max frequency, the device runs at its >> max frequency. If the CPUs are running at their min frequency, the >> device runs at its min frequency. It is interpolated for frequencies >> in between. >> >> Andrew-sh.Cheng change >> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq >> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value >> after kernel-5.7 >> Don't return -EINVAL in devfreq_passive_event_handler() >> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases. >> >> Signed-off-by: Saravana Kannan >> [Sibi: Integrated cpu-freqmap governor into passive_governor] >> Signed-off-by: Sibi Sankar >> Signed-off-by: Andrew-sh.Cheng >> --- >> drivers/devfreq/Kconfig| 2 + >> drivers/devfreq/governor_passive.c | 329 >> +++-- >> include/linux/devfreq.h
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
On Thu, Mar 25, 2021 at 3:58 PM Chanwoo Choi wrote:
>
> Hi,
>
> You are missing to add these patches to linux-pm mailing list.
> Need to send them to linu-pm ML.
>
> Also, before received this series, I tried to clean-up these patches
> on testing branch[1]. So that I add my comment with my clean-up case.
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
>
> And 'Saravana Kannan ' is wrong email address.
> Please update the email or drop this email.
>
>
> On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
> > From: Saravana Kannan
> >
> > Many CPU architectures have caches that can scale independent of the
> > CPUs. Frequency scaling of the caches is necessary to make sure that the
> > cache is not a performance bottleneck that leads to poor performance and
> > power. The same idea applies for RAM/DDR.
> >
> > To achieve this, this patch adds support for cpu based scaling to the
> > passive governor. This is accomplished by taking the current frequency
> > of each CPU frequency domain and then adjust the frequency of the cache
> > (or any devfreq device) based on the frequency of the CPUs. It listens
> > to CPU frequency transition notifiers to keep itself up to date on the
> > current CPU frequency.
> >
> > To decide the frequency of the device, the governor does one of the
> > following:
> > * Derives the optimal devfreq device opp from required-opps property of
> > the parent cpu opp_table.
> >
> > * Scales the device frequency in proportion to the CPU frequency. So, if
> > the CPUs are running at their max frequency, the device runs at its
> > max frequency. If the CPUs are running at their min frequency, the
> > device runs at its min frequency. It is interpolated for frequencies
> > in between.
> >
> > Andrew-sh.Cheng change
> > dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> > to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> > after kernel-5.7
> > Don't return -EINVAL in devfreq_passive_event_handler()
> > since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
> >
> > Signed-off-by: Saravana Kannan
> > [Sibi: Integrated cpu-freqmap governor into passive_governor]
> > Signed-off-by: Sibi Sankar
> > Signed-off-by: Andrew-sh.Cheng
> > ---
> > drivers/devfreq/Kconfig| 2 +
> > drivers/devfreq/governor_passive.c | 329
> > +++--
> > include/linux/devfreq.h| 29 +++-
> > 3 files changed, 342 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> > index 00704efe6398..f56132b0ae64 100644
> > --- a/drivers/devfreq/Kconfig
> > +++ b/drivers/devfreq/Kconfig
> > @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> > device. This governor does not change the frequency by itself
> > through sysfs entries. The passive governor recommends that
> > devfreq device uses the OPP table to get the frequency/voltage.
> > + Alternatively the governor can also be chosen to scale based on
> > + the online CPUs current frequency.
> >
> > comment "DEVFREQ Drivers"
> >
> > diff --git a/drivers/devfreq/governor_passive.c
> > b/drivers/devfreq/governor_passive.c
> > index b094132bd20b..9cc57b083839 100644
> > --- a/drivers/devfreq/governor_passive.c
> > +++ b/drivers/devfreq/governor_passive.c
> > @@ -8,11 +8,103 @@
> > */
> >
> > #include
> > +#include
> > +#include
> > +#include
> > #include
> > #include
> > +#include
> > #include "governor.h"
> >
> > -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> > +struct devfreq_cpu_state {
> > + unsigned int curr_freq;
> > + unsigned int min_freq;
> > + unsigned int max_freq;
> > + unsigned int first_cpu;
> > + struct device *cpu_dev;
> > + struct opp_table *opp_table;
> > +};
>
> As I knew, the previous version has the description of structure
> as following: I wan to add the description like below.
>
> And if you have no any objection, I'd like you to order
> the variables as following and use 'dev' instead of 'cpu_dev'
> because this patch use the 'cpu_state->cpu_dev' at the multiple points.
> I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
> Also, I prefer to use 'cur_freq' instead of 'curr_freq'
> because devfreq subsystem uses 'cur_freq' for expressing the 'current
> frequency'.
>
> /**
> * struct devfreq_cpu_state - Hold the per-cpu data
> * @dev:reference to cpu device.
> * @first_cpu: the cpumask of the first cpu of a policy.
> * @opp_table: reference to cpu opp table.
> * @cur_freq: the current frequency of the cpu.
> * @min_freq: the min frequency of the cpu.
> * @max_freq: the max frequency of the cpu.
> *
> * This structure stores the required cpu_data of a cpu.
> * This is auto-populated by the governor.
> */
> struct devfreq_cpu_state {
> struct device *dev;
> unsigned int first_cpu;
>
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
On 3/31/21 5:27 PM, Chanwoo Choi wrote: > Hi, > > On 3/31/21 5:03 PM, andrew-sh.cheng wrote: >> On Thu, 2021-03-25 at 17:14 +0900, Chanwoo Choi wrote: >>> Hi, >>> >>> You are missing to add these patches to linux-pm mailing list. >>> Need to send them to linu-pm ML. >>> >>> Also, before received this series, I tried to clean-up these patches >>> on testing branch[1]. So that I add my comment with my clean-up case. >>> [1] >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgAPh9XRRs$ >>> >>> >>> And 'Saravana Kannan ' is wrong email address. >>> Please update the email or drop this email. >> >> Hi Chanwoo, >> >> Thank you for the advices. >> I will resend patch v9 (add to linux-pm ML), remove this patch, and note >> that my patch set base on >> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov > > I has not yet test this patch[1] on devfreq-testing-passive-gov branch. > So that if possible, I'd like you to test your patches with this patch[1] > and then if there is no problem, could you send the next patches with > patch[1]? > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=39c80d11a8f42dd63ecea1e0df595a0ceb83b454 Sorry for the confusion. I make the devfreq-testing-passive-gov[1] branch based on latest devfreq-next branch. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov First of all, if possible, I want to test them[1] with your patches in this series. And then if there are no any problem, please let me know. After confirmed from you, I'll send the patches of devfreq-testing-passive-gov[1] branch. How about that? > >> >> >>> >>> >>> On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote: From: Saravana Kannan Many CPU architectures have caches that can scale independent of the CPUs. Frequency scaling of the caches is necessary to make sure that the cache is not a performance bottleneck that leads to poor performance and power. The same idea applies for RAM/DDR. To achieve this, this patch adds support for cpu based scaling to the passive governor. This is accomplished by taking the current frequency of each CPU frequency domain and then adjust the frequency of the cache (or any devfreq device) based on the frequency of the CPUs. It listens to CPU frequency transition notifiers to keep itself up to date on the current CPU frequency. To decide the frequency of the device, the governor does one of the following: * Derives the optimal devfreq device opp from required-opps property of the parent cpu opp_table. * Scales the device frequency in proportion to the CPU frequency. So, if the CPUs are running at their max frequency, the device runs at its max frequency. If the CPUs are running at their min frequency, the device runs at its min frequency. It is interpolated for frequencies in between. Andrew-sh.Cheng change dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value after kernel-5.7 Don't return -EINVAL in devfreq_passive_event_handler() since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases. Signed-off-by: Saravana Kannan [Sibi: Integrated cpu-freqmap governor into passive_governor] Signed-off-by: Sibi Sankar Signed-off-by: Andrew-sh.Cheng --- drivers/devfreq/Kconfig| 2 + drivers/devfreq/governor_passive.c | 329 +++-- include/linux/devfreq.h| 29 +++- 3 files changed, 342 insertions(+), 18 deletions(-) diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig index 00704efe6398..f56132b0ae64 100644 --- a/drivers/devfreq/Kconfig +++ b/drivers/devfreq/Kconfig @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE device. This governor does not change the frequency by itself through sysfs entries. The passive governor recommends that devfreq device uses the OPP table to get the frequency/voltage. +Alternatively the governor can also be chosen to scale based on +the online CPUs current frequency. comment "DEVFREQ Drivers" diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c index b094132bd20b..9cc57b083839 100644 --- a/drivers/devfreq/governor_passive.c +++ b/drivers/devfreq/governor_passive.c @@ -8,11 +8,103 @@ */ #include +#include +#include +#include #include #include +#include #i
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
Hi,
On 3/31/21 5:03 PM, andrew-sh.cheng wrote:
> On Thu, 2021-03-25 at 17:14 +0900, Chanwoo Choi wrote:
>> Hi,
>>
>> You are missing to add these patches to linux-pm mailing list.
>> Need to send them to linu-pm ML.
>>
>> Also, before received this series, I tried to clean-up these patches
>> on testing branch[1]. So that I add my comment with my clean-up case.
>> [1]
>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov__;!!CTRNKA9wMg0ARbw!zIrzeDp9vPnm1_SDzVPuzqdHn3zWie9DnfBXaA-j9-CSrVc6aR9_rJQQiw81_CgAPh9XRRs$
>>
>>
>> And 'Saravana Kannan ' is wrong email address.
>> Please update the email or drop this email.
>
> Hi Chanwoo,
>
> Thank you for the advices.
> I will resend patch v9 (add to linux-pm ML), remove this patch, and note
> that my patch set base on
> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
I has not yet test this patch[1] on devfreq-testing-passive-gov branch.
So that if possible, I'd like you to test your patches with this patch[1]
and then if there is no problem, could you send the next patches with patch[1]?
[1]https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing-passive-gov&id=39c80d11a8f42dd63ecea1e0df595a0ceb83b454
>
>
>>
>>
>> On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
>>> From: Saravana Kannan
>>>
>>> Many CPU architectures have caches that can scale independent of the
>>> CPUs. Frequency scaling of the caches is necessary to make sure that the
>>> cache is not a performance bottleneck that leads to poor performance and
>>> power. The same idea applies for RAM/DDR.
>>>
>>> To achieve this, this patch adds support for cpu based scaling to the
>>> passive governor. This is accomplished by taking the current frequency
>>> of each CPU frequency domain and then adjust the frequency of the cache
>>> (or any devfreq device) based on the frequency of the CPUs. It listens
>>> to CPU frequency transition notifiers to keep itself up to date on the
>>> current CPU frequency.
>>>
>>> To decide the frequency of the device, the governor does one of the
>>> following:
>>> * Derives the optimal devfreq device opp from required-opps property of
>>> the parent cpu opp_table.
>>>
>>> * Scales the device frequency in proportion to the CPU frequency. So, if
>>> the CPUs are running at their max frequency, the device runs at its
>>> max frequency. If the CPUs are running at their min frequency, the
>>> device runs at its min frequency. It is interpolated for frequencies
>>> in between.
>>>
>>> Andrew-sh.Cheng change
>>> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
>>> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
>>> after kernel-5.7
>>> Don't return -EINVAL in devfreq_passive_event_handler()
>>> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
>>>
>>> Signed-off-by: Saravana Kannan
>>> [Sibi: Integrated cpu-freqmap governor into passive_governor]
>>> Signed-off-by: Sibi Sankar
>>> Signed-off-by: Andrew-sh.Cheng
>>> ---
>>> drivers/devfreq/Kconfig| 2 +
>>> drivers/devfreq/governor_passive.c | 329
>>> +++--
>>> include/linux/devfreq.h| 29 +++-
>>> 3 files changed, 342 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> index 00704efe6398..f56132b0ae64 100644
>>> --- a/drivers/devfreq/Kconfig
>>> +++ b/drivers/devfreq/Kconfig
>>> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
>>> device. This governor does not change the frequency by itself
>>> through sysfs entries. The passive governor recommends that
>>> devfreq device uses the OPP table to get the frequency/voltage.
>>> + Alternatively the governor can also be chosen to scale based on
>>> + the online CPUs current frequency.
>>>
>>> comment "DEVFREQ Drivers"
>>>
>>> diff --git a/drivers/devfreq/governor_passive.c
>>> b/drivers/devfreq/governor_passive.c
>>> index b094132bd20b..9cc57b083839 100644
>>> --- a/drivers/devfreq/governor_passive.c
>>> +++ b/drivers/devfreq/governor_passive.c
>>> @@ -8,11 +8,103 @@
>>> */
>>>
>>> #include
>>> +#include
>>> +#include
>>> +#include
>>> #include
>>> #include
>>> +#include
>>> #include "governor.h"
>>>
>>> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>>> +struct devfreq_cpu_state {
>>> + unsigned int curr_freq;
>>> + unsigned int min_freq;
>>> + unsigned int max_freq;
>>> + unsigned int first_cpu;
>>> + struct device *cpu_dev;
>>> + struct opp_table *opp_table;
>>> +};
>>
>> As I knew, the previous version has the description of structure
>> as following: I wan to add the description like below.
>>
>> And if you have no any objection, I'd like you to order
>> the variables as following and use 'dev' instead of 'cpu_dev'
>> because this patch use the 'cp
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
Hi,
You are missing to add these patches to linux-pm mailing list.
Need to send them to linu-pm ML.
Also, before received this series, I tried to clean-up these patches
on testing branch[1]. So that I add my comment with my clean-up case.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
And 'Saravana Kannan ' is wrong email address.
Please update the email or drop this email.
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
> From: Saravana Kannan
>
> Many CPU architectures have caches that can scale independent of the
> CPUs. Frequency scaling of the caches is necessary to make sure that the
> cache is not a performance bottleneck that leads to poor performance and
> power. The same idea applies for RAM/DDR.
>
> To achieve this, this patch adds support for cpu based scaling to the
> passive governor. This is accomplished by taking the current frequency
> of each CPU frequency domain and then adjust the frequency of the cache
> (or any devfreq device) based on the frequency of the CPUs. It listens
> to CPU frequency transition notifiers to keep itself up to date on the
> current CPU frequency.
>
> To decide the frequency of the device, the governor does one of the
> following:
> * Derives the optimal devfreq device opp from required-opps property of
> the parent cpu opp_table.
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
> the CPUs are running at their max frequency, the device runs at its
> max frequency. If the CPUs are running at their min frequency, the
> device runs at its min frequency. It is interpolated for frequencies
> in between.
>
> Andrew-sh.Cheng change
> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> after kernel-5.7
> Don't return -EINVAL in devfreq_passive_event_handler()
> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
>
> Signed-off-by: Saravana Kannan
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar
> Signed-off-by: Andrew-sh.Cheng
> ---
> drivers/devfreq/Kconfig| 2 +
> drivers/devfreq/governor_passive.c | 329
> +++--
> include/linux/devfreq.h| 29 +++-
> 3 files changed, 342 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 00704efe6398..f56132b0ae64 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> device. This governor does not change the frequency by itself
> through sysfs entries. The passive governor recommends that
> devfreq device uses the OPP table to get the frequency/voltage.
> + Alternatively the governor can also be chosen to scale based on
> + the online CPUs current frequency.
>
> comment "DEVFREQ Drivers"
>
> diff --git a/drivers/devfreq/governor_passive.c
> b/drivers/devfreq/governor_passive.c
> index b094132bd20b..9cc57b083839 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -8,11 +8,103 @@
> */
>
> #include
> +#include
> +#include
> +#include
> #include
> #include
> +#include
> #include "governor.h"
>
> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> +struct devfreq_cpu_state {
> + unsigned int curr_freq;
> + unsigned int min_freq;
> + unsigned int max_freq;
> + unsigned int first_cpu;
> + struct device *cpu_dev;
> + struct opp_table *opp_table;
> +};
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current
frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev:reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is au
Re: [PATCH V8 1/8] PM / devfreq: Add cpu based scaling support to passive_governor
Hi,
You are missing to add these patches to linux-pm mailing list.
Need to send them to linu-pm ML.
Also, before received this series, I tried to clean-up these patches
on testing branch[1]. So that I add my comment with my clean-up case.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing-passive-gov
On 3/23/21 8:33 PM, Andrew-sh.Cheng wrote:
> From: Saravana Kannan
>
> Many CPU architectures have caches that can scale independent of the
> CPUs. Frequency scaling of the caches is necessary to make sure that the
> cache is not a performance bottleneck that leads to poor performance and
> power. The same idea applies for RAM/DDR.
>
> To achieve this, this patch adds support for cpu based scaling to the
> passive governor. This is accomplished by taking the current frequency
> of each CPU frequency domain and then adjust the frequency of the cache
> (or any devfreq device) based on the frequency of the CPUs. It listens
> to CPU frequency transition notifiers to keep itself up to date on the
> current CPU frequency.
>
> To decide the frequency of the device, the governor does one of the
> following:
> * Derives the optimal devfreq device opp from required-opps property of
> the parent cpu opp_table.
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
> the CPUs are running at their max frequency, the device runs at its
> max frequency. If the CPUs are running at their min frequency, the
> device runs at its min frequency. It is interpolated for frequencies
> in between.
>
> Andrew-sh.Cheng change
> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> after kernel-5.7
> Don't return -EINVAL in devfreq_passive_event_handler()
> since it doesn't handle DEVFREQ_GOV_SUSPEND DEVFREQ_GOV_RESUME cases.
>
> Signed-off-by: Saravana Kannan
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar
> Signed-off-by: Andrew-sh.Cheng
> ---
> drivers/devfreq/Kconfig| 2 +
> drivers/devfreq/governor_passive.c | 329
> +++--
> include/linux/devfreq.h| 29 +++-
> 3 files changed, 342 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 00704efe6398..f56132b0ae64 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> device. This governor does not change the frequency by itself
> through sysfs entries. The passive governor recommends that
> devfreq device uses the OPP table to get the frequency/voltage.
> + Alternatively the governor can also be chosen to scale based on
> + the online CPUs current frequency.
>
> comment "DEVFREQ Drivers"
>
> diff --git a/drivers/devfreq/governor_passive.c
> b/drivers/devfreq/governor_passive.c
> index b094132bd20b..9cc57b083839 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -8,11 +8,103 @@
> */
>
> #include
> +#include
> +#include
> +#include
> #include
> #include
> +#include
> #include "governor.h"
>
> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> +struct devfreq_cpu_state {
> + unsigned int curr_freq;
> + unsigned int min_freq;
> + unsigned int max_freq;
> + unsigned int first_cpu;
> + struct device *cpu_dev;
> + struct opp_table *opp_table;
> +};
As I knew, the previous version has the description of structure
as following: I wan to add the description like below.
And if you have no any objection, I'd like you to order
the variables as following and use 'dev' instead of 'cpu_dev'
because this patch use the 'cpu_state->cpu_dev' at the multiple points.
I think that 'cpu_state->dev' is better than 'cpu_state->cpu_dev'.
Also, I prefer to use 'cur_freq' instead of 'curr_freq'
because devfreq subsystem uses 'cur_freq' for expressing the 'current
frequency'.
/**
* struct devfreq_cpu_state - Hold the per-cpu data
* @dev:reference to cpu device.
* @first_cpu: the cpumask of the first cpu of a policy.
* @opp_table: reference to cpu opp table.
* @cur_freq: the current frequency of the cpu.
* @min_freq: the min frequency of the cpu.
* @max_freq: the max frequency of the cpu.
*
* This structure stores the required cpu_data of a cpu.
* This is auto-populated by the governor.
*/
