[PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[gao.yunxiao6]

From: Jeson Gao <jeson.gao@unisoc.com>

On some platforms, due to the high power consumption, thermal frequency
reduction policy cannot control the desired temperature, platform have to
use the hotplug mechanism to mitigate temperature rising,so adding the
platform callback to support this function.

platform will hotplug out CPU when the normalised power is lower than
the power corresponding to the minimum frequency limit that is set by
platform.

Signed-off-by: Jeson Gao <jeson.gao@unisoc.com>
---
 drivers/thermal/cpufreq_cooling.c | 52 +++++++++++++++++++++++++++++++++++++++
 include/linux/cpu_cooling.h       | 30 ++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index e297e13..16cbf58 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -64,6 +64,7 @@ struct time_in_idle {
  * @node: list_head to link all cpufreq_cooling_device together.
  * @idle_time: idle time stats
  * @qos_req: PM QoS contraint to apply
+ * @plat_ops: point to platform callback function.
  *
  * This structure is required for keeping information of each registered
  * cpufreq_cooling_device.
@@ -78,6 +79,7 @@ struct cpufreq_cooling_device {
 	struct list_head node;
 	struct time_in_idle *idle_time;
 	struct freq_qos_request qos_req;
+	struct cpufreq_cooling_plat_ops *plat_ops;
 };
 
 static DEFINE_IDA(cpufreq_ida);
@@ -313,12 +315,24 @@ static int cpufreq_power2state(struct thermal_cooling_device *cdev,
 	u32 last_load, normalised_power;
 	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
 	struct cpufreq_policy *policy = cpufreq_cdev->policy;
+	struct cpufreq_cooling_plat_ops *plat_ops = cpufreq_cdev->plat_ops;
 
 	last_load = cpufreq_cdev->last_load ?: 1;
 	normalised_power = (power * 100) / last_load;
 	target_freq = cpu_power_to_freq(cpufreq_cdev, normalised_power);
 
 	*state = get_level(cpufreq_cdev, target_freq);
+	if (*state == cpufreq_cdev->max_level &&
+			plat_ops && plat_ops->cpufreq_plat_min_freq_limit) {
+		plat_ops->cpufreq_plat_min_freq_limit(policy, &target_freq);
+		*state = get_level(cpufreq_cdev, target_freq);
+	}
+
+	if (plat_ops && plat_ops->cpufreq_plat_cpu_ctrl)
+		plat_ops->cpufreq_plat_cpu_ctrl(policy,
+				last_load, normalised_power,
+				cpu_freq_to_power(cpufreq_cdev, target_freq));
+
 	trace_thermal_power_cpu_limit(policy->related_cpus, target_freq, *state,
 				      power);
 	return 0;
@@ -684,3 +698,41 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 	kfree(cpufreq_cdev);
 }
 EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
+
+/**
+ * cpufreq_cooling_plat_ops_register - register platform callback function.
+ * @cdev: thermal cooling device pointer.
+ * @plat_ops: platform callback function pointer.
+ */
+int  cpufreq_cooling_plat_ops_register(struct thermal_cooling_device *cdev,
+			struct cpufreq_cooling_plat_ops *plat_ops)
+{
+	struct cpufreq_cooling_device *cpufreq_cdev;
+
+	if (!cdev && !cdev->devdata && !plat_ops)
+		return -EINVAL;
+
+	cpufreq_cdev = cdev->devdata;
+	cpufreq_cdev->plat_ops = plat_ops;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpufreq_cooling_plat_ops_register);
+
+/**
+ * cpufreq_cooling_plat_ops_unregister - unregister platform callback function.
+ * @cdev: thermal cooling device pointer.
+ */
+int  cpufreq_cooling_plat_ops_unregister(struct thermal_cooling_device *cdev)
+{
+	struct cpufreq_cooling_device *cpufreq_cdev;
+
+	if (!cdev && !cdev->devdata)
+		return -EINVAL;
+
+	cpufreq_cdev = cdev->devdata;
+	cpufreq_cdev->plat_ops = NULL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cpufreq_cooling_plat_ops_unregister);
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index 65501d8..3934918 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -19,6 +19,23 @@
 
 struct cpufreq_policy;
 
+/**
+ * struct cpufreq_cooling_plat_ops - platfom cpu cooling policy ops
+ *
+ * @cpufreq_plat_cpu_ctrl: this function provides a further core control
+ * policy when the current policies cannot cool down to an expected
+ * temperature value.
+ *
+ * @cpufreq_plat_min_freq_limit: set cpu frequency limit, cooling devices
+ * are not allowed to adjust cpu frequency to out of that limit.
+ */
+struct cpufreq_cooling_plat_ops {
+	int (*cpufreq_plat_cpu_ctrl)(struct cpufreq_policy *policy,
+				u32 load, u32 normalised_power, u32 freq_power);
+	void (*cpufreq_plat_min_freq_limit)(struct cpufreq_policy *policy,
+						u32 *target_freq);
+};
+
 #ifdef CONFIG_CPU_FREQ_THERMAL
 /**
  * cpufreq_cooling_register - function to create cpufreq cooling device.
@@ -40,6 +57,19 @@ struct thermal_cooling_device *
 struct thermal_cooling_device *
 of_cpufreq_cooling_register(struct cpufreq_policy *policy);
 
+/**
+ * cpufreq_cooling_plat_ops_register - register platform callback function.
+ * @cdev: thermal cooling device pointer.
+ * @plat_ops: platform callback function pointer.
+ */
+int cpufreq_cooling_plat_ops_register(struct thermal_cooling_device *cdev,
+			struct cpufreq_cooling_plat_ops *plat_ops);
+/**
+ * cpufreq_cooling_plat_ops_unregister - unregister platform callback function.
+ * @cdev: thermal cooling device pointer.
+ */
+int  cpufreq_cooling_plat_ops_unregister(struct thermal_cooling_device *cdev);
+
 #else /* !CONFIG_CPU_FREQ_THERMAL */
 static inline struct thermal_cooling_device *
 cpufreq_cooling_register(struct cpufreq_policy *policy)
-- 
1.9.1

[PATCH 2/2] thermal/drivers/sprd_cpu_cooling: Add platform mitigation thermal driver

[gao.yunxiao6]

From: Jeson Gao <jeson.gao@unisoc.com>

This driver provides a further core ctrl policy to reduce temperature for
unisoc platform.

CPU unplug or isolation usually would be used for core ctrl policy.

This dirver is showing an example of using core ctrl policy.

Signed-off-by: Jeson Gao <jeson.gao@unisoc.com>
---
 drivers/thermal/Kconfig            |   8 +
 drivers/thermal/Makefile           |   1 +
 drivers/thermal/sprd_cpu_cooling.c | 340 +++++++++++++++++++++++++++++++++++++
 3 files changed, 349 insertions(+)
 create mode 100644 drivers/thermal/sprd_cpu_cooling.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 91af271..41a57b0 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -490,4 +490,12 @@ config SPRD_THERMAL
 	help
 	  Support for the Spreadtrum thermal sensor driver in the Linux thermal
 	  framework.
+
+config SPRD_CPU_COOLING
+	tristate "sprd cpu cooling support"
+	depends on CPU_FREQ_THERMAL
+	help
+	  This implements the sprd cpu cooling mechanism to mitigate temperature
+	  rising.
+
 endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 8c8ed7b..4031842 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_ZX2967_THERMAL)	+= zx2967_thermal.o
 obj-$(CONFIG_UNIPHIER_THERMAL)	+= uniphier_thermal.o
 obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
 obj-$(CONFIG_SPRD_THERMAL)	+= sprd_thermal.o
+obj-$(CONFIG_SPRD_CPU_COOLING)	+= sprd_cpu_cooling.o
diff --git a/drivers/thermal/sprd_cpu_cooling.c b/drivers/thermal/sprd_cpu_cooling.c
new file mode 100644
index 0000000..2af2714
--- /dev/null
+++ b/drivers/thermal/sprd_cpu_cooling.c
@@ -0,0 +1,340 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Spreadtrum Communications Inc.
+
+#include <linux/thermal.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/cpu.h>
+#include <linux/sched.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/cpufreq.h>
+
+#include <linux/cpu_cooling.h>
+
+struct sprd_cooling_cluster {
+	int id;
+	int cpu;
+	int cpus;
+	u32 min_freq;
+	struct cpufreq_policy policy;
+};
+
+struct sprd_cooling_data {
+	int nr_clusters;
+	struct sprd_cooling_cluster *cluster;
+};
+
+static void sprd_update_target_cpus(struct cpufreq_policy *policy,
+				    u32 target_cpus)
+{
+	int ret, cpu, first;
+	u32 curr_online_cpus, ncpus;
+
+	ncpus = cpumask_weight(policy->related_cpus);
+	curr_online_cpus = cpumask_weight(policy->cpus);
+	first = cpumask_first(policy->related_cpus);
+
+	if (target_cpus > curr_online_cpus) {
+		cpu = first;
+		for_each_cpu(cpu, policy->related_cpus) {
+			if (curr_online_cpus == target_cpus)
+				break;
+			if ((target_cpus > curr_online_cpus) &&
+			    !cpu_online(cpu)) {
+				ret = add_cpu(cpu);
+				if (!ret && cpu_online(cpu))
+					curr_online_cpus++;
+			}
+		}
+	} else if (target_cpus < curr_online_cpus) {
+		for (cpu = (first + ncpus - 1); cpu >= first; cpu--) {
+			if (curr_online_cpus == target_cpus)
+				break;
+			if ((target_cpus < curr_online_cpus) &&
+			    cpu_online(cpu)) {
+				ret = remove_cpu(cpu);
+				if (!ret && !cpu_online(cpu))
+					curr_online_cpus--;
+			}
+		}
+	}
+}
+
+static void sprd_estimate_down_cpus(struct cpufreq_policy *policy,
+				    u32 load, u32 normalised_power,
+				    u32 freq_power)
+{
+	int i = 0;
+	u32 target_cpus, online_cpus;
+	u32 avg_load, estimate_power =  0;
+
+	if (normalised_power) {
+		online_cpus = cpumask_weight(policy->cpus);
+		if (!online_cpus)
+			return;
+
+		avg_load = (load / online_cpus) ?: 1;
+		do {
+			estimate_power += (freq_power * avg_load) / 100;
+			if (estimate_power > normalised_power)
+				break;
+			i++;
+		} while (i < online_cpus);
+
+		target_cpus = max(i, 0);
+	} else
+		target_cpus = 0;
+
+	sprd_update_target_cpus(policy, target_cpus);
+}
+
+static void sprd_estimate_up_cpus(struct cpufreq_policy *policy,
+				  u32 load, u32 normalised_power,
+				  u32 freq_power)
+{
+	int i = 0, ncpus;
+	u32 target_cpus;
+	u32 avg_load, estimate_power =  0;
+
+	ncpus = cpumask_weight(policy->related_cpus);
+	avg_load = (load / ncpus) ?: 1;
+
+	do {
+		estimate_power += (freq_power * avg_load) / 100;
+		if (estimate_power > normalised_power)
+			break;
+		i++;
+	} while (i < ncpus);
+
+	target_cpus = min(i, ncpus);
+	sprd_update_target_cpus(policy, target_cpus);
+}
+
+static void sprd_keep_cpus(struct cpufreq_policy *policy)
+{
+	u32 online_cpus;
+
+	online_cpus = cpumask_weight(policy->cpus);
+	sprd_update_target_cpus(policy, online_cpus);
+}
+
+static int sprd_cpufreq_cpu_ctrl(struct cpufreq_policy *policy,
+				 u32 load, u32 normalised_power,
+				 u32 freq_power)
+{
+	unsigned int ncpus, online_cpus;
+
+	ncpus = cpumask_weight(policy->related_cpus);
+	online_cpus = cpumask_weight(policy->cpus);
+
+	if (normalised_power <  freq_power)
+		sprd_estimate_down_cpus(policy, load,
+					normalised_power, freq_power);
+	else if (online_cpus < ncpus)
+		sprd_estimate_up_cpus(policy, load,
+				      normalised_power, freq_power);
+	else
+		sprd_keep_cpus(policy);
+
+	return 0;
+}
+
+static void sprd_cpufreq_min_freq_limit(struct cpufreq_policy *policy,
+					u32 *target_freq)
+{
+	struct sprd_cooling_cluster *cluster =
+		container_of(policy, struct sprd_cooling_cluster, policy);
+
+	if (*target_freq < cluster->min_freq)
+		*target_freq = cluster->min_freq;
+}
+
+struct cpufreq_cooling_plat_ops plat_ops = {
+	.cpufreq_plat_cpu_ctrl = sprd_cpufreq_cpu_ctrl,
+	.cpufreq_plat_min_freq_limit = sprd_cpufreq_min_freq_limit,
+};
+
+static int sprd_get_cluster_counts(void)
+{
+	int cpu = 0, core_num = -1;
+	int cluster_num = 0;
+
+	do {
+		core_num = cpumask_weight(topology_core_cpumask(cpu));
+		if (core_num > 0) {
+			cpu = cpu + core_num;
+			cluster_num++;
+		} else
+			break;
+	} while (cpu > 0);
+
+	return cluster_num;
+}
+
+static int sprd_cpu_cooling_ops_register(struct platform_device *pdev)
+{
+	int id, ret = 0;
+	struct thermal_cooling_device *cdev;
+	struct sprd_cooling_cluster *cluster;
+	struct sprd_cooling_data *data = platform_get_drvdata(pdev);
+
+	for (id = 0; id < data->nr_clusters; id++) {
+		cluster = &data->cluster[id];
+		cdev = cluster->policy.cdev;
+		if (cdev) {
+			ret = cpufreq_cooling_plat_ops_register(cdev,
+								&plat_ops);
+			if (ret < 0) {
+				dev_err(&pdev->dev,
+					"CPU%d: failed to register platform function\n",
+					cluster->cpu);
+				break;
+			}
+		} else {
+			ret = -ENODEV;
+			dev_err(&pdev->dev,
+				"CPU%d: failed to get thermal device\n",
+				cluster->cpu);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int sprd_cpu_cooling_ops_unregister(struct platform_device *pdev)
+{
+	int id, ret = 0;
+	struct thermal_cooling_device *cdev;
+	struct sprd_cooling_cluster *cluster;
+	struct sprd_cooling_data *data = platform_get_drvdata(pdev);
+
+	for (id = 0; id < data->nr_clusters; id++) {
+		cluster = &data->cluster[id];
+		cdev = cluster->policy.cdev;
+		if (cdev) {
+			ret = cpufreq_cooling_plat_ops_unregister(cdev);
+			if (ret < 0) {
+				dev_err(&pdev->dev,
+					"cpu%d: failed to unregister platform function\n",
+					cluster->cpu);
+				break;
+			}
+		} else {
+			dev_err(&pdev->dev,
+				"cpu%d: failed to get thermal device\n",
+				cluster->cpu);
+			ret = -ENODEV;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int sprd_init_cooling_cluster(struct platform_device *pdev)
+{
+	int cpu = 0, id, core_num, ret = 0;
+	struct sprd_cooling_cluster *cluster;
+	struct sprd_cooling_data *data = platform_get_drvdata(pdev);
+
+	do {
+		core_num = cpumask_weight(topology_core_cpumask(cpu));
+		id = topology_physical_package_id((cpu));
+		if (core_num > 0 && id >= 0) {
+			cluster = &data->cluster[id];
+			cluster->id = id;
+			cluster->cpu = cpu;
+			cluster->cpus = core_num;
+			ret = cpufreq_get_policy(&(cluster->policy), cpu);
+			if (ret < 0) {
+				dev_err(&pdev->dev,
+					"CPU%d failed to get policy\n",
+					cpu);
+				break;
+			}
+
+			cpu = cpu + core_num;
+		} else
+			break;
+
+	} while (cpu > 0);
+
+	return ret;
+}
+
+static int sprd_cpu_cooling_probe(struct platform_device *pdev)
+{
+	int ret = -1;
+	int counts = 0;
+	struct sprd_cooling_data *data;
+	struct device *dev = &pdev->dev;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	counts = sprd_get_cluster_counts();
+	data->nr_clusters = counts;
+	data->cluster = devm_kzalloc(dev,
+				     sizeof(*data->cluster) * data->nr_clusters,
+				     GFP_KERNEL);
+	if (!data->cluster)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, data);
+
+	ret = sprd_init_cooling_cluster(pdev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to init cooling cluster\n");
+		return ret;
+	}
+
+	ret = sprd_cpu_cooling_ops_register(pdev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register cooling callback function\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int sprd_cpu_cooling_remove(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = sprd_cpu_cooling_ops_unregister(pdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to unregister cooling callback function\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver sprd_cpu_cooling_driver = {
+	.probe = sprd_cpu_cooling_probe,
+	.remove = sprd_cpu_cooling_remove,
+	.driver = {
+		   .owner = THIS_MODULE,
+		   .name = "sprd_cpu_cooling",
+		   },
+};
+static int __init sprd_cpu_cooling_init(void)
+{
+	return platform_driver_register(&sprd_cpu_cooling_driver);
+}
+
+static void __exit sprd_cpu_cooling_exit(void)
+{
+	platform_driver_unregister(&sprd_cpu_cooling_driver);
+}
+
+module_init(sprd_cpu_cooling_init)
+module_exit(sprd_cpu_cooling_exit);
+
+MODULE_DESCRIPTION("sprd cpu cooling");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[Viresh Kumar]

On 20-04-20, 14:48, gao.yunxiao6@gmail.com wrote:
>  static DEFINE_IDA(cpufreq_ida);
> @@ -313,12 +315,24 @@ static int cpufreq_power2state(struct thermal_cooling_device *cdev,
>  	u32 last_load, normalised_power;
>  	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
>  	struct cpufreq_policy *policy = cpufreq_cdev->policy;
> +	struct cpufreq_cooling_plat_ops *plat_ops = cpufreq_cdev->plat_ops;
>  
>  	last_load = cpufreq_cdev->last_load ?: 1;
>  	normalised_power = (power * 100) / last_load;
>  	target_freq = cpu_power_to_freq(cpufreq_cdev, normalised_power);
>  
>  	*state = get_level(cpufreq_cdev, target_freq);
> +	if (*state == cpufreq_cdev->max_level &&
> +			plat_ops && plat_ops->cpufreq_plat_min_freq_limit) {
> +		plat_ops->cpufreq_plat_min_freq_limit(policy, &target_freq);
> +		*state = get_level(cpufreq_cdev, target_freq);
> +	}
> +
> +	if (plat_ops && plat_ops->cpufreq_plat_cpu_ctrl)
> +		plat_ops->cpufreq_plat_cpu_ctrl(policy,
> +				last_load, normalised_power,
> +				cpu_freq_to_power(cpufreq_cdev, target_freq));
> +
>  	trace_thermal_power_cpu_limit(policy->related_cpus, target_freq, *state,
>  				      power);
>  	return 0;
> @@ -684,3 +698,41 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>  	kfree(cpufreq_cdev);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);

Instead of adding such callbacks to constraint the min freq of CPUs,
you can directly use frequency constraints used by QoS framework to
put such limit directly on cpufreq. Look at freq_qos_add_request().

-- 
viresh

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[gao yunxiao]

viresh

Thank you very much for your advice.

In here, only check whether the frequency point given by cpu_cooling
module is the minimum frequency point.

On 22/04/2020, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 20-04-20, 14:48, gao.yunxiao6@gmail.com wrote:
>>  static DEFINE_IDA(cpufreq_ida);
>> @@ -313,12 +315,24 @@ static int cpufreq_power2state(struct
>> thermal_cooling_device *cdev,
>>  	u32 last_load, normalised_power;
>>  	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
>>  	struct cpufreq_policy *policy = cpufreq_cdev->policy;
>> +	struct cpufreq_cooling_plat_ops *plat_ops = cpufreq_cdev->plat_ops;
>>
>>  	last_load = cpufreq_cdev->last_load ?: 1;
>>  	normalised_power = (power * 100) / last_load;
>>  	target_freq = cpu_power_to_freq(cpufreq_cdev, normalised_power);
>>
>>  	*state = get_level(cpufreq_cdev, target_freq);
>> +	if (*state == cpufreq_cdev->max_level &&
>> +			plat_ops && plat_ops->cpufreq_plat_min_freq_limit) {
>> +		plat_ops->cpufreq_plat_min_freq_limit(policy, &target_freq);
>> +		*state = get_level(cpufreq_cdev, target_freq);
>> +	}
>> +
>> +	if (plat_ops && plat_ops->cpufreq_plat_cpu_ctrl)
>> +		plat_ops->cpufreq_plat_cpu_ctrl(policy,
>> +				last_load, normalised_power,
>> +				cpu_freq_to_power(cpufreq_cdev, target_freq));
>> +
>>  	trace_thermal_power_cpu_limit(policy->related_cpus, target_freq,
>> *state,
>>  				      power);
>>  	return 0;
>> @@ -684,3 +698,41 @@ void cpufreq_cooling_unregister(struct
>> thermal_cooling_device *cdev)
>>  	kfree(cpufreq_cdev);
>>  }
>>  EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
>
> Instead of adding such callbacks to constraint the min freq of CPUs,
> you can directly use frequency constraints used by QoS framework to
> put such limit directly on cpufreq. Look at freq_qos_add_request().
>
> --
> viresh
>

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[Viresh Kumar]

On 22-04-20, 17:53, gao yunxiao wrote:
> viresh
> 
> Thank you very much for your advice.
> 
> In here, only check whether the frequency point given by cpu_cooling
> module is the minimum frequency point.

I am not sure I understood what you are doing here. Please elaborate a
bit.

-- 
viresh

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[gao yunxiao]

On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
> viresh
>
> On UNISOC platform, CPU's temperature can not be controlled when
> cpufreq has been limited to the lowest frequency, we have to hotplug
> out CPUS to mitigate temperature rising.
>
> adding platform callback to have a chance to check whether the
> normalised power at power2state() is lower than the power
> corresponding to the lowest frequency. provide an example in another
> patch
> +static int sprd_cpufreq_cpu_ctrl(struct cpufreq_policy *policy,
> +                                u32 load, u32 normalised_power,
> +                                u32 freq_power)
> +{
> +       unsigned int ncpus, online_cpus;
> +
> +       ncpus = cpumask_weight(policy->related_cpus);
> +       online_cpus = cpumask_weight(policy->cpus);
> +
> +       if (normalised_power <  freq_power)
> +               sprd_estimate_down_cpus(policy, load,
> +                                       normalised_power, freq_power);
> +       else if (online_cpus < ncpus)
> +               sprd_estimate_up_cpus(policy, load,
> +                                     normalised_power, freq_power);
> +       else
> +               sprd_keep_cpus(policy);
> +
> +       return 0;
> +}
>
> On 22/04/2020, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 22-04-20, 17:53, gao yunxiao wrote:
>>> viresh
>>>
>>> Thank you very much for your advice.
>>>
>>> In here, only check whether the frequency point given by cpu_cooling
>>> module is the minimum frequency point.
>>
>> I am not sure I understood what you are doing here. Please elaborate a
>> bit.
>>
>> --
>> viresh
>>
>

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[Daniel Lezcano]

On 22/04/2020 13:11, gao yunxiao wrote:
> On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
>> viresh
>>
>> On UNISOC platform, CPU's temperature can not be controlled when
>> cpufreq has been limited to the lowest frequency, we have to hotplug
>> out CPUS to mitigate temperature rising.
>>
>> adding platform callback to have a chance to check whether the
>> normalised power at power2state() is lower than the power
>> corresponding to the lowest frequency. provide an example in another
>> patch

You can use in addition the cpuidle cooling device if the cpufreq
cooling device fails.

Add two trip points. The first one mitigated by the cpufreq cooling
device and the second one, with a higher temperature, mitigated by the
cpuidle cooling device [1][2].

For my personal information, does the platform support voltage scaling?


[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/thermal/cpu-idle-cooling.rst

[2] https://lkml.org/lkml/2020/4/14/1442



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[gao yunxiao]

Daniel

Thank you for your suggestion

Yes， the platform can support voltage scaling.
I will porting cpuidle cooling and double check it on our platform.

By the way, I have a question trouble to you
when one cpu is forced into the cpuidle, the running task on it are
stopped or migrated other cpu?



On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 22/04/2020 13:11, gao yunxiao wrote:
>> On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
>>> viresh
>>>
>>> On UNISOC platform, CPU's temperature can not be controlled when
>>> cpufreq has been limited to the lowest frequency, we have to hotplug
>>> out CPUS to mitigate temperature rising.
>>>
>>> adding platform callback to have a chance to check whether the
>>> normalised power at power2state() is lower than the power
>>> corresponding to the lowest frequency. provide an example in another
>>> patch
>
> You can use in addition the cpuidle cooling device if the cpufreq
> cooling device fails.
>
> Add two trip points. The first one mitigated by the cpufreq cooling
> device and the second one, with a higher temperature, mitigated by the
> cpuidle cooling device [1][2].
>
> For my personal information, does the platform support voltage scaling?
>
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/thermal/cpu-idle-cooling.rst
>
> [2] https://lkml.org/lkml/2020/4/14/1442
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[Daniel Lezcano]

Hi,

On 28/04/2020 08:51, gao yunxiao wrote:
> Daniel
> 
> Thank you for your suggestion
> 
> Yes， the platform can support voltage scaling.

Given your issue, I would double check if the voltage scaling is really
effective.

> I will porting cpuidle cooling and double check it on our platform.
> 
> By the way, I have a question trouble to you
> when one cpu is forced into the cpuidle, the running task on it are
> stopped or migrated other cpu?

The task is scheduled out by the idle injection which has a real time
priority. When this one finishes the idle cycle, it schedules itself and
the previous task continue its work. So the short answer is the task is
stopped, the idle injection happens, then the task runs again.

Concerning the migration, that is a scheduler thing and will depend on
the thermal configuration and sensors layout.

Let's assume the platform is 4 x cores (one cluster).

1. The platform has one sensor per core and the configuration sets one
thermal zone with one idle cooling device per core

In this case, the mitigation will insert idle cycles, those will be seen
as chunk of system load cycle and will enter in the CPU load
computation. Thus, when there is an imbalance, the scheduler can migrate
the task to an idle CPU (or less busy CPU).

2. The platform has one sensor per cluster and the configuration sets
one thermal zone with *four* idle cooling devices (one per core)

When the mitigation happens, the idle injection will be on all the cores
at the same time, thus the load will increase on all the CPUs and won't
 enter in the balance computation (well actually it will enter but as
they are the same on all the CPUs, the difference is zero).


In practical, we found the configuration #2, and in order to reach the
temperature limit, all the cores are fully busy, so task migration
depends on what the tasks do and the idle injection has few impact on this.

Does it answer your question ?


> On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>> On 22/04/2020 13:11, gao yunxiao wrote:
>>> On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
>>>> viresh
>>>>
>>>> On UNISOC platform, CPU's temperature can not be controlled when
>>>> cpufreq has been limited to the lowest frequency, we have to hotplug
>>>> out CPUS to mitigate temperature rising.
>>>>
>>>> adding platform callback to have a chance to check whether the
>>>> normalised power at power2state() is lower than the power
>>>> corresponding to the lowest frequency. provide an example in another
>>>> patch
>>
>> You can use in addition the cpuidle cooling device if the cpufreq
>> cooling device fails.
>>
>> Add two trip points. The first one mitigated by the cpufreq cooling
>> device and the second one, with a higher temperature, mitigated by the
>> cpuidle cooling device [1][2].
>>
>> For my personal information, does the platform support voltage scaling?
>>
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/thermal/cpu-idle-cooling.rst
>>
>> [2] https://lkml.org/lkml/2020/4/14/1442
>>
>>
>>
>> --
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[gao yunxiao]

Yes，thank you for your introduction

another question trouble to you.
I'm worried that the idle cycle is set too long, there may be a
jamming phenomenon for the mobile phone, so I am not sure how to
determine the idle cycle?

On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> Hi,
>
> On 28/04/2020 08:51, gao yunxiao wrote:
>> Daniel
>>
>> Thank you for your suggestion
>>
>> Yes， the platform can support voltage scaling.
>
> Given your issue, I would double check if the voltage scaling is really
> effective.
>
>> I will porting cpuidle cooling and double check it on our platform.
>>
>> By the way, I have a question trouble to you
>> when one cpu is forced into the cpuidle, the running task on it are
>> stopped or migrated other cpu?
>
> The task is scheduled out by the idle injection which has a real time
> priority. When this one finishes the idle cycle, it schedules itself and
> the previous task continue its work. So the short answer is the task is
> stopped, the idle injection happens, then the task runs again.
>
> Concerning the migration, that is a scheduler thing and will depend on
> the thermal configuration and sensors layout.
>
> Let's assume the platform is 4 x cores (one cluster).
>
> 1. The platform has one sensor per core and the configuration sets one
> thermal zone with one idle cooling device per core
>
> In this case, the mitigation will insert idle cycles, those will be seen
> as chunk of system load cycle and will enter in the CPU load
> computation. Thus, when there is an imbalance, the scheduler can migrate
> the task to an idle CPU (or less busy CPU).
>
> 2. The platform has one sensor per cluster and the configuration sets
> one thermal zone with *four* idle cooling devices (one per core)
>
> When the mitigation happens, the idle injection will be on all the cores
> at the same time, thus the load will increase on all the CPUs and won't
>  enter in the balance computation (well actually it will enter but as
> they are the same on all the CPUs, the difference is zero).
>
>
> In practical, we found the configuration #2, and in order to reach the
> temperature limit, all the cores are fully busy, so task migration
> depends on what the tasks do and the idle injection has few impact on this.
>
> Does it answer your question ?
>
>
>> On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>> On 22/04/2020 13:11, gao yunxiao wrote:
>>>> On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
>>>>> viresh
>>>>>
>>>>> On UNISOC platform, CPU's temperature can not be controlled when
>>>>> cpufreq has been limited to the lowest frequency, we have to hotplug
>>>>> out CPUS to mitigate temperature rising.
>>>>>
>>>>> adding platform callback to have a chance to check whether the
>>>>> normalised power at power2state() is lower than the power
>>>>> corresponding to the lowest frequency. provide an example in another
>>>>> patch
>>>
>>> You can use in addition the cpuidle cooling device if the cpufreq
>>> cooling device fails.
>>>
>>> Add two trip points. The first one mitigated by the cpufreq cooling
>>> device and the second one, with a higher temperature, mitigated by the
>>> cpuidle cooling device [1][2].
>>>
>>> For my personal information, does the platform support voltage scaling?
>>>
>>>
>>> [1]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/thermal/cpu-idle-cooling.rst
>>>
>>> [2] https://lkml.org/lkml/2020/4/14/1442
>>>
>>>
>>>
>>> --
>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>
>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>> <http://twitter.com/#!/linaroorg> Twitter |
>>> <http://www.linaro.org/linaro-blog/> Blog
>>>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[Daniel Lezcano]

On 28/04/2020 12:01, gao yunxiao wrote:
> Yes，thank you for your introduction
> 
> another question trouble to you.
> I'm worried that the idle cycle is set too long, there may be a
> jamming phenomenon for the mobile phone, so I am not sure how to
> determine the idle cycle?

That will depend on the speed transition of the available idle states.
The DT gives the binding for the idle duration and the exit latency.

The optional exit latency constraint will prevent to choose deep idle
states (but which have better cooling effect).

So choosing the right constraint and the idle duration (which should be
at least greater than target residency of the idle state) is a question
of experimentation.

Concerning the jamming effect, if the phone is failing to cool down with
cpufreq and has to use the idle injection in addition, clearly there is
a problem with the heat dissipation and the latency introduced is not
the biggest problem in this situation.

If you think hotplugging the CPU is better, you still have the
possibility to rely on the hot trip point notification (not sure it
works) to unplug from userspace the CPU [1].

  -- Daniel

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/thermal/thermal_core.c#n290


> On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> Hi,
>>
>> On 28/04/2020 08:51, gao yunxiao wrote:
>>> Daniel
>>>
>>> Thank you for your suggestion
>>>
>>> Yes， the platform can support voltage scaling.
>>
>> Given your issue, I would double check if the voltage scaling is really
>> effective.
>>
>>> I will porting cpuidle cooling and double check it on our platform.
>>>
>>> By the way, I have a question trouble to you
>>> when one cpu is forced into the cpuidle, the running task on it are
>>> stopped or migrated other cpu?
>>
>> The task is scheduled out by the idle injection which has a real time
>> priority. When this one finishes the idle cycle, it schedules itself and
>> the previous task continue its work. So the short answer is the task is
>> stopped, the idle injection happens, then the task runs again.
>>
>> Concerning the migration, that is a scheduler thing and will depend on
>> the thermal configuration and sensors layout.
>>
>> Let's assume the platform is 4 x cores (one cluster).
>>
>> 1. The platform has one sensor per core and the configuration sets one
>> thermal zone with one idle cooling device per core
>>
>> In this case, the mitigation will insert idle cycles, those will be seen
>> as chunk of system load cycle and will enter in the CPU load
>> computation. Thus, when there is an imbalance, the scheduler can migrate
>> the task to an idle CPU (or less busy CPU).
>>
>> 2. The platform has one sensor per cluster and the configuration sets
>> one thermal zone with *four* idle cooling devices (one per core)
>>
>> When the mitigation happens, the idle injection will be on all the cores
>> at the same time, thus the load will increase on all the CPUs and won't
>>  enter in the balance computation (well actually it will enter but as
>> they are the same on all the CPUs, the difference is zero).
>>
>>
>> In practical, we found the configuration #2, and in order to reach the
>> temperature limit, all the cores are fully busy, so task migration
>> depends on what the tasks do and the idle injection has few impact on this.
>>
>> Does it answer your question ?
>>
>>
>>> On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>>> On 22/04/2020 13:11, gao yunxiao wrote:
>>>>> On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
>>>>>> viresh
>>>>>>
>>>>>> On UNISOC platform, CPU's temperature can not be controlled when
>>>>>> cpufreq has been limited to the lowest frequency, we have to hotplug
>>>>>> out CPUS to mitigate temperature rising.
>>>>>>
>>>>>> adding platform callback to have a chance to check whether the
>>>>>> normalised power at power2state() is lower than the power
>>>>>> corresponding to the lowest frequency. provide an example in another
>>>>>> patch
>>>>
>>>> You can use in addition the cpuidle cooling device if the cpufreq
>>>> cooling device fails.
>>>>
>>>> Add two trip points. The first one mitigated by the cpufreq cooling
>>>> device and the second one, with a higher temperature, mitigated by the
>>>> cpuidle cooling device [1][2].
>>>>
>>>> For my personal information, does the platform support voltage scaling?
>>>>
>>>>
>>>> [1]
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/thermal/cpu-idle-cooling.rst
>>>>
>>>> [2] https://lkml.org/lkml/2020/4/14/1442
>>>>
>>>>
>>>>
>>>> --
>>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>>
>>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>>> <http://twitter.com/#!/linaroorg> Twitter |
>>>> <http://www.linaro.org/linaro-blog/> Blog
>>>>
>>
>>
>> --
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/2] thermal/drivers/cpufreq_cooling: Add platform callback functions

[gao yunxiao]

Daniel, thank you very much

On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> On 28/04/2020 12:01, gao yunxiao wrote:
>> Yes，thank you for your introduction
>>
>> another question trouble to you.
>> I'm worried that the idle cycle is set too long, there may be a
>> jamming phenomenon for the mobile phone, so I am not sure how to
>> determine the idle cycle?
>
> That will depend on the speed transition of the available idle states.
> The DT gives the binding for the idle duration and the exit latency.
>
> The optional exit latency constraint will prevent to choose deep idle
> states (but which have better cooling effect).
>
> So choosing the right constraint and the idle duration (which should be
> at least greater than target residency of the idle state) is a question
> of experimentation.
>
> Concerning the jamming effect, if the phone is failing to cool down with
> cpufreq and has to use the idle injection in addition, clearly there is
> a problem with the heat dissipation and the latency introduced is not
> the biggest problem in this situation.
>
> If you think hotplugging the CPU is better, you still have the
> possibility to rely on the hot trip point notification (not sure it
> works) to unplug from userspace the CPU [1].
>
>   -- Daniel
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/thermal/thermal_core.c#n290
>
>
>> On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>>
>>> Hi,
>>>
>>> On 28/04/2020 08:51, gao yunxiao wrote:
>>>> Daniel
>>>>
>>>> Thank you for your suggestion
>>>>
>>>> Yes， the platform can support voltage scaling.
>>>
>>> Given your issue, I would double check if the voltage scaling is really
>>> effective.
>>>
>>>> I will porting cpuidle cooling and double check it on our platform.
>>>>
>>>> By the way, I have a question trouble to you
>>>> when one cpu is forced into the cpuidle, the running task on it are
>>>> stopped or migrated other cpu?
>>>
>>> The task is scheduled out by the idle injection which has a real time
>>> priority. When this one finishes the idle cycle, it schedules itself and
>>> the previous task continue its work. So the short answer is the task is
>>> stopped, the idle injection happens, then the task runs again.
>>>
>>> Concerning the migration, that is a scheduler thing and will depend on
>>> the thermal configuration and sensors layout.
>>>
>>> Let's assume the platform is 4 x cores (one cluster).
>>>
>>> 1. The platform has one sensor per core and the configuration sets one
>>> thermal zone with one idle cooling device per core
>>>
>>> In this case, the mitigation will insert idle cycles, those will be seen
>>> as chunk of system load cycle and will enter in the CPU load
>>> computation. Thus, when there is an imbalance, the scheduler can migrate
>>> the task to an idle CPU (or less busy CPU).
>>>
>>> 2. The platform has one sensor per cluster and the configuration sets
>>> one thermal zone with *four* idle cooling devices (one per core)
>>>
>>> When the mitigation happens, the idle injection will be on all the cores
>>> at the same time, thus the load will increase on all the CPUs and won't
>>>  enter in the balance computation (well actually it will enter but as
>>> they are the same on all the CPUs, the difference is zero).
>>>
>>>
>>> In practical, we found the configuration #2, and in order to reach the
>>> temperature limit, all the cores are fully busy, so task migration
>>> depends on what the tasks do and the idle injection has few impact on
>>> this.
>>>
>>> Does it answer your question ?
>>>
>>>
>>>> On 28/04/2020, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>>>> On 22/04/2020 13:11, gao yunxiao wrote:
>>>>>> On 22/04/2020, gao yunxiao <gao.yunxiao6@gmail.com> wrote:
>>>>>>> viresh
>>>>>>>
>>>>>>> On UNISOC platform, CPU's temperature can not be controlled when
>>>>>>> cpufreq has been limited to the lowest frequency, we have to hotplug
>>>>>>> out CPUS to mitigate temperature rising.
>>>>>>>
>>>>>>> adding platform callback to have a chance to check whether the
>>>>>>> normalised power at power2state() is lower than the power
>>>>>>> corresponding to the lowest frequency. provide an example in another
>>>>>>> patch
>>>>>
>>>>> You can use in addition the cpuidle cooling device if the cpufreq
>>>>> cooling device fails.
>>>>>
>>>>> Add two trip points. The first one mitigated by the cpufreq cooling
>>>>> device and the second one, with a higher temperature, mitigated by the
>>>>> cpuidle cooling device [1][2].
>>>>>
>>>>> For my personal information, does the platform support voltage
>>>>> scaling?
>>>>>
>>>>>
>>>>> [1]
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/thermal/cpu-idle-cooling.rst
>>>>>
>>>>> [2] https://lkml.org/lkml/2020/4/14/1442
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM
>>>>> SoCs
>>>>>
>>>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>>>> <http://twitter.com/#!/linaroorg> Twitter |
>>>>> <http://www.linaro.org/linaro-blog/> Blog
>>>>>
>>>
>>>
>>> --
>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>
>>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>>> <http://twitter.com/#!/linaroorg> Twitter |
>>> <http://www.linaro.org/linaro-blog/> Blog
>>>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>