* [PATCH 1/2] ACPI: PM: Avoid evaluating _PS3 on transitions from D3hot to D3cold
From: Rafael J. Wysocki @ 2019-06-25 12:04 UTC (permalink / raw)
To: Linux ACPI; +Cc: Bjorn Helgaas, Linux PM, Mika Westerberg, LKML, Zhang Rui
In-Reply-To: <10419005.Mb09WM6RCc@kreacher>
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If the power state of a device with ACPI PM is changed from D3hot to
D3cold, it merely is a matter of dropping references to additional
power resources (specifically, those in the list returned by _PR3),
and the _PS3 method should not be invoked for the device then (as
it has already been evaluated during the previous transition to
D3hot).
Fixes: 20dacb71ad28 (ACPI / PM: Rework device power management to follow ACPI 6)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/device_pm.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -215,9 +215,15 @@ int acpi_device_set_power(struct acpi_de
return -ENODEV;
}
- result = acpi_dev_pm_explicit_set(device, state);
- if (result)
- goto end;
+ /*
+ * If the device goes from D3hot to D3cold, _PS3 has been
+ * evaluated for it already, so skip it in that case.
+ */
+ if (device->power.state < ACPI_STATE_D3_HOT) {
+ result = acpi_dev_pm_explicit_set(device, state);
+ if (result)
+ goto end;
+ }
if (device->power.flags.power_resources)
result = acpi_power_transition(device, target_state);
^ permalink raw reply
* [PATCH 2/2] ACPI: PM: Allow transitions to D0 to occur in special cases
From: Rafael J. Wysocki @ 2019-06-25 12:06 UTC (permalink / raw)
To: Linux ACPI; +Cc: Bjorn Helgaas, Linux PM, Mika Westerberg, LKML, Zhang Rui
In-Reply-To: <10419005.Mb09WM6RCc@kreacher>
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If a device with ACPI PM is left in D0 during a system-wide
transition to the S3 (suspend-to-RAM) or S4 (hibernation) sleep
state, the actual state of the device need not be D0 during resume
from it, although its power.state value will still reflect D0 (that
is, the power state from before the system-wide transition).
In that case, the acpi_device_set_power() call made to ensure that
the power state of the device will be D0 going forward has no effect,
because the new state (D0) is equal to the one reflected by the
device's power.state value. That does not affect power resources,
which are taken care of by acpi_resume_power_resources() called from
acpi_pm_finish() during resume from system-wide sleep states, but it
still may be necessary to invoke _PS0 for the device on top of that
in order to finalize its transition to D0.
For this reason, modify acpi_device_set_power() to allow transitions
to D0 to occur even if D0 is the current power state of the device
according to its power.state value.
That will not affect power resources, which are assumed to be in
the right configuration already (as reflected by the current values
of their reference counters), but it may cause _PS0 to be evaluated
for the device. However, evaluating _PS0 for a device already in D0
may lead to confusion in general, so invoke _PSC (if present) to
check the device's current power state upfront and only evaluate
_PS0 for it if _PSC has returned a power state different from D0.
[If _PSC is not present or the evaluation of it fails, the power
state of the device is assumed to be D0 at this point.]
Fixes: 20dacb71ad28 (ACPI / PM: Rework device power management to follow ACPI 6)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/device_pm.c | 53 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 45 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -45,6 +45,19 @@ const char *acpi_power_state_string(int
}
}
+static int acpi_dev_pm_explicit_get(struct acpi_device *device, int *state)
+{
+ unsigned long long psc;
+ acpi_status status;
+
+ status = acpi_evaluate_integer(device->handle, "_PSC", NULL, &psc);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ *state = psc;
+ return 0;
+}
+
/**
* acpi_device_get_power - Get power state of an ACPI device.
* @device: Device to get the power state of.
@@ -57,6 +70,7 @@ const char *acpi_power_state_string(int
int acpi_device_get_power(struct acpi_device *device, int *state)
{
int result = ACPI_STATE_UNKNOWN;
+ int error;
if (!device || !state)
return -EINVAL;
@@ -73,18 +87,16 @@ int acpi_device_get_power(struct acpi_de
* if available.
*/
if (device->power.flags.power_resources) {
- int error = acpi_power_get_inferred_state(device, &result);
+ error = acpi_power_get_inferred_state(device, &result);
if (error)
return error;
}
if (device->power.flags.explicit_get) {
- acpi_handle handle = device->handle;
- unsigned long long psc;
- acpi_status status;
+ int psc;
- status = acpi_evaluate_integer(handle, "_PSC", NULL, &psc);
- if (ACPI_FAILURE(status))
- return -ENODEV;
+ error = acpi_dev_pm_explicit_get(device, &psc);
+ if (error)
+ return error;
/*
* The power resources settings may indicate a power state
@@ -152,7 +164,8 @@ int acpi_device_set_power(struct acpi_de
/* Make sure this is a valid target state */
- if (state == device->power.state) {
+ /* There is a special case for D0 addressed below. */
+ if (state > ACPI_STATE_D0 && state == device->power.state) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n",
device->pnp.bus_id,
acpi_power_state_string(state)));
@@ -214,6 +227,30 @@ int acpi_device_set_power(struct acpi_de
if (result)
goto end;
}
+
+ if (device->power.state == ACPI_STATE_D0) {
+ int psc;
+
+ /* Nothing to do here if _PSC is not present. */
+ if (!device->power.flags.explicit_get)
+ return 0;
+
+ /*
+ * The power state of the device was set to D0 last
+ * time, but that might have happened before a
+ * system-wide transition involving the platform
+ * firmware, so it may be necessary to evaluate _PS0
+ * for the device here. However, use extra care here
+ * and evaluate _PSC to check the device's current power
+ * state, and only invoke _PS0 if the evaluation of _PSC
+ * is successful and it returns a power state different
+ * from D0.
+ */
+ result = acpi_dev_pm_explicit_get(device, &psc);
+ if (result || psc == ACPI_STATE_D0)
+ return 0;
+ }
+
result = acpi_dev_pm_explicit_set(device, ACPI_STATE_D0);
}
^ permalink raw reply
* [PATCH V3 3/3] thermal/drivers/cpu_cooling: cpufreq_cooling_register returns an int
From: Daniel Lezcano @ 2019-06-25 11:32 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel, Amit Daniel Kachhap, Javi Merino,
Zhang Rui, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, Keerthy,
open list:THERMAL/CPU_COOLING,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190625113244.18146-1-daniel.lezcano@linaro.org>
It looks like after the changes in the patch the only reason for
returning (struct thermal_cooling_device *) from
cpufreq_cooling_register() is error checking, but it would be much
more straightforward to return int for this purpose.
Moreover, that would prevent the callers of it from doing incorrect
things with the returned pointers (like using it to unregister the
cooling device).
Replace the returned value an integer instead of a pointer to a
thermal cooling device structure.
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/cpu_cooling.c | 63 +++++++++----------
drivers/thermal/imx_thermal.c | 6 +-
.../ti-soc-thermal/ti-thermal-common.c | 7 +--
include/linux/cpu_cooling.h | 16 ++---
4 files changed, 40 insertions(+), 52 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 007c7c6bf845..f5fa31a57658 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -530,13 +530,12 @@ static struct notifier_block thermal_cpufreq_notifier_block = {
* cooling devices. It also gives the opportunity to link the cooling device
* with a device tree node, in order to bind it via the thermal DT code.
*
- * Return: a valid struct thermal_cooling_device pointer on success,
- * on failure, it returns a corresponding ERR_PTR().
+ * Return: zero on success, less than zero corresponding to the
+ * negative error code.
*/
-static struct thermal_cooling_device *
-__cpufreq_cooling_register(struct device_node *np,
- struct cpufreq_policy *policy,
- struct em_perf_domain *em)
+static int __cpufreq_cooling_register(struct device_node *np,
+ struct cpufreq_policy *policy,
+ struct em_perf_domain *em)
{
struct thermal_cooling_device *cdev;
struct cpufreq_cooling_device *cpufreq_cdev;
@@ -548,19 +547,19 @@ __cpufreq_cooling_register(struct device_node *np,
if (IS_ERR_OR_NULL(policy)) {
pr_err("%s: cpufreq policy isn't valid: %p\n", __func__, policy);
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
i = cpufreq_table_count_valid_entries(policy);
if (!i) {
pr_debug("%s: CPUFreq table not found or has no valid entries\n",
__func__);
- return ERR_PTR(-ENODEV);
+ return -ENODEV;
}
cpufreq_cdev = kzalloc(sizeof(*cpufreq_cdev), GFP_KERNEL);
if (!cpufreq_cdev)
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
cpufreq_cdev->policy = policy;
num_cpus = cpumask_weight(policy->related_cpus);
@@ -568,7 +567,7 @@ __cpufreq_cooling_register(struct device_node *np,
sizeof(*cpufreq_cdev->idle_time),
GFP_KERNEL);
if (!cpufreq_cdev->idle_time) {
- cdev = ERR_PTR(-ENOMEM);
+ ret = -ENOMEM;
goto free_cdev;
}
@@ -576,10 +575,8 @@ __cpufreq_cooling_register(struct device_node *np,
cpufreq_cdev->max_level = i - 1;
ret = ida_simple_get(&cpufreq_ida, 0, 0, GFP_KERNEL);
- if (ret < 0) {
- cdev = ERR_PTR(ret);
+ if (ret < 0)
goto free_idle_time;
- }
cpufreq_cdev->id = ret;
snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d",
@@ -597,14 +594,16 @@ __cpufreq_cooling_register(struct device_node *np,
if (policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED) {
pr_err("%s: unsorted frequency tables are not supported\n",
__func__);
- cdev = ERR_PTR(-EINVAL);
+ ret = -EINVAL;
goto remove_ida;
}
cdev = thermal_of_cooling_device_register(np, dev_name, cpufreq_cdev,
cooling_ops);
- if (IS_ERR(cdev))
+ if (IS_ERR(cdev)) {
+ ret = PTR_ERR(cdev);
goto remove_ida;
+ }
cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
cpufreq_cdev->cdev = cdev;
@@ -619,7 +618,7 @@ __cpufreq_cooling_register(struct device_node *np,
cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
CPUFREQ_POLICY_NOTIFIER);
- return cdev;
+ return 0;
remove_ida:
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
@@ -627,7 +626,7 @@ __cpufreq_cooling_register(struct device_node *np,
kfree(cpufreq_cdev->idle_time);
free_cdev:
kfree(cpufreq_cdev);
- return cdev;
+ return ret;
}
/**
@@ -638,11 +637,10 @@ __cpufreq_cooling_register(struct device_node *np,
* "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
* cooling devices.
*
- * Return: a valid struct thermal_cooling_device pointer on success,
- * on failure, it returns a corresponding ERR_PTR().
+ * Return: zero on success, less than zero corresponding to the
+ * negative error code.
*/
-struct thermal_cooling_device *
-cpufreq_cooling_register(struct cpufreq_policy *policy)
+int cpufreq_cooling_register(struct cpufreq_policy *policy)
{
return __cpufreq_cooling_register(NULL, policy, NULL);
}
@@ -664,34 +662,31 @@ EXPORT_SYMBOL_GPL(cpufreq_cooling_register);
* It also takes into account, if property present in policy CPU node, the
* static power consumed by the cpu.
*
- * Return: a valid struct thermal_cooling_device pointer on success,
- * and NULL on failure.
+ * Return: zero on success, less than zero corresponding to the
+ * negative error code.
*/
-struct thermal_cooling_device *
-of_cpufreq_cooling_register(struct cpufreq_policy *policy)
+int of_cpufreq_cooling_register(struct cpufreq_policy *policy)
{
struct device_node *np = of_get_cpu_node(policy->cpu, NULL);
- struct thermal_cooling_device *cdev = NULL;
+ int ret = -EINVAL;
if (!np) {
pr_err("cpu_cooling: OF node not available for cpu%d\n",
policy->cpu);
- return NULL;
+ return -EINVAL;
}
if (of_find_property(np, "#cooling-cells", NULL)) {
struct em_perf_domain *em = em_cpu_get(policy->cpu);
- cdev = __cpufreq_cooling_register(np, policy, em);
- if (IS_ERR(cdev)) {
- pr_err("cpu_cooling: cpu%d failed to register as cooling device: %ld\n",
- policy->cpu, PTR_ERR(cdev));
- cdev = NULL;
- }
+ ret = __cpufreq_cooling_register(np, policy, em);
+ if (ret)
+ pr_err("cpu_cooling: cpu%d failed to register as cooling device: %d\n",
+ policy->cpu, ret);
}
of_node_put(np);
- return cdev;
+ return ret;
}
EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 021c0948b740..1c4b49b583bc 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -655,7 +655,6 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
{
struct device_node *np;
- struct thermal_cooling_device *cdev;
int ret;
data->policy = cpufreq_cpu_get(0);
@@ -667,9 +666,8 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
np = of_get_cpu_node(data->policy->cpu, NULL);
if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
- cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(cdev)) {
- ret = PTR_ERR(cdev);
+ ret = cpufreq_cooling_register(data->policy);
+ if (ret) {
cpufreq_cpu_put(data->policy);
return ret;
}
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 170b70b6ec61..eacc46d7bd1c 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -232,7 +232,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
{
struct ti_thermal_data *data;
struct device_node *np = bgp->dev->of_node;
- struct thermal_cooling_device *cdev;
+ int ret;
/*
* We are assuming here that if one deploys the zone
@@ -256,9 +256,8 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
}
/* Register cooling device */
- cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(cdev)) {
- int ret = PTR_ERR(cdev);
+ ret = cpufreq_cooling_register(data->policy);
+ if (ret) {
dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
ret);
cpufreq_cpu_put(data->policy);
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index 89f469ee4be4..98f7c8a9cab6 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -24,8 +24,7 @@ struct cpufreq_policy;
* cpufreq_cooling_register - function to create cpufreq cooling device.
* @policy: cpufreq policy.
*/
-struct thermal_cooling_device *
-cpufreq_cooling_register(struct cpufreq_policy *policy);
+int cpufreq_cooling_register(struct cpufreq_policy *policy);
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling device.
@@ -34,10 +33,9 @@ cpufreq_cooling_register(struct cpufreq_policy *policy);
void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
#else /* !CONFIG_CPU_THERMAL */
-static inline struct thermal_cooling_device *
-cpufreq_cooling_register(struct cpufreq_policy *policy)
+static inline int cpufreq_cooling_register(struct cpufreq_policy *policy)
{
- return ERR_PTR(-ENOSYS);
+ return -ENOSYS;
}
static inline
@@ -52,13 +50,11 @@ void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
* of_cpufreq_cooling_register - create cpufreq cooling device based on DT.
* @policy: cpufreq policy.
*/
-struct thermal_cooling_device *
-of_cpufreq_cooling_register(struct cpufreq_policy *policy);
+int of_cpufreq_cooling_register(struct cpufreq_policy *policy);
#else
-static inline struct thermal_cooling_device *
-of_cpufreq_cooling_register(struct cpufreq_policy *policy)
+static inline int of_cpufreq_cooling_register(struct cpufreq_policy *policy)
{
- return NULL;
+ return -ENOSYS;
}
#endif /* defined(CONFIG_THERMAL_OF) && defined(CONFIG_CPU_THERMAL) */
--
2.17.1
^ permalink raw reply related
* [PATCH V3 2/3] thermal/drivers/cpu_cooling: Unregister with the policy
From: Daniel Lezcano @ 2019-06-25 11:32 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel, Sudeep Holla, Amit Daniel Kachhap,
Javi Merino, Zhang Rui, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190625113244.18146-1-daniel.lezcano@linaro.org>
Currently the function cpufreq_cooling_register() returns a cooling
device pointer which is used back as a pointer to call the function
cpufreq_cooling_unregister(). Even if it is correct, it would make
sense to not leak the structure inside a cpufreq driver and keep the
code thermal code self-encapsulate. Moreover, that forces to add an
extra variable in each driver using this function.
Instead of passing the cooling device to unregister, pass the policy.
Because the cpufreq_cooling_unregister() function uses the policy to
unregister itself. The only purpose of the cooling device pointer is
to unregister the cpu cooling device.
As there is no more need of this pointer, remove it.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/arm_big_little.c | 9 +++------
drivers/cpufreq/cpufreq.c | 10 ++++------
drivers/thermal/cpu_cooling.c | 18 ++++++++++--------
drivers/thermal/imx_thermal.c | 12 ++++++------
.../thermal/ti-soc-thermal/ti-thermal-common.c | 10 +++++-----
include/linux/cpu_cooling.h | 6 +++---
include/linux/cpufreq.h | 3 ---
7 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 7fe52fcddcf1..718c63231e66 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -56,7 +56,6 @@ static bool bL_switching_enabled;
#define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 : freq)
#define VIRT_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
-static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
static const struct cpufreq_arm_bL_ops *arm_bL_ops;
static struct clk *clk[MAX_CLUSTERS];
static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -501,10 +500,8 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
struct device *cpu_dev;
int cur_cluster = cpu_to_cluster(policy->cpu);
- if (cur_cluster < MAX_CLUSTERS) {
- cpufreq_cooling_unregister(cdev[cur_cluster]);
- cdev[cur_cluster] = NULL;
- }
+ if (cur_cluster < MAX_CLUSTERS)
+ cpufreq_cooling_unregister(policy);
cpu_dev = get_cpu_device(policy->cpu);
if (!cpu_dev) {
@@ -527,7 +524,7 @@ static void bL_cpufreq_ready(struct cpufreq_policy *policy)
if (cur_cluster >= MAX_CLUSTERS)
return;
- cdev[cur_cluster] = of_cpufreq_cooling_register(policy);
+ of_cpufreq_cooling_register(policy);
}
static struct cpufreq_driver bL_cpufreq_driver = {
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index aee024e42618..f07454249fbc 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu)
cpufreq_driver->ready(policy);
if (cpufreq_thermal_control_enabled(cpufreq_driver))
- policy->cdev = of_cpufreq_cooling_register(policy);
-
+ of_cpufreq_cooling_register(policy);
+
pr_debug("initialization complete\n");
return 0;
@@ -1468,10 +1468,8 @@ static int cpufreq_offline(unsigned int cpu)
goto unlock;
}
- if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
- cpufreq_cooling_unregister(policy->cdev);
- policy->cdev = NULL;
- }
+ if (cpufreq_thermal_control_enabled(cpufreq_driver))
+ cpufreq_cooling_unregister(policy);
if (cpufreq_driver->stop_cpu)
cpufreq_driver->stop_cpu(policy);
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 83486775e593..007c7c6bf845 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -78,6 +78,7 @@ struct cpufreq_cooling_device {
struct cpufreq_policy *policy;
struct list_head node;
struct time_in_idle *idle_time;
+ struct thermal_cooling_device *cdev;
};
static DEFINE_IDA(cpufreq_ida);
@@ -606,6 +607,7 @@ __cpufreq_cooling_register(struct device_node *np,
goto remove_ida;
cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
+ cpufreq_cdev->cdev = cdev;
mutex_lock(&cooling_list_lock);
/* Register the notifier for first cpufreq cooling device */
@@ -699,18 +701,18 @@ EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
*
* This interface function unregisters the "thermal-cpufreq-%x" cooling device.
*/
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
struct cpufreq_cooling_device *cpufreq_cdev;
bool last;
- if (!cdev)
- return;
-
- cpufreq_cdev = cdev->devdata;
-
mutex_lock(&cooling_list_lock);
- list_del(&cpufreq_cdev->node);
+ list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
+ if (cpufreq_cdev->policy == policy) {
+ list_del(&cpufreq_cdev->node);
+ break;
+ }
+ }
/* Unregister the notifier for the last cpufreq cooling device */
last = list_empty(&cpufreq_cdev_list);
mutex_unlock(&cooling_list_lock);
@@ -719,7 +721,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
CPUFREQ_POLICY_NOTIFIER);
- thermal_cooling_device_unregister(cdev);
+ thermal_cooling_device_unregister(cpufreq_cdev->cdev);
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
kfree(cpufreq_cdev->idle_time);
kfree(cpufreq_cdev);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index bb6754a5342c..021c0948b740 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -203,7 +203,6 @@ static struct thermal_soc_data thermal_imx7d_data = {
struct imx_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *tz;
- struct thermal_cooling_device *cdev;
enum thermal_device_mode mode;
struct regmap *tempmon;
u32 c1, c2; /* See formula in imx_init_calib() */
@@ -656,6 +655,7 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
{
struct device_node *np;
+ struct thermal_cooling_device *cdev;
int ret;
data->policy = cpufreq_cpu_get(0);
@@ -667,9 +667,9 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
np = of_get_cpu_node(data->policy->cpu, NULL);
if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
- data->cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cdev)) {
- ret = PTR_ERR(data->cdev);
+ cdev = cpufreq_cooling_register(data->policy);
+ if (IS_ERR(cdev)) {
+ ret = PTR_ERR(cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -680,7 +680,7 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
static void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data *data)
{
- cpufreq_cooling_unregister(data->cdev);
+ cpufreq_cooling_unregister(data->policy);
cpufreq_cpu_put(data->policy);
}
@@ -872,7 +872,7 @@ static int imx_thermal_remove(struct platform_device *pdev)
clk_disable_unprepare(data->thermal_clk);
thermal_zone_device_unregister(data->tz);
- cpufreq_cooling_unregister(data->cdev);
+ cpufreq_cooling_unregister(data->policy);
cpufreq_cpu_put(data->policy);
return 0;
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index b4f981daeaf2..170b70b6ec61 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -41,7 +41,6 @@ struct ti_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *ti_thermal;
struct thermal_zone_device *pcb_tz;
- struct thermal_cooling_device *cool_dev;
struct ti_bandgap *bgp;
enum thermal_device_mode mode;
struct work_struct thermal_wq;
@@ -233,6 +232,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
{
struct ti_thermal_data *data;
struct device_node *np = bgp->dev->of_node;
+ struct thermal_cooling_device *cdev;
/*
* We are assuming here that if one deploys the zone
@@ -256,9 +256,9 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
}
/* Register cooling device */
- data->cool_dev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cool_dev)) {
- int ret = PTR_ERR(data->cool_dev);
+ cdev = cpufreq_cooling_register(data->policy);
+ if (IS_ERR(cdev)) {
+ int ret = PTR_ERR(cdev);
dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
ret);
cpufreq_cpu_put(data->policy);
@@ -277,7 +277,7 @@ int ti_thermal_unregister_cpu_cooling(struct ti_bandgap *bgp, int id)
data = ti_bandgap_get_sensor_data(bgp, id);
if (data) {
- cpufreq_cooling_unregister(data->cool_dev);
+ cpufreq_cooling_unregister(data->policy);
if (data->policy)
cpufreq_cpu_put(data->policy);
}
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index bae54bb7c048..89f469ee4be4 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -29,9 +29,9 @@ cpufreq_cooling_register(struct cpufreq_policy *policy);
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling device.
- * @cdev: thermal cooling device pointer.
+ * @policy: cpufreq policy
*/
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
#else /* !CONFIG_CPU_THERMAL */
static inline struct thermal_cooling_device *
@@ -41,7 +41,7 @@ cpufreq_cooling_register(struct cpufreq_policy *policy)
}
static inline
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
return;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a1467aa7f58b..ce13204df972 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -144,9 +144,6 @@ struct cpufreq_policy {
/* For cpufreq driver's internal use */
void *driver_data;
-
- /* Pointer to the cooling device if used for thermal mitigation */
- struct thermal_cooling_device *cdev;
};
struct cpufreq_freqs {
--
2.17.1
^ permalink raw reply related
* [PATCH V3 1/3] cpufreq: Move the IS_ENABLED(CPU_THERMAL) macro in a stub
From: Daniel Lezcano @ 2019-06-25 11:32 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel,
open list:CPU FREQUENCY SCALING FRAMEWORK
The cpufreq_online and the cpufreq_offline [un]register the driver as
a cooling device. This is done if the driver is flagged as a cooling
device in addition with a IS_ENABLED macro to compile out the branching
code.
Group this test in a stub function added in the cpufreq header instead
of having the IS_ENABLED in the code path.
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 6 ++----
include/linux/cpufreq.h | 6 ++++++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 85ff958e01f1..aee024e42618 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1378,8 +1378,7 @@ static int cpufreq_online(unsigned int cpu)
if (cpufreq_driver->ready)
cpufreq_driver->ready(policy);
- if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
- cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
+ if (cpufreq_thermal_control_enabled(cpufreq_driver))
policy->cdev = of_cpufreq_cooling_register(policy);
pr_debug("initialization complete\n");
@@ -1469,8 +1468,7 @@ static int cpufreq_offline(unsigned int cpu)
goto unlock;
}
- if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
- cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {
+ if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
cpufreq_cooling_unregister(policy->cdev);
policy->cdev = NULL;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d01a74fbc4db..a1467aa7f58b 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -409,6 +409,12 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
const char *cpufreq_get_current_driver(void);
void *cpufreq_get_driver_data(void);
+static inline int cpufreq_thermal_control_enabled(struct cpufreq_driver *drv)
+{
+ return IS_ENABLED(CONFIG_CPU_THERMAL) &&
+ (drv->flags & CPUFREQ_IS_COOLING_DEV);
+}
+
static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
unsigned int min, unsigned int max)
{
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v4 13/16] PM / devfreq: tegra: Support Tegra30
From: Dmitry Osipenko @ 2019-06-25 11:30 UTC (permalink / raw)
To: myungjoo.ham
Cc: Chanwoo Choi, linux-pm@vger.kernel.org,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
jonathanh@nvidia.com, thierry.reding@gmail.com
In-Reply-To: <20190625014214epcms1p1b8f2d76cd8cfdf3fdf517be08a92ccdf@epcms1p1>
25.06.2019 4:42, MyungJoo Ham пишет:
> Sender : Dmitry Osipenko <digetx@gmail.com>
>> 24.06.2019 14:11, MyungJoo Ham пишет:
>>>>
>>>> --------- Original Message ---------
>>>> Sender : Dmitry Osipenko <digetx@gmail.com>
>>>>
>>>> 24.06.2019 10:34, MyungJoo Ham пишет:
>>>>>>
>>>>>> A question:
>>>>>>
>>>>>> Does this driver support Tegra20 as well?
>>>>>> I'm asking this because ARCH_TEGRA includes ARCH_TEGRA_2x_SOC
>>>>>> according to /drivers/soc/tegra/Kconfig.
>>>>>>
>>>>>
>>>>> For this matter, how about updating your 13/16 patch as follows?
>>>>>
>>> []
>>>>
>>>> Good call! I'll update this patch following yours suggestion, thanks.
>>>
>>> Or, you may approve the modified commits here:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git/log/?h=for-next
>>
>> Looks almost good to me!
>>
>> I just recalled that there is also a 64bit variant of Tegra124, the Tegra132. Hence
>> the Tegra30+ Kconfig entry should look like this (it's also worthy to break the lines
>> for readability):
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index ccb1a68c4b51..bd2efbc27725 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -94,7 +94,10 @@ config ARM_EXYNOS_BUS_DEVFREQ
>>
>> config ARM_TEGRA_DEVFREQ
>> tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>> - depends on ARCH_TEGRA || COMPILE_TEST
>> + depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>> + ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
>> + ARCH_TEGRA_210_SOC || \
>> + COMPILE_TEST
>> select PM_OPP
>> help
>> This adds the DEVFREQ driver for the Tegra family of SoCs.
>>
>> Could you please adjust the patches like I'm suggesting? I'll approve yours change
>> then and won't re-spin the first batch of the patches.
>
> I've adjusted as you suggested. It's pushed to the git repo as well.
Thank you very much, looking good now!
^ permalink raw reply
* Re: [PATCH v10 08/13] drivers: memory: add DMC driver for Exynos5422
From: Lukasz Luba @ 2019-06-25 11:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org, linux-clk, mturquette, sboyd,
Bartłomiej Żołnierkiewicz, kgene, Chanwoo Choi,
kyungmin.park, Marek Szyprowski, s.nawrocki, myungjoo.ham,
keescook, tony, jroedel, treding, digetx, gregkh,
willy.mh.wolff.ml
In-Reply-To: <CAJKOXPdjXhfcNRL-XMS6K1jrUQoqoNHXGirjXoeoL4GyXzarpg@mail.gmail.com>
Hi Krzysztof,
On 6/14/19 3:47 PM, Krzysztof Kozlowski wrote:
> On Fri, 14 Jun 2019 at 11:53, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>
>> This patch adds driver for Exynos5422 Dynamic Memory Controller.
>> The driver provides support for dynamic frequency and voltage scaling for
>> DMC and DRAM. It supports changing timings of DRAM running with different
>> frequency. There is also an algorithm to calculate timigns based on
>> memory description provided in DT.
>> The patch also contains needed MAINTAINERS file update.
>>
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>> MAINTAINERS | 8 +
>> drivers/memory/samsung/Kconfig | 17 +
>> drivers/memory/samsung/Makefile | 1 +
>> drivers/memory/samsung/exynos5422-dmc.c | 1262 +++++++++++++++++++++++
>> 4 files changed, 1288 insertions(+)
>> create mode 100644 drivers/memory/samsung/exynos5422-dmc.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 57f496cff999..6ffccfd95351 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3470,6 +3470,14 @@ S: Maintained
>> F: drivers/devfreq/exynos-bus.c
>> F: Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>
>> +DMC FREQUENCY DRIVER FOR SAMSUNG EXYNOS5422
>> +M: Lukasz Luba <l.luba@partner.samsung.com>
>> +L: linux-pm@vger.kernel.org
>> +L: linux-samsung-soc@vger.kernel.org
>> +S: Maintained
>> +F: drivers/memory/samsung/exynos5422-dmc.c
>> +F: Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt
>> +
>> BUSLOGIC SCSI DRIVER
>> M: Khalid Aziz <khalid@gonehiking.org>
>> L: linux-scsi@vger.kernel.org
>> diff --git a/drivers/memory/samsung/Kconfig b/drivers/memory/samsung/Kconfig
>> index 79ce7ea58903..c93baa029654 100644
>> --- a/drivers/memory/samsung/Kconfig
>> +++ b/drivers/memory/samsung/Kconfig
>> @@ -5,6 +5,23 @@ config SAMSUNG_MC
>> Support for the Memory Controller (MC) devices found on
>> Samsung Exynos SoCs.
>>
>> +config ARM_EXYNOS5422_DMC
>> + tristate "ARM EXYNOS5422 Dynamic Memory Controller driver"
>> + depends on ARCH_EXYNOS
>> + select DDR
>> + select PM_DEVFREQ
>> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
>> + select DEVFREQ_GOV_USERSPACE
>> + select PM_DEVFREQ_EVENT
>> + select PM_OPP
>> + help
>> + This adds driver for Exynos5422 DMC (Dynamic Memory Controller).
>> + The driver provides support for Dynamic Voltage and Frequency Scaling in
>> + DMC and DRAM. It also supports changing timings of DRAM running with
>> + different frequency. The timings are calculated based on DT memory
>> + information.
>> +
>> +
>> if SAMSUNG_MC
>>
>> config EXYNOS_SROM
>> diff --git a/drivers/memory/samsung/Makefile b/drivers/memory/samsung/Makefile
>> index 00587be66211..4f6e4383bab7 100644
>> --- a/drivers/memory/samsung/Makefile
>> +++ b/drivers/memory/samsung/Makefile
>> @@ -1,2 +1,3 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_ARM_EXYNOS5422_DMC) += exynos5422-dmc.o
>> obj-$(CONFIG_EXYNOS_SROM) += exynos-srom.o
>> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
>> new file mode 100644
>> index 000000000000..b397efe0da57
>> --- /dev/null
>> +++ b/drivers/memory/samsung/exynos5422-dmc.c
>> @@ -0,0 +1,1262 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 Samsung Electronics Co., Ltd.
>> + * Author: Lukasz Luba <l.luba@partner.samsung.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/devfreq-event.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/pm_opp.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/slab.h>
>> +#include <memory/jedec_ddr.h>
>> +#include "../of_memory.h"
>> +
>> +#define EXYNOS5_DREXI_TIMINGAREF (0x0030)
>> +#define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
>> +#define EXYNOS5_DREXI_TIMINGDATA0 (0x0038)
>> +#define EXYNOS5_DREXI_TIMINGPOWER0 (0x003C)
>> +#define EXYNOS5_DREXI_TIMINGROW1 (0x00E4)
>> +#define EXYNOS5_DREXI_TIMINGDATA1 (0x00E8)
>> +#define EXYNOS5_DREXI_TIMINGPOWER1 (0x00EC)
>> +#define CDREX_PAUSE (0x2091c)
>> +#define CDREX_LPDDR3PHY_CON3 (0x20a20)
>> +#define EXYNOS5_TIMING_SET_SWI (1UL << 28)
>> +#define USE_MX_MSPLL_TIMINGS (1)
>> +#define USE_BPLL_TIMINGS (0)
>> +#define EXYNOS5_AREF_NORMAL (0x2e)
>> +
>> +/**
>> + * struct dmc_opp_table - Operating level desciption
>> + *
>> + * Covers frequency and voltage settings of the DMC operating mode.
>> + */
>> +struct dmc_opp_table {
>> + u32 freq_hz;
>> + u32 volt_uv;
>> +};
>> +
>> +/**
>> + * struct exynos5_dmc - main structure describing DMC device
>> + *
>> + * The main structure for the Dynamic Memory Controller which covers clocks,
>> + * memory regions, HW information, parameters and current operating mode.
>> + */
>> +struct exynos5_dmc {
>> + struct device *dev;
>> + struct devfreq *df;
>> + struct devfreq_simple_ondemand_data gov_data;
>> + void __iomem *base_drexi0;
>> + void __iomem *base_drexi1;
>> + struct regmap *clk_regmap;
>> + struct mutex lock;
>> + unsigned long curr_rate;
>> + unsigned long curr_volt;
>> + unsigned long bypass_rate;
>> + struct dmc_opp_table *opp;
>> + struct dmc_opp_table opp_bypass;
>> + int opp_count;
>> + u32 timings_arr_size;
>> + u32 *timing_row;
>> + u32 *timing_data;
>> + u32 *timing_power;
>> + const struct lpddr3_timings *timings;
>> + const struct lpddr3_min_tck *min_tck;
>> + u32 bypass_timing_row;
>> + u32 bypass_timing_data;
>> + u32 bypass_timing_power;
>> + struct regulator *vdd_mif;
>> + struct clk *fout_spll;
>> + struct clk *fout_bpll;
>> + struct clk *mout_spll;
>> + struct clk *mout_bpll;
>> + struct clk *mout_mclk_cdrex;
>> + struct clk *mout_mx_mspll_ccore;
>> + struct clk *mx_mspll_ccore_phy;
>> + struct clk *mout_mx_mspll_ccore_phy;
>> + struct devfreq_event_dev **counter;
>> + int num_counters;
>> +};
>> +
>> +#define TIMING_FIELD(t_name, t_bit_beg, t_bit_end) \
>> + { .name = t_name, .bit_beg = t_bit_beg, .bit_end = t_bit_end }
>> +
>> +#define TIMING_VAL(timing_array, id, t_val) \
>> +({ \
>> + u32 __val; \
>> + __val = t_val << timing_array[id].bit_beg; \
>> + __val; \
>> +})
>> +
>> +#define TIMING_VAL2REG(timing, t_val) \
>> +({ \
>> + u32 __val; \
>> + __val = t_val << timing->bit_beg; \
>> + __val; \
>> +})
>> +
>> +#define TIMING_REG2VAL(reg, timing) \
>
> It seems that only some of these defines are used. Please clean them up.
Indeed, they were used in previous version which had more features and
debug options. So TIMING_REG2VAL and TIMING_VAL will be cleaned
> You have also a lot of checkpatch --strict suggestions:
> CHECK: Macro argument 'reg' may be better as '(reg)' to avoid
> precedence issues
> which seems to be valid.
Will not be valid since it is in TIMING_REG2VAL macro which was used
for debugfs information only.
>
> While at it please also fix few other --strict errors:
> CHECK: Please don't use multiple blank lines
> CHECK: Alignment should match open parenthesis
> CHECK: Prefer using the BIT macro
> CHECK: struct mutex definition without comment
I wouldn't call them 'errors' but will do a cleanup.
Thank you for the review.
Regards,
Lukasz
>
> Best regards,
> Krzysztof
>
>
^ permalink raw reply
* Re: [PATCH v4 1/2] power: supply: add input power and voltage limit properties
From: Enric Balletbo Serra @ 2019-06-25 10:04 UTC (permalink / raw)
To: Benson Leung
Cc: Enric Balletbo i Serra, Linux PM list, Sebastian Reichel,
Sameer Nanda, Benson Leung, Rafael J. Wysocki, Gwendal Grignou,
linux-kernel, Len Brown, Guenter Roeck, Adam.Thomson.Opensource,
Collabora Kernel ML, Pavel Machek
In-Reply-To: <20190523195438.GA110498@google.com>
Hi Sebastian, Pavel,
Missatge de Benson Leung <bleung@google.com> del dia dj., 23 de maig
2019 a les 21:55:
>
> Hi Enric,
>
> On Tue, May 07, 2019 at 11:52:47AM +0200, Enric Balletbo i Serra wrote:
> > For thermal management strategy you might be interested on limit the
> > input power for a power supply. We already have current limit but
> > basically what we probably want is to limit power. So, introduce the
> > input_power_limit property.
> >
> > Although the common use case is limit the input power, in some
> > specific cases it is the voltage that is problematic (i.e some regulators
> > have different efficiencies at higher voltage resulting in more heat).
> > So introduce also the input_voltage_limit property.
> >
> > This happens in one Chromebook and is used on the Pixel C's thermal
> > management strategy to effectively limit the input power to 5V 3A when
> > the screen is on. When the screen is on, the display, the CPU, and the GPU
> > all contribute more heat to the system than while the screen is off, and
> > we made a tradeoff to throttle the charger in order to give more of the
> > thermal budget to those other components.
> >
> > So there's nothing fundamentally broken about the hardware that would
> > cause the Pixel C to malfunction if we were charging at 9V or 12V instead
> > of 5V when the screen is on, i.e. if userspace doesn't change this.
> >
> > What would happen is that you wouldn't meet Google's skin temperature
> > targets on the system if the charger was allowed to run at 9V or 12V with
> > the screen on.
> >
> > For folks hacking on Pixel Cs (which is now outside of Google's official
> > support window for Android) and customizing their own kernel and userspace
> > this would be acceptable, but we wanted to expose this feature in the
> > power supply properties because the feature does exist in the Emedded
> > Controller firmware of the Pixel C and all of Google's Chromebooks with
> > USB-C made since 2015 in case someone running an up to date kernel wanted
> > to limit the charging power for thermal or other reasons.
> >
> > This patch exposes a new property, similar to input current limit, to
> > re-configure the maximum voltage from the external supply at runtime
> > based on system-level knowledge or user input.
> >
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Reviewed-by: Guenter Roeck <groeck@chromium.org>
> > Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
>
> Reviewed-by: Benson Leung <bleung@chromium.org>
>
We're close to the merge window so I'm wondering if there are more
concerns on this patchset?
Thanks,
~ Enric
> > ---
> >
> > Changes in v4:
> > - Add also input_power_limit.
> >
> > Changes in v3:
> > - Improve commit log and documentation with Benson comments.
> >
> > Changes in v2:
> > - Document the new property in ABI/testing/sysfs-class-power.
> > - Add the Reviewed-by Guenter Roeck tag.
> >
> > Documentation/ABI/testing/sysfs-class-power | 32 +++++++++++++++++++++
> > Documentation/power/power_supply_class.txt | 4 +++
> > drivers/power/supply/power_supply_sysfs.c | 2 ++
> > include/linux/power_supply.h | 2 ++
> > 4 files changed, 40 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > index 5e23e22dce1b..962a27a1daf8 100644
> > --- a/Documentation/ABI/testing/sysfs-class-power
> > +++ b/Documentation/ABI/testing/sysfs-class-power
> > @@ -331,10 +331,42 @@ Description:
> > supply. Normally this is configured based on the type of
> > connection made (e.g. A configured SDP should output a maximum
> > of 500mA so the input current limit is set to the same value).
> > + Use preferably input_power_limit, and for problems that can be
> > + solved using power limit use input_current_limit.
> >
> > Access: Read, Write
> > Valid values: Represented in microamps
> >
> > +What: /sys/class/power_supply/<supply_name>/input_voltage_limit
> > +Date: May 2019
> > +Contact: linux-pm@vger.kernel.org
> > +Description:
> > + This entry configures the incoming VBUS voltage limit currently
> > + set in the supply. Normally this is configured based on
> > + system-level knowledge or user input (e.g. This is part of the
> > + Pixel C's thermal management strategy to effectively limit the
> > + input power to 5V when the screen is on to meet Google's skin
> > + temperature targets). Note that this feature should not be
> > + used for safety critical things.
> > + Use preferably input_power_limit, and for problems that can be
> > + solved using power limit use input_voltage_limit.
> > +
> > + Access: Read, Write
> > + Valid values: Represented in microvolts
> > +
> > +What: /sys/class/power_supply/<supply_name>/input_power_limit
> > +Date: May 2019
> > +Contact: linux-pm@vger.kernel.org
> > +Description:
> > + This entry configures the incoming power limit currently set
> > + in the supply. Normally this is configured based on
> > + system-level knowledge or user input. Use preferably this
> > + feature to limit the incoming power and use current/voltage
> > + limit only for problems that can be solved using power limit.
> > +
> > + Access: Read, Write
> > + Valid values: Represented in microwatts
> > +
> > What: /sys/class/power_supply/<supply_name>/online,
> > Date: May 2007
> > Contact: linux-pm@vger.kernel.org
> > diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
> > index 300d37896e51..1e3c705111db 100644
> > --- a/Documentation/power/power_supply_class.txt
> > +++ b/Documentation/power/power_supply_class.txt
> > @@ -137,6 +137,10 @@ power supply object.
> >
> > INPUT_CURRENT_LIMIT - input current limit programmed by charger. Indicates
> > the current drawn from a charging source.
> > +INPUT_VOLTAGE_LIMIT - input voltage limit programmed by charger. Indicates
> > +the voltage limit from a charging source.
> > +INPUT_POWER_LIMIT - input power limit programmed by charger. Indicates
> > +the power limit from a charging source.
> >
> > CHARGE_CONTROL_LIMIT - current charge control limit setting
> > CHARGE_CONTROL_LIMIT_MAX - maximum charge control limit setting
> > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > index 5358a80d854f..860db617d241 100644
> > --- a/drivers/power/supply/power_supply_sysfs.c
> > +++ b/drivers/power/supply/power_supply_sysfs.c
> > @@ -275,6 +275,8 @@ static struct device_attribute power_supply_attrs[] = {
> > POWER_SUPPLY_ATTR(charge_control_limit),
> > POWER_SUPPLY_ATTR(charge_control_limit_max),
> > POWER_SUPPLY_ATTR(input_current_limit),
> > + POWER_SUPPLY_ATTR(input_voltage_limit),
> > + POWER_SUPPLY_ATTR(input_power_limit),
> > POWER_SUPPLY_ATTR(energy_full_design),
> > POWER_SUPPLY_ATTR(energy_empty_design),
> > POWER_SUPPLY_ATTR(energy_full),
> > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > index 2f9c201a54d1..ba135a5d8996 100644
> > --- a/include/linux/power_supply.h
> > +++ b/include/linux/power_supply.h
> > @@ -122,6 +122,8 @@ enum power_supply_property {
> > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT,
> > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
> > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> > + POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
> > + POWER_SUPPLY_PROP_INPUT_POWER_LIMIT,
> > POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
> > POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
> > POWER_SUPPLY_PROP_ENERGY_FULL,
> > --
> > 2.20.1
> >
>
> --
> Benson Leung
> Staff Software Engineer
> Chrome OS Kernel
> Google Inc.
> bleung@google.com
> Chromium OS Project
> bleung@chromium.org
^ permalink raw reply
* Re: [PATCH v10 12/16] sched/core: uclamp: Extend CPU's cgroup controller
From: Patrick Bellasi @ 2019-06-25 9:31 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-kernel, linux-pm, Ingo Molnar, Peter Zijlstra,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
Alessio Balsini
In-Reply-To: <20190624175215.GR657710@devbig004.ftw2.facebook.com>
On 24-Jun 10:52, Tejun Heo wrote:
> Hey, Patrick.
Hi,
> On Mon, Jun 24, 2019 at 06:29:06PM +0100, Patrick Bellasi wrote:
> > > I kinda wonder whether the term bandwidth is a bit confusing because
> > > it's also used for cpu.max/min. Would just calling it frequency be
> > > clearer?
> >
> > Maybe I should find a better way to express the concept above.
> >
> > I agree that bandwidth is already used by cpu.{max,min}, what I want
> > to call out is that clamps allows to enrich that concept.
> >
> > By hinting the scheduler on min/max required utilization we can better
> > defined the amount of actual CPU cycles required/allowed.
> > That's a bit more precise bandwidth control compared to just rely on
> > temporal runnable/period limits.
>
> I see. I wonder whether it's overloading the same term too subtly
> tho. It's great to document how they interact but it *might* be
> easier for readers if a different term is used even if the meaning is
> essentially the same. Anyways, it's a nitpick. Please feel free to
> ignore.
Got it, will try come up with a better description in the v11 to avoid
confusion and better explain the "improvements" without polluting too
much the original concept.
> > > > + tg = css_tg(of_css(of));
> > > > + if (tg == &root_task_group) {
> > > > + ret = -EINVAL;
> > > > + goto out;
> > > > + }
> > >
> > > I don't think you need the above check.
> >
> > Don't we want to forbid attributes tuning from the root group?
>
> Yeah, that's enforced by NOT_ON_ROOT flag, right?
Oh right, since we don't show them we can't write them :)
> > > So, uclamp.max limits the maximum freq% can get and uclamp.min limits
> > > hte maximum freq% protection can get in the subtree. Let's say
> > > uclamp.max is 50% and uclamp.min is 100%.
> >
> > That's not possible, in the current implementation we always enforce
> > the limit (uclamp.max) to be _not smaller_ then the protection
> > (uclamp.min).
> >
> > Indeed, in principle, it does not make sense to ask for a minimum
> > utilization (i.e. frequency boosting) which is higher then the
> > maximum allowed utilization (i.e. frequency capping).
>
> Yeah, I'm trying to explain actually it does.
>
> > > It means that protection is not limited but the actual freq% is
> > > limited upto 50%, which isn't necessarily invalid.
> > > For a simple example, a user might be saying
> > > that they want to get whatever protection they can get from its parent
> > > but wanna limit eventual freq at 50% and it isn't too difficult to
> > > imagine cases where the two knobs are configured separately especially
> > > configuration is being managed hierarchically / automatically.
> >
> > That's not my understanding, in v10 by default when we create a
> > subgroup we assign it uclamp.min=0%, meaning that we don't boost
> > frequencies.
> >
> > It seems instead that you are asking to set uclamp.min=100% by
> > default, so that the effective value will give us whatever the father
> > allow. Is that correct?
>
> No, the defaults are fine. I'm trying to say that min/max
> configurations don't need to be coupled like this and there are valid
> use cases where the configured min is higher than max when
> configurations are nested and managed automatically.
>
> Limits always trump protection in effect of course but please don't
> limit what can be configured.
Got it, thanks!
Will fix it in v11.
> Thanks.
>
> --
> tejun
Cheers,
Patrick
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v4 08/11] thermal: sun8i: support ahb clocks
From: Maxime Ripard @ 2019-06-25 8:25 UTC (permalink / raw)
To: Yangtao Li, mark.rutland, devicetree, linux-pm, gregkh,
linus.walleij, daniel.lezcano, linux-kernel, edubezval, wens,
robh+dt, mchehab+samsung, rui.zhang, paulmck, davem,
linux-arm-kernel
In-Reply-To: <20190625003416.pxve36mrxmotg2bq@core.my.home>
[-- Attachment #1: Type: text/plain, Size: 1916 bytes --]
On Tue, Jun 25, 2019 at 02:34:16AM +0200, Ondřej Jirman wrote:
> On Mon, Jun 24, 2019 at 08:23:33PM +0200, Maxime Ripard wrote:
> > On Sun, Jun 23, 2019 at 12:42:03PM -0400, Yangtao Li wrote:
> > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device
> > > and adds the process of the clock.
> > >
> > > This is pre-work for supprt it.
> > >
> > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > > ---
> > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++-
> > > 1 file changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> > > index ed1c19bb27cf..04f53ffb6a14 100644
> > > --- a/drivers/thermal/sun8i_thermal.c
> > > +++ b/drivers/thermal/sun8i_thermal.c
> > > @@ -54,6 +54,7 @@ struct tsensor {
> > > };
> > >
> > > struct ths_thermal_chip {
> > > + bool has_ahb_clk;
> > > int sensor_num;
> > > int offset;
> > > int scale;
> > > @@ -69,6 +70,7 @@ struct ths_device {
> > > struct regmap *regmap;
> > > struct reset_control *reset;
> > > struct clk *bus_clk;
> > > + struct clk *ahb_clk;
> >
> > Hmm, thinking a bit about this, the name of those two clocks doesn't
> > make sense. AHB is the bus being used to access that device, so the
> > bus clock is the AHB clock.
> >
> > What is that clock being used for?
>
> To control the A/D and sample averaging logic, I suppose. It's controlled by the
> THS_CLK_REG (THS Clock Register) in H3 user manual.
>
> bus_clk controls THS_GATING in BUS_CLK_GATING_REG2 (THS module is connected to
> APB bus).
>
> I'd call it ths_clk and bus_clk.
Thanks. We've tried to make clock names a bit more generic and
consistent, so let's use mod instead.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] notifier: Fix broken error handling pattern
From: Peter Zijlstra @ 2019-06-25 7:38 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Jessica Yu, linux-kernel, jikos, mbenes, pmladek, ast, daniel,
akpm, Rafael J. Wysocki, Pavel Machek, Len Brown, Sam Protsenko,
Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras,
Allison Randal, Vasily Averin, Todd Brandt, linux-pm
In-Reply-To: <20190624222107.wrmtww6b2be26wwl@treble>
On Mon, Jun 24, 2019 at 05:21:07PM -0500, Josh Poimboeuf wrote:
> On Mon, Jun 24, 2019 at 11:18:44AM +0200, Peter Zijlstra wrote:
> > The current notifiers have the following error handling pattern all
> > over the place:
> >
> > int nr;
> >
> > ret = __foo_notifier_call_chain(&chain, val_up, v, -1, &nr);
> > if (err & NOTIFIER_STOP_MASK)
>
> s/err/ret/
-ETOOWARM :-)
> > __foo_notifier_call_chain(&chain, val_down, v, nr-1, NULL)
> >
> > And aside from the endless repetition thereof, it is broken. Consider
> > blocking notifiers; both calls take and drop the rwsem, this means
> > that the notifier list can change in between the two calls, making @nr
> > meaningless.
> >
> > Fix this by replacing all the __foo_notifier_call_chain() functions
> > with foo_notifier_call_chain_error() that embeds the above patter, but
> > ensures it is inside a single lock region.
>
> The name "notifier_call_chain_error()" seems confusing, it almost sounds
> like it's notifying an error code. Then again, I can't really think of
> a more reasonably succinct name.
I;m not attached to the name; I very much ran out of ideas and just
typed something.
> > @@ -25,8 +25,23 @@ static int cpu_pm_notify(enum cpu_pm_eve
> > * RCU know this.
> > */
> > rcu_irq_enter_irqson();
> > - ret = __atomic_notifier_call_chain(&cpu_pm_notifier_chain, event, NULL,
> > - nr_to_call, nr_calls);
> > + ret = atomic_notifier_call_chain(&cpu_pm_notifier_chain, event, NULL);
> > + rcu_irq_exit_irqson();
> > +
> > + return notifier_to_errno(ret);
> > +}
> > +
> > +static int cpu_pm_notify_error(enum cpu_pm_event event_up, enum cpu_pm_event event_down)
> > +{
> > + int ret;
> > +
> > + /*
> > + * __atomic_notifier_call_chain has a RCU read critical section, which
>
> __atomic_notifier_call_chain() no longer exists.
>
> > + * could be disfunctional in cpu idle. Copy RCU_NONIDLE code to let
>
> "dysfunctional"
That's copy paste, I don't think I've read the comment, my bad.
> > @@ -156,43 +169,30 @@ int atomic_notifier_chain_unregister(str
> > }
> > EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
> >
> > -/**
> > - * __atomic_notifier_call_chain - Call functions in an atomic notifier chain
> > - * @nh: Pointer to head of the atomic notifier chain
> > - * @val: Value passed unmodified to notifier function
> > - * @v: Pointer passed unmodified to notifier function
> > - * @nr_to_call: See the comment for notifier_call_chain.
> > - * @nr_calls: See the comment for notifier_call_chain.
> > - *
> > - * Calls each function in a notifier chain in turn. The functions
> > - * run in an atomic context, so they must not block.
> > - * This routine uses RCU to synchronize with changes to the chain.
> > - *
> > - * If the return value of the notifier can be and'ed
> > - * with %NOTIFY_STOP_MASK then atomic_notifier_call_chain()
> > - * will return immediately, with the return value of
> > - * the notifier function which halted execution.
> > - * Otherwise the return value is the return value
> > - * of the last notifier function called.
> > - */
>
> Why remove the useful comment?
Because I delete the whole function ?
^ permalink raw reply
* Re: [PATCH v10 09/13] drivers: devfreq: events: add Exynos PPMU new events
From: Chanwoo Choi @ 2019-06-25 7:38 UTC (permalink / raw)
To: Lukasz Luba, cwchoi00
Cc: devicetree, linux-kernel, Linux PM list, linux-samsung-soc,
linux-clk, Michael Turquette, Stephen Boyd,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Kukjin Kim,
Kyungmin Park, Marek Szyprowski, Sylwester Nawrocki, MyungJoo Ham,
keescook, Tony Lindgren, jroedel, treding, digetx, Greg KH,
willy.mh.wolff.ml
In-Reply-To: <2e1be1d3-b6ae-e40d-48cd-6b6adb26860d@partner.samsung.com>
On 19. 6. 25. 오후 4:31, Lukasz Luba wrote:
>
>
> On 6/22/19 3:10 PM, Chanwoo Choi wrote:
>> Hi,
>>
>> 2019년 6월 14일 (금) 오후 6:54, Lukasz Luba <l.luba@partner.samsung.com>님이 작성:
>>>
>>> Define new performance events supported by Exynos5422 SoC counters.
>>> The counters are built-in in Dynamic Memory Controller and provide
>>> information regarding memory utilization.
>>>
>>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>>> ---
>>> drivers/devfreq/event/exynos-ppmu.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
>>> index c2ea94957501..ce658c262c27 100644
>>> --- a/drivers/devfreq/event/exynos-ppmu.c
>>> +++ b/drivers/devfreq/event/exynos-ppmu.c
>>> @@ -89,6 +89,12 @@ static struct __exynos_ppmu_events {
>>> PPMU_EVENT(d1-cpu),
>>> PPMU_EVENT(d1-general),
>>> PPMU_EVENT(d1-rt),
>>> +
>>> + /* For Exynos5422 SoC */
>>> + PPMU_EVENT(dmc0_0),
>>> + PPMU_EVENT(dmc0_1),
>>> + PPMU_EVENT(dmc1_0),
>>> + PPMU_EVENT(dmc1_1),
>>> };
>>>
>>> static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *edev)
>>> --
>>> 2.17.1
>>>
>>
>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>>
> Thank you Chanwoo.
It[1] was merged to devfreq.git.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git/commit/?h=for-next
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v10 09/13] drivers: devfreq: events: add Exynos PPMU new events
From: Lukasz Luba @ 2019-06-25 7:31 UTC (permalink / raw)
To: cwchoi00
Cc: devicetree, linux-kernel, Linux PM list, linux-samsung-soc,
linux-clk, Michael Turquette, Stephen Boyd,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Kukjin Kim,
Chanwoo Choi, Kyungmin Park, Marek Szyprowski, Sylwester Nawrocki,
MyungJoo Ham, keescook, Tony Lindgren, jroedel, treding, digetx,
Greg KH, willy.mh.wolff.ml
In-Reply-To: <CAGTfZH35X0zE2LhGWJJp2xZNNk1ew7zNMoMqL+eZ5rcBFcPvew@mail.gmail.com>
On 6/22/19 3:10 PM, Chanwoo Choi wrote:
> Hi,
>
> 2019년 6월 14일 (금) 오후 6:54, Lukasz Luba <l.luba@partner.samsung.com>님이 작성:
>>
>> Define new performance events supported by Exynos5422 SoC counters.
>> The counters are built-in in Dynamic Memory Controller and provide
>> information regarding memory utilization.
>>
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>> drivers/devfreq/event/exynos-ppmu.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
>> index c2ea94957501..ce658c262c27 100644
>> --- a/drivers/devfreq/event/exynos-ppmu.c
>> +++ b/drivers/devfreq/event/exynos-ppmu.c
>> @@ -89,6 +89,12 @@ static struct __exynos_ppmu_events {
>> PPMU_EVENT(d1-cpu),
>> PPMU_EVENT(d1-general),
>> PPMU_EVENT(d1-rt),
>> +
>> + /* For Exynos5422 SoC */
>> + PPMU_EVENT(dmc0_0),
>> + PPMU_EVENT(dmc0_1),
>> + PPMU_EVENT(dmc1_0),
>> + PPMU_EVENT(dmc1_1),
>> };
>>
>> static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *edev)
>> --
>> 2.17.1
>>
>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>
Thank you Chanwoo.
Regards,
Lukasz
^ permalink raw reply
* Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Saravana Kannan @ 2019-06-25 5:29 UTC (permalink / raw)
To: Viresh Kumar
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM, LKML
In-Reply-To: <20190625052227.3v74l6xtrkydzx6w@vireshk-i7>
On Mon, Jun 24, 2019 at 10:22 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 24-06-19, 22:00, Saravana Kannan wrote:
> > All of the cases above are some real world scenarios I've come across.
> > CPU and L2/L3 on ARM systems are a good example of (2) but the passive
> > governor doesn't work with CPUs yet. But I plan to work on that later
> > as that's not related to this patch series.
>
> So in case of CPUs, the cache will be the parent device and CPU be the
> children ? And CPUs nodes will contain the required-opps property ?
No, the CPUs will be the "parent" and the cache will be the "child".
CPU is a special case when it comes to the actual software (not DT) as
we'll need the devfreq governor to look at all the CPUfreq policies to
decide the cache frequency (max of all their requirements).
I think "master" and "slave" would have been a better term as the
master device determines its frequency using whatever means and the
"slave" device just "follows" the master device.
-Saravana
^ permalink raw reply
* Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Viresh Kumar @ 2019-06-25 5:22 UTC (permalink / raw)
To: Saravana Kannan
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM, LKML
In-Reply-To: <CAGETcx8MuXkQyD5qZBC948-hOu=kWd4hPk2Qiu-zWOcHBCc=FA@mail.gmail.com>
On 24-06-19, 22:00, Saravana Kannan wrote:
> All of the cases above are some real world scenarios I've come across.
> CPU and L2/L3 on ARM systems are a good example of (2) but the passive
> governor doesn't work with CPUs yet. But I plan to work on that later
> as that's not related to this patch series.
So in case of CPUs, the cache will be the parent device and CPU be the
children ? And CPUs nodes will contain the required-opps property ?
--
viresh
^ permalink raw reply
* Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Saravana Kannan @ 2019-06-25 5:00 UTC (permalink / raw)
To: Viresh Kumar
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM, LKML
In-Reply-To: <20190625041054.2ceuvnuuebc6hsr5@vireshk-i7>
On Mon, Jun 24, 2019 at 9:11 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 24-06-19, 15:17, Saravana Kannan wrote:
> > Here's an example. This can't be done today, but can be done with this change.
> >
> > In arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi you have something
> > like this with the following changes:
> >
> > bus_g2d_400: bus0 {
> > compatible = "samsung,exynos-bus";
> > clocks = <&cmu_top CLK_ACLK_G2D_400>;
> > clock-names = "bus";
> > operating-points-v2 = <&bus_g2d_400_opp_table>;
> > status = "disabled";
> > };
> >
> > bus_noc2: bus9 {
> > compatible = "samsung,exynos-bus";
> > clocks = <&cmu_mif CLK_ACLK_BUS2_400>;
> > clock-names = "bus";
> > operating-points-v2 = <&bus_noc2_opp_table>;
> > status = "disabled";
> > };
>
> And what is the relation between these two busses ?
I can't speak for the Exynos hardware. Maybe Chanwoo knows.
But a couple of common reasons to do this between devices are:
1. These were the combination of frequencies that were
validated/screen during the manufacturing process.
2. These are the sensible performance combinations between two devices
interacting with each other. So that when one runs fast the other
doesn't become the bottleneck.
3. Hardware bugs requiring some kind of frequency ratio between devices.
All of the cases above are some real world scenarios I've come across.
CPU and L2/L3 on ARM systems are a good example of (2) but the passive
governor doesn't work with CPUs yet. But I plan to work on that later
as that's not related to this patch series.
-Saravana
> > bus_g2d_400_opp_table: opp_table2 {
> > compatible = "operating-points-v2";
> > opp-shared;
> >
> > opp-400000000 {
> > opp-hz = /bits/ 64 <400000000>;
> > opp-microvolt = <1075000>;
> > + required-opps = <&noc2_400>;
> > };
> > opp-267000000 {
> > opp-hz = /bits/ 64 <267000000>;
> > opp-microvolt = <1000000>;
> > + required-opps = <&noc2_200>;
> > };
> > opp-200000000 {
> > opp-hz = /bits/ 64 <200000000>;
> > opp-microvolt = <975000>;
> > + required-opps = <&noc2_200>;
> > };
> > opp-160000000 {
> > opp-hz = /bits/ 64 <160000000>;
> > opp-microvolt = <962500>;
> > + required-opps = <&noc2_134>;
> > };
> > opp-134000000 {
> > opp-hz = /bits/ 64 <134000000>;
> > opp-microvolt = <950000>;
> > + required-opps = <&noc2_134>;
> > };
> > opp-100000000 {
> > opp-hz = /bits/ 64 <100000000>;
> > opp-microvolt = <937500>;
> > + required-opps = <&noc2_100>;
> > };
> > };
> >
> > bus_noc2_opp_table: opp_table6 {
> > compatible = "operating-points-v2";
> >
> > - opp-400000000 {
> > + noc2_400: opp-400000000 {
> > opp-hz = /bits/ 64 <400000000>;
> > };
> > - opp-200000000 {
> > + noc2_200: opp-200000000 {
> > opp-hz = /bits/ 64 <200000000>;
> > };
> > - opp-134000000 {
> > + noc2_134: opp-134000000 {
> > opp-hz = /bits/ 64 <134000000>;
> > };
> > - opp-100000000 {
> > + noc2_100: opp-100000000 {
> > opp-hz = /bits/ 64 <100000000>;
> > };
> > };
> >
> > Thanks,
> > Saravana
>
> --
> viresh
^ permalink raw reply
* [RFCv3 1/8] sched/core: Add manual jitter classification using sched_setattr syscall
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
Jitter tasks are short/bursty tasks,typically performing some housekeeping
and are less important in the overall scheme of things. In this patch we
provide a mechanism based on Patrick Bellasi's UCLAMP framework to classify
jitter tasks"
We define jitter tasks as those whose util.max in the UCLAMP framework is
the least (=0). This also provides benefit of giving the least frequency to
those jitter tasks, which is useful if all jitters are packed onto a
separate core."
UCLAMP already provides a way to set util.max for a task by using a syscall
interface. This patch uses the `sched_setattr` syscall to set
sched_util_max attribute of the task which is used to classify the task as
jitter.
Use Case with turbo_bench.c
===================
```
i=8;
./turbo_bench -t 30 -h $i -n $((2*i)) -j
```
This spawns 2*i total threads: of which i-CPU bound and i-jitter threads.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
include/linux/sched.h | 6 ++++++
kernel/sched/core.c | 9 +++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e2d80e6a187d..2bd9f75a3abb 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -696,6 +696,12 @@ struct task_struct {
struct uclamp_se uclamp_req[UCLAMP_CNT];
/* Effective clamp values used for a scheduling entity */
struct uclamp_se uclamp[UCLAMP_CNT];
+ /*
+ * Tag the task as jitter.
+ * 0 = regular. Follows regular CFS policy for task placement.
+ * 1 = Jitter tasks. Should be packed to reduce active core count.
+ */
+ unsigned int is_jitter;
#endif
#ifdef CONFIG_PREEMPT_NOTIFIERS
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ab0aa319fe60..19c7204d6351 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1189,6 +1189,15 @@ static void __setscheduler_uclamp(struct task_struct *p,
if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) {
uclamp_se_set(&p->uclamp_req[UCLAMP_MAX],
attr->sched_util_max, true);
+
+ /*
+ * Set task to jitter class if Max util is clamped to the least
+ * possible value
+ */
+ if (p->uclamp_req[UCLAMP_MAX].bucket_id == 0 && !p->is_jitter)
+ p->is_jitter = 1;
+ else if (p->is_jitter)
+ p->is_jitter = 0;
}
}
--
2.17.1
^ permalink raw reply related
* [RFCv3 4/8] sched/fair: Define core capacity to limit task packing
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
This patch defines a method name arch_scale_core_capacity which should
return the capacity of the core. This method will be used in the future
patches to determine if the spare capacity is left in the core to pack
jitter tasks.
For some architectures, core capacity does not increase much with the
number of threads( or CPUs) in the core. For such cases, architecture
specific calculations needs to be done to find core capacity.
This patch provides a default implementation for the scaling core capacity.
ToDo: As per Peter's comments, if we are getting rid of SMT capacity then
we need to find a workaround for limiting task packing. I'm working around
that trying to find a solution for the same but would like to get community
response first to have better view.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
kernel/sched/fair.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e8c5eb339e35..ff3f88d788d8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5929,6 +5929,25 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t
return cpu;
}
+#ifdef CONFIG_SCHED_SMT
+
+#ifndef arch_scale_core_capacity
+static inline unsigned long arch_scale_core_capacity(int first_thread,
+ unsigned long smt_cap)
+{
+ /* Default capacity of core is sum of cap of all the threads */
+ unsigned long ret = 0;
+ int sibling;
+
+ for_each_cpu(sibling, cpu_smt_mask(first_thread))
+ ret += cpu_rq(sibling)->cpu_capacity;
+
+ return ret;
+}
+#endif
+
+#endif
+
/*
* Try and locate an idle core/thread in the LLC cache domain.
*/
--
2.17.1
^ permalink raw reply related
* [RFCv3 3/8] sched/core: Update turbo_sched count only when required
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
Use the get/put methods to add/remove the use of TurboSched support, such
that the feature is turned on only if there is atleast one jitter task.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
kernel/sched/core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ef83725bd4b0..c7b628d0be2b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1214,10 +1214,13 @@ static void __setscheduler_uclamp(struct task_struct *p,
* Set task to jitter class if Max util is clamped to the least
* possible value
*/
- if (p->uclamp_req[UCLAMP_MAX].bucket_id == 0 && !p->is_jitter)
+ if (p->uclamp_req[UCLAMP_MAX].bucket_id == 0 && !p->is_jitter) {
p->is_jitter = 1;
- else if (p->is_jitter)
+ turbo_sched_get();
+ } else if (p->is_jitter) {
p->is_jitter = 0;
+ turbo_sched_put();
+ }
}
}
@@ -3215,6 +3218,9 @@ static struct rq *finish_task_switch(struct task_struct *prev)
mmdrop(mm);
}
if (unlikely(prev_state == TASK_DEAD)) {
+ if (unlikely(prev->is_jitter))
+ turbo_sched_put();
+
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
--
2.17.1
^ permalink raw reply related
* [RFCv3 5/8] powerpc: Define Core Capacity for POWER systems
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
This patch tunes arch_scale_core_capacity for powerpc arch by scaling
capacity w.r.t to the number of online SMT in the core such that for SMT-4,
core capacity is 1.5x the capacity of sibling thread.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
arch/powerpc/include/asm/topology.h | 4 ++++
arch/powerpc/kernel/smp.c | 33 +++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index f85e2b01c3df..1c777ee67180 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -132,6 +132,10 @@ static inline void shared_proc_topology_init(void) {}
#define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
#define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
#define topology_core_id(cpu) (cpu_to_core_id(cpu))
+#define arch_scale_core_capacity powerpc_scale_core_capacity
+
+unsigned long powerpc_scale_core_capacity(int first_smt,
+ unsigned long smt_cap);
int dlpar_cpu_readd(int cpu);
#endif
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ea6adbf6a221..149a3fbf8ed3 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1169,6 +1169,39 @@ static void remove_cpu_from_masks(int cpu)
}
#endif
+#ifdef CONFIG_SCHED_SMT
+/*
+ * Calculate capacity of a core based on the active threads in the core
+ * Scale the capacity of first SM-thread based on total number of
+ * active threads in the respective smt_mask.
+ *
+ * The scaling is done such that for
+ * SMT-4, core_capacity = 1.5x first_cpu_capacity
+ * and for SMT-8, core_capacity multiplication factor is 2x
+ *
+ * So, core_capacity multiplication factor = (1 + smt_mode*0.125)
+ *
+ * @first_cpu: First/any CPU id in the core
+ * @cap: Capacity of the first_cpu
+ */
+unsigned long powerpc_scale_core_capacity(int first_cpu,
+ unsigned long cap)
+{
+ struct cpumask select_idles;
+ struct cpumask *cpus = &select_idles;
+ int cpu, smt_mode = 0;
+
+ cpumask_and(cpus, cpu_smt_mask(first_cpu), cpu_online_mask);
+
+ /* Find SMT mode from active SM-threads */
+ for_each_cpu(cpu, cpus)
+ smt_mode++;
+
+ /* Scale core capacity based on smt mode */
+ return smt_mode == 1 ? cap : ((cap * smt_mode) >> 3) + cap;
+}
+#endif
+
static inline void add_cpu_to_smallcore_masks(int cpu)
{
struct cpumask *this_l1_cache_map = per_cpu(cpu_l1_cache_map, cpu);
--
2.17.1
^ permalink raw reply related
* [RFCv3 6/8] sched/fair: Tune task wake-up logic to pack jitter tasks
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
The algorithm finds the first non idle core in the system and tries to
place a task in the least utilized CPU in the chosen core. To maintain
cache hotness, work of finding non idle core starts from the prev_cpu,
which also reduces task ping-pong behaviour inside of the core.
This patch defines a new method named core_underutilized() which will
determine if the core utilization is less than 12.5% of its capacity.
Since core with low utilization should not be selected for packing, the
margin of under-utilization is kept at 12.5% of core capacity.
12.5% is an experimental number which identifies whether the core is
considered to be idle or not. For task packing, the algorithm should
select the best core where the task can be accommodated such that it does
not wake up an idle core. But the jitter tasks should not be placed on the
core which is about to go idle. If the core has aggregated utilization of
<12.5%, it may go idle soon and hence packing on such core should be
ignored. The experiment showed that keeping this threshold to 12.5% gives
better decision capability on not selecting the core which will idle out
soon.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
kernel/sched/fair.c | 116 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 114 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ff3f88d788d8..9d11631ce18c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5318,6 +5318,8 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
/* Working cpumask for: load_balance, load_balance_newidle. */
DEFINE_PER_CPU(cpumask_var_t, load_balance_mask);
DEFINE_PER_CPU(cpumask_var_t, select_idle_mask);
+/* A cpumask to find active cores in the system. */
+DEFINE_PER_CPU(cpumask_var_t, turbo_sched_mask);
#ifdef CONFIG_NO_HZ_COMMON
@@ -5929,8 +5931,22 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t
return cpu;
}
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_UCLAMP_TASK
+static inline bool is_task_jitter(struct task_struct *p)
+{
+ if (p->is_jitter == 1)
+ return true;
+ return false;
+}
+#else
+static inline bool is_task_jitter(struct task_struct *p)
+{
+ return false;
+}
+#endif
+
+#ifdef CONFIG_SCHED_SMT
#ifndef arch_scale_core_capacity
static inline unsigned long arch_scale_core_capacity(int first_thread,
unsigned long smt_cap)
@@ -5946,6 +5962,81 @@ static inline unsigned long arch_scale_core_capacity(int first_thread,
}
#endif
+/*
+ * Core is defined as under-utilized in case if the aggregated utilization of a
+ * all the CPUs in a core is less than 12.5%
+ */
+#define UNDERUTILIZED_THRESHOLD 3
+static inline bool core_underutilized(unsigned long core_util,
+ unsigned long core_capacity)
+{
+ return core_util < (core_capacity >> UNDERUTILIZED_THRESHOLD);
+}
+
+/*
+ * Try to find a non idle core in the system with spare capacity
+ * available for task packing, thereby keeping minimal cores active.
+ * Uses first fit algorithm to pack low util jitter tasks on active cores.
+ */
+static int select_non_idle_core(struct task_struct *p, int prev_cpu, int target)
+{
+ struct cpumask *cpus = this_cpu_cpumask_var_ptr(turbo_sched_mask);
+ int iter_cpu, sibling;
+
+ cpumask_and(cpus, cpu_online_mask, p->cpus_ptr);
+
+ for_each_cpu_wrap(iter_cpu, cpus, prev_cpu) {
+ unsigned long core_util = 0;
+ unsigned long core_cap = arch_scale_core_capacity(iter_cpu,
+ capacity_of(iter_cpu));
+ unsigned long est_util = 0, est_util_enqueued = 0;
+ unsigned long util_best_cpu = ULONG_MAX;
+ int best_cpu = iter_cpu;
+ struct cfs_rq *cfs_rq;
+
+ for_each_cpu(sibling, cpu_smt_mask(iter_cpu)) {
+ __cpumask_clear_cpu(sibling, cpus);
+ core_util += cpu_util(sibling);
+
+ /*
+ * Keep track of least utilized CPU in the core
+ */
+ if (cpu_util(sibling) < util_best_cpu) {
+ util_best_cpu = cpu_util(sibling);
+ best_cpu = sibling;
+ }
+ }
+
+ /*
+ * Find if the selected task will fit into this core or not by
+ * estimating the utilization of the core.
+ */
+ if (!core_underutilized(core_util, core_cap)) {
+ cfs_rq = &cpu_rq(best_cpu)->cfs;
+ est_util =
+ READ_ONCE(cfs_rq->avg.util_avg) + task_util(p);
+ est_util_enqueued =
+ READ_ONCE(cfs_rq->avg.util_est.enqueued);
+ est_util_enqueued += _task_util_est(p);
+ est_util = max(est_util, est_util_enqueued);
+ est_util = core_util - util_best_cpu + est_util;
+
+ if (est_util < core_cap) {
+ /*
+ * Try to bias towards prev_cpu to avoid task
+ * ping-pong behaviour inside the core.
+ */
+ if (cpumask_test_cpu(prev_cpu,
+ cpu_smt_mask(iter_cpu)))
+ return prev_cpu;
+
+ return best_cpu;
+ }
+ }
+ }
+
+ return select_idle_sibling(p, prev_cpu, target);
+}
#endif
/*
@@ -6402,6 +6493,23 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
return -1;
}
+#ifdef CONFIG_SCHED_SMT
+/*
+ * Select all tasks of type 1(jitter) for task packing
+ */
+static inline int turbosched_select_non_idle_core(struct task_struct *p,
+ int prev_cpu, int target)
+{
+ return select_non_idle_core(p, prev_cpu, target);
+}
+#else
+static inline int turbosched_select_non_idle_core(struct task_struct *p,
+ int prev_cpu, int target)
+{
+ return select_idle_sibling(p, prev_cpu, target);
+}
+#endif
+
/*
* select_task_rq_fair: Select target runqueue for the waking task in domains
* that have the 'sd_flag' flag set. In practice, this is SD_BALANCE_WAKE,
@@ -6467,7 +6575,11 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int sd_flag, int wake_f
} else if (sd_flag & SD_BALANCE_WAKE) { /* XXX always ? */
/* Fast path */
- new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);
+ if (is_turbosched_enabled() && unlikely(is_task_jitter(p)))
+ new_cpu = turbosched_select_non_idle_core(p, prev_cpu,
+ new_cpu);
+ else
+ new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);
if (want_affine)
current->recent_used_cpu = cpu;
--
2.17.1
^ permalink raw reply related
* [RFCv3 7/8] sched/fair: Bound non idle core search within LLC domain
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
This patch specifies the method which returns sched domain to limit the
search for a non idle core. By default, limit the search in LLC domain
which usually includes all the cores across the system.
The select_non_idle_core searches for the non idle cores across whole
system. But in the systems with multiple NUMA domains, the Turbo frequency
can be sustained within the NUMA domain without being affected from other
NUMA. For such case, arch_turbo_domain can be tuned to change domain for
non idle core search.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
kernel/sched/fair.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 9d11631ce18c..b049c9d73f1d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5973,6 +5973,13 @@ static inline bool core_underutilized(unsigned long core_util,
return core_util < (core_capacity >> UNDERUTILIZED_THRESHOLD);
}
+#ifndef arch_turbo_domain
+static __always_inline struct cpumask *arch_turbo_domain(int cpu)
+{
+ return sched_domain_span(rcu_dereference(per_cpu(sd_llc, cpu)));
+}
+#endif
+
/*
* Try to find a non idle core in the system with spare capacity
* available for task packing, thereby keeping minimal cores active.
@@ -5983,7 +5990,8 @@ static int select_non_idle_core(struct task_struct *p, int prev_cpu, int target)
struct cpumask *cpus = this_cpu_cpumask_var_ptr(turbo_sched_mask);
int iter_cpu, sibling;
- cpumask_and(cpus, cpu_online_mask, p->cpus_ptr);
+ cpumask_and(cpus, cpu_online_mask, arch_turbo_domain(prev_cpu));
+ cpumask_and(cpus, cpus, p->cpus_ptr);
for_each_cpu_wrap(iter_cpu, cpus, prev_cpu) {
unsigned long core_util = 0;
--
2.17.1
^ permalink raw reply related
* [RFCv3 8/8] powerpc: Set turbo domain to NUMA node for task packing
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
This patch provides an powerpc architecture specific implementation for
defining the turbo domain to make searching of the core to be bound within
the NUMA. This provides a way to decrease the searching time for specific
architectures.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
arch/powerpc/include/asm/topology.h | 3 +++
arch/powerpc/kernel/smp.c | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 1c777ee67180..410b94c9e1a2 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -133,10 +133,13 @@ static inline void shared_proc_topology_init(void) {}
#define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
#define topology_core_id(cpu) (cpu_to_core_id(cpu))
#define arch_scale_core_capacity powerpc_scale_core_capacity
+#define arch_turbo_domain powerpc_turbo_domain
unsigned long powerpc_scale_core_capacity(int first_smt,
unsigned long smt_cap);
+struct cpumask *powerpc_turbo_domain(int cpu);
+
int dlpar_cpu_readd(int cpu);
#endif
#endif
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 149a3fbf8ed3..856f7233190e 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1200,6 +1200,11 @@ unsigned long powerpc_scale_core_capacity(int first_cpu,
/* Scale core capacity based on smt mode */
return smt_mode == 1 ? cap : ((cap * smt_mode) >> 3) + cap;
}
+
+inline struct cpumask *powerpc_turbo_domain(int cpu)
+{
+ return cpumask_of_node(cpu_to_node(cpu));
+}
#endif
static inline void add_cpu_to_smallcore_masks(int cpu)
--
2.17.1
^ permalink raw reply related
* [RFCv3 2/8] sched: Introduce switch to enable TurboSched mode
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
In-Reply-To: <20190625043726.21490-1-parth@linux.ibm.com>
This patch creates a static key which allows to enable or disable
TurboSched feature at runtime.
This key is added in order to enable the TurboSched feature. The static key
helps in optimizing the scheduler fast-path when the TurboSched feature is
disabled.
The patch also provides get/put methods to keep track of the tasks using
the TurboSched feature. This allows to enable the feature on setting first
task classified as jitter, similarly disable the feature on unsetting of
such last task.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
kernel/sched/core.c | 20 ++++++++++++++++++++
kernel/sched/sched.h | 9 +++++++++
2 files changed, 29 insertions(+)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 19c7204d6351..ef83725bd4b0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -61,6 +61,26 @@ __read_mostly int scheduler_running;
*/
int sysctl_sched_rt_runtime = 950000;
+DEFINE_STATIC_KEY_FALSE(__turbo_sched_enabled);
+static DEFINE_MUTEX(turbo_sched_lock);
+static int turbo_sched_count;
+
+void turbo_sched_get(void)
+{
+ mutex_lock(&turbo_sched_lock);
+ if (!turbo_sched_count++)
+ static_branch_enable(&__turbo_sched_enabled);
+ mutex_unlock(&turbo_sched_lock);
+}
+
+void turbo_sched_put(void)
+{
+ mutex_lock(&turbo_sched_lock);
+ if (!--turbo_sched_count)
+ static_branch_disable(&__turbo_sched_enabled);
+ mutex_unlock(&turbo_sched_lock);
+}
+
/*
* __task_rq_lock - lock the rq @p resides on.
*/
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index e5599b33ffb8..1f239a960a6d 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2426,3 +2426,12 @@ static inline bool sched_energy_enabled(void)
static inline bool sched_energy_enabled(void) { return false; }
#endif /* CONFIG_ENERGY_MODEL && CONFIG_CPU_FREQ_GOV_SCHEDUTIL */
+
+void turbo_sched_get(void);
+void turbo_sched_put(void);
+DECLARE_STATIC_KEY_FALSE(__turbo_sched_enabled);
+
+static inline bool is_turbosched_enabled(void)
+{
+ return static_branch_unlikely(&__turbo_sched_enabled);
+}
--
2.17.1
^ permalink raw reply related
* [RFCv3 0/8] TurboSched: A scheduler for sustaining Turbo Frequencies for longer durations
From: Parth Shah @ 2019-06-25 4:37 UTC (permalink / raw)
To: linux-kernel, linux-pm; +Cc: mingo, peterz, dietmar.eggemann, patrick.bellasi
This is the 3rd version of the patchset to sustain Turbo frequencies for
longer durations.
The previous versions can be found here:
v2: https://lkml.org/lkml/2019/5/15/1258
v1: https://lwn.net/Articles/783959/
The changes in this versions are:
v[2] -> v[3]:
- Added a new attribute in task_struct to allow per task jitter
classification so that scheduler can use this as request to change wakeup
path for task packing
- Use syscall for jitter classification, removed cgroup based task
classification
- Use mutex over spinlock to get rid of task sleeping problem
- Changed _Bool->int everywhere
- Split few patches to have arch specific code separate from core scheduler
code
ToDo:
- Recompute core capacity only during CPU-Hotplug operation
- Remove smt capacity
v[1] -> v[2]:
- No CPU bound tasks' classification, only jitter tasks are classified from
the cpu cgroup controller
- Use of Spinlock rather than mutex to count number of jitters in the
system classified from cgroup
- Architecture specific implementation of Core capacity multiplication
factor changes dynamically based on the number of active threads in the
core
- Selection of non idle core in the system is bounded by DIE domain
- Use of UCLAMP mechanism to classify jitter tasks
- Removed "highutil_cpu_mask", and rather uses sd for DIE domain to find
better fit
Abstract
========
The modern servers allows multiple cores to run at range of frequencies
higher than rated range of frequencies. But the power budget of the system
inhibits sustaining these higher frequencies for longer durations.
However when certain cores are put to idle states, the power can be
effectively channelled to other busy cores, allowing them to sustain the
higher frequency.
One way to achieve this is to pack tasks onto fewer cores keeping others
idle, but it may lead to performance penalty for such tasks and sustaining
higher frequencies proves to be of no benefit. But if one can identify
unimportant low utilization tasks which can be packed on the already active
cores then waking up of new cores can be avoided. Such tasks are short
and/or bursty "jitter tasks" and waking up new core is expensive for such
case.
Current CFS algorithm in kernel scheduler is performance oriented and hence
tries to assign any idle CPU first for the waking up of new tasks. This
policy is perfect for major categories of the workload, but for jitter
tasks, one can save energy by packing them onto the active cores and allow
those cores to run at higher frequencies.
These patch-set tunes the task wake up logic in scheduler to pack
exclusively classified jitter tasks onto busy cores. The work involves the
jitter tasks classifications by using syscall based mechanisms.
In brief, if we can pack jitter tasks on busy cores then we can save power
by keeping other cores idle and allow busier cores to run at turbo
frequencies, patch-set tries to meet this solution in simplest manner.
Though, there are some challenges in implementing it(like smt_capacity,
un-needed arch hooks, etc) and I'm trying to work around that, it would be
great to have a discussion around this patches.
Implementation
==============
These patches uses UCLAMP mechanism[2] used to clamp utilization from the
userspace, which can be used to classify the jitter tasks. The task wakeup
logic uses this information to pack such tasks onto cores which are already
running busy with CPU intensive tasks. The task packing is done at
`select_task_rq_fair` only so that in case of wrong decision load balancer
may pull the classified jitter tasks for maximizing performance.
Any tasks clamped with cpu.util.max=1 (with sched_setattr syscall) are
classified as jitter tasks. We define a core to be non-idle if it is over
12.5% utilized of its capacity; the jitters are packed over these cores
using First-fit approach.
To demonstrate/benchmark, one can use a synthetic workload generator
`turbo_bench.c`[1] available at
https://github.com/parthsl/tools/blob/master/benchmarks/turbo_bench.c
Following snippet demonstrates the use of TurboSched feature:
```
i=8; ./turbo_bench -t 30 -h $i -n $((i*2)) -j
```
Current implementation uses only jitter classified tasks to be packed on
the first busy cores, but can be further optimized by getting userspace
input of important tasks and keeping track of such tasks. This leads to
optimized searching of non idle cores and also more accurate as userspace
hints are safer than auto classified busy cores/tasks.
Result
======
The patch-set proves to be useful for the system and the workload where
frequency boost is found to be useful than packing tasks into cores. IBM
POWER 9 system shows the benefit for a workload can be up to 13%.
Performance benefit of TurboSched w.r.t. CFS
+--+--+--+--+--+--+-+--+--+--+--+--+--+--+--+--+--+--+-+--+--+--+--+--+
| + + + + + + + + + + + + + + + + + + + + + + + |
15 +-+ Performance benefit in % +-+
| ** |
| ** ** |
10 +-+ ** ** ** +-+
| ** ** ** |
| ** ** ** |
5 +-+ ** ** ** ** ** ** +-+
| ** ** ** ** ** ** ** ** |
| ** ** ** ** ** ** ** ** ** ** |
| * ** ** ** ** ** ** ** ** ** ** ** * |
0 +-+** ** ** ** ** * ** ** ** ** ** ** ** ** ** ** ** * ** ** ** ** **-+
| ** ** ** ** |
| ** |
-5 +-+ +-+
| + + + + + + + + + + + + + + + + + + + + + + + |
+--+--+--+--+--+--+-+--+--+--+--+--+--+--+--+--+--+--+-+--+--+--+--+--+
2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1920 21 22 23 24
No. of workload threads
Frequency benefit of TurboSched w.r.t. CFS
+--+--+--+--+--+--+-+--+--+--+--+--+--+--+--+--+--+--+-+--+--+--+--+--+
| + + + + + + + + + + + + + + + + + + + + + + + |
15 +-+ Frequency benefit in % +-+
| ** |
| ** |
10 +-+ ** ** +-+
| ** ** ** |
| ** ** * ** ** ** |
5 +-+ ** ** ** * ** ** ** ** +-+
| ** ** ** ** * ** ** ** ** ** |
| ** ** ** ** ** * ** ** ** ** ** ** |
| ** ** ** ** ** * ** ** ** ** ** ** ** ** ** |
0 +-+** ** ** ** ** * ** ** ** ** ** ** ** ** ** ** ** * ** ** ** ** **-+
| |
| |
-5 +-+ +-+
| + + + + + + + + + + + + + + + + + + + + + + + |
+--+--+--+--+--+--+-+--+--+--+--+--+--+--+--+--+--+--+-+--+--+--+--+--+
2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1920 21 22 23 24
No. of workload threads
These numbers are w.r.t. `turbo_bench.c` multi-threaded test benchmark
which can create two kinds of tasks: CPU bound (High Utilization) and
Jitters (Low Utilization). N in X-axis represents N-CPU bound and N-Jitter
tasks spawned.
Series organization
==============
- Patches [01-03]: Jitter tasks classification using syscall
- Patches [04-05]: Defines Core Capacity to limit task packing
- Patches [06-08]: Tune CFS task wakeup logic to pack tasks onto busy
cores
Series can be applied on top of Patrick Bellasi's UCLAMP RFCv9[2]
patches with branch on tip/sched/core and UCLAMP_TASK_GROUP config
options enabled.
References
==========
[1] "Turbo_bench: Synthetic workload generator"
https://github.com/parthsl/tools/blob/master/benchmarks/turbo_bench.c
[2] "Patrick Bellasi, Add utilization clamping support"
https://lkml.org/lkml/2019/5/15/212
Parth Shah (8):
sched/core: Add manual jitter classification using sched_setattr
syscall
sched: Introduce switch to enable TurboSched mode
sched/core: Update turbo_sched count only when required
sched/fair: Define core capacity to limit task packing
powerpc: Define Core Capacity for POWER systems
sched/fair: Tune task wake-up logic to pack jitter tasks
sched/fair: Bound non idle core search within LLC domain
powerpc: Set turbo domain to NUMA node for task packing
arch/powerpc/include/asm/topology.h | 7 ++
arch/powerpc/kernel/smp.c | 38 ++++++++
include/linux/sched.h | 6 ++
kernel/sched/core.c | 35 +++++++
kernel/sched/fair.c | 141 +++++++++++++++++++++++++++-
kernel/sched/sched.h | 9 ++
6 files changed, 235 insertions(+), 1 deletion(-)
--
2.17.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox