* [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier()
2019-06-10 10:51 [PATCH V3 0/5] cpufreq: Use QoS layer to manage freq-constraints Viresh Kumar
@ 2019-06-10 10:51 ` Viresh Kumar
2019-06-11 23:45 ` Matthias Kaehlcke
` (2 more replies)
2019-06-10 10:51 ` [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value() Viresh Kumar
` (3 subsequent siblings)
4 siblings, 3 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-10 10:51 UTC (permalink / raw)
To: Rafael Wysocki, Len Brown, Pavel Machek, Kevin Hilman, Ulf Hansson
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Qais.Yousef, mka,
juri.lelli, linux-kernel
In order to use the same set of routines to register notifiers for
different request types, update the existing
dev_pm_qos_{add|remove}_notifier() routines with an additional
parameter: request-type.
For now, it only supports resume-latency request type.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/power/pm_qos_interface.txt | 10 ++++++----
drivers/base/power/domain.c | 8 +++++---
drivers/base/power/qos.c | 14 ++++++++++++--
include/linux/pm_qos.h | 12 ++++++++----
4 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
index 19c5f7b1a7ba..ec7d662d1707 100644
--- a/Documentation/power/pm_qos_interface.txt
+++ b/Documentation/power/pm_qos_interface.txt
@@ -164,12 +164,14 @@ directory.
Notification mechanisms:
The per-device PM QoS framework has a per-device notification tree.
-int dev_pm_qos_add_notifier(device, notifier):
-Adds a notification callback function for the device.
+int dev_pm_qos_add_notifier(device, notifier, type):
+Adds a notification callback function for the device for a particular request
+type.
+
The callback is called when the aggregated value of the device constraints list
-is changed (for resume latency device PM QoS only).
+is changed.
-int dev_pm_qos_remove_notifier(device, notifier):
+int dev_pm_qos_remove_notifier(device, notifier, type):
Removes the notification callback function for the device.
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 33c30c1e6a30..b063bc41b0a9 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1536,7 +1536,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
if (ret)
genpd_free_dev_data(dev, gpd_data);
else
- dev_pm_qos_add_notifier(dev, &gpd_data->nb);
+ dev_pm_qos_add_notifier(dev, &gpd_data->nb,
+ DEV_PM_QOS_RESUME_LATENCY);
return ret;
}
@@ -1569,7 +1570,8 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
pdd = dev->power.subsys_data->domain_data;
gpd_data = to_gpd_data(pdd);
- dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
+ dev_pm_qos_remove_notifier(dev, &gpd_data->nb,
+ DEV_PM_QOS_RESUME_LATENCY);
genpd_lock(genpd);
@@ -1597,7 +1599,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
out:
genpd_unlock(genpd);
- dev_pm_qos_add_notifier(dev, &gpd_data->nb);
+ dev_pm_qos_add_notifier(dev, &gpd_data->nb, DEV_PM_QOS_RESUME_LATENCY);
return ret;
}
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 6c91f8df1d59..cfd463212513 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -467,6 +467,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
*
* @dev: target device for the constraint
* @notifier: notifier block managed by caller.
+ * @type: request type.
*
* Will register the notifier into a notification chain that gets called
* upon changes to the target value for the device.
@@ -474,10 +475,14 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
* If the device's constraints object doesn't exist when this routine is called,
* it will be created (or error code will be returned if that fails).
*/
-int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier)
+int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
+ enum dev_pm_qos_req_type type)
{
int ret = 0;
+ if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
+ return -EINVAL;
+
mutex_lock(&dev_pm_qos_mtx);
if (IS_ERR(dev->power.qos))
@@ -500,15 +505,20 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier);
*
* @dev: target device for the constraint
* @notifier: notifier block to be removed.
+ * @type: request type.
*
* Will remove the notifier from the notification chain that gets called
* upon changes to the target value.
*/
int dev_pm_qos_remove_notifier(struct device *dev,
- struct notifier_block *notifier)
+ struct notifier_block *notifier,
+ enum dev_pm_qos_req_type type)
{
int retval = 0;
+ if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
+ return -EINVAL;
+
mutex_lock(&dev_pm_qos_mtx);
/* Silently return if the constraints object is not present. */
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 6ea1ae373d77..1f4d456e8fff 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -146,9 +146,11 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
int dev_pm_qos_add_notifier(struct device *dev,
- struct notifier_block *notifier);
+ struct notifier_block *notifier,
+ enum dev_pm_qos_req_type type);
int dev_pm_qos_remove_notifier(struct device *dev,
- struct notifier_block *notifier);
+ struct notifier_block *notifier,
+ enum dev_pm_qos_req_type type);
void dev_pm_qos_constraints_init(struct device *dev);
void dev_pm_qos_constraints_destroy(struct device *dev);
int dev_pm_qos_add_ancestor_request(struct device *dev,
@@ -202,10 +204,12 @@ static inline int dev_pm_qos_update_request(struct dev_pm_qos_request *req,
static inline int dev_pm_qos_remove_request(struct dev_pm_qos_request *req)
{ return 0; }
static inline int dev_pm_qos_add_notifier(struct device *dev,
- struct notifier_block *notifier)
+ struct notifier_block *notifier,
+ enum dev_pm_qos_req_type type);
{ return 0; }
static inline int dev_pm_qos_remove_notifier(struct device *dev,
- struct notifier_block *notifier)
+ struct notifier_block *notifier,
+ enum dev_pm_qos_req_type type)
{ return 0; }
static inline void dev_pm_qos_constraints_init(struct device *dev)
{
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier()
2019-06-10 10:51 ` [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier() Viresh Kumar
@ 2019-06-11 23:45 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
2019-06-17 22:52 ` Rafael J. Wysocki
2 siblings, 0 replies; 27+ messages in thread
From: Matthias Kaehlcke @ 2019-06-11 23:45 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Len Brown, Pavel Machek, Kevin Hilman,
Ulf Hansson, linux-pm, Vincent Guittot, Qais.Yousef, juri.lelli,
linux-kernel
On Mon, Jun 10, 2019 at 04:21:32PM +0530, Viresh Kumar wrote:
> In order to use the same set of routines to register notifiers for
> different request types, update the existing
> dev_pm_qos_{add|remove}_notifier() routines with an additional
> parameter: request-type.
>
> For now, it only supports resume-latency request type.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Documentation/power/pm_qos_interface.txt | 10 ++++++----
> drivers/base/power/domain.c | 8 +++++---
> drivers/base/power/qos.c | 14 ++++++++++++--
> include/linux/pm_qos.h | 12 ++++++++----
> 4 files changed, 31 insertions(+), 13 deletions(-)
My QoS background is nil, but this looks reasonable to me:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier()
2019-06-10 10:51 ` [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier() Viresh Kumar
2019-06-11 23:45 ` Matthias Kaehlcke
@ 2019-06-17 9:23 ` Ulf Hansson
2019-06-17 22:52 ` Rafael J. Wysocki
2 siblings, 0 replies; 27+ messages in thread
From: Ulf Hansson @ 2019-06-17 9:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Len Brown, Pavel Machek, Kevin Hilman, Linux PM,
Vincent Guittot, Qais.Yousef, Matthias Kaehlcke, juri.lelli,
Linux Kernel Mailing List
On Mon, 10 Jun 2019 at 12:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> In order to use the same set of routines to register notifiers for
> different request types, update the existing
> dev_pm_qos_{add|remove}_notifier() routines with an additional
> parameter: request-type.
>
> For now, it only supports resume-latency request type.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> Documentation/power/pm_qos_interface.txt | 10 ++++++----
> drivers/base/power/domain.c | 8 +++++---
> drivers/base/power/qos.c | 14 ++++++++++++--
> include/linux/pm_qos.h | 12 ++++++++----
> 4 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
> index 19c5f7b1a7ba..ec7d662d1707 100644
> --- a/Documentation/power/pm_qos_interface.txt
> +++ b/Documentation/power/pm_qos_interface.txt
> @@ -164,12 +164,14 @@ directory.
> Notification mechanisms:
> The per-device PM QoS framework has a per-device notification tree.
>
> -int dev_pm_qos_add_notifier(device, notifier):
> -Adds a notification callback function for the device.
> +int dev_pm_qos_add_notifier(device, notifier, type):
> +Adds a notification callback function for the device for a particular request
> +type.
> +
> The callback is called when the aggregated value of the device constraints list
> -is changed (for resume latency device PM QoS only).
> +is changed.
>
> -int dev_pm_qos_remove_notifier(device, notifier):
> +int dev_pm_qos_remove_notifier(device, notifier, type):
> Removes the notification callback function for the device.
>
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 33c30c1e6a30..b063bc41b0a9 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1536,7 +1536,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
> if (ret)
> genpd_free_dev_data(dev, gpd_data);
> else
> - dev_pm_qos_add_notifier(dev, &gpd_data->nb);
> + dev_pm_qos_add_notifier(dev, &gpd_data->nb,
> + DEV_PM_QOS_RESUME_LATENCY);
>
> return ret;
> }
> @@ -1569,7 +1570,8 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>
> pdd = dev->power.subsys_data->domain_data;
> gpd_data = to_gpd_data(pdd);
> - dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> + dev_pm_qos_remove_notifier(dev, &gpd_data->nb,
> + DEV_PM_QOS_RESUME_LATENCY);
>
> genpd_lock(genpd);
>
> @@ -1597,7 +1599,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>
> out:
> genpd_unlock(genpd);
> - dev_pm_qos_add_notifier(dev, &gpd_data->nb);
> + dev_pm_qos_add_notifier(dev, &gpd_data->nb, DEV_PM_QOS_RESUME_LATENCY);
>
> return ret;
> }
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 6c91f8df1d59..cfd463212513 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -467,6 +467,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
> *
> * @dev: target device for the constraint
> * @notifier: notifier block managed by caller.
> + * @type: request type.
> *
> * Will register the notifier into a notification chain that gets called
> * upon changes to the target value for the device.
> @@ -474,10 +475,14 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
> * If the device's constraints object doesn't exist when this routine is called,
> * it will be created (or error code will be returned if that fails).
> */
> -int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier)
> +int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
> + enum dev_pm_qos_req_type type)
> {
> int ret = 0;
>
> + if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
> + return -EINVAL;
> +
> mutex_lock(&dev_pm_qos_mtx);
>
> if (IS_ERR(dev->power.qos))
> @@ -500,15 +505,20 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier);
> *
> * @dev: target device for the constraint
> * @notifier: notifier block to be removed.
> + * @type: request type.
> *
> * Will remove the notifier from the notification chain that gets called
> * upon changes to the target value.
> */
> int dev_pm_qos_remove_notifier(struct device *dev,
> - struct notifier_block *notifier)
> + struct notifier_block *notifier,
> + enum dev_pm_qos_req_type type)
> {
> int retval = 0;
>
> + if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
> + return -EINVAL;
> +
> mutex_lock(&dev_pm_qos_mtx);
>
> /* Silently return if the constraints object is not present. */
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 6ea1ae373d77..1f4d456e8fff 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -146,9 +146,11 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
> int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
> int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
> int dev_pm_qos_add_notifier(struct device *dev,
> - struct notifier_block *notifier);
> + struct notifier_block *notifier,
> + enum dev_pm_qos_req_type type);
> int dev_pm_qos_remove_notifier(struct device *dev,
> - struct notifier_block *notifier);
> + struct notifier_block *notifier,
> + enum dev_pm_qos_req_type type);
> void dev_pm_qos_constraints_init(struct device *dev);
> void dev_pm_qos_constraints_destroy(struct device *dev);
> int dev_pm_qos_add_ancestor_request(struct device *dev,
> @@ -202,10 +204,12 @@ static inline int dev_pm_qos_update_request(struct dev_pm_qos_request *req,
> static inline int dev_pm_qos_remove_request(struct dev_pm_qos_request *req)
> { return 0; }
> static inline int dev_pm_qos_add_notifier(struct device *dev,
> - struct notifier_block *notifier)
> + struct notifier_block *notifier,
> + enum dev_pm_qos_req_type type);
> { return 0; }
> static inline int dev_pm_qos_remove_notifier(struct device *dev,
> - struct notifier_block *notifier)
> + struct notifier_block *notifier,
> + enum dev_pm_qos_req_type type)
> { return 0; }
> static inline void dev_pm_qos_constraints_init(struct device *dev)
> {
> --
> 2.21.0.rc0.269.g1a574e7a288b
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier()
2019-06-10 10:51 ` [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier() Viresh Kumar
2019-06-11 23:45 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
@ 2019-06-17 22:52 ` Rafael J. Wysocki
2 siblings, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2019-06-17 22:52 UTC (permalink / raw)
To: Viresh Kumar
Cc: Len Brown, Pavel Machek, Kevin Hilman, Ulf Hansson, linux-pm,
Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On Monday, June 10, 2019 12:51:32 PM CEST Viresh Kumar wrote:
> In order to use the same set of routines to register notifiers for
> different request types, update the existing
> dev_pm_qos_{add|remove}_notifier() routines with an additional
> parameter: request-type.
>
> For now, it only supports resume-latency request type.
It would be good to mention the broader rationale of this change in its changelog
(that is, the frequency limits use case).
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value()
2019-06-10 10:51 [PATCH V3 0/5] cpufreq: Use QoS layer to manage freq-constraints Viresh Kumar
2019-06-10 10:51 ` [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier() Viresh Kumar
@ 2019-06-10 10:51 ` Viresh Kumar
2019-06-12 0:08 ` Matthias Kaehlcke
` (2 more replies)
2019-06-10 10:51 ` [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints Viresh Kumar
` (2 subsequent siblings)
4 siblings, 3 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-10 10:51 UTC (permalink / raw)
To: Rafael Wysocki, Len Brown, Pavel Machek, Kevin Hilman,
Ulf Hansson, Daniel Lezcano
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Qais.Yousef, mka,
juri.lelli, linux-kernel
In order to use dev_pm_qos_read_value(), and other internal routines to
it, to read values for different QoS requests, pass request type as a
parameter to these routines.
For now, it only supports resume-latency request type.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/power/pm_qos_interface.txt | 2 +-
drivers/base/power/domain_governor.c | 4 +--
drivers/base/power/qos.c | 10 +++---
drivers/base/power/runtime.c | 2 +-
drivers/cpuidle/governor.c | 2 +-
include/linux/pm_qos.h | 41 +++++++++++++++++-------
6 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
index ec7d662d1707..cfcb1df39799 100644
--- a/Documentation/power/pm_qos_interface.txt
+++ b/Documentation/power/pm_qos_interface.txt
@@ -123,7 +123,7 @@ Will remove the element. After removal it will update the aggregate target and
call the notification trees if the target was changed as a result of removing
the request.
-s32 dev_pm_qos_read_value(device):
+s32 dev_pm_qos_read_value(device, type):
Returns the aggregated value for a given device's constraints list.
enum pm_qos_flags_status dev_pm_qos_flags(device, mask)
diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index 3838045c9277..f66ac46d07d0 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -33,7 +33,7 @@ static int dev_update_qos_constraint(struct device *dev, void *data)
* take its current PM QoS constraint (that's the only thing
* known at this point anyway).
*/
- constraint_ns = dev_pm_qos_read_value(dev);
+ constraint_ns = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
constraint_ns *= NSEC_PER_USEC;
}
@@ -66,7 +66,7 @@ static bool default_suspend_ok(struct device *dev)
td->constraint_changed = false;
td->cached_suspend_ok = false;
td->effective_constraint_ns = 0;
- constraint_ns = __dev_pm_qos_read_value(dev);
+ constraint_ns = __dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
spin_unlock_irqrestore(&dev->power.lock, flags);
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index cfd463212513..7bb88174371f 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -92,27 +92,29 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_flags);
/**
* __dev_pm_qos_read_value - Get PM QoS constraint for a given device.
* @dev: Device to get the PM QoS constraint value for.
+ * @type: QoS request type.
*
* This routine must be called with dev->power.lock held.
*/
-s32 __dev_pm_qos_read_value(struct device *dev)
+s32 __dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
{
lockdep_assert_held(&dev->power.lock);
- return dev_pm_qos_raw_read_value(dev);
+ return dev_pm_qos_raw_read_value(dev, type);
}
/**
* dev_pm_qos_read_value - Get PM QoS constraint for a given device (locked).
* @dev: Device to get the PM QoS constraint value for.
+ * @type: QoS request type.
*/
-s32 dev_pm_qos_read_value(struct device *dev)
+s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
{
unsigned long flags;
s32 ret;
spin_lock_irqsave(&dev->power.lock, flags);
- ret = __dev_pm_qos_read_value(dev);
+ ret = __dev_pm_qos_read_value(dev, type);
spin_unlock_irqrestore(&dev->power.lock, flags);
return ret;
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 952a1e7057c7..92ffc2a0151d 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -275,7 +275,7 @@ static int rpm_check_suspend_allowed(struct device *dev)
|| (dev->power.request_pending
&& dev->power.request == RPM_REQ_RESUME))
retval = -EAGAIN;
- else if (__dev_pm_qos_read_value(dev) == 0)
+ else if (__dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY) == 0)
retval = -EPERM;
else if (dev->power.runtime_status == RPM_SUSPENDED)
retval = 1;
diff --git a/drivers/cpuidle/governor.c b/drivers/cpuidle/governor.c
index 9fddf828a76f..fb5659eb2009 100644
--- a/drivers/cpuidle/governor.c
+++ b/drivers/cpuidle/governor.c
@@ -110,7 +110,7 @@ int cpuidle_governor_latency_req(unsigned int cpu)
{
int global_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
struct device *device = get_cpu_device(cpu);
- int device_req = dev_pm_qos_raw_read_value(device);
+ int device_req = dev_pm_qos_raw_read_value(device, DEV_PM_QOS_RESUME_LATENCY);
return device_req < global_req ? device_req : global_req;
}
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 1f4d456e8fff..55814d48c39c 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -139,8 +139,8 @@ s32 pm_qos_read_value(struct pm_qos_constraints *c);
#ifdef CONFIG_PM
enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, s32 mask);
enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev, s32 mask);
-s32 __dev_pm_qos_read_value(struct device *dev);
-s32 dev_pm_qos_read_value(struct device *dev);
+s32 __dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type);
+s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type);
int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
enum dev_pm_qos_req_type type, s32 value);
int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
@@ -176,11 +176,19 @@ static inline s32 dev_pm_qos_requested_flags(struct device *dev)
return dev->power.qos->flags_req->data.flr.flags;
}
-static inline s32 dev_pm_qos_raw_read_value(struct device *dev)
+static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
+ enum dev_pm_qos_req_type type)
{
- return IS_ERR_OR_NULL(dev->power.qos) ?
- PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
- pm_qos_read_value(&dev->power.qos->resume_latency);
+ struct dev_pm_qos *qos = dev->power.qos;
+
+ switch (type) {
+ case DEV_PM_QOS_RESUME_LATENCY:
+ return IS_ERR_OR_NULL(qos) ? PM_QOS_RESUME_LATENCY_NO_CONSTRAINT
+ : pm_qos_read_value(&qos->resume_latency);
+ default:
+ WARN_ON(1);
+ return 0;
+ }
}
#else
static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
@@ -189,10 +197,6 @@ static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
static inline enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev,
s32 mask)
{ return PM_QOS_FLAGS_UNDEFINED; }
-static inline s32 __dev_pm_qos_read_value(struct device *dev)
- { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; }
-static inline s32 dev_pm_qos_read_value(struct device *dev)
- { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; }
static inline int dev_pm_qos_add_request(struct device *dev,
struct dev_pm_qos_request *req,
enum dev_pm_qos_req_type type,
@@ -245,10 +249,23 @@ static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
}
static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; }
-static inline s32 dev_pm_qos_raw_read_value(struct device *dev)
+
+static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
+ enum dev_pm_qos_req_type type)
{
- return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+ switch type {
+ case DEV_PM_QOS_RESUME_LATENCY:
+ return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+ default:
+ WARN_ON(1);
+ return 0;
+ }
}
+
+static inline s32 __dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
+ { return dev_pm_qos_raw_read_value(dev, type); }
+static inline s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
+ { return dev_pm_qos_raw_read_value(dev, type); }
#endif
#endif
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value()
2019-06-10 10:51 ` [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value() Viresh Kumar
@ 2019-06-12 0:08 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
2019-06-17 23:14 ` Rafael J. Wysocki
2 siblings, 0 replies; 27+ messages in thread
From: Matthias Kaehlcke @ 2019-06-12 0:08 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Len Brown, Pavel Machek, Kevin Hilman,
Ulf Hansson, Daniel Lezcano, linux-pm, Vincent Guittot,
Qais.Yousef, juri.lelli, linux-kernel
On Mon, Jun 10, 2019 at 04:21:33PM +0530, Viresh Kumar wrote:
> In order to use dev_pm_qos_read_value(), and other internal routines to
> it, to read values for different QoS requests, pass request type as a
> parameter to these routines.
nit: I find that somewhat hard to parse, a possible improvement:
"In order to allow dev_pm_qos_read_value() and other {related,similar}
internal routines to read values for different QoS requests, pass
request type as a parameter to these routines."
(I'm not a native English speaker, so I don't claim this to be
correct ;-)
> For now, it only supports resume-latency request type.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Documentation/power/pm_qos_interface.txt | 2 +-
> drivers/base/power/domain_governor.c | 4 +--
> drivers/base/power/qos.c | 10 +++---
> drivers/base/power/runtime.c | 2 +-
> drivers/cpuidle/governor.c | 2 +-
> include/linux/pm_qos.h | 41 +++++++++++++++++-------
> 6 files changed, 40 insertions(+), 21 deletions(-)
looks good to me, however I'm just a QoS n00b:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value()
2019-06-10 10:51 ` [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value() Viresh Kumar
2019-06-12 0:08 ` Matthias Kaehlcke
@ 2019-06-17 9:23 ` Ulf Hansson
2019-06-17 23:14 ` Rafael J. Wysocki
2 siblings, 0 replies; 27+ messages in thread
From: Ulf Hansson @ 2019-06-17 9:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Len Brown, Pavel Machek, Kevin Hilman,
Daniel Lezcano, Linux PM, Vincent Guittot, Qais.Yousef,
Matthias Kaehlcke, juri.lelli, Linux Kernel Mailing List
On Mon, 10 Jun 2019 at 12:51, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> In order to use dev_pm_qos_read_value(), and other internal routines to
> it, to read values for different QoS requests, pass request type as a
> parameter to these routines.
>
> For now, it only supports resume-latency request type.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> Documentation/power/pm_qos_interface.txt | 2 +-
> drivers/base/power/domain_governor.c | 4 +--
> drivers/base/power/qos.c | 10 +++---
> drivers/base/power/runtime.c | 2 +-
> drivers/cpuidle/governor.c | 2 +-
> include/linux/pm_qos.h | 41 +++++++++++++++++-------
> 6 files changed, 40 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
> index ec7d662d1707..cfcb1df39799 100644
> --- a/Documentation/power/pm_qos_interface.txt
> +++ b/Documentation/power/pm_qos_interface.txt
> @@ -123,7 +123,7 @@ Will remove the element. After removal it will update the aggregate target and
> call the notification trees if the target was changed as a result of removing
> the request.
>
> -s32 dev_pm_qos_read_value(device):
> +s32 dev_pm_qos_read_value(device, type):
> Returns the aggregated value for a given device's constraints list.
>
> enum pm_qos_flags_status dev_pm_qos_flags(device, mask)
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index 3838045c9277..f66ac46d07d0 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -33,7 +33,7 @@ static int dev_update_qos_constraint(struct device *dev, void *data)
> * take its current PM QoS constraint (that's the only thing
> * known at this point anyway).
> */
> - constraint_ns = dev_pm_qos_read_value(dev);
> + constraint_ns = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
> constraint_ns *= NSEC_PER_USEC;
> }
>
> @@ -66,7 +66,7 @@ static bool default_suspend_ok(struct device *dev)
> td->constraint_changed = false;
> td->cached_suspend_ok = false;
> td->effective_constraint_ns = 0;
> - constraint_ns = __dev_pm_qos_read_value(dev);
> + constraint_ns = __dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
>
> spin_unlock_irqrestore(&dev->power.lock, flags);
>
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index cfd463212513..7bb88174371f 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -92,27 +92,29 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_flags);
> /**
> * __dev_pm_qos_read_value - Get PM QoS constraint for a given device.
> * @dev: Device to get the PM QoS constraint value for.
> + * @type: QoS request type.
> *
> * This routine must be called with dev->power.lock held.
> */
> -s32 __dev_pm_qos_read_value(struct device *dev)
> +s32 __dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> {
> lockdep_assert_held(&dev->power.lock);
>
> - return dev_pm_qos_raw_read_value(dev);
> + return dev_pm_qos_raw_read_value(dev, type);
> }
>
> /**
> * dev_pm_qos_read_value - Get PM QoS constraint for a given device (locked).
> * @dev: Device to get the PM QoS constraint value for.
> + * @type: QoS request type.
> */
> -s32 dev_pm_qos_read_value(struct device *dev)
> +s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> {
> unsigned long flags;
> s32 ret;
>
> spin_lock_irqsave(&dev->power.lock, flags);
> - ret = __dev_pm_qos_read_value(dev);
> + ret = __dev_pm_qos_read_value(dev, type);
> spin_unlock_irqrestore(&dev->power.lock, flags);
>
> return ret;
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 952a1e7057c7..92ffc2a0151d 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -275,7 +275,7 @@ static int rpm_check_suspend_allowed(struct device *dev)
> || (dev->power.request_pending
> && dev->power.request == RPM_REQ_RESUME))
> retval = -EAGAIN;
> - else if (__dev_pm_qos_read_value(dev) == 0)
> + else if (__dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY) == 0)
> retval = -EPERM;
> else if (dev->power.runtime_status == RPM_SUSPENDED)
> retval = 1;
> diff --git a/drivers/cpuidle/governor.c b/drivers/cpuidle/governor.c
> index 9fddf828a76f..fb5659eb2009 100644
> --- a/drivers/cpuidle/governor.c
> +++ b/drivers/cpuidle/governor.c
> @@ -110,7 +110,7 @@ int cpuidle_governor_latency_req(unsigned int cpu)
> {
> int global_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
> struct device *device = get_cpu_device(cpu);
> - int device_req = dev_pm_qos_raw_read_value(device);
> + int device_req = dev_pm_qos_raw_read_value(device, DEV_PM_QOS_RESUME_LATENCY);
>
> return device_req < global_req ? device_req : global_req;
> }
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 1f4d456e8fff..55814d48c39c 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -139,8 +139,8 @@ s32 pm_qos_read_value(struct pm_qos_constraints *c);
> #ifdef CONFIG_PM
> enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, s32 mask);
> enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev, s32 mask);
> -s32 __dev_pm_qos_read_value(struct device *dev);
> -s32 dev_pm_qos_read_value(struct device *dev);
> +s32 __dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type);
> +s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type);
> int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
> enum dev_pm_qos_req_type type, s32 value);
> int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
> @@ -176,11 +176,19 @@ static inline s32 dev_pm_qos_requested_flags(struct device *dev)
> return dev->power.qos->flags_req->data.flr.flags;
> }
>
> -static inline s32 dev_pm_qos_raw_read_value(struct device *dev)
> +static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
> + enum dev_pm_qos_req_type type)
> {
> - return IS_ERR_OR_NULL(dev->power.qos) ?
> - PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
> - pm_qos_read_value(&dev->power.qos->resume_latency);
> + struct dev_pm_qos *qos = dev->power.qos;
> +
> + switch (type) {
> + case DEV_PM_QOS_RESUME_LATENCY:
> + return IS_ERR_OR_NULL(qos) ? PM_QOS_RESUME_LATENCY_NO_CONSTRAINT
> + : pm_qos_read_value(&qos->resume_latency);
> + default:
> + WARN_ON(1);
> + return 0;
> + }
> }
> #else
> static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
> @@ -189,10 +197,6 @@ static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
> static inline enum pm_qos_flags_status dev_pm_qos_flags(struct device *dev,
> s32 mask)
> { return PM_QOS_FLAGS_UNDEFINED; }
> -static inline s32 __dev_pm_qos_read_value(struct device *dev)
> - { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; }
> -static inline s32 dev_pm_qos_read_value(struct device *dev)
> - { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; }
> static inline int dev_pm_qos_add_request(struct device *dev,
> struct dev_pm_qos_request *req,
> enum dev_pm_qos_req_type type,
> @@ -245,10 +249,23 @@ static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
> return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> }
> static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; }
> -static inline s32 dev_pm_qos_raw_read_value(struct device *dev)
> +
> +static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
> + enum dev_pm_qos_req_type type)
> {
> - return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> + switch type {
> + case DEV_PM_QOS_RESUME_LATENCY:
> + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> + default:
> + WARN_ON(1);
> + return 0;
> + }
> }
> +
> +static inline s32 __dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> + { return dev_pm_qos_raw_read_value(dev, type); }
> +static inline s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
> + { return dev_pm_qos_raw_read_value(dev, type); }
> #endif
>
> #endif
> --
> 2.21.0.rc0.269.g1a574e7a288b
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value()
2019-06-10 10:51 ` [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value() Viresh Kumar
2019-06-12 0:08 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
@ 2019-06-17 23:14 ` Rafael J. Wysocki
2 siblings, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2019-06-17 23:14 UTC (permalink / raw)
To: Viresh Kumar
Cc: Len Brown, Pavel Machek, Kevin Hilman, Ulf Hansson,
Daniel Lezcano, linux-pm, Vincent Guittot, Qais.Yousef, mka,
juri.lelli, linux-kernel
On Monday, June 10, 2019 12:51:33 PM CEST Viresh Kumar wrote:
> In order to use dev_pm_qos_read_value(), and other internal routines to
> it, to read values for different QoS requests, pass request type as a
> parameter to these routines.
>
> For now, it only supports resume-latency request type.
I don't quite like the structure by which the type arg is passed through the
entire call chain until the switch in dep_pm_qos_raw_read_value().
There is only one direct user of dev_pm_qos_raw_read_value() AFAICS which
is cpuidle. It shouldn't need to suffer the general case overhead, so I would
rename that function to dev_pm_qos_raw_resume_latency() and update cpuidle
accordingly.
Moreover, the callers of __dev_pm_qos_read_value() are interested in the
resume latency value too, so it might make sense to rename this as
__dev_pm_qos_resume_latency(), update its callers and put the switch
into dev_pm_qos_read_value().
Plus the changelog should explain the broader rationale of this change like
for the first patch IMO.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints
2019-06-10 10:51 [PATCH V3 0/5] cpufreq: Use QoS layer to manage freq-constraints Viresh Kumar
2019-06-10 10:51 ` [PATCH V3 1/5] PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier() Viresh Kumar
2019-06-10 10:51 ` [PATCH V3 2/5] PM / QOS: Pass request type to dev_pm_qos_read_value() Viresh Kumar
@ 2019-06-10 10:51 ` Viresh Kumar
2019-06-13 0:04 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
2019-06-10 10:51 ` [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework Viresh Kumar
2019-06-10 10:51 ` [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints Viresh Kumar
4 siblings, 2 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-10 10:51 UTC (permalink / raw)
To: Rafael Wysocki, Pavel Machek, Len Brown
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Qais.Yousef, mka,
juri.lelli, linux-kernel
This patch introduces the min-frequency and max-frequency device
constraints, which will be used by the cpufreq core to begin with.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/qos.c | 103 +++++++++++++++++++++++++++++++++------
include/linux/pm_qos.h | 18 +++++++
2 files changed, 107 insertions(+), 14 deletions(-)
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 7bb88174371f..e977aef437a5 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -151,6 +151,14 @@ static int apply_constraint(struct dev_pm_qos_request *req,
req->dev->power.set_latency_tolerance(req->dev, value);
}
break;
+ case DEV_PM_QOS_MIN_FREQUENCY:
+ ret = pm_qos_update_target(&qos->min_frequency,
+ &req->data.pnode, action, value);
+ break;
+ case DEV_PM_QOS_MAX_FREQUENCY:
+ ret = pm_qos_update_target(&qos->max_frequency,
+ &req->data.pnode, action, value);
+ break;
case DEV_PM_QOS_FLAGS:
ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
action, value);
@@ -179,12 +187,11 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
if (!qos)
return -ENOMEM;
- n = kzalloc(sizeof(*n), GFP_KERNEL);
+ n = kzalloc(3 * sizeof(*n), GFP_KERNEL);
if (!n) {
kfree(qos);
return -ENOMEM;
}
- BLOCKING_INIT_NOTIFIER_HEAD(n);
c = &qos->resume_latency;
plist_head_init(&c->list);
@@ -193,6 +200,7 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
c->type = PM_QOS_MIN;
c->notifiers = n;
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
c = &qos->latency_tolerance;
plist_head_init(&c->list);
@@ -201,6 +209,24 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
c->type = PM_QOS_MIN;
+ c = &qos->min_frequency;
+ plist_head_init(&c->list);
+ c->target_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
+ c->default_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
+ c->no_constraint_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
+ c->type = PM_QOS_MAX;
+ c->notifiers = ++n;
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
+
+ c = &qos->max_frequency;
+ plist_head_init(&c->list);
+ c->target_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
+ c->default_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
+ c->no_constraint_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
+ c->type = PM_QOS_MIN;
+ c->notifiers = ++n;
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
+
INIT_LIST_HEAD(&qos->flags.list);
spin_lock_irq(&dev->power.lock);
@@ -254,11 +280,25 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
memset(req, 0, sizeof(*req));
}
+
c = &qos->latency_tolerance;
plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
memset(req, 0, sizeof(*req));
}
+
+ c = &qos->min_frequency;
+ plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
+ apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE);
+ memset(req, 0, sizeof(*req));
+ }
+
+ c = &qos->max_frequency;
+ plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
+ apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
+ memset(req, 0, sizeof(*req));
+ }
+
f = &qos->flags;
list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
@@ -370,6 +410,8 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
switch(req->type) {
case DEV_PM_QOS_RESUME_LATENCY:
case DEV_PM_QOS_LATENCY_TOLERANCE:
+ case DEV_PM_QOS_MIN_FREQUENCY:
+ case DEV_PM_QOS_MAX_FREQUENCY:
curr_value = req->data.pnode.prio;
break;
case DEV_PM_QOS_FLAGS:
@@ -482,9 +524,6 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
{
int ret = 0;
- if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
- return -EINVAL;
-
mutex_lock(&dev_pm_qos_mtx);
if (IS_ERR(dev->power.qos))
@@ -492,10 +531,28 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
else if (!dev->power.qos)
ret = dev_pm_qos_constraints_allocate(dev);
- if (!ret)
+ if (ret)
+ goto unlock;
+
+ switch (type) {
+ case DEV_PM_QOS_RESUME_LATENCY:
ret = blocking_notifier_chain_register(dev->power.qos->resume_latency.notifiers,
notifier);
+ break;
+ case DEV_PM_QOS_MIN_FREQUENCY:
+ ret = blocking_notifier_chain_register(dev->power.qos->min_frequency.notifiers,
+ notifier);
+ break;
+ case DEV_PM_QOS_MAX_FREQUENCY:
+ ret = blocking_notifier_chain_register(dev->power.qos->max_frequency.notifiers,
+ notifier);
+ break;
+ default:
+ WARN_ON(1);
+ ret = -EINVAL;
+ }
+unlock:
mutex_unlock(&dev_pm_qos_mtx);
return ret;
}
@@ -516,20 +573,35 @@ int dev_pm_qos_remove_notifier(struct device *dev,
struct notifier_block *notifier,
enum dev_pm_qos_req_type type)
{
- int retval = 0;
-
- if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
- return -EINVAL;
+ int ret = 0;
mutex_lock(&dev_pm_qos_mtx);
/* Silently return if the constraints object is not present. */
- if (!IS_ERR_OR_NULL(dev->power.qos))
- retval = blocking_notifier_chain_unregister(dev->power.qos->resume_latency.notifiers,
- notifier);
+ if (IS_ERR_OR_NULL(dev->power.qos))
+ goto unlock;
+ switch (type) {
+ case DEV_PM_QOS_RESUME_LATENCY:
+ ret = blocking_notifier_chain_unregister(dev->power.qos->resume_latency.notifiers,
+ notifier);
+ break;
+ case DEV_PM_QOS_MIN_FREQUENCY:
+ ret = blocking_notifier_chain_unregister(dev->power.qos->min_frequency.notifiers,
+ notifier);
+ break;
+ case DEV_PM_QOS_MAX_FREQUENCY:
+ ret = blocking_notifier_chain_unregister(dev->power.qos->max_frequency.notifiers,
+ notifier);
+ break;
+ default:
+ WARN_ON(1);
+ ret = -EINVAL;
+ }
+
+unlock:
mutex_unlock(&dev_pm_qos_mtx);
- return retval;
+ return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_qos_remove_notifier);
@@ -589,6 +661,9 @@ static void __dev_pm_qos_drop_user_request(struct device *dev,
req = dev->power.qos->flags_req;
dev->power.qos->flags_req = NULL;
break;
+ default:
+ WARN_ON(1);
+ return;
}
__dev_pm_qos_remove_request(req);
kfree(req);
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 55814d48c39c..3f994283a5ae 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -40,6 +40,8 @@ enum pm_qos_flags_status {
#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY
#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS PM_QOS_LATENCY_ANY_NS
#define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0
+#define PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE 0
+#define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE (-1)
#define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1)
#define PM_QOS_FLAG_NO_POWER_OFF (1 << 0)
@@ -58,6 +60,8 @@ struct pm_qos_flags_request {
enum dev_pm_qos_req_type {
DEV_PM_QOS_RESUME_LATENCY = 1,
DEV_PM_QOS_LATENCY_TOLERANCE,
+ DEV_PM_QOS_MIN_FREQUENCY,
+ DEV_PM_QOS_MAX_FREQUENCY,
DEV_PM_QOS_FLAGS,
};
@@ -99,10 +103,14 @@ struct pm_qos_flags {
struct dev_pm_qos {
struct pm_qos_constraints resume_latency;
struct pm_qos_constraints latency_tolerance;
+ struct pm_qos_constraints min_frequency;
+ struct pm_qos_constraints max_frequency;
struct pm_qos_flags flags;
struct dev_pm_qos_request *resume_latency_req;
struct dev_pm_qos_request *latency_tolerance_req;
struct dev_pm_qos_request *flags_req;
+ struct dev_pm_qos_request *min_frequency_req;
+ struct dev_pm_qos_request *max_frequency_req;
};
/* Action requested to pm_qos_update_target */
@@ -185,6 +193,12 @@ static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
case DEV_PM_QOS_RESUME_LATENCY:
return IS_ERR_OR_NULL(qos) ? PM_QOS_RESUME_LATENCY_NO_CONSTRAINT
: pm_qos_read_value(&qos->resume_latency);
+ case DEV_PM_QOS_MIN_FREQUENCY:
+ return IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE
+ : pm_qos_read_value(&qos->min_frequency);
+ case DEV_PM_QOS_MAX_FREQUENCY:
+ return IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
+ : pm_qos_read_value(&qos->max_frequency);
default:
WARN_ON(1);
return 0;
@@ -256,6 +270,10 @@ static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
switch type {
case DEV_PM_QOS_RESUME_LATENCY:
return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+ case DEV_PM_QOS_MIN_FREQUENCY:
+ return PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
+ case DEV_PM_QOS_MAX_FREQUENCY:
+ return PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
default:
WARN_ON(1);
return 0;
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints
2019-06-10 10:51 ` [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints Viresh Kumar
@ 2019-06-13 0:04 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
1 sibling, 0 replies; 27+ messages in thread
From: Matthias Kaehlcke @ 2019-06-13 0:04 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Pavel Machek, Len Brown, linux-pm,
Vincent Guittot, Qais.Yousef, juri.lelli, linux-kernel
On Mon, Jun 10, 2019 at 04:21:34PM +0530, Viresh Kumar wrote:
> This patch introduces the min-frequency and max-frequency device
> constraints, which will be used by the cpufreq core to begin with.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Again, I'm mostly ignorant about QoS, in the context of the
existing code (particularly looking at DEV_PM_QOS_RESUME_LATENCY)
this looks reasonable to me. FWIW:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints
2019-06-10 10:51 ` [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints Viresh Kumar
2019-06-13 0:04 ` Matthias Kaehlcke
@ 2019-06-17 9:23 ` Ulf Hansson
1 sibling, 0 replies; 27+ messages in thread
From: Ulf Hansson @ 2019-06-17 9:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Pavel Machek, Len Brown, Linux PM,
Vincent Guittot, Qais.Yousef, Matthias Kaehlcke, juri.lelli,
Linux Kernel Mailing List
On Mon, 10 Jun 2019 at 12:52, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This patch introduces the min-frequency and max-frequency device
> constraints, which will be used by the cpufreq core to begin with.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/base/power/qos.c | 103 +++++++++++++++++++++++++++++++++------
> include/linux/pm_qos.h | 18 +++++++
> 2 files changed, 107 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 7bb88174371f..e977aef437a5 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -151,6 +151,14 @@ static int apply_constraint(struct dev_pm_qos_request *req,
> req->dev->power.set_latency_tolerance(req->dev, value);
> }
> break;
> + case DEV_PM_QOS_MIN_FREQUENCY:
> + ret = pm_qos_update_target(&qos->min_frequency,
> + &req->data.pnode, action, value);
> + break;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> + ret = pm_qos_update_target(&qos->max_frequency,
> + &req->data.pnode, action, value);
> + break;
> case DEV_PM_QOS_FLAGS:
> ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
> action, value);
> @@ -179,12 +187,11 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
> if (!qos)
> return -ENOMEM;
>
> - n = kzalloc(sizeof(*n), GFP_KERNEL);
> + n = kzalloc(3 * sizeof(*n), GFP_KERNEL);
> if (!n) {
> kfree(qos);
> return -ENOMEM;
> }
> - BLOCKING_INIT_NOTIFIER_HEAD(n);
>
> c = &qos->resume_latency;
> plist_head_init(&c->list);
> @@ -193,6 +200,7 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
> c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> c->type = PM_QOS_MIN;
> c->notifiers = n;
> + BLOCKING_INIT_NOTIFIER_HEAD(n);
>
> c = &qos->latency_tolerance;
> plist_head_init(&c->list);
> @@ -201,6 +209,24 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
> c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
> c->type = PM_QOS_MIN;
>
> + c = &qos->min_frequency;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
> + c->type = PM_QOS_MAX;
> + c->notifiers = ++n;
> + BLOCKING_INIT_NOTIFIER_HEAD(n);
> +
> + c = &qos->max_frequency;
> + plist_head_init(&c->list);
> + c->target_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> + c->default_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> + c->no_constraint_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> + c->type = PM_QOS_MIN;
> + c->notifiers = ++n;
> + BLOCKING_INIT_NOTIFIER_HEAD(n);
> +
> INIT_LIST_HEAD(&qos->flags.list);
>
> spin_lock_irq(&dev->power.lock);
> @@ -254,11 +280,25 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
> apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> memset(req, 0, sizeof(*req));
> }
> +
> c = &qos->latency_tolerance;
> plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
> apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> memset(req, 0, sizeof(*req));
> }
> +
> + c = &qos->min_frequency;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> + c = &qos->max_frequency;
> + plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
> + apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
> + memset(req, 0, sizeof(*req));
> + }
> +
> f = &qos->flags;
> list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
> apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
> @@ -370,6 +410,8 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
> switch(req->type) {
> case DEV_PM_QOS_RESUME_LATENCY:
> case DEV_PM_QOS_LATENCY_TOLERANCE:
> + case DEV_PM_QOS_MIN_FREQUENCY:
> + case DEV_PM_QOS_MAX_FREQUENCY:
> curr_value = req->data.pnode.prio;
> break;
> case DEV_PM_QOS_FLAGS:
> @@ -482,9 +524,6 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
> {
> int ret = 0;
>
> - if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
> - return -EINVAL;
> -
> mutex_lock(&dev_pm_qos_mtx);
>
> if (IS_ERR(dev->power.qos))
> @@ -492,10 +531,28 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
> else if (!dev->power.qos)
> ret = dev_pm_qos_constraints_allocate(dev);
>
> - if (!ret)
> + if (ret)
> + goto unlock;
> +
> + switch (type) {
> + case DEV_PM_QOS_RESUME_LATENCY:
> ret = blocking_notifier_chain_register(dev->power.qos->resume_latency.notifiers,
> notifier);
> + break;
> + case DEV_PM_QOS_MIN_FREQUENCY:
> + ret = blocking_notifier_chain_register(dev->power.qos->min_frequency.notifiers,
> + notifier);
> + break;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> + ret = blocking_notifier_chain_register(dev->power.qos->max_frequency.notifiers,
> + notifier);
> + break;
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + }
>
> +unlock:
> mutex_unlock(&dev_pm_qos_mtx);
> return ret;
> }
> @@ -516,20 +573,35 @@ int dev_pm_qos_remove_notifier(struct device *dev,
> struct notifier_block *notifier,
> enum dev_pm_qos_req_type type)
> {
> - int retval = 0;
> -
> - if (WARN_ON(type != DEV_PM_QOS_RESUME_LATENCY))
> - return -EINVAL;
> + int ret = 0;
>
> mutex_lock(&dev_pm_qos_mtx);
>
> /* Silently return if the constraints object is not present. */
> - if (!IS_ERR_OR_NULL(dev->power.qos))
> - retval = blocking_notifier_chain_unregister(dev->power.qos->resume_latency.notifiers,
> - notifier);
> + if (IS_ERR_OR_NULL(dev->power.qos))
> + goto unlock;
>
> + switch (type) {
> + case DEV_PM_QOS_RESUME_LATENCY:
> + ret = blocking_notifier_chain_unregister(dev->power.qos->resume_latency.notifiers,
> + notifier);
> + break;
> + case DEV_PM_QOS_MIN_FREQUENCY:
> + ret = blocking_notifier_chain_unregister(dev->power.qos->min_frequency.notifiers,
> + notifier);
> + break;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> + ret = blocking_notifier_chain_unregister(dev->power.qos->max_frequency.notifiers,
> + notifier);
> + break;
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + }
> +
> +unlock:
> mutex_unlock(&dev_pm_qos_mtx);
> - return retval;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(dev_pm_qos_remove_notifier);
>
> @@ -589,6 +661,9 @@ static void __dev_pm_qos_drop_user_request(struct device *dev,
> req = dev->power.qos->flags_req;
> dev->power.qos->flags_req = NULL;
> break;
> + default:
> + WARN_ON(1);
> + return;
> }
> __dev_pm_qos_remove_request(req);
> kfree(req);
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 55814d48c39c..3f994283a5ae 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -40,6 +40,8 @@ enum pm_qos_flags_status {
> #define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY
> #define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS PM_QOS_LATENCY_ANY_NS
> #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0
> +#define PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE 0
> +#define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE (-1)
> #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1)
>
> #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0)
> @@ -58,6 +60,8 @@ struct pm_qos_flags_request {
> enum dev_pm_qos_req_type {
> DEV_PM_QOS_RESUME_LATENCY = 1,
> DEV_PM_QOS_LATENCY_TOLERANCE,
> + DEV_PM_QOS_MIN_FREQUENCY,
> + DEV_PM_QOS_MAX_FREQUENCY,
> DEV_PM_QOS_FLAGS,
> };
>
> @@ -99,10 +103,14 @@ struct pm_qos_flags {
> struct dev_pm_qos {
> struct pm_qos_constraints resume_latency;
> struct pm_qos_constraints latency_tolerance;
> + struct pm_qos_constraints min_frequency;
> + struct pm_qos_constraints max_frequency;
> struct pm_qos_flags flags;
> struct dev_pm_qos_request *resume_latency_req;
> struct dev_pm_qos_request *latency_tolerance_req;
> struct dev_pm_qos_request *flags_req;
> + struct dev_pm_qos_request *min_frequency_req;
> + struct dev_pm_qos_request *max_frequency_req;
> };
>
> /* Action requested to pm_qos_update_target */
> @@ -185,6 +193,12 @@ static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
> case DEV_PM_QOS_RESUME_LATENCY:
> return IS_ERR_OR_NULL(qos) ? PM_QOS_RESUME_LATENCY_NO_CONSTRAINT
> : pm_qos_read_value(&qos->resume_latency);
> + case DEV_PM_QOS_MIN_FREQUENCY:
> + return IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE
> + : pm_qos_read_value(&qos->min_frequency);
> + case DEV_PM_QOS_MAX_FREQUENCY:
> + return IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
> + : pm_qos_read_value(&qos->max_frequency);
> default:
> WARN_ON(1);
> return 0;
> @@ -256,6 +270,10 @@ static inline s32 dev_pm_qos_raw_read_value(struct device *dev,
> switch type {
> case DEV_PM_QOS_RESUME_LATENCY:
> return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> + case DEV_PM_QOS_MIN_FREQUENCY:
> + return PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
> + case DEV_PM_QOS_MAX_FREQUENCY:
> + return PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
> default:
> WARN_ON(1);
> return 0;
> --
> 2.21.0.rc0.269.g1a574e7a288b
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-10 10:51 [PATCH V3 0/5] cpufreq: Use QoS layer to manage freq-constraints Viresh Kumar
` (2 preceding siblings ...)
2019-06-10 10:51 ` [PATCH V3 3/5] PM / QoS: Add support for MIN/MAX frequency constraints Viresh Kumar
@ 2019-06-10 10:51 ` Viresh Kumar
2019-06-14 16:46 ` Matthias Kaehlcke
` (3 more replies)
2019-06-10 10:51 ` [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints Viresh Kumar
4 siblings, 4 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-10 10:51 UTC (permalink / raw)
To: Rafael Wysocki
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Qais.Yousef, mka,
juri.lelli, linux-kernel
This registers the notifiers for min/max frequency constraints with the
PM QoS framework. The constraints are also taken into consideration in
cpufreq_set_policy().
This also relocates cpufreq_policy_put_kobj() as it is required to be
called from cpufreq_policy_alloc() now.
No constraints are added until now though.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 139 +++++++++++++++++++++++++++++++-------
include/linux/cpufreq.h | 4 ++
2 files changed, 120 insertions(+), 23 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 85ff958e01f1..547d221b2ff2 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -26,6 +26,7 @@
#include <linux/kernel_stat.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/suspend.h>
#include <linux/syscore_ops.h>
@@ -1126,11 +1127,77 @@ static void handle_update(struct work_struct *work)
cpufreq_update_policy(cpu);
}
+static void cpufreq_update_freq_work(struct work_struct *work)
+{
+ struct cpufreq_policy *policy =
+ container_of(work, struct cpufreq_policy, req_work);
+ struct cpufreq_policy new_policy = *policy;
+
+ /* We should read constraint values from QoS layer */
+ new_policy.min = 0;
+ new_policy.max = UINT_MAX;
+
+ down_write(&policy->rwsem);
+
+ if (!policy_is_inactive(policy))
+ cpufreq_set_policy(policy, &new_policy);
+
+ up_write(&policy->rwsem);
+}
+
+static int cpufreq_update_freq(struct cpufreq_policy *policy)
+{
+ schedule_work(&policy->req_work);
+ return 0;
+}
+
+static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
+ void *data)
+{
+ struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
+
+ return cpufreq_update_freq(policy);
+}
+
+static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
+ void *data)
+{
+ struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
+
+ return cpufreq_update_freq(policy);
+}
+
+static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
+{
+ struct kobject *kobj;
+ struct completion *cmp;
+
+ down_write(&policy->rwsem);
+ cpufreq_stats_free_table(policy);
+ kobj = &policy->kobj;
+ cmp = &policy->kobj_unregister;
+ up_write(&policy->rwsem);
+ kobject_put(kobj);
+
+ /*
+ * We need to make sure that the underlying kobj is
+ * actually not referenced anymore by anybody before we
+ * proceed with unloading.
+ */
+ pr_debug("waiting for dropping of refcount\n");
+ wait_for_completion(cmp);
+ pr_debug("wait complete\n");
+}
+
static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
{
struct cpufreq_policy *policy;
+ struct device *dev = get_cpu_device(cpu);
int ret;
+ if (!dev)
+ return NULL;
+
policy = kzalloc(sizeof(*policy), GFP_KERNEL);
if (!policy)
return NULL;
@@ -1147,7 +1214,7 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
ret = kobject_init_and_add(&policy->kobj, &ktype_cpufreq,
cpufreq_global_kobject, "policy%u", cpu);
if (ret) {
- pr_err("%s: failed to init policy->kobj: %d\n", __func__, ret);
+ dev_err(dev, "%s: failed to init policy->kobj: %d\n", __func__, ret);
/*
* The entire policy object will be freed below, but the extra
* memory allocated for the kobject name needs to be freed by
@@ -1157,16 +1224,41 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_free_real_cpus;
}
+ policy->nb_min.notifier_call = cpufreq_notifier_min;
+ policy->nb_max.notifier_call = cpufreq_notifier_max;
+
+ ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+ if (ret) {
+ dev_err(dev, "Failed to register MIN QoS notifier: %d (%*pbl)\n",
+ ret, cpumask_pr_args(policy->cpus));
+ goto err_kobj_remove;
+ }
+
+ ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
+ if (ret) {
+ dev_err(dev, "Failed to register MAX QoS notifier: %d (%*pbl)\n",
+ ret, cpumask_pr_args(policy->cpus));
+ goto err_min_qos_notifier;
+ }
+
INIT_LIST_HEAD(&policy->policy_list);
init_rwsem(&policy->rwsem);
spin_lock_init(&policy->transition_lock);
init_waitqueue_head(&policy->transition_wait);
init_completion(&policy->kobj_unregister);
INIT_WORK(&policy->update, handle_update);
+ INIT_WORK(&policy->req_work, cpufreq_update_freq_work);
policy->cpu = cpu;
return policy;
+err_min_qos_notifier:
+ dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
+err_kobj_remove:
+ cpufreq_policy_put_kobj(policy);
err_free_real_cpus:
free_cpumask_var(policy->real_cpus);
err_free_rcpumask:
@@ -1179,30 +1271,9 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
return NULL;
}
-static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
-{
- struct kobject *kobj;
- struct completion *cmp;
-
- down_write(&policy->rwsem);
- cpufreq_stats_free_table(policy);
- kobj = &policy->kobj;
- cmp = &policy->kobj_unregister;
- up_write(&policy->rwsem);
- kobject_put(kobj);
-
- /*
- * We need to make sure that the underlying kobj is
- * actually not referenced anymore by anybody before we
- * proceed with unloading.
- */
- pr_debug("waiting for dropping of refcount\n");
- wait_for_completion(cmp);
- pr_debug("wait complete\n");
-}
-
static void cpufreq_policy_free(struct cpufreq_policy *policy)
{
+ struct device *dev = get_cpu_device(policy->cpu);
unsigned long flags;
int cpu;
@@ -1214,6 +1285,10 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
per_cpu(cpufreq_cpu_data, cpu) = NULL;
write_unlock_irqrestore(&cpufreq_driver_lock, flags);
+ dev_pm_qos_remove_notifier(dev, &policy->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
+ dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ DEV_PM_QOS_MIN_FREQUENCY);
cpufreq_policy_put_kobj(policy);
free_cpumask_var(policy->real_cpus);
free_cpumask_var(policy->related_cpus);
@@ -2290,6 +2365,8 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
struct cpufreq_policy *new_policy)
{
struct cpufreq_governor *old_gov;
+ struct device *cpu_dev = get_cpu_device(policy->cpu);
+ unsigned long min, max;
int ret;
pr_debug("setting new policy for CPU %u: %u - %u kHz\n",
@@ -2304,11 +2381,27 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
if (new_policy->min > new_policy->max)
return -EINVAL;
+ /*
+ * PM QoS framework collects all the requests from users and provide us
+ * the final aggregated value here.
+ */
+ min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
+ max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
+
+ if (min > new_policy->min)
+ new_policy->min = min;
+ if (max < new_policy->max)
+ new_policy->max = max;
+
/* verify the cpu speed can be set within this limit */
ret = cpufreq_driver->verify(new_policy);
if (ret)
return ret;
+ /*
+ * The notifier-chain shall be removed once all the users of
+ * CPUFREQ_ADJUST are moved to use the QoS framework.
+ */
/* adjust if necessary - all reasons */
blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
CPUFREQ_ADJUST, new_policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d01a74fbc4db..0fe7678da9c2 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -83,6 +83,7 @@ struct cpufreq_policy {
struct work_struct update; /* if update_policy() needs to be
* called, but you're in IRQ context */
+ struct work_struct req_work;
struct cpufreq_user_policy user_policy;
struct cpufreq_frequency_table *freq_table;
@@ -147,6 +148,9 @@ struct cpufreq_policy {
/* Pointer to the cooling device if used for thermal mitigation */
struct thermal_cooling_device *cdev;
+
+ struct notifier_block nb_min;
+ struct notifier_block nb_max;
};
struct cpufreq_freqs {
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-10 10:51 ` [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework Viresh Kumar
@ 2019-06-14 16:46 ` Matthias Kaehlcke
2019-06-17 3:02 ` Viresh Kumar
2019-06-17 9:23 ` Ulf Hansson
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Matthias Kaehlcke @ 2019-06-14 16:46 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, Qais.Yousef,
juri.lelli, linux-kernel
Hi Viresh,
On Mon, Jun 10, 2019 at 04:21:35PM +0530, Viresh Kumar wrote:
> This registers the notifiers for min/max frequency constraints with the
> PM QoS framework. The constraints are also taken into consideration in
> cpufreq_set_policy().
>
> This also relocates cpufreq_policy_put_kobj() as it is required to be
> called from cpufreq_policy_alloc() now.
>
> No constraints are added until now though.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 139 +++++++++++++++++++++++++++++++-------
> include/linux/cpufreq.h | 4 ++
> 2 files changed, 120 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..547d221b2ff2 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/pm_qos.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> @@ -1126,11 +1127,77 @@ static void handle_update(struct work_struct *work)
> cpufreq_update_policy(cpu);
> }
>
> +static void cpufreq_update_freq_work(struct work_struct *work)
> +{
> + struct cpufreq_policy *policy =
> + container_of(work, struct cpufreq_policy, req_work);
> + struct cpufreq_policy new_policy = *policy;
> +
> + /* We should read constraint values from QoS layer */
> + new_policy.min = 0;
> + new_policy.max = UINT_MAX;
> +
> + down_write(&policy->rwsem);
> +
> + if (!policy_is_inactive(policy))
> + cpufreq_set_policy(policy, &new_policy);
> +
> + up_write(&policy->rwsem);
> +}
> +
> +static int cpufreq_update_freq(struct cpufreq_policy *policy)
> +{
> + schedule_work(&policy->req_work);
I think you need to add a cancel_work_sync() in cpufreq_policy_free()
to make sure the work doesn't run after the policy has been freed.
Otherwise it looks good to me.
Cheers
Matthias
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-14 16:46 ` Matthias Kaehlcke
@ 2019-06-17 3:02 ` Viresh Kumar
0 siblings, 0 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-17 3:02 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, Qais.Yousef,
juri.lelli, linux-kernel
On 14-06-19, 09:46, Matthias Kaehlcke wrote:
> Hi Viresh,
>
> On Mon, Jun 10, 2019 at 04:21:35PM +0530, Viresh Kumar wrote:
> > This registers the notifiers for min/max frequency constraints with the
> > PM QoS framework. The constraints are also taken into consideration in
> > cpufreq_set_policy().
> >
> > This also relocates cpufreq_policy_put_kobj() as it is required to be
> > called from cpufreq_policy_alloc() now.
> >
> > No constraints are added until now though.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/cpufreq.c | 139 +++++++++++++++++++++++++++++++-------
> > include/linux/cpufreq.h | 4 ++
> > 2 files changed, 120 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 85ff958e01f1..547d221b2ff2 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -26,6 +26,7 @@
> > #include <linux/kernel_stat.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > +#include <linux/pm_qos.h>
> > #include <linux/slab.h>
> > #include <linux/suspend.h>
> > #include <linux/syscore_ops.h>
> > @@ -1126,11 +1127,77 @@ static void handle_update(struct work_struct *work)
> > cpufreq_update_policy(cpu);
> > }
> >
> > +static void cpufreq_update_freq_work(struct work_struct *work)
> > +{
> > + struct cpufreq_policy *policy =
> > + container_of(work, struct cpufreq_policy, req_work);
> > + struct cpufreq_policy new_policy = *policy;
> > +
> > + /* We should read constraint values from QoS layer */
> > + new_policy.min = 0;
> > + new_policy.max = UINT_MAX;
> > +
> > + down_write(&policy->rwsem);
> > +
> > + if (!policy_is_inactive(policy))
> > + cpufreq_set_policy(policy, &new_policy);
> > +
> > + up_write(&policy->rwsem);
> > +}
> > +
> > +static int cpufreq_update_freq(struct cpufreq_policy *policy)
> > +{
> > + schedule_work(&policy->req_work);
>
> I think you need to add a cancel_work_sync() in cpufreq_policy_free()
> to make sure the work doesn't run after the policy has been freed.
Right, added this to the commit.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 547d221b2ff2..878add2cac3c 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1289,6 +1289,8 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
DEV_PM_QOS_MAX_FREQUENCY);
dev_pm_qos_remove_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
+ cancel_work_sync(&policy->req_work);
+
cpufreq_policy_put_kobj(policy);
free_cpumask_var(policy->real_cpus);
free_cpumask_var(policy->related_cpus);
--
viresh
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-10 10:51 ` [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework Viresh Kumar
2019-06-14 16:46 ` Matthias Kaehlcke
@ 2019-06-17 9:23 ` Ulf Hansson
2019-06-17 23:26 ` Rafael J. Wysocki
2019-06-17 23:37 ` Rafael J. Wysocki
3 siblings, 0 replies; 27+ messages in thread
From: Ulf Hansson @ 2019-06-17 9:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Linux PM, Vincent Guittot, Qais.Yousef,
Matthias Kaehlcke, juri.lelli, Linux Kernel Mailing List
On Mon, 10 Jun 2019 at 12:52, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This registers the notifiers for min/max frequency constraints with the
> PM QoS framework. The constraints are also taken into consideration in
> cpufreq_set_policy().
>
> This also relocates cpufreq_policy_put_kobj() as it is required to be
> called from cpufreq_policy_alloc() now.
>
> No constraints are added until now though.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/cpufreq/cpufreq.c | 139 +++++++++++++++++++++++++++++++-------
> include/linux/cpufreq.h | 4 ++
> 2 files changed, 120 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..547d221b2ff2 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/pm_qos.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> @@ -1126,11 +1127,77 @@ static void handle_update(struct work_struct *work)
> cpufreq_update_policy(cpu);
> }
>
> +static void cpufreq_update_freq_work(struct work_struct *work)
> +{
> + struct cpufreq_policy *policy =
> + container_of(work, struct cpufreq_policy, req_work);
> + struct cpufreq_policy new_policy = *policy;
> +
> + /* We should read constraint values from QoS layer */
> + new_policy.min = 0;
> + new_policy.max = UINT_MAX;
> +
> + down_write(&policy->rwsem);
> +
> + if (!policy_is_inactive(policy))
> + cpufreq_set_policy(policy, &new_policy);
> +
> + up_write(&policy->rwsem);
> +}
> +
> +static int cpufreq_update_freq(struct cpufreq_policy *policy)
> +{
> + schedule_work(&policy->req_work);
> + return 0;
> +}
> +
> +static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
> + void *data)
> +{
> + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
> +
> + return cpufreq_update_freq(policy);
> +}
> +
> +static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
> + void *data)
> +{
> + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
> +
> + return cpufreq_update_freq(policy);
> +}
> +
> +static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
> +{
> + struct kobject *kobj;
> + struct completion *cmp;
> +
> + down_write(&policy->rwsem);
> + cpufreq_stats_free_table(policy);
> + kobj = &policy->kobj;
> + cmp = &policy->kobj_unregister;
> + up_write(&policy->rwsem);
> + kobject_put(kobj);
> +
> + /*
> + * We need to make sure that the underlying kobj is
> + * actually not referenced anymore by anybody before we
> + * proceed with unloading.
> + */
> + pr_debug("waiting for dropping of refcount\n");
> + wait_for_completion(cmp);
> + pr_debug("wait complete\n");
> +}
> +
> static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> {
> struct cpufreq_policy *policy;
> + struct device *dev = get_cpu_device(cpu);
> int ret;
>
> + if (!dev)
> + return NULL;
> +
> policy = kzalloc(sizeof(*policy), GFP_KERNEL);
> if (!policy)
> return NULL;
> @@ -1147,7 +1214,7 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> ret = kobject_init_and_add(&policy->kobj, &ktype_cpufreq,
> cpufreq_global_kobject, "policy%u", cpu);
> if (ret) {
> - pr_err("%s: failed to init policy->kobj: %d\n", __func__, ret);
> + dev_err(dev, "%s: failed to init policy->kobj: %d\n", __func__, ret);
> /*
> * The entire policy object will be freed below, but the extra
> * memory allocated for the kobject name needs to be freed by
> @@ -1157,16 +1224,41 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> goto err_free_real_cpus;
> }
>
> + policy->nb_min.notifier_call = cpufreq_notifier_min;
> + policy->nb_max.notifier_call = cpufreq_notifier_max;
> +
> + ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
> + DEV_PM_QOS_MIN_FREQUENCY);
> + if (ret) {
> + dev_err(dev, "Failed to register MIN QoS notifier: %d (%*pbl)\n",
> + ret, cpumask_pr_args(policy->cpus));
> + goto err_kobj_remove;
> + }
> +
> + ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
> + DEV_PM_QOS_MAX_FREQUENCY);
> + if (ret) {
> + dev_err(dev, "Failed to register MAX QoS notifier: %d (%*pbl)\n",
> + ret, cpumask_pr_args(policy->cpus));
> + goto err_min_qos_notifier;
> + }
> +
> INIT_LIST_HEAD(&policy->policy_list);
> init_rwsem(&policy->rwsem);
> spin_lock_init(&policy->transition_lock);
> init_waitqueue_head(&policy->transition_wait);
> init_completion(&policy->kobj_unregister);
> INIT_WORK(&policy->update, handle_update);
> + INIT_WORK(&policy->req_work, cpufreq_update_freq_work);
>
> policy->cpu = cpu;
> return policy;
>
> +err_min_qos_notifier:
> + dev_pm_qos_remove_notifier(dev, &policy->nb_min,
> + DEV_PM_QOS_MIN_FREQUENCY);
> +err_kobj_remove:
> + cpufreq_policy_put_kobj(policy);
> err_free_real_cpus:
> free_cpumask_var(policy->real_cpus);
> err_free_rcpumask:
> @@ -1179,30 +1271,9 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> return NULL;
> }
>
> -static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
> -{
> - struct kobject *kobj;
> - struct completion *cmp;
> -
> - down_write(&policy->rwsem);
> - cpufreq_stats_free_table(policy);
> - kobj = &policy->kobj;
> - cmp = &policy->kobj_unregister;
> - up_write(&policy->rwsem);
> - kobject_put(kobj);
> -
> - /*
> - * We need to make sure that the underlying kobj is
> - * actually not referenced anymore by anybody before we
> - * proceed with unloading.
> - */
> - pr_debug("waiting for dropping of refcount\n");
> - wait_for_completion(cmp);
> - pr_debug("wait complete\n");
> -}
> -
> static void cpufreq_policy_free(struct cpufreq_policy *policy)
> {
> + struct device *dev = get_cpu_device(policy->cpu);
> unsigned long flags;
> int cpu;
>
> @@ -1214,6 +1285,10 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> per_cpu(cpufreq_cpu_data, cpu) = NULL;
> write_unlock_irqrestore(&cpufreq_driver_lock, flags);
>
> + dev_pm_qos_remove_notifier(dev, &policy->nb_max,
> + DEV_PM_QOS_MAX_FREQUENCY);
> + dev_pm_qos_remove_notifier(dev, &policy->nb_min,
> + DEV_PM_QOS_MIN_FREQUENCY);
> cpufreq_policy_put_kobj(policy);
> free_cpumask_var(policy->real_cpus);
> free_cpumask_var(policy->related_cpus);
> @@ -2290,6 +2365,8 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
> struct cpufreq_policy *new_policy)
> {
> struct cpufreq_governor *old_gov;
> + struct device *cpu_dev = get_cpu_device(policy->cpu);
> + unsigned long min, max;
> int ret;
>
> pr_debug("setting new policy for CPU %u: %u - %u kHz\n",
> @@ -2304,11 +2381,27 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
> if (new_policy->min > new_policy->max)
> return -EINVAL;
>
> + /*
> + * PM QoS framework collects all the requests from users and provide us
> + * the final aggregated value here.
> + */
> + min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
> + max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
> +
> + if (min > new_policy->min)
> + new_policy->min = min;
> + if (max < new_policy->max)
> + new_policy->max = max;
> +
> /* verify the cpu speed can be set within this limit */
> ret = cpufreq_driver->verify(new_policy);
> if (ret)
> return ret;
>
> + /*
> + * The notifier-chain shall be removed once all the users of
> + * CPUFREQ_ADJUST are moved to use the QoS framework.
> + */
> /* adjust if necessary - all reasons */
> blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
> CPUFREQ_ADJUST, new_policy);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index d01a74fbc4db..0fe7678da9c2 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -83,6 +83,7 @@ struct cpufreq_policy {
>
> struct work_struct update; /* if update_policy() needs to be
> * called, but you're in IRQ context */
> + struct work_struct req_work;
>
> struct cpufreq_user_policy user_policy;
> struct cpufreq_frequency_table *freq_table;
> @@ -147,6 +148,9 @@ struct cpufreq_policy {
>
> /* Pointer to the cooling device if used for thermal mitigation */
> struct thermal_cooling_device *cdev;
> +
> + struct notifier_block nb_min;
> + struct notifier_block nb_max;
> };
>
> struct cpufreq_freqs {
> --
> 2.21.0.rc0.269.g1a574e7a288b
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-10 10:51 ` [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework Viresh Kumar
2019-06-14 16:46 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
@ 2019-06-17 23:26 ` Rafael J. Wysocki
2019-06-18 11:25 ` Viresh Kumar
2019-06-17 23:37 ` Rafael J. Wysocki
3 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2019-06-17 23:26 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-pm, Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On Monday, June 10, 2019 12:51:35 PM CEST Viresh Kumar wrote:
> This registers the notifiers for min/max frequency constraints with the
> PM QoS framework. The constraints are also taken into consideration in
> cpufreq_set_policy().
>
> This also relocates cpufreq_policy_put_kobj() as it is required to be
> called from cpufreq_policy_alloc() now.
>
> No constraints are added until now though.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 139 +++++++++++++++++++++++++++++++-------
> include/linux/cpufreq.h | 4 ++
> 2 files changed, 120 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..547d221b2ff2 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/pm_qos.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> @@ -1126,11 +1127,77 @@ static void handle_update(struct work_struct *work)
> cpufreq_update_policy(cpu);
> }
>
> +static void cpufreq_update_freq_work(struct work_struct *work)
> +{
> + struct cpufreq_policy *policy =
> + container_of(work, struct cpufreq_policy, req_work);
> + struct cpufreq_policy new_policy = *policy;
> +
> + /* We should read constraint values from QoS layer */
> + new_policy.min = 0;
> + new_policy.max = UINT_MAX;
> +
> + down_write(&policy->rwsem);
> +
> + if (!policy_is_inactive(policy))
> + cpufreq_set_policy(policy, &new_policy);
> +
> + up_write(&policy->rwsem);
> +}
> +
> +static int cpufreq_update_freq(struct cpufreq_policy *policy)
> +{
> + schedule_work(&policy->req_work);
> + return 0;
> +}
> +
> +static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
> + void *data)
> +{
> + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
> +
> + return cpufreq_update_freq(policy);
> +}
> +
> +static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
> + void *data)
> +{
> + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
> +
> + return cpufreq_update_freq(policy);
> +}
This is a bit convoluted.
Two different notifiers are registered basically for the same thing.
Any chance to use just one?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-17 23:26 ` Rafael J. Wysocki
@ 2019-06-18 11:25 ` Viresh Kumar
2019-06-18 22:23 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Viresh Kumar @ 2019-06-18 11:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On 18-06-19, 01:26, Rafael J. Wysocki wrote:
> On Monday, June 10, 2019 12:51:35 PM CEST Viresh Kumar wrote:
> > +static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
> > + void *data)
> > +{
> > + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
> > +
> > + return cpufreq_update_freq(policy);
> > +}
> > +
> > +static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
> > + void *data)
> > +{
> > + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
> > +
> > + return cpufreq_update_freq(policy);
> > +}
>
> This is a bit convoluted.
>
> Two different notifiers are registered basically for the same thing.
>
> Any chance to use just one?
The way QoS is designed, it handles one value only at a time and we need two,
min/max. I thought a lot about it earlier and this is what I came up with :(
You have any suggestions here ?
--
viresh
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-18 11:25 ` Viresh Kumar
@ 2019-06-18 22:23 ` Rafael J. Wysocki
2019-06-19 6:39 ` Viresh Kumar
0 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2019-06-18 22:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-pm, Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On Tuesday, June 18, 2019 1:25:22 PM CEST Viresh Kumar wrote:
> On 18-06-19, 01:26, Rafael J. Wysocki wrote:
> > On Monday, June 10, 2019 12:51:35 PM CEST Viresh Kumar wrote:
> > > +static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
> > > + void *data)
> > > +{
> > > + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
> > > +
> > > + return cpufreq_update_freq(policy);
> > > +}
> > > +
> > > +static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
> > > + void *data)
> > > +{
> > > + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
> > > +
> > > + return cpufreq_update_freq(policy);
> > > +}
> >
> > This is a bit convoluted.
> >
> > Two different notifiers are registered basically for the same thing.
> >
> > Any chance to use just one?
>
> The way QoS is designed, it handles one value only at a time and we need two,
> min/max. I thought a lot about it earlier and this is what I came up with :(
>
> You have any suggestions here ?
In patch [3/5] you could point notifiers for both min and max freq to the same
notifier head. Both of your notifiers end up calling cpufreq_update_policy()
anyway.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-18 22:23 ` Rafael J. Wysocki
@ 2019-06-19 6:39 ` Viresh Kumar
2019-06-19 9:15 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Viresh Kumar @ 2019-06-19 6:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On 19-06-19, 00:23, Rafael J. Wysocki wrote:
> In patch [3/5] you could point notifiers for both min and max freq to the same
> notifier head. Both of your notifiers end up calling cpufreq_update_policy()
> anyway.
I tried it and the changes in qos.c file look fine. But I don't like at all how
cpufreq.c looks now. We only register for min-freq notifier now and that takes
care of max as well. What could have been better is if we could have registered
a freq-notifier instead of min/max, which isn't possible as well because of how
qos framework works.
Honestly, the cpufreq changes look hacky to me :(
What do you say.
--
viresh
---
drivers/base/power/qos.c | 15 ++++++++-------
drivers/cpufreq/cpufreq.c | 38 ++++++++------------------------------
include/linux/cpufreq.h | 3 +--
3 files changed, 17 insertions(+), 39 deletions(-)
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index cde2692b97f9..9bbf2d2a3376 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -202,20 +202,20 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
if (!qos)
return -ENOMEM;
- n = kzalloc(3 * sizeof(*n), GFP_KERNEL);
+ n = kzalloc(2 * sizeof(*n), GFP_KERNEL);
if (!n) {
kfree(qos);
return -ENOMEM;
}
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
c = &qos->resume_latency;
plist_head_init(&c->list);
c->target_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE;
c->default_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE;
c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
c->type = PM_QOS_MIN;
- c->notifiers = n;
- BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c->notifiers = n++;
c = &qos->latency_tolerance;
plist_head_init(&c->list);
@@ -224,14 +224,16 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
c->type = PM_QOS_MIN;
+ /* Same notifier head is used for both min/max frequency */
+ BLOCKING_INIT_NOTIFIER_HEAD(n);
+
c = &qos->min_frequency;
plist_head_init(&c->list);
c->target_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
c->default_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
c->no_constraint_value = PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
c->type = PM_QOS_MAX;
- c->notifiers = ++n;
- BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c->notifiers = n;
c = &qos->max_frequency;
plist_head_init(&c->list);
@@ -239,8 +241,7 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
c->default_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
c->no_constraint_value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
c->type = PM_QOS_MIN;
- c->notifiers = ++n;
- BLOCKING_INIT_NOTIFIER_HEAD(n);
+ c->notifiers = n;
INIT_LIST_HEAD(&qos->flags.list);
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 1344e1b1307f..1605dba1327e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1139,19 +1139,10 @@ static int cpufreq_update_freq(struct cpufreq_policy *policy)
return 0;
}
-static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
+static int cpufreq_notifier_qos(struct notifier_block *nb, unsigned long freq,
void *data)
{
- struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
-
- return cpufreq_update_freq(policy);
-}
-
-static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
- void *data)
-{
- struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
+ struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_qos);
return cpufreq_update_freq(policy);
@@ -1214,10 +1205,10 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_free_real_cpus;
}
- policy->nb_min.notifier_call = cpufreq_notifier_min;
- policy->nb_max.notifier_call = cpufreq_notifier_max;
+ policy->nb_qos.notifier_call = cpufreq_notifier_qos;
- ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
+ /* Notifier for min frequency also takes care of max frequency notifier */
+ ret = dev_pm_qos_add_notifier(dev, &policy->nb_qos,
DEV_PM_QOS_MIN_FREQUENCY);
if (ret) {
dev_err(dev, "Failed to register MIN QoS notifier: %d (%*pbl)\n",
@@ -1225,18 +1216,10 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_kobj_remove;
}
- ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
- DEV_PM_QOS_MAX_FREQUENCY);
- if (ret) {
- dev_err(dev, "Failed to register MAX QoS notifier: %d (%*pbl)\n",
- ret, cpumask_pr_args(policy->cpus));
- goto err_min_qos_notifier;
- }
-
policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
GFP_KERNEL);
if (!policy->min_freq_req)
- goto err_max_qos_notifier;
+ goto err_min_qos_notifier;
policy->max_freq_req = policy->min_freq_req + 1;
INIT_LIST_HEAD(&policy->policy_list);
@@ -1250,11 +1233,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
policy->cpu = cpu;
return policy;
-err_max_qos_notifier:
- dev_pm_qos_remove_notifier(dev, &policy->nb_max,
- DEV_PM_QOS_MAX_FREQUENCY);
err_min_qos_notifier:
- dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ dev_pm_qos_remove_notifier(dev, &policy->nb_qos,
DEV_PM_QOS_MIN_FREQUENCY);
err_kobj_remove:
cpufreq_policy_put_kobj(policy);
@@ -1284,9 +1264,7 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
per_cpu(cpufreq_cpu_data, cpu) = NULL;
write_unlock_irqrestore(&cpufreq_driver_lock, flags);
- dev_pm_qos_remove_notifier(dev, &policy->nb_max,
- DEV_PM_QOS_MAX_FREQUENCY);
- dev_pm_qos_remove_notifier(dev, &policy->nb_min,
+ dev_pm_qos_remove_notifier(dev, &policy->nb_qos,
DEV_PM_QOS_MIN_FREQUENCY);
cancel_work_sync(&policy->req_work);
dev_pm_qos_remove_request(policy->max_freq_req);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6bbed9af4fd2..2080d6490ed1 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -145,8 +145,7 @@ struct cpufreq_policy {
/* Pointer to the cooling device if used for thermal mitigation */
struct thermal_cooling_device *cdev;
- struct notifier_block nb_min;
- struct notifier_block nb_max;
+ struct notifier_block nb_qos;
};
struct cpufreq_freqs {
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-19 6:39 ` Viresh Kumar
@ 2019-06-19 9:15 ` Rafael J. Wysocki
0 siblings, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2019-06-19 9:15 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Linux PM, Vincent Guittot, Qais.Yousef,
Matthias Kaehlcke, Juri Lelli, Linux Kernel Mailing List
On Wed, Jun 19, 2019 at 8:39 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 19-06-19, 00:23, Rafael J. Wysocki wrote:
> > In patch [3/5] you could point notifiers for both min and max freq to the same
> > notifier head. Both of your notifiers end up calling cpufreq_update_policy()
> > anyway.
>
> I tried it and the changes in qos.c file look fine. But I don't like at all how
> cpufreq.c looks now. We only register for min-freq notifier now and that takes
> care of max as well. What could have been better is if we could have registered
> a freq-notifier instead of min/max, which isn't possible as well because of how
> qos framework works.
>
> Honestly, the cpufreq changes look hacky to me :(
>
> What do you say.
OK, leave it as is. That's not a big deal.
It is slightly awkward, but oh well.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-10 10:51 ` [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework Viresh Kumar
` (2 preceding siblings ...)
2019-06-17 23:26 ` Rafael J. Wysocki
@ 2019-06-17 23:37 ` Rafael J. Wysocki
2019-06-18 11:34 ` Viresh Kumar
3 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2019-06-17 23:37 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-pm, Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On Monday, June 10, 2019 12:51:35 PM CEST Viresh Kumar wrote:
> This registers the notifiers for min/max frequency constraints with the
> PM QoS framework. The constraints are also taken into consideration in
> cpufreq_set_policy().
>
> This also relocates cpufreq_policy_put_kobj() as it is required to be
> called from cpufreq_policy_alloc() now.
>
> No constraints are added until now though.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 139 +++++++++++++++++++++++++++++++-------
> include/linux/cpufreq.h | 4 ++
> 2 files changed, 120 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..547d221b2ff2 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel_stat.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/pm_qos.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> #include <linux/syscore_ops.h>
> @@ -1126,11 +1127,77 @@ static void handle_update(struct work_struct *work)
> cpufreq_update_policy(cpu);
> }
>
> +static void cpufreq_update_freq_work(struct work_struct *work)
> +{
> + struct cpufreq_policy *policy =
> + container_of(work, struct cpufreq_policy, req_work);
> + struct cpufreq_policy new_policy = *policy;
> +
> + /* We should read constraint values from QoS layer */
> + new_policy.min = 0;
> + new_policy.max = UINT_MAX;
> +
> + down_write(&policy->rwsem);
> +
> + if (!policy_is_inactive(policy))
> + cpufreq_set_policy(policy, &new_policy);
> +
> + up_write(&policy->rwsem);
> +}
> +
> +static int cpufreq_update_freq(struct cpufreq_policy *policy)
> +{
> + schedule_work(&policy->req_work);
> + return 0;
> +}
> +
> +static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
> + void *data)
> +{
> + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
> +
> + return cpufreq_update_freq(policy);
> +}
> +
> +static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
> + void *data)
> +{
> + struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
> +
> + return cpufreq_update_freq(policy);
> +}
> +
> +static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy)
> +{
> + struct kobject *kobj;
> + struct completion *cmp;
> +
> + down_write(&policy->rwsem);
> + cpufreq_stats_free_table(policy);
> + kobj = &policy->kobj;
> + cmp = &policy->kobj_unregister;
> + up_write(&policy->rwsem);
> + kobject_put(kobj);
> +
> + /*
> + * We need to make sure that the underlying kobj is
> + * actually not referenced anymore by anybody before we
> + * proceed with unloading.
> + */
> + pr_debug("waiting for dropping of refcount\n");
> + wait_for_completion(cmp);
> + pr_debug("wait complete\n");
> +}
> +
> static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> {
> struct cpufreq_policy *policy;
> + struct device *dev = get_cpu_device(cpu);
> int ret;
>
> + if (!dev)
> + return NULL;
> +
> policy = kzalloc(sizeof(*policy), GFP_KERNEL);
> if (!policy)
> return NULL;
> @@ -1147,7 +1214,7 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> ret = kobject_init_and_add(&policy->kobj, &ktype_cpufreq,
> cpufreq_global_kobject, "policy%u", cpu);
> if (ret) {
> - pr_err("%s: failed to init policy->kobj: %d\n", __func__, ret);
> + dev_err(dev, "%s: failed to init policy->kobj: %d\n", __func__, ret);
> /*
> * The entire policy object will be freed below, but the extra
> * memory allocated for the kobject name needs to be freed by
> @@ -1157,16 +1224,41 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> goto err_free_real_cpus;
> }
>
> + policy->nb_min.notifier_call = cpufreq_notifier_min;
> + policy->nb_max.notifier_call = cpufreq_notifier_max;
> +
> + ret = dev_pm_qos_add_notifier(dev, &policy->nb_min,
> + DEV_PM_QOS_MIN_FREQUENCY);
> + if (ret) {
> + dev_err(dev, "Failed to register MIN QoS notifier: %d (%*pbl)\n",
> + ret, cpumask_pr_args(policy->cpus));
> + goto err_kobj_remove;
> + }
> +
> + ret = dev_pm_qos_add_notifier(dev, &policy->nb_max,
> + DEV_PM_QOS_MAX_FREQUENCY);
> + if (ret) {
> + dev_err(dev, "Failed to register MAX QoS notifier: %d (%*pbl)\n",
> + ret, cpumask_pr_args(policy->cpus));
> + goto err_min_qos_notifier;
> + }
> +
> INIT_LIST_HEAD(&policy->policy_list);
> init_rwsem(&policy->rwsem);
> spin_lock_init(&policy->transition_lock);
> init_waitqueue_head(&policy->transition_wait);
> init_completion(&policy->kobj_unregister);
> INIT_WORK(&policy->update, handle_update);
> + INIT_WORK(&policy->req_work, cpufreq_update_freq_work);
One more thing.
handle_update() is very similar to cpufreq_update_freq_work().
Why are both of them needed?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework
2019-06-17 23:37 ` Rafael J. Wysocki
@ 2019-06-18 11:34 ` Viresh Kumar
0 siblings, 0 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-18 11:34 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, Vincent Guittot, Qais.Yousef, mka, juri.lelli, linux-kernel
On 18-06-19, 01:37, Rafael J. Wysocki wrote:
> One more thing.
>
> handle_update() is very similar to cpufreq_update_freq_work().
>
> Why are both of them needed?
I probably did that because of locking required in cpufreq_update_freq_work()
but maybe I can do better. Lemme try and come back on it.
--
viresh
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints
2019-06-10 10:51 [PATCH V3 0/5] cpufreq: Use QoS layer to manage freq-constraints Viresh Kumar
` (3 preceding siblings ...)
2019-06-10 10:51 ` [PATCH V3 4/5] cpufreq: Register notifiers with the PM QoS framework Viresh Kumar
@ 2019-06-10 10:51 ` Viresh Kumar
2019-06-14 17:14 ` Matthias Kaehlcke
2019-06-17 9:23 ` Ulf Hansson
4 siblings, 2 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-10 10:51 UTC (permalink / raw)
To: Rafael Wysocki
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Qais.Yousef, mka,
juri.lelli, linux-kernel
This implements QoS requests to manage userspace configuration of min
and max frequency.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 92 +++++++++++++++++++--------------------
include/linux/cpufreq.h | 8 +---
2 files changed, 47 insertions(+), 53 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 547d221b2ff2..ff754981fcb4 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -720,23 +720,15 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
static ssize_t store_##file_name \
(struct cpufreq_policy *policy, const char *buf, size_t count) \
{ \
- int ret, temp; \
- struct cpufreq_policy new_policy; \
+ unsigned long val; \
+ int ret; \
\
- memcpy(&new_policy, policy, sizeof(*policy)); \
- new_policy.min = policy->user_policy.min; \
- new_policy.max = policy->user_policy.max; \
- \
- ret = sscanf(buf, "%u", &new_policy.object); \
+ ret = sscanf(buf, "%lu", &val); \
if (ret != 1) \
return -EINVAL; \
\
- temp = new_policy.object; \
- ret = cpufreq_set_policy(policy, &new_policy); \
- if (!ret) \
- policy->user_policy.object = temp; \
- \
- return ret ? ret : count; \
+ ret = dev_pm_qos_update_request(policy->object##_freq_req, val);\
+ return ret && ret != 1 ? ret : count; \
}
store_one(scaling_min_freq, min);
@@ -1133,10 +1125,6 @@ static void cpufreq_update_freq_work(struct work_struct *work)
container_of(work, struct cpufreq_policy, req_work);
struct cpufreq_policy new_policy = *policy;
- /* We should read constraint values from QoS layer */
- new_policy.min = 0;
- new_policy.max = UINT_MAX;
-
down_write(&policy->rwsem);
if (!policy_is_inactive(policy))
@@ -1243,6 +1231,12 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_min_qos_notifier;
}
+ policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
+ GFP_KERNEL);
+ if (!policy->min_freq_req)
+ goto err_max_qos_notifier;
+
+ policy->max_freq_req = policy->min_freq_req + 1;
INIT_LIST_HEAD(&policy->policy_list);
init_rwsem(&policy->rwsem);
spin_lock_init(&policy->transition_lock);
@@ -1254,6 +1248,9 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
policy->cpu = cpu;
return policy;
+err_max_qos_notifier:
+ dev_pm_qos_remove_notifier(dev, &policy->nb_max,
+ DEV_PM_QOS_MAX_FREQUENCY);
err_min_qos_notifier:
dev_pm_qos_remove_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
@@ -1289,6 +1286,10 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
DEV_PM_QOS_MAX_FREQUENCY);
dev_pm_qos_remove_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
+ dev_pm_qos_remove_request(policy->max_freq_req);
+ dev_pm_qos_remove_request(policy->min_freq_req);
+ kfree(policy->min_freq_req);
+
cpufreq_policy_put_kobj(policy);
free_cpumask_var(policy->real_cpus);
free_cpumask_var(policy->related_cpus);
@@ -1366,16 +1367,30 @@ static int cpufreq_online(unsigned int cpu)
cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
if (new_policy) {
- policy->user_policy.min = policy->min;
- policy->user_policy.max = policy->max;
+ struct device *dev = get_cpu_device(cpu);
for_each_cpu(j, policy->related_cpus) {
per_cpu(cpufreq_cpu_data, j) = policy;
add_cpu_dev_symlink(policy, j);
}
- } else {
- policy->min = policy->user_policy.min;
- policy->max = policy->user_policy.max;
+
+ ret = dev_pm_qos_add_request(dev, policy->min_freq_req,
+ DEV_PM_QOS_MIN_FREQUENCY,
+ policy->min);
+ if (ret < 0) {
+ dev_err(dev, "Failed to add min-freq constraint (%d)\n",
+ ret);
+ goto out_destroy_policy;
+ }
+
+ ret = dev_pm_qos_add_request(dev, policy->max_freq_req,
+ DEV_PM_QOS_MAX_FREQUENCY,
+ policy->max);
+ if (ret < 0) {
+ dev_err(dev, "Failed to add max-freq constraint (%d)\n",
+ ret);
+ goto out_destroy_policy;
+ }
}
if (cpufreq_driver->get && !cpufreq_driver->setpolicy) {
@@ -2366,7 +2381,6 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
{
struct cpufreq_governor *old_gov;
struct device *cpu_dev = get_cpu_device(policy->cpu);
- unsigned long min, max;
int ret;
pr_debug("setting new policy for CPU %u: %u - %u kHz\n",
@@ -2374,24 +2388,12 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
memcpy(&new_policy->cpuinfo, &policy->cpuinfo, sizeof(policy->cpuinfo));
- /*
- * This check works well when we store new min/max freq attributes,
- * because new_policy is a copy of policy with one field updated.
- */
- if (new_policy->min > new_policy->max)
- return -EINVAL;
-
/*
* PM QoS framework collects all the requests from users and provide us
* the final aggregated value here.
*/
- min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
- max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
-
- if (min > new_policy->min)
- new_policy->min = min;
- if (max < new_policy->max)
- new_policy->max = max;
+ new_policy->min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
+ new_policy->max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
/* verify the cpu speed can be set within this limit */
ret = cpufreq_driver->verify(new_policy);
@@ -2480,10 +2482,9 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
* @cpu: CPU to re-evaluate the policy for.
*
* Update the current frequency for the cpufreq policy of @cpu and use
- * cpufreq_set_policy() to re-apply the min and max limits saved in the
- * user_policy sub-structure of that policy, which triggers the evaluation
- * of policy notifiers and the cpufreq driver's ->verify() callback for the
- * policy in question, among other things.
+ * cpufreq_set_policy() to re-apply the min and max limits, which triggers the
+ * evaluation of policy notifiers and the cpufreq driver's ->verify() callback
+ * for the policy in question, among other things.
*/
void cpufreq_update_policy(unsigned int cpu)
{
@@ -2503,8 +2504,6 @@ void cpufreq_update_policy(unsigned int cpu)
pr_debug("updating policy for CPU %u\n", cpu);
memcpy(&new_policy, policy, sizeof(*policy));
- new_policy.min = policy->user_policy.min;
- new_policy.max = policy->user_policy.max;
cpufreq_set_policy(policy, &new_policy);
@@ -2549,10 +2548,9 @@ static int cpufreq_boost_set_sw(int state)
break;
}
- down_write(&policy->rwsem);
- policy->user_policy.max = policy->max;
- cpufreq_governor_limits(policy);
- up_write(&policy->rwsem);
+ ret = dev_pm_qos_update_request(policy->max_freq_req, policy->max);
+ if (ret)
+ break;
}
return ret;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 0fe7678da9c2..6bbed9af4fd2 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -50,11 +50,6 @@ struct cpufreq_cpuinfo {
unsigned int transition_latency;
};
-struct cpufreq_user_policy {
- unsigned int min; /* in kHz */
- unsigned int max; /* in kHz */
-};
-
struct cpufreq_policy {
/* CPUs sharing clock, require sw coordination */
cpumask_var_t cpus; /* Online CPUs only */
@@ -85,7 +80,8 @@ struct cpufreq_policy {
* called, but you're in IRQ context */
struct work_struct req_work;
- struct cpufreq_user_policy user_policy;
+ struct dev_pm_qos_request *min_freq_req;
+ struct dev_pm_qos_request *max_freq_req;
struct cpufreq_frequency_table *freq_table;
enum cpufreq_table_sorting freq_table_sorted;
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints
2019-06-10 10:51 ` [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints Viresh Kumar
@ 2019-06-14 17:14 ` Matthias Kaehlcke
2019-06-17 3:07 ` Viresh Kumar
2019-06-17 9:23 ` Ulf Hansson
1 sibling, 1 reply; 27+ messages in thread
From: Matthias Kaehlcke @ 2019-06-14 17:14 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, Qais.Yousef,
juri.lelli, linux-kernel
Hi Viresh,
On Mon, Jun 10, 2019 at 04:21:36PM +0530, Viresh Kumar wrote:
> This implements QoS requests to manage userspace configuration of min
> and max frequency.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 92 +++++++++++++++++++--------------------
> include/linux/cpufreq.h | 8 +---
> 2 files changed, 47 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 547d221b2ff2..ff754981fcb4 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -720,23 +720,15 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
> static ssize_t store_##file_name \
> (struct cpufreq_policy *policy, const char *buf, size_t count) \
> { \
> - int ret, temp; \
> - struct cpufreq_policy new_policy; \
> + unsigned long val; \
> + int ret; \
> \
> - memcpy(&new_policy, policy, sizeof(*policy)); \
> - new_policy.min = policy->user_policy.min; \
> - new_policy.max = policy->user_policy.max; \
> - \
> - ret = sscanf(buf, "%u", &new_policy.object); \
> + ret = sscanf(buf, "%lu", &val); \
> if (ret != 1) \
> return -EINVAL; \
> \
> - temp = new_policy.object; \
> - ret = cpufreq_set_policy(policy, &new_policy); \
> - if (!ret) \
> - policy->user_policy.object = temp; \
> - \
> - return ret ? ret : count; \
> + ret = dev_pm_qos_update_request(policy->object##_freq_req, val);\
> + return ret && ret != 1 ? ret : count; \
nit: I wonder if
return (ret >= 0) ? count : ret;
would be clearer.
Other than that:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints
2019-06-14 17:14 ` Matthias Kaehlcke
@ 2019-06-17 3:07 ` Viresh Kumar
0 siblings, 0 replies; 27+ messages in thread
From: Viresh Kumar @ 2019-06-17 3:07 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, Qais.Yousef,
juri.lelli, linux-kernel
On 14-06-19, 10:14, Matthias Kaehlcke wrote:
> Hi Viresh,
>
> On Mon, Jun 10, 2019 at 04:21:36PM +0530, Viresh Kumar wrote:
> > This implements QoS requests to manage userspace configuration of min
> > and max frequency.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/cpufreq.c | 92 +++++++++++++++++++--------------------
> > include/linux/cpufreq.h | 8 +---
> > 2 files changed, 47 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 547d221b2ff2..ff754981fcb4 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -720,23 +720,15 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
> > static ssize_t store_##file_name \
> > (struct cpufreq_policy *policy, const char *buf, size_t count) \
> > { \
> > - int ret, temp; \
> > - struct cpufreq_policy new_policy; \
> > + unsigned long val; \
> > + int ret; \
> > \
> > - memcpy(&new_policy, policy, sizeof(*policy)); \
> > - new_policy.min = policy->user_policy.min; \
> > - new_policy.max = policy->user_policy.max; \
> > - \
> > - ret = sscanf(buf, "%u", &new_policy.object); \
> > + ret = sscanf(buf, "%lu", &val); \
> > if (ret != 1) \
> > return -EINVAL; \
> > \
> > - temp = new_policy.object; \
> > - ret = cpufreq_set_policy(policy, &new_policy); \
> > - if (!ret) \
> > - policy->user_policy.object = temp; \
> > - \
> > - return ret ? ret : count; \
> > + ret = dev_pm_qos_update_request(policy->object##_freq_req, val);\
> > + return ret && ret != 1 ? ret : count; \
>
> nit: I wonder if
>
> return (ret >= 0) ? count : ret;
>
> would be clearer.
Done. Thanks.
> Other than that:
>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
--
viresh
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints
2019-06-10 10:51 ` [PATCH V3 5/5] cpufreq: Add QoS requests for userspace constraints Viresh Kumar
2019-06-14 17:14 ` Matthias Kaehlcke
@ 2019-06-17 9:23 ` Ulf Hansson
1 sibling, 0 replies; 27+ messages in thread
From: Ulf Hansson @ 2019-06-17 9:23 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Linux PM, Vincent Guittot, Qais.Yousef,
Matthias Kaehlcke, juri.lelli, Linux Kernel Mailing List
On Mon, 10 Jun 2019 at 12:52, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This implements QoS requests to manage userspace configuration of min
> and max frequency.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/cpufreq/cpufreq.c | 92 +++++++++++++++++++--------------------
> include/linux/cpufreq.h | 8 +---
> 2 files changed, 47 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 547d221b2ff2..ff754981fcb4 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -720,23 +720,15 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
> static ssize_t store_##file_name \
> (struct cpufreq_policy *policy, const char *buf, size_t count) \
> { \
> - int ret, temp; \
> - struct cpufreq_policy new_policy; \
> + unsigned long val; \
> + int ret; \
> \
> - memcpy(&new_policy, policy, sizeof(*policy)); \
> - new_policy.min = policy->user_policy.min; \
> - new_policy.max = policy->user_policy.max; \
> - \
> - ret = sscanf(buf, "%u", &new_policy.object); \
> + ret = sscanf(buf, "%lu", &val); \
> if (ret != 1) \
> return -EINVAL; \
> \
> - temp = new_policy.object; \
> - ret = cpufreq_set_policy(policy, &new_policy); \
> - if (!ret) \
> - policy->user_policy.object = temp; \
> - \
> - return ret ? ret : count; \
> + ret = dev_pm_qos_update_request(policy->object##_freq_req, val);\
> + return ret && ret != 1 ? ret : count; \
> }
>
> store_one(scaling_min_freq, min);
> @@ -1133,10 +1125,6 @@ static void cpufreq_update_freq_work(struct work_struct *work)
> container_of(work, struct cpufreq_policy, req_work);
> struct cpufreq_policy new_policy = *policy;
>
> - /* We should read constraint values from QoS layer */
> - new_policy.min = 0;
> - new_policy.max = UINT_MAX;
> -
> down_write(&policy->rwsem);
>
> if (!policy_is_inactive(policy))
> @@ -1243,6 +1231,12 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> goto err_min_qos_notifier;
> }
>
> + policy->min_freq_req = kzalloc(2 * sizeof(*policy->min_freq_req),
> + GFP_KERNEL);
> + if (!policy->min_freq_req)
> + goto err_max_qos_notifier;
> +
> + policy->max_freq_req = policy->min_freq_req + 1;
> INIT_LIST_HEAD(&policy->policy_list);
> init_rwsem(&policy->rwsem);
> spin_lock_init(&policy->transition_lock);
> @@ -1254,6 +1248,9 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
> policy->cpu = cpu;
> return policy;
>
> +err_max_qos_notifier:
> + dev_pm_qos_remove_notifier(dev, &policy->nb_max,
> + DEV_PM_QOS_MAX_FREQUENCY);
> err_min_qos_notifier:
> dev_pm_qos_remove_notifier(dev, &policy->nb_min,
> DEV_PM_QOS_MIN_FREQUENCY);
> @@ -1289,6 +1286,10 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> DEV_PM_QOS_MAX_FREQUENCY);
> dev_pm_qos_remove_notifier(dev, &policy->nb_min,
> DEV_PM_QOS_MIN_FREQUENCY);
> + dev_pm_qos_remove_request(policy->max_freq_req);
> + dev_pm_qos_remove_request(policy->min_freq_req);
> + kfree(policy->min_freq_req);
> +
> cpufreq_policy_put_kobj(policy);
> free_cpumask_var(policy->real_cpus);
> free_cpumask_var(policy->related_cpus);
> @@ -1366,16 +1367,30 @@ static int cpufreq_online(unsigned int cpu)
> cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
>
> if (new_policy) {
> - policy->user_policy.min = policy->min;
> - policy->user_policy.max = policy->max;
> + struct device *dev = get_cpu_device(cpu);
>
> for_each_cpu(j, policy->related_cpus) {
> per_cpu(cpufreq_cpu_data, j) = policy;
> add_cpu_dev_symlink(policy, j);
> }
> - } else {
> - policy->min = policy->user_policy.min;
> - policy->max = policy->user_policy.max;
> +
> + ret = dev_pm_qos_add_request(dev, policy->min_freq_req,
> + DEV_PM_QOS_MIN_FREQUENCY,
> + policy->min);
> + if (ret < 0) {
> + dev_err(dev, "Failed to add min-freq constraint (%d)\n",
> + ret);
> + goto out_destroy_policy;
> + }
> +
> + ret = dev_pm_qos_add_request(dev, policy->max_freq_req,
> + DEV_PM_QOS_MAX_FREQUENCY,
> + policy->max);
> + if (ret < 0) {
> + dev_err(dev, "Failed to add max-freq constraint (%d)\n",
> + ret);
> + goto out_destroy_policy;
> + }
> }
>
> if (cpufreq_driver->get && !cpufreq_driver->setpolicy) {
> @@ -2366,7 +2381,6 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
> {
> struct cpufreq_governor *old_gov;
> struct device *cpu_dev = get_cpu_device(policy->cpu);
> - unsigned long min, max;
> int ret;
>
> pr_debug("setting new policy for CPU %u: %u - %u kHz\n",
> @@ -2374,24 +2388,12 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
>
> memcpy(&new_policy->cpuinfo, &policy->cpuinfo, sizeof(policy->cpuinfo));
>
> - /*
> - * This check works well when we store new min/max freq attributes,
> - * because new_policy is a copy of policy with one field updated.
> - */
> - if (new_policy->min > new_policy->max)
> - return -EINVAL;
> -
> /*
> * PM QoS framework collects all the requests from users and provide us
> * the final aggregated value here.
> */
> - min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
> - max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
> -
> - if (min > new_policy->min)
> - new_policy->min = min;
> - if (max < new_policy->max)
> - new_policy->max = max;
> + new_policy->min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY);
> + new_policy->max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY);
>
> /* verify the cpu speed can be set within this limit */
> ret = cpufreq_driver->verify(new_policy);
> @@ -2480,10 +2482,9 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
> * @cpu: CPU to re-evaluate the policy for.
> *
> * Update the current frequency for the cpufreq policy of @cpu and use
> - * cpufreq_set_policy() to re-apply the min and max limits saved in the
> - * user_policy sub-structure of that policy, which triggers the evaluation
> - * of policy notifiers and the cpufreq driver's ->verify() callback for the
> - * policy in question, among other things.
> + * cpufreq_set_policy() to re-apply the min and max limits, which triggers the
> + * evaluation of policy notifiers and the cpufreq driver's ->verify() callback
> + * for the policy in question, among other things.
> */
> void cpufreq_update_policy(unsigned int cpu)
> {
> @@ -2503,8 +2504,6 @@ void cpufreq_update_policy(unsigned int cpu)
>
> pr_debug("updating policy for CPU %u\n", cpu);
> memcpy(&new_policy, policy, sizeof(*policy));
> - new_policy.min = policy->user_policy.min;
> - new_policy.max = policy->user_policy.max;
>
> cpufreq_set_policy(policy, &new_policy);
>
> @@ -2549,10 +2548,9 @@ static int cpufreq_boost_set_sw(int state)
> break;
> }
>
> - down_write(&policy->rwsem);
> - policy->user_policy.max = policy->max;
> - cpufreq_governor_limits(policy);
> - up_write(&policy->rwsem);
> + ret = dev_pm_qos_update_request(policy->max_freq_req, policy->max);
> + if (ret)
> + break;
> }
>
> return ret;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 0fe7678da9c2..6bbed9af4fd2 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -50,11 +50,6 @@ struct cpufreq_cpuinfo {
> unsigned int transition_latency;
> };
>
> -struct cpufreq_user_policy {
> - unsigned int min; /* in kHz */
> - unsigned int max; /* in kHz */
> -};
> -
> struct cpufreq_policy {
> /* CPUs sharing clock, require sw coordination */
> cpumask_var_t cpus; /* Online CPUs only */
> @@ -85,7 +80,8 @@ struct cpufreq_policy {
> * called, but you're in IRQ context */
> struct work_struct req_work;
>
> - struct cpufreq_user_policy user_policy;
> + struct dev_pm_qos_request *min_freq_req;
> + struct dev_pm_qos_request *max_freq_req;
> struct cpufreq_frequency_table *freq_table;
> enum cpufreq_table_sorting freq_table_sorted;
>
> --
> 2.21.0.rc0.269.g1a574e7a288b
>
^ permalink raw reply [flat|nested] 27+ messages in thread