* [RFC PATCH] PM / devfreq: Add policy notifier @ 2018-05-15 21:24 ` Matthias Kaehlcke 2018-05-17 2:01 ` Chanwoo Choi 0 siblings, 1 reply; 5+ messages in thread From: Matthias Kaehlcke @ 2018-05-15 21:24 UTC (permalink / raw) To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi Cc: linux-pm, linux-kernel, Brian Norris, Matthias Kaehlcke Policy notifiers are called before a frequency change and may adjust the frequency, similar to CPUFREQ_ADJUST. The purpose of policy notifiers is to support non-thermal throttling of devfreq devices. As of now notifiers may only select a lower frequency, but not increase it. The reason for this limitation is that devfreq currently doesn't have an actual policy for frequencies and OPPs > freq might be disabled by a devfreq cooling device (see drivers/thermal/devfreq_cooling.c). Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- Marked as RFC since I'm aware that this isn't quite a 'policy' notifier, but I also didn't want to make it too specific. Maybe devfreq devices should have a policy similar to cpufreq? Should devfreq core be in charge of disabling/enabling OPPs instead of the thermal cooling device? Typically we don't want to use higher OPPs than those whitelisted by thermal, so in practice the current situation shouldn't be much of an issue. drivers/devfreq/devfreq.c | 20 ++++++++++++++++---- include/linux/devfreq.h | 6 ++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index fe2af6aa88fc..a7294c056f65 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -273,6 +273,9 @@ int update_devfreq(struct devfreq *devfreq) if (err) return err; + srcu_notifier_call_chain(&devfreq->policy_notifier_list, + DEVFREQ_ADJUST, &freq); + /* * Adjust the frequency with user freq, QoS and available freq. * @@ -640,6 +643,7 @@ struct devfreq *devfreq_add_device(struct device *dev, devfreq->last_stat_updated = jiffies; srcu_init_notifier_head(&devfreq->transition_notifier_list); + srcu_init_notifier_head(&devfreq->policy_notifier_list); mutex_unlock(&devfreq->lock); @@ -1414,7 +1418,7 @@ EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier); * devfreq_register_notifier() - Register a driver with devfreq * @devfreq: The devfreq object. * @nb: The notifier block to register. - * @list: DEVFREQ_TRANSITION_NOTIFIER. + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. */ int devfreq_register_notifier(struct devfreq *devfreq, struct notifier_block *nb, @@ -1430,6 +1434,10 @@ int devfreq_register_notifier(struct devfreq *devfreq, ret = srcu_notifier_chain_register( &devfreq->transition_notifier_list, nb); break; + case DEVFREQ_POLICY_NOTIFIER: + ret = srcu_notifier_chain_register( + &devfreq->policy_notifier_list, nb); + break; default: ret = -EINVAL; } @@ -1442,7 +1450,7 @@ EXPORT_SYMBOL(devfreq_register_notifier); * devfreq_unregister_notifier() - Unregister a driver with devfreq * @devfreq: The devfreq object. * @nb: The notifier block to be unregistered. - * @list: DEVFREQ_TRANSITION_NOTIFIER. + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. */ int devfreq_unregister_notifier(struct devfreq *devfreq, struct notifier_block *nb, @@ -1458,6 +1466,10 @@ int devfreq_unregister_notifier(struct devfreq *devfreq, ret = srcu_notifier_chain_unregister( &devfreq->transition_notifier_list, nb); break; + case DEVFREQ_POLICY_NOTIFIER: + ret = srcu_notifier_chain_unregister( + &devfreq->policy_notifier_list, nb); + break; default: ret = -EINVAL; } @@ -1485,7 +1497,7 @@ static void devm_devfreq_notifier_release(struct device *dev, void *res) * @dev: The devfreq user device. (parent of devfreq) * @devfreq: The devfreq object. * @nb: The notifier block to be unregistered. - * @list: DEVFREQ_TRANSITION_NOTIFIER. + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. */ int devm_devfreq_register_notifier(struct device *dev, struct devfreq *devfreq, @@ -1521,7 +1533,7 @@ EXPORT_SYMBOL(devm_devfreq_register_notifier); * @dev: The devfreq user device. (parent of devfreq) * @devfreq: The devfreq object. * @nb: The notifier block to be unregistered. - * @list: DEVFREQ_TRANSITION_NOTIFIER. + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. */ void devm_devfreq_unregister_notifier(struct device *dev, struct devfreq *devfreq, diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h index 3aae5b3af87c..8edc538d78f7 100644 --- a/include/linux/devfreq.h +++ b/include/linux/devfreq.h @@ -33,6 +33,10 @@ #define DEVFREQ_PRECHANGE (0) #define DEVFREQ_POSTCHANGE (1) +#define DEVFREQ_POLICY_NOTIFIER 1 + +#define DEVFREQ_ADJUST 0 + struct devfreq; struct devfreq_governor; @@ -136,6 +140,7 @@ struct devfreq_dev_profile { * @time_in_state: Statistics of devfreq states * @last_stat_updated: The last time stat updated * @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER notifier + * @policy_notifier_list: list head of DEVFREQ_POLICY_NOTIFIER notifier * * This structure stores the devfreq information for a give device. * @@ -174,6 +179,7 @@ struct devfreq { unsigned long last_stat_updated; struct srcu_notifier_head transition_notifier_list; + struct srcu_notifier_head policy_notifier_list; }; struct devfreq_freqs { -- 2.17.0.441.gb46fe60e1d-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] PM / devfreq: Add policy notifier 2018-05-15 21:24 ` [RFC PATCH] PM / devfreq: Add policy notifier Matthias Kaehlcke @ 2018-05-17 2:01 ` Chanwoo Choi 2018-05-17 23:07 ` Matthias Kaehlcke 0 siblings, 1 reply; 5+ messages in thread From: Chanwoo Choi @ 2018-05-17 2:01 UTC (permalink / raw) To: Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park Cc: linux-pm, linux-kernel, Brian Norris Hi, Could you give some use-case of DEVFREQ_POLICY_NOTIFIER or send use-case patch with this patch? I already knew the CPUFREQ_POLICY_NOTIFIER. But, until now, there are no any requirements of DEVFREQ_POLICY_NOTIFIER. If there are no any use-case, it is not necessary codes. On 2018년 05월 16일 06:24, Matthias Kaehlcke wrote: > Policy notifiers are called before a frequency change and may adjust the > frequency, similar to CPUFREQ_ADJUST. The purpose of policy notifiers is > to support non-thermal throttling of devfreq devices. > > As of now notifiers may only select a lower frequency, but not increase it. > The reason for this limitation is that devfreq currently doesn't have an > actual policy for frequencies and OPPs > freq might be disabled by a > devfreq cooling device (see drivers/thermal/devfreq_cooling.c). > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > --- > Marked as RFC since I'm aware that this isn't quite a 'policy' notifier, > but I also didn't want to make it too specific. Maybe devfreq devices should > have a policy similar to cpufreq? > > Should devfreq core be in charge of disabling/enabling OPPs instead of > the thermal cooling device? Typically we don't want to use higher OPPs > than those whitelisted by thermal, so in practice the current situation > shouldn't be much of an issue. > > drivers/devfreq/devfreq.c | 20 ++++++++++++++++---- > include/linux/devfreq.h | 6 ++++++ > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index fe2af6aa88fc..a7294c056f65 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -273,6 +273,9 @@ int update_devfreq(struct devfreq *devfreq) > if (err) > return err; > > + srcu_notifier_call_chain(&devfreq->policy_notifier_list, > + DEVFREQ_ADJUST, &freq); It is not proper to used 'freq' as the passed data. In current step,'freq' is not adjusted and is not final decided frequency. > + > /* > * Adjust the frequency with user freq, QoS and available freq. > * > @@ -640,6 +643,7 @@ struct devfreq *devfreq_add_device(struct device *dev, > devfreq->last_stat_updated = jiffies; > > srcu_init_notifier_head(&devfreq->transition_notifier_list); > + srcu_init_notifier_head(&devfreq->policy_notifier_list); > > mutex_unlock(&devfreq->lock); > > @@ -1414,7 +1418,7 @@ EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier); > * devfreq_register_notifier() - Register a driver with devfreq > * @devfreq: The devfreq object. > * @nb: The notifier block to register. > - * @list: DEVFREQ_TRANSITION_NOTIFIER. > + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. > */ > int devfreq_register_notifier(struct devfreq *devfreq, > struct notifier_block *nb, > @@ -1430,6 +1434,10 @@ int devfreq_register_notifier(struct devfreq *devfreq, > ret = srcu_notifier_chain_register( > &devfreq->transition_notifier_list, nb); > break; > + case DEVFREQ_POLICY_NOTIFIER: > + ret = srcu_notifier_chain_register( > + &devfreq->policy_notifier_list, nb); > + break; > default: > ret = -EINVAL; > } > @@ -1442,7 +1450,7 @@ EXPORT_SYMBOL(devfreq_register_notifier); > * devfreq_unregister_notifier() - Unregister a driver with devfreq > * @devfreq: The devfreq object. > * @nb: The notifier block to be unregistered. > - * @list: DEVFREQ_TRANSITION_NOTIFIER. > + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. > */ > int devfreq_unregister_notifier(struct devfreq *devfreq, > struct notifier_block *nb, > @@ -1458,6 +1466,10 @@ int devfreq_unregister_notifier(struct devfreq *devfreq, > ret = srcu_notifier_chain_unregister( > &devfreq->transition_notifier_list, nb); > break; > + case DEVFREQ_POLICY_NOTIFIER: > + ret = srcu_notifier_chain_unregister( > + &devfreq->policy_notifier_list, nb); > + break; > default: > ret = -EINVAL; > } > @@ -1485,7 +1497,7 @@ static void devm_devfreq_notifier_release(struct device *dev, void *res) > * @dev: The devfreq user device. (parent of devfreq) > * @devfreq: The devfreq object. > * @nb: The notifier block to be unregistered. > - * @list: DEVFREQ_TRANSITION_NOTIFIER. > + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. > */ > int devm_devfreq_register_notifier(struct device *dev, > struct devfreq *devfreq, > @@ -1521,7 +1533,7 @@ EXPORT_SYMBOL(devm_devfreq_register_notifier); > * @dev: The devfreq user device. (parent of devfreq) > * @devfreq: The devfreq object. > * @nb: The notifier block to be unregistered. > - * @list: DEVFREQ_TRANSITION_NOTIFIER. > + * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER. > */ > void devm_devfreq_unregister_notifier(struct device *dev, > struct devfreq *devfreq, > diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h > index 3aae5b3af87c..8edc538d78f7 100644 > --- a/include/linux/devfreq.h > +++ b/include/linux/devfreq.h > @@ -33,6 +33,10 @@ > #define DEVFREQ_PRECHANGE (0) > #define DEVFREQ_POSTCHANGE (1) > > +#define DEVFREQ_POLICY_NOTIFIER 1 > + > +#define DEVFREQ_ADJUST 0 > + > struct devfreq; > struct devfreq_governor; > > @@ -136,6 +140,7 @@ struct devfreq_dev_profile { > * @time_in_state: Statistics of devfreq states > * @last_stat_updated: The last time stat updated > * @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER notifier > + * @policy_notifier_list: list head of DEVFREQ_POLICY_NOTIFIER notifier > * > * This structure stores the devfreq information for a give device. > * > @@ -174,6 +179,7 @@ struct devfreq { > unsigned long last_stat_updated; > > struct srcu_notifier_head transition_notifier_list; > + struct srcu_notifier_head policy_notifier_list; > }; > > struct devfreq_freqs { > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] PM / devfreq: Add policy notifier 2018-05-17 2:01 ` Chanwoo Choi @ 2018-05-17 23:07 ` Matthias Kaehlcke 2018-05-17 23:26 ` Chanwoo Choi 0 siblings, 1 reply; 5+ messages in thread From: Matthias Kaehlcke @ 2018-05-17 23:07 UTC (permalink / raw) To: Chanwoo Choi Cc: MyungJoo Ham, Kyungmin Park, linux-pm, linux-kernel, Brian Norris Hi, On Thu, May 17, 2018 at 11:01:34AM +0900, Chanwoo Choi wrote: > Hi, > > Could you give some use-case of DEVFREQ_POLICY_NOTIFIER > or send use-case patch with this patch? This is a WIP patch that makes use of the DEVFREQ_POLICY_NOTIFIER: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1065122 > I already knew the CPUFREQ_POLICY_NOTIFIER. > But, until now, there are no any requirements of DEVFREQ_POLICY_NOTIFIER. > If there are no any use-case, it is not necessary codes. Sure, I intend to land the above driver upstream if devfreq can provide the necessary interfaces. > On 2018년 05월 16일 06:24, Matthias Kaehlcke wrote: > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > > index fe2af6aa88fc..a7294c056f65 100644 > > --- a/drivers/devfreq/devfreq.c > > +++ b/drivers/devfreq/devfreq.c > > @@ -273,6 +273,9 @@ int update_devfreq(struct devfreq *devfreq) > > if (err) > > return err; > > > > + srcu_notifier_call_chain(&devfreq->policy_notifier_list, > > + DEVFREQ_ADJUST, &freq); > > It is not proper to used 'freq' as the passed data. > In current step,'freq' is not adjusted and is not final decided > frequency. Right, the next revision will pass a struct devfreq_policy instead, where the notifiers can adjust the min/max values, similar to what cpufreq does. Thanks Matthias ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] PM / devfreq: Add policy notifier 2018-05-17 23:07 ` Matthias Kaehlcke @ 2018-05-17 23:26 ` Chanwoo Choi 2018-05-18 16:25 ` Matthias Kaehlcke 0 siblings, 1 reply; 5+ messages in thread From: Chanwoo Choi @ 2018-05-17 23:26 UTC (permalink / raw) To: Matthias Kaehlcke Cc: MyungJoo Ham, Kyungmin Park, linux-pm, linux-kernel, Brian Norris Hi, On 2018년 05월 18일 08:07, Matthias Kaehlcke wrote: > Hi, > > On Thu, May 17, 2018 at 11:01:34AM +0900, Chanwoo Choi wrote: >> Hi, >> >> Could you give some use-case of DEVFREQ_POLICY_NOTIFIER >> or send use-case patch with this patch? > > This is a WIP patch that makes use of the DEVFREQ_POLICY_NOTIFIER: > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1065122 > >> I already knew the CPUFREQ_POLICY_NOTIFIER. >> But, until now, there are no any requirements of DEVFREQ_POLICY_NOTIFIER. >> If there are no any use-case, it is not necessary codes. > > Sure, I intend to land the above driver upstream if devfreq can > provide the necessary interfaces. I recommend that you should send the patch with the use-case patch. > >> On 2018년 05월 16일 06:24, Matthias Kaehlcke wrote: >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index fe2af6aa88fc..a7294c056f65 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -273,6 +273,9 @@ int update_devfreq(struct devfreq *devfreq) >>> if (err) >>> return err; >>> >>> + srcu_notifier_call_chain(&devfreq->policy_notifier_list, >>> + DEVFREQ_ADJUST, &freq); >> >> It is not proper to used 'freq' as the passed data. >> In current step,'freq' is not adjusted and is not final decided >> frequency. > > Right, the next revision will pass a struct devfreq_policy instead, > where the notifiers can adjust the min/max values, similar to what > cpufreq does. Actually, I don't know the devfreq_policy(?). As I already commented, it is not proper to discuss it because there is no any real code and patches. It is difficult to understand for me. > > Thanks > > Matthias > > > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] PM / devfreq: Add policy notifier 2018-05-17 23:26 ` Chanwoo Choi @ 2018-05-18 16:25 ` Matthias Kaehlcke 0 siblings, 0 replies; 5+ messages in thread From: Matthias Kaehlcke @ 2018-05-18 16:25 UTC (permalink / raw) To: Chanwoo Choi Cc: MyungJoo Ham, Kyungmin Park, linux-pm, linux-kernel, Brian Norris On Fri, May 18, 2018 at 08:26:30AM +0900, Chanwoo Choi wrote: > Hi, > > On 2018년 05월 18일 08:07, Matthias Kaehlcke wrote: > > Hi, > > > > On Thu, May 17, 2018 at 11:01:34AM +0900, Chanwoo Choi wrote: > >> Hi, > >> > >> Could you give some use-case of DEVFREQ_POLICY_NOTIFIER > >> or send use-case patch with this patch? > > > > This is a WIP patch that makes use of the DEVFREQ_POLICY_NOTIFIER: > > > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1065122 > > > >> I already knew the CPUFREQ_POLICY_NOTIFIER. > >> But, until now, there are no any requirements of DEVFREQ_POLICY_NOTIFIER. > >> If there are no any use-case, it is not necessary codes. > > > > Sure, I intend to land the above driver upstream if devfreq can > > provide the necessary interfaces. > > I recommend that you should send the patch with the use-case patch. One of the reasons this patch was sent as an RFC was to get an idea whether it would be feasible to add support for DEVFREQ_POLICY_NOTIFIER before spending much time implementing a driver that relies on it, and then possibly hear that it's not going to fly. However since you didn't voice general concerns about the idea I made progress with the driver in the last days. I'll polish it a bit more and send it in a series with related devfreq patches. > >> On 2018년 05월 16일 06:24, Matthias Kaehlcke wrote: > >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > >>> index fe2af6aa88fc..a7294c056f65 100644 > >>> --- a/drivers/devfreq/devfreq.c > >>> +++ b/drivers/devfreq/devfreq.c > >>> @@ -273,6 +273,9 @@ int update_devfreq(struct devfreq *devfreq) > >>> if (err) > >>> return err; > >>> > >>> + srcu_notifier_call_chain(&devfreq->policy_notifier_list, > >>> + DEVFREQ_ADJUST, &freq); > >> > >> It is not proper to used 'freq' as the passed data. > >> In current step,'freq' is not adjusted and is not final decided > >> frequency. > > > > Right, the next revision will pass a struct devfreq_policy instead, > > where the notifiers can adjust the min/max values, similar to what > > cpufreq does. > > Actually, I don't know the devfreq_policy(?). As I already commented, > it is not proper to discuss it because there is no any real code and patches. > It is difficult to understand for me. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-18 16:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20180515212545epcas1p2bdfd9c90f2ce6b74ac0fdd8cd9f9f63b@epcas1p2.samsung.com>
2018-05-15 21:24 ` [RFC PATCH] PM / devfreq: Add policy notifier Matthias Kaehlcke
2018-05-17 2:01 ` Chanwoo Choi
2018-05-17 23:07 ` Matthias Kaehlcke
2018-05-17 23:26 ` Chanwoo Choi
2018-05-18 16:25 ` Matthias Kaehlcke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).