* [PATCH v6 0/3] PM / devfreq: misc changes (spinned out from throttler series)
@ 2018-08-03 20:05 Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 1/3] PM / devfreq: Fix handling of min/max_freq == 0 Matthias Kaehlcke
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Matthias Kaehlcke @ 2018-08-03 20:05 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi
Cc: linux-pm, linux-kernel, Brian Norris, Matthias Kaehlcke
This series includes miscellaneous changes that were originally part
of the throttler patch series (https://lore.kernel.org/patchwork/project/lkml/list/?series=357937).
They have been spinned out to facilitate merging them independently
from the throttler changes.
The series is based on
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
Matthias Kaehlcke (3):
PM / devfreq: Fix handling of min/max_freq == 0
PM / devfreq: Don't adjust to user limits in governors
PM / devfreq: Make update_devfreq() public
drivers/devfreq/devfreq.c | 42 ++++++++++++++++-------
drivers/devfreq/governor.h | 6 ++--
drivers/devfreq/governor_performance.c | 5 +--
drivers/devfreq/governor_powersave.c | 2 +-
drivers/devfreq/governor_simpleondemand.c | 12 ++-----
drivers/devfreq/governor_userspace.c | 16 +++------
include/linux/devfreq.h | 8 +++++
7 files changed, 50 insertions(+), 41 deletions(-)
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v6 1/3] PM / devfreq: Fix handling of min/max_freq == 0
2018-08-03 20:05 [PATCH v6 0/3] PM / devfreq: misc changes (spinned out from throttler series) Matthias Kaehlcke
@ 2018-08-03 20:05 ` Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 2/3] PM / devfreq: Don't adjust to user limits in governors Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 3/3] PM / devfreq: Make update_devfreq() public Matthias Kaehlcke
2 siblings, 0 replies; 4+ messages in thread
From: Matthias Kaehlcke @ 2018-08-03 20:05 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi
Cc: linux-pm, linux-kernel, Brian Norris, Matthias Kaehlcke
Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the
devfreq device") initializes df->min/max_freq with the min/max OPP when
the device is added. Later commit f1d981eaecf8 ("PM / devfreq: Use the
available min/max frequency") adds df->scaling_min/max_freq and the
following to the frequency adjustment code:
max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);
With the current handling of min/max_freq this is incorrect:
Even though df->max_freq is now initialized to a value != 0 user space
can still set it to 0, in this case max_freq would be 0 instead of
df->scaling_max_freq as intended. In consequence the frequency adjustment
is not performed:
if (max_freq && freq > max_freq) {
freq = max_freq;
To fix this set df->min/max freq to the min/max OPP in max/max_freq_store,
when the user passes a value of 0. This also prevents df->max_freq from
being set below the min OPP when df->min_freq is 0, and similar for
min_freq. Since it is now guaranteed that df->min/max_freq can't be 0 the
checks for this case can be removed.
Fixes: f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency")
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes in v6:
- min/max_freq_store(): updated comment about sorting order of
frequency table
- min/max_freq_store(): moved declaration of 'freq_table' into the
block that uses it
- added 'Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>' tag
Changes in v5:
- none
Changes in v4:
- added 'Reviewed-by: Brian Norris <briannorris@chromium.org>' tag
Changes in v3:
- none
Changes in v2:
- handle freq tables sorted in ascending and descending order in
min/max_freq_store()
- use same order for conditional statements in min/max_freq_store()
---
drivers/devfreq/devfreq.c | 42 ++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 4c49bb1330b5..772e8aab1627 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -283,11 +283,11 @@ int update_devfreq(struct devfreq *devfreq)
max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);
min_freq = MAX(devfreq->scaling_min_freq, devfreq->min_freq);
- if (min_freq && freq < min_freq) {
+ if (freq < min_freq) {
freq = min_freq;
flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
}
- if (max_freq && freq > max_freq) {
+ if (freq > max_freq) {
freq = max_freq;
flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */
}
@@ -1126,17 +1126,26 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
struct devfreq *df = to_devfreq(dev);
unsigned long value;
int ret;
- unsigned long max;
ret = sscanf(buf, "%lu", &value);
if (ret != 1)
return -EINVAL;
mutex_lock(&df->lock);
- max = df->max_freq;
- if (value && max && value > max) {
- ret = -EINVAL;
- goto unlock;
+
+ if (value) {
+ if (value > df->max_freq) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+ } else {
+ unsigned long *freq_table = df->profile->freq_table;
+
+ /* Get minimum frequency according to sorting order */
+ if (freq_table[0] < freq_table[df->profile->max_state - 1])
+ value = freq_table[0];
+ else
+ value = freq_table[df->profile->max_state - 1];
}
df->min_freq = value;
@@ -1161,17 +1170,26 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
struct devfreq *df = to_devfreq(dev);
unsigned long value;
int ret;
- unsigned long min;
ret = sscanf(buf, "%lu", &value);
if (ret != 1)
return -EINVAL;
mutex_lock(&df->lock);
- min = df->min_freq;
- if (value && min && value < min) {
- ret = -EINVAL;
- goto unlock;
+
+ if (value) {
+ if (value < df->min_freq) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+ } else {
+ unsigned long *freq_table = df->profile->freq_table;
+
+ /* Get maximum frequency according to sorting order */
+ if (freq_table[0] < freq_table[df->profile->max_state - 1])
+ value = freq_table[df->profile->max_state - 1];
+ else
+ value = freq_table[0];
}
df->max_freq = value;
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v6 2/3] PM / devfreq: Don't adjust to user limits in governors
2018-08-03 20:05 [PATCH v6 0/3] PM / devfreq: misc changes (spinned out from throttler series) Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 1/3] PM / devfreq: Fix handling of min/max_freq == 0 Matthias Kaehlcke
@ 2018-08-03 20:05 ` Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 3/3] PM / devfreq: Make update_devfreq() public Matthias Kaehlcke
2 siblings, 0 replies; 4+ messages in thread
From: Matthias Kaehlcke @ 2018-08-03 20:05 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi
Cc: linux-pm, linux-kernel, Brian Norris, Matthias Kaehlcke
Several governors use the user space limits df->min/max_freq to adjust
the target frequency. This is not necessary, since update_devfreq()
already takes care of this. Instead the governor can request the available
min/max frequency by setting the target frequency to DEVFREQ_MIN/MAX_FREQ
and let update_devfreq() take care of any adjustments.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes in v6:
- added 'Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>' tag
Changes in v5:
- none
Changes in v4:
- added 'Reviewed-by: Brian Norris <briannorris@chromium.org>' tag
Changes in v3:
- none
Changes in v2:
- squashed "PM / devfreq: Remove redundant frequency adjustment from governors"
and "PM / devfreq: governors: Return device frequency limits instead of user
limits"
- updated subject and commit message
- use DEVFREQ_MIN/MAX_FREQ instead of df->scaling_min/max_freq
---
drivers/devfreq/governor.h | 3 +++
drivers/devfreq/governor_performance.c | 5 +----
drivers/devfreq/governor_powersave.c | 2 +-
drivers/devfreq/governor_simpleondemand.c | 12 +++---------
drivers/devfreq/governor_userspace.c | 16 ++++------------
5 files changed, 12 insertions(+), 26 deletions(-)
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index cfc50a61a90d..b81700244ce3 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -25,6 +25,9 @@
#define DEVFREQ_GOV_SUSPEND 0x4
#define DEVFREQ_GOV_RESUME 0x5
+#define DEVFREQ_MIN_FREQ 0
+#define DEVFREQ_MAX_FREQ ULONG_MAX
+
/**
* struct devfreq_governor - Devfreq policy governor
* @node: list node - contains registered devfreq governors
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index 4d23ecfbd948..ded429fd51be 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -20,10 +20,7 @@ static int devfreq_performance_func(struct devfreq *df,
* target callback should be able to get floor value as
* said in devfreq.h
*/
- if (!df->max_freq)
- *freq = UINT_MAX;
- else
- *freq = df->max_freq;
+ *freq = DEVFREQ_MAX_FREQ;
return 0;
}
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 0c42f23249ef..9e8897f5ac42 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -20,7 +20,7 @@ static int devfreq_powersave_func(struct devfreq *df,
* target callback should be able to get ceiling value as
* said in devfreq.h
*/
- *freq = df->min_freq;
+ *freq = DEVFREQ_MIN_FREQ;
return 0;
}
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index 28e0f2de7100..c0417f0e081e 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -27,7 +27,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
struct devfreq_simple_ondemand_data *data = df->data;
- unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
err = devfreq_update_stats(df);
if (err)
@@ -47,7 +46,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
/* Assume MAX if it is going to be divided by zero */
if (stat->total_time == 0) {
- *freq = max;
+ *freq = DEVFREQ_MAX_FREQ;
return 0;
}
@@ -60,13 +59,13 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
/* Set MAX if it's busy enough */
if (stat->busy_time * 100 >
stat->total_time * dfso_upthreshold) {
- *freq = max;
+ *freq = DEVFREQ_MAX_FREQ;
return 0;
}
/* Set MAX if we do not know the initial frequency */
if (stat->current_frequency == 0) {
- *freq = max;
+ *freq = DEVFREQ_MAX_FREQ;
return 0;
}
@@ -85,11 +84,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
*freq = (unsigned long) b;
- if (df->min_freq && *freq < df->min_freq)
- *freq = df->min_freq;
- if (df->max_freq && *freq > df->max_freq)
- *freq = df->max_freq;
-
return 0;
}
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 080607c3f34d..378d84c011df 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -26,19 +26,11 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
{
struct userspace_data *data = df->data;
- if (data->valid) {
- unsigned long adjusted_freq = data->user_frequency;
-
- if (df->max_freq && adjusted_freq > df->max_freq)
- adjusted_freq = df->max_freq;
-
- if (df->min_freq && adjusted_freq < df->min_freq)
- adjusted_freq = df->min_freq;
-
- *freq = adjusted_freq;
- } else {
+ if (data->valid)
+ *freq = data->user_frequency;
+ else
*freq = df->previous_freq; /* No user freq specified yet */
- }
+
return 0;
}
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v6 3/3] PM / devfreq: Make update_devfreq() public
2018-08-03 20:05 [PATCH v6 0/3] PM / devfreq: misc changes (spinned out from throttler series) Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 1/3] PM / devfreq: Fix handling of min/max_freq == 0 Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 2/3] PM / devfreq: Don't adjust to user limits in governors Matthias Kaehlcke
@ 2018-08-03 20:05 ` Matthias Kaehlcke
2 siblings, 0 replies; 4+ messages in thread
From: Matthias Kaehlcke @ 2018-08-03 20:05 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi
Cc: linux-pm, linux-kernel, Brian Norris, Matthias Kaehlcke
Currently update_devfreq() is only visible to devfreq governors outside
of devfreq.c. Make it public to allow drivers that adjust devfreq policies
to cause a re-evaluation of the frequency after a policy change.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
--
Changes in v6:
- added 'Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>' tag
Changes in v5:
- none
Changed in v4:
- added 'Reviewed-by: Brian Norris <briannorris@chromium.org>' tag
Changes in v3:
- none
Changes in v2:
- added 'Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>' tag
---
drivers/devfreq/governor.h | 3 ---
include/linux/devfreq.h | 8 ++++++++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index b81700244ce3..f53339ca610f 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -57,9 +57,6 @@ struct devfreq_governor {
unsigned int event, void *data);
};
-/* Caution: devfreq->lock must be locked before calling update_devfreq */
-extern int update_devfreq(struct devfreq *devfreq);
-
extern void devfreq_monitor_start(struct devfreq *devfreq);
extern void devfreq_monitor_stop(struct devfreq *devfreq);
extern void devfreq_monitor_suspend(struct devfreq *devfreq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 3aae5b3af87c..e4963b0f45da 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -198,6 +198,14 @@ extern void devm_devfreq_remove_device(struct device *dev,
extern int devfreq_suspend_device(struct devfreq *devfreq);
extern int devfreq_resume_device(struct devfreq *devfreq);
+/**
+ * update_devfreq() - Reevaluate the device and configure frequency
+ * @devfreq: the devfreq device
+ *
+ * Note: devfreq->lock must be held
+ */
+extern int update_devfreq(struct devfreq *devfreq);
+
/* Helper functions for devfreq user device driver with OPP. */
extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
unsigned long *freq, u32 flags);
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-03 20:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 20:05 [PATCH v6 0/3] PM / devfreq: misc changes (spinned out from throttler series) Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 1/3] PM / devfreq: Fix handling of min/max_freq == 0 Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 2/3] PM / devfreq: Don't adjust to user limits in governors Matthias Kaehlcke
2018-08-03 20:05 ` [PATCH v6 3/3] PM / devfreq: Make update_devfreq() public Matthias Kaehlcke
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.