Certain resources modeled as a generic power domain in linux kernel can be
used to warm up the SoC (mx power domain on sdm845) if the temperature
falls below certain threshold. These power domains can be considered as
thermal warming devices. (opposite of thermal cooling devices).
In kernel, these warming devices can be modeled as a thermal cooling
device. Since linux kernel today has no instance of a resource modeled as
a power domain acting as a thermal warming device, a generic power domain
based thermal warming device driver that can be used pan-Socs is the
approach taken in this patch series. Since thermal warming devices can be
thought of as the mirror opposite of thermal cooling devices, this patch
series re-uses thermal cooling device framework. To use these power
domains as warming devices require further tweaks in the thermal framework
which are out of scope of this patch series. These tweaks have been posted
as a separate series[1].
The first patch in this series extends the genpd framework to export out
the performance states of a power domain so that when a power domain is
modeled as a cooling device, the number of possible states and current
state of the cooling device can be retrieved from the genpd framework.
The second patch implements the newly added genpd callback for Qualcomm
RPMH power domain driver which hosts the mx power domain.
The third patch introduces a new cooling device register API that allows
a parent to be specified for the cooling device.
The fourth patch introduces the generic power domain warming device
driver.
The fifth patch extends Qualcomm RPMh power controller driver to register
mx power domain as a thermal warming device in the kernel.
The sixth patch describes the dt binding extensions for mx power domain to
be a thermal warming device.
The seventh patch introduces the DT entreis for sdm845 to register mx
power domain as a thermal warming device.
v1->v2:
- Rename the patch series from "qcom: Model RPMH power domains as
thermal cooling devices" to "Introduce Power domain based
thermal warming devices" as it is more appropriate.
- Introduce a new patch(patch 3) describing the dt-bindings for
generic power domain warming device.
- Patch specific changes mentioned in respective patches.
v2->v3:
- Changed power domain warming device from a virtual device node
entry in DT to being a subnode of power domain controller
binding following Rob's review comments.
- Implemented Ulf's review comments.
- The changes above introduced two new patches (patch 3 and 4)
v3->v4:
- Dropped late_init hook in cooling device ops. Instead introduced
a new cooling device register API that allows to define a parent
for the cooling device.
- Patch specific changes mentioned in respective patches.
v4->v5:
- Dropped the patch that introduced the cooling device register
API with parent as per review comments from Ulf.
- Patch specific changes mentioned in respective patches.
1. https://lkml.org/lkml/2019/9/18/1180
Thara Gopinath (6):
PM/Domains: Add support for retrieving genpd performance states
information
soc: qcom: rpmhpd: Introduce function to retrieve power domain
performance state count
thermal: Add generic power domain warming device driver.
soc: qcom: Extend RPMh power controller driver to register warming
devices.
dt-bindings: power: Extend RPMh power controller binding to describe
thermal warming device
arm64: dts: qcom: Indicate rpmhpd hosts a power domain that can be
used as a warming device.
.../devicetree/bindings/power/qcom,rpmpd.yaml | 3 +
arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 +
drivers/base/power/domain.c | 37 ++++
drivers/soc/qcom/rpmhpd.c | 46 ++++-
drivers/thermal/Kconfig | 10 ++
drivers/thermal/Makefile | 2 +
drivers/thermal/pd_warming.c | 168 ++++++++++++++++++
include/linux/pd_warming.h | 29 +++
include/linux/pm_domain.h | 13 ++
9 files changed, 308 insertions(+), 1 deletion(-)
create mode 100644 drivers/thermal/pd_warming.c
create mode 100644 include/linux/pd_warming.h
--
2.20.1
Add two new APIs in the genpd framework, dev_pm_genpd_get_performance_state
to return the current performance state of a power domain and
dev_pm_genpd_performance_state_count to return the total number of
performance states supported by a power domain. Since the genpd framework
does not maintain a count of number of performance states supported by a
power domain, introduce a new callback(.get_performance_state_count) that
can be used to retrieve this information from power domain drivers.
These APIs are added to aid the implementation of a power domain as a
warming device. Linux kernel cooling device framework(into which warming
device can be plugged in) requires during initialization to be provided
with the maximum number of states that can be supported. When a power
domain acts as a warming device, the max state is the max number of
perfomrance states supported by the power domain. The cooling device
framework implements API to retrieve the current state of the cooling
device. This in turn translates to the current performance state of the
power domain.
Reviewed-by: Ulf Hansson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/base/power/domain.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 13 +++++++++++++
2 files changed, 50 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 959d6d5eb000..d0297c48fa79 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -408,6 +408,43 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state)
}
EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state);
+int dev_pm_genpd_get_performance_state(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ unsigned int state;
+
+ genpd = dev_to_genpd_safe(dev);
+ if (IS_ERR(genpd))
+ return -ENODEV;
+
+ genpd_lock(genpd);
+ state = genpd->performance_state;
+ genpd_unlock(genpd);
+
+ return state;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_get_performance_state);
+
+int dev_pm_genpd_performance_state_count(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int count;
+
+ genpd = dev_to_genpd_safe(dev);
+ if (IS_ERR(genpd))
+ return -ENODEV;
+
+ if (unlikely(!genpd->get_performance_state_count))
+ return -EINVAL;
+
+ genpd_lock(genpd);
+ count = genpd->get_performance_state_count(genpd);
+ genpd_unlock(genpd);
+
+ return count;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_performance_state_count);
+
static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
{
unsigned int state_idx = genpd->state_idx;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9ec78ee53652..7d415350380f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -117,6 +117,7 @@ struct generic_pm_domain {
struct dev_pm_opp *opp);
int (*set_performance_state)(struct generic_pm_domain *genpd,
unsigned int state);
+ int (*get_performance_state_count)(struct generic_pm_domain *genpd);
struct gpd_dev_ops dev_ops;
s64 max_off_time_ns; /* Maximum allowed "suspended" time. */
bool max_off_time_changed;
@@ -204,6 +205,8 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
struct dev_power_governor *gov, bool is_off);
int pm_genpd_remove(struct generic_pm_domain *genpd);
int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
+int dev_pm_genpd_get_performance_state(struct device *dev);
+int dev_pm_genpd_performance_state_count(struct device *dev);
extern struct dev_power_governor simple_qos_governor;
extern struct dev_power_governor pm_domain_always_on_gov;
@@ -251,6 +254,16 @@ static inline int dev_pm_genpd_set_performance_state(struct device *dev,
return -ENOTSUPP;
}
+static inline int dev_pm_genpd_get_performance_state(struct device *dev)
+{
+ return -ENOTSUPP;
+}
+
+static inline int dev_pm_genpd_performance_state_count(struct device *dev)
+{
+ return -ENOTSUPP;
+}
+
#define simple_qos_governor (*(struct dev_power_governor *)(NULL))
#define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
#endif
--
2.20.1
Resources modeled as power domains in linux kernel can be used to warm the
SoC(eg. mx power domain on sdm845). To support this feature, introduce a
generic power domain warming device driver that can be plugged into the
thermal framework (The thermal framework itself requires further
modifiction to support a warming device in place of a cooling device.
Those extensions are not introduced in this patch series).
Signed-off-by: Thara Gopinath <[email protected]>
---
v3->v4:
- Removed late_init hook pd_warming_device_ops.
- Use of_genpd_add_device instead of pm_genpd_add_device to attach
device to the generic power domain.
- Use thermal_of_cooling_device_parent_register to register the
cooling device so that the device with genpd attached can be
made parent of the cooling device.
- With above changes, remove reference to generic_pm_domain in
pd_warming_device.
v4->v5:
- All the below changes are as per Ulf's review comments.
- Renamed pwr_domain_warming.c and pwr_domain_warming.h to
pd_warming.c and pd_warming.h.
- Renamed pwr_domain_warming_register API to
of_pd_warming_register.
- Dropped in-param pd_name to of_pd_warming_register.
- Introduced ID allocator to uniquely identify each power domain
warming device.
- Introduced pd_warming_release to handle device kfree for
pd_warming_device.
- Introduced pm_genpd_remove_device in the error exit path
of of_pd_warming_register.
drivers/thermal/Kconfig | 10 +++
drivers/thermal/Makefile | 2 +
drivers/thermal/pd_warming.c | 168 +++++++++++++++++++++++++++++++++++
include/linux/pd_warming.h | 29 ++++++
4 files changed, 209 insertions(+)
create mode 100644 drivers/thermal/pd_warming.c
create mode 100644 include/linux/pd_warming.h
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 4d6753f2b18f..92522d541d0e 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -206,6 +206,16 @@ config DEVFREQ_THERMAL
If you want this support, you should say Y here.
+config PWR_DOMAIN_WARMING_THERMAL
+ bool "Power Domain based warming device"
+ depends on PM_GENERIC_DOMAINS_OF
+ help
+ This implements the generic power domain based warming
+ mechanism through increasing the performance state of
+ a power domain.
+
+ If you want this support, you should say Y here.
+
config THERMAL_EMULATION
bool "Thermal emulation mode support"
help
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 8c8ed7b79915..7db87a779126 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -28,6 +28,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o
# devfreq cooling
thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
+thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pd_warming.o
+
# platform thermal drivers
obj-y += broadcom/
obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o
diff --git a/drivers/thermal/pd_warming.c b/drivers/thermal/pd_warming.c
new file mode 100644
index 000000000000..c0854d2e4b92
--- /dev/null
+++ b/drivers/thermal/pd_warming.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Linaro Ltd
+ */
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/pd_warming.h>
+
+struct pd_warming_device {
+ struct thermal_cooling_device *cdev;
+ struct device dev;
+ int id;
+ int max_state;
+ int cur_state;
+ bool runtime_resumed;
+};
+
+static DEFINE_IDA(pd_ida);
+
+static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ struct pd_warming_device *pd_wdev = cdev->devdata;
+
+ *state = pd_wdev->max_state;
+ return 0;
+}
+
+static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ struct pd_warming_device *pd_wdev = cdev->devdata;
+
+ *state = dev_pm_genpd_get_performance_state(&pd_wdev->dev);
+
+ return 0;
+}
+
+static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long state)
+{
+ struct pd_warming_device *pd_wdev = cdev->devdata;
+ struct device *dev = &pd_wdev->dev;
+ int ret;
+
+ ret = dev_pm_genpd_set_performance_state(dev, state);
+
+ if (ret)
+ return ret;
+
+ if (state && !pd_wdev->runtime_resumed) {
+ ret = pm_runtime_get_sync(dev);
+ pd_wdev->runtime_resumed = true;
+ } else if (!state && pd_wdev->runtime_resumed) {
+ ret = pm_runtime_put(dev);
+ pd_wdev->runtime_resumed = false;
+ }
+
+ return ret;
+}
+
+static struct thermal_cooling_device_ops pd_warming_device_ops = {
+ .get_max_state = pd_wdev_get_max_state,
+ .get_cur_state = pd_wdev_get_cur_state,
+ .set_cur_state = pd_wdev_set_cur_state,
+};
+
+static void pd_warming_release(struct device *dev)
+{
+ kfree(dev);
+}
+
+struct thermal_cooling_device *
+of_pd_warming_register(struct device *parent, int pd_id)
+{
+ struct pd_warming_device *pd_wdev;
+ struct of_phandle_args pd_args;
+ char cdev_name[THERMAL_NAME_LENGTH];
+ int ret;
+
+ pd_wdev = kzalloc(sizeof(*pd_wdev), GFP_KERNEL);
+ if (!pd_wdev)
+ return ERR_PTR(-ENOMEM);
+
+ dev_set_name(&pd_wdev->dev, "%s_%d_warming_dev",
+ dev_name(parent), pd_id);
+ pd_wdev->dev.parent = parent;
+ pd_wdev->dev.release = pd_warming_release;
+
+ ret = device_register(&pd_wdev->dev);
+ if (ret) {
+ put_device(&pd_wdev->dev);
+ goto free_pd_wdev;
+ }
+
+ ret = ida_simple_get(&pd_ida, 0, 0, GFP_KERNEL);
+ if (ret < 0)
+ goto unregister_device;
+
+ pd_wdev->id = ret;
+
+ pd_args.np = parent->of_node;
+ pd_args.args[0] = pd_id;
+ pd_args.args_count = 1;
+
+ ret = of_genpd_add_device(&pd_args, &pd_wdev->dev);
+
+ if (ret)
+ goto remove_ida;
+
+ ret = dev_pm_genpd_performance_state_count(&pd_wdev->dev);
+ if (ret < 0)
+ goto out_genpd;
+
+ pd_wdev->max_state = ret - 1;
+ pm_runtime_enable(&pd_wdev->dev);
+ pd_wdev->runtime_resumed = false;
+
+ snprintf(cdev_name, sizeof(cdev_name), "thermal-pd-%d", pd_wdev->id);
+ pd_wdev->cdev = thermal_of_cooling_device_register
+ (NULL, cdev_name, pd_wdev,
+ &pd_warming_device_ops);
+ if (IS_ERR(pd_wdev->cdev)) {
+ pr_err("unable to register %s cooling device\n", cdev_name);
+ ret = PTR_ERR(pd_wdev->cdev);
+ goto out_runtime_disable;
+ }
+
+ return pd_wdev->cdev;
+
+out_runtime_disable:
+ pm_runtime_disable(&pd_wdev->dev);
+out_genpd:
+ pm_genpd_remove_device(&pd_wdev->dev);
+remove_ida:
+ ida_simple_remove(&pd_ida, pd_wdev->id);
+unregister_device:
+ device_unregister(&pd_wdev->dev);
+ pd_warming_release(&pd_wdev->dev);
+free_pd_wdev:
+ kfree(pd_wdev);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(of_pd_warming_register);
+
+void pd_warming_unregister(struct thermal_cooling_device *cdev)
+{
+ struct pd_warming_device *pd_wdev = cdev->devdata;
+ struct device *dev = &pd_wdev->dev;
+
+ if (pd_wdev->runtime_resumed) {
+ dev_pm_genpd_set_performance_state(dev, 0);
+ pm_runtime_put(dev);
+ pd_wdev->runtime_resumed = false;
+ }
+ pm_runtime_disable(dev);
+ pm_genpd_remove_device(dev);
+ ida_simple_remove(&pd_ida, pd_wdev->id);
+ thermal_cooling_device_unregister(cdev);
+ kfree(pd_wdev);
+}
+EXPORT_SYMBOL_GPL(pd_warming_unregister);
diff --git a/include/linux/pd_warming.h b/include/linux/pd_warming.h
new file mode 100644
index 000000000000..550a5683b56d
--- /dev/null
+++ b/include/linux/pd_warming.h
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Linaro Ltd.
+ */
+#ifndef __PWR_DOMAIN_WARMING_H__
+#define __PWR_DOMAIN_WARMING_H__
+
+#include <linux/pm_domain.h>
+#include <linux/thermal.h>
+
+#ifdef CONFIG_PWR_DOMAIN_WARMING_THERMAL
+struct thermal_cooling_device *
+of_pd_warming_register(struct device *parent, int pd_id);
+
+void pd_warming_unregister(struct thermal_cooling_device *cdev);
+
+#else
+static inline struct thermal_cooling_device *
+of_pd_warming_register(struct device *parent, int pd_id)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void
+pd_warming_unregister(struct thermal_cooling_device *cdev)
+{
+}
+#endif /* CONFIG_PWR_DOMAIN_WARMING_THERMAL */
+#endif /* __PWR_DOMAIN_WARMING_H__ */
--
2.20.1
RPMh hosts mx power domain that can be used to warm up the SoC. Indicate
this by using #cooling-cells property.
Reviewed-by: Ulf Hansson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
v3->v4:
- Removed subnode to indicate that mx power domain is a warming
device. Instead #cooling-cells is used as a power domain
provider property to indicate if the provider hosts a power
domain that can be used as a warming device.
arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index fe35d37a11cc..0d878b2ca351 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3703,6 +3703,7 @@
rpmhpd: power-controller {
compatible = "qcom,sdm845-rpmhpd";
#power-domain-cells = <1>;
+ #cooling-cells = <2>;
operating-points-v2 = <&rpmhpd_opp_table>;
rpmhpd_opp_table: opp-table {
--
2.20.1
RPMh power controller hosts mx domain that can be used as thermal warming
device. Add #cooling-cells property to the power domain provider node to
indicate this.
Signed-off-by: Thara Gopinath <[email protected]>
---
v3->v4:
- Removed subnode to indicate that mx power domain is a warming
device. Instead #cooling-cells is used as a power domain
provider property to indicate if the provider hosts a power
domain that can be used as a warming device.
v4->v5:
Moved the property from .txt format to .yaml format.
Documentation/devicetree/bindings/power/qcom,rpmpd.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
index ba605310abeb..16b713d295c4 100644
--- a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
@@ -27,6 +27,9 @@ properties:
'#power-domain-cells':
const: 1
+ '#cooling-cells':
+ const: 2
+
operating-points-v2: true
opp-table:
--
2.20.1
RPMh power control hosts power domains that can be used as
thermal warming devices. Register these power domains
with the generic power domain warming device thermal framework.
Reviewed-by: Ulf Hansson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
v3->v4:
- Introduce a boolean value is_warming_dev in rpmhpd structure to
indicate if a generic power domain can be used as a warming
device or not.With this change, device tree no longer has to
specify which power domain inside the rpmh power domain provider
is a warming device.
- Move registering of warming devices into a late initcall to
ensure that warming devices are registered after thermal
framework is initialized.
drivers/soc/qcom/rpmhpd.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index 7142409a3b77..4e9c0bbb8826 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -11,6 +11,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
+#include <linux/pd_warming.h>
#include <soc/qcom/cmd-db.h>
#include <soc/qcom/rpmh.h>
#include <dt-bindings/power/qcom-rpmpd.h>
@@ -48,6 +49,7 @@ struct rpmhpd {
bool enabled;
const char *res_name;
u32 addr;
+ bool is_warming_dev;
};
struct rpmhpd_desc {
@@ -55,6 +57,8 @@ struct rpmhpd_desc {
size_t num_pds;
};
+const struct rpmhpd_desc *global_desc;
+
static DEFINE_MUTEX(rpmhpd_lock);
/* SDM845 RPMH powerdomains */
@@ -89,6 +93,7 @@ static struct rpmhpd sdm845_mx = {
.pd = { .name = "mx", },
.peer = &sdm845_mx_ao,
.res_name = "mx.lvl",
+ .is_warming_dev = true,
};
static struct rpmhpd sdm845_mx_ao = {
@@ -452,7 +457,14 @@ static int rpmhpd_probe(struct platform_device *pdev)
&rpmhpds[i]->pd);
}
- return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
+ ret = of_genpd_add_provider_onecell(pdev->dev.of_node, data);
+
+ if (ret)
+ return ret;
+
+ global_desc = desc;
+
+ return 0;
}
static struct platform_driver rpmhpd_driver = {
@@ -469,3 +481,26 @@ static int __init rpmhpd_init(void)
return platform_driver_register(&rpmhpd_driver);
}
core_initcall(rpmhpd_init);
+
+static int __init rpmhpd_init_warming_device(void)
+{
+ size_t num_pds;
+ struct rpmhpd **rpmhpds;
+ int i;
+
+ if (!global_desc)
+ return -EINVAL;
+
+ rpmhpds = global_desc->rpmhpds;
+ num_pds = global_desc->num_pds;
+
+ if (!of_find_property(rpmhpds[0]->dev->of_node, "#cooling-cells", NULL))
+ return 0;
+
+ for (i = 0; i < num_pds; i++)
+ if (rpmhpds[i]->is_warming_dev)
+ of_pd_warming_register(rpmhpds[i]->dev, i);
+
+ return 0;
+}
+late_initcall(rpmhpd_init_warming_device);
--
2.20.1
Populate .get_performace_state_count in genpd ops to retrieve the count of
performance states supported by a rpmh power domain.
Reviewed-by: Ulf Hansson <[email protected]>
Signed-off-by: Thara Gopinath <[email protected]>
---
drivers/soc/qcom/rpmhpd.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index 4d264d0672c4..7142409a3b77 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -341,6 +341,13 @@ static unsigned int rpmhpd_get_performance_state(struct generic_pm_domain *genpd
return dev_pm_opp_get_level(opp);
}
+static int rpmhpd_performance_states_count(struct generic_pm_domain *domain)
+{
+ struct rpmhpd *pd = domain_to_rpmhpd(domain);
+
+ return pd->level_count;
+}
+
static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
{
int i;
@@ -429,6 +436,8 @@ static int rpmhpd_probe(struct platform_device *pdev)
rpmhpds[i]->pd.power_on = rpmhpd_power_on;
rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance_state;
rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance_state;
+ rpmhpds[i]->pd.get_performance_state_count =
+ rpmhpd_performance_states_count;
pm_genpd_init(&rpmhpds[i]->pd, NULL, true);
data->domains[i] = &rpmhpds[i]->pd;
--
2.20.1
On Thu, 19 Mar 2020 21:41:06 -0400, Thara Gopinath wrote:
> RPMh power controller hosts mx domain that can be used as thermal warming
> device. Add #cooling-cells property to the power domain provider node to
> indicate this.
>
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
>
> v3->v4:
> - Removed subnode to indicate that mx power domain is a warming
> device. Instead #cooling-cells is used as a power domain
> provider property to indicate if the provider hosts a power
> domain that can be used as a warming device.
>
> v4->v5:
> Moved the property from .txt format to .yaml format.
>
> Documentation/devicetree/bindings/power/qcom,rpmpd.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Fri, 20 Mar 2020 at 02:41, Thara Gopinath <[email protected]> wrote:
>
> Resources modeled as power domains in linux kernel can be used to warm the
> SoC(eg. mx power domain on sdm845). To support this feature, introduce a
> generic power domain warming device driver that can be plugged into the
> thermal framework (The thermal framework itself requires further
> modifiction to support a warming device in place of a cooling device.
> Those extensions are not introduced in this patch series).
>
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
>
> v3->v4:
> - Removed late_init hook pd_warming_device_ops.
> - Use of_genpd_add_device instead of pm_genpd_add_device to attach
> device to the generic power domain.
> - Use thermal_of_cooling_device_parent_register to register the
> cooling device so that the device with genpd attached can be
> made parent of the cooling device.
> - With above changes, remove reference to generic_pm_domain in
> pd_warming_device.
>
> v4->v5:
> - All the below changes are as per Ulf's review comments.
> - Renamed pwr_domain_warming.c and pwr_domain_warming.h to
> pd_warming.c and pd_warming.h.
> - Renamed pwr_domain_warming_register API to
> of_pd_warming_register.
> - Dropped in-param pd_name to of_pd_warming_register.
> - Introduced ID allocator to uniquely identify each power domain
> warming device.
> - Introduced pd_warming_release to handle device kfree for
> pd_warming_device.
> - Introduced pm_genpd_remove_device in the error exit path
> of of_pd_warming_register.
>
> drivers/thermal/Kconfig | 10 +++
> drivers/thermal/Makefile | 2 +
> drivers/thermal/pd_warming.c | 168 +++++++++++++++++++++++++++++++++++
> include/linux/pd_warming.h | 29 ++++++
> 4 files changed, 209 insertions(+)
> create mode 100644 drivers/thermal/pd_warming.c
> create mode 100644 include/linux/pd_warming.h
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 4d6753f2b18f..92522d541d0e 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -206,6 +206,16 @@ config DEVFREQ_THERMAL
>
> If you want this support, you should say Y here.
>
> +config PWR_DOMAIN_WARMING_THERMAL
> + bool "Power Domain based warming device"
> + depends on PM_GENERIC_DOMAINS_OF
> + help
> + This implements the generic power domain based warming
> + mechanism through increasing the performance state of
> + a power domain.
> +
> + If you want this support, you should say Y here.
> +
> config THERMAL_EMULATION
> bool "Thermal emulation mode support"
> help
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 8c8ed7b79915..7db87a779126 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -28,6 +28,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o
> # devfreq cooling
> thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>
> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pd_warming.o
> +
> # platform thermal drivers
> obj-y += broadcom/
> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o
> diff --git a/drivers/thermal/pd_warming.c b/drivers/thermal/pd_warming.c
> new file mode 100644
> index 000000000000..c0854d2e4b92
> --- /dev/null
> +++ b/drivers/thermal/pd_warming.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, Linaro Ltd
> + */
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/pd_warming.h>
> +
> +struct pd_warming_device {
> + struct thermal_cooling_device *cdev;
> + struct device dev;
> + int id;
> + int max_state;
> + int cur_state;
> + bool runtime_resumed;
> +};
> +
> +static DEFINE_IDA(pd_ida);
> +
> +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct pd_warming_device *pd_wdev = cdev->devdata;
> +
> + *state = pd_wdev->max_state;
> + return 0;
> +}
> +
> +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + struct pd_warming_device *pd_wdev = cdev->devdata;
> +
> + *state = dev_pm_genpd_get_performance_state(&pd_wdev->dev);
> +
> + return 0;
> +}
> +
> +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long state)
> +{
> + struct pd_warming_device *pd_wdev = cdev->devdata;
> + struct device *dev = &pd_wdev->dev;
> + int ret;
> +
> + ret = dev_pm_genpd_set_performance_state(dev, state);
> +
> + if (ret)
> + return ret;
> +
> + if (state && !pd_wdev->runtime_resumed) {
> + ret = pm_runtime_get_sync(dev);
> + pd_wdev->runtime_resumed = true;
> + } else if (!state && pd_wdev->runtime_resumed) {
> + ret = pm_runtime_put(dev);
> + pd_wdev->runtime_resumed = false;
> + }
> +
> + return ret;
> +}
> +
> +static struct thermal_cooling_device_ops pd_warming_device_ops = {
> + .get_max_state = pd_wdev_get_max_state,
> + .get_cur_state = pd_wdev_get_cur_state,
> + .set_cur_state = pd_wdev_set_cur_state,
> +};
> +
> +static void pd_warming_release(struct device *dev)
> +{
> + kfree(dev);
This is wrong, you should free a "struct pd_warming_device *". Use the
"container of" macro to get it from 'dev'.
> +}
> +
> +struct thermal_cooling_device *
> +of_pd_warming_register(struct device *parent, int pd_id)
> +{
> + struct pd_warming_device *pd_wdev;
> + struct of_phandle_args pd_args;
> + char cdev_name[THERMAL_NAME_LENGTH];
> + int ret;
> +
> + pd_wdev = kzalloc(sizeof(*pd_wdev), GFP_KERNEL);
> + if (!pd_wdev)
> + return ERR_PTR(-ENOMEM);
> +
> + dev_set_name(&pd_wdev->dev, "%s_%d_warming_dev",
> + dev_name(parent), pd_id);
> + pd_wdev->dev.parent = parent;
> + pd_wdev->dev.release = pd_warming_release;
> +
> + ret = device_register(&pd_wdev->dev);
> + if (ret) {
> + put_device(&pd_wdev->dev);
> + goto free_pd_wdev;
> + }
> +
> + ret = ida_simple_get(&pd_ida, 0, 0, GFP_KERNEL);
> + if (ret < 0)
> + goto unregister_device;
If you use and ida, you might as well use it as a part of the
dev_set_name() above.
That should give you a unique name, similar to how you use it for the
cdev_name below.
> +
> + pd_wdev->id = ret;
> +
> + pd_args.np = parent->of_node;
> + pd_args.args[0] = pd_id;
> + pd_args.args_count = 1;
> +
> + ret = of_genpd_add_device(&pd_args, &pd_wdev->dev);
> +
> + if (ret)
> + goto remove_ida;
> +
> + ret = dev_pm_genpd_performance_state_count(&pd_wdev->dev);
> + if (ret < 0)
> + goto out_genpd;
> +
> + pd_wdev->max_state = ret - 1;
> + pm_runtime_enable(&pd_wdev->dev);
> + pd_wdev->runtime_resumed = false;
> +
> + snprintf(cdev_name, sizeof(cdev_name), "thermal-pd-%d", pd_wdev->id);
> + pd_wdev->cdev = thermal_of_cooling_device_register
> + (NULL, cdev_name, pd_wdev,
> + &pd_warming_device_ops);
> + if (IS_ERR(pd_wdev->cdev)) {
> + pr_err("unable to register %s cooling device\n", cdev_name);
> + ret = PTR_ERR(pd_wdev->cdev);
> + goto out_runtime_disable;
> + }
> +
> + return pd_wdev->cdev;
> +
> +out_runtime_disable:
> + pm_runtime_disable(&pd_wdev->dev);
> +out_genpd:
> + pm_genpd_remove_device(&pd_wdev->dev);
> +remove_ida:
> + ida_simple_remove(&pd_ida, pd_wdev->id);
> +unregister_device:
> + device_unregister(&pd_wdev->dev);
> + pd_warming_release(&pd_wdev->dev);
This is wrong, drop this.
> +free_pd_wdev:
> + kfree(pd_wdev);
Since you should free this from the ->release() callback, there is no
need to do this here.
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(of_pd_warming_register);
> +
> +void pd_warming_unregister(struct thermal_cooling_device *cdev)
> +{
> + struct pd_warming_device *pd_wdev = cdev->devdata;
> + struct device *dev = &pd_wdev->dev;
> +
> + if (pd_wdev->runtime_resumed) {
> + dev_pm_genpd_set_performance_state(dev, 0);
> + pm_runtime_put(dev);
> + pd_wdev->runtime_resumed = false;
> + }
> + pm_runtime_disable(dev);
> + pm_genpd_remove_device(dev);
> + ida_simple_remove(&pd_ida, pd_wdev->id);
> + thermal_cooling_device_unregister(cdev);
> + kfree(pd_wdev);
Don't use kfree here, but instead device_unregister(dev);
> +}
> +EXPORT_SYMBOL_GPL(pd_warming_unregister);
> diff --git a/include/linux/pd_warming.h b/include/linux/pd_warming.h
> new file mode 100644
> index 000000000000..550a5683b56d
> --- /dev/null
> +++ b/include/linux/pd_warming.h
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, Linaro Ltd.
> + */
> +#ifndef __PWR_DOMAIN_WARMING_H__
> +#define __PWR_DOMAIN_WARMING_H__
> +
> +#include <linux/pm_domain.h>
> +#include <linux/thermal.h>
> +
> +#ifdef CONFIG_PWR_DOMAIN_WARMING_THERMAL
> +struct thermal_cooling_device *
> +of_pd_warming_register(struct device *parent, int pd_id);
> +
> +void pd_warming_unregister(struct thermal_cooling_device *cdev);
> +
> +#else
> +static inline struct thermal_cooling_device *
> +of_pd_warming_register(struct device *parent, int pd_id)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline void
> +pd_warming_unregister(struct thermal_cooling_device *cdev)
> +{
> +}
> +#endif /* CONFIG_PWR_DOMAIN_WARMING_THERMAL */
> +#endif /* __PWR_DOMAIN_WARMING_H__ */
> --
> 2.20.1
>
Besides the few things above, this looks good to me.
Kind regards
Uffe
Hi Ulf,
Thanks for the review!
On 3/23/20 11:57 AM, Ulf Hansson wrote:
--snip
>> +
>> +static void pd_warming_release(struct device *dev)
>> +{
>> + kfree(dev);
>
> This is wrong, you should free a "struct pd_warming_device *". Use the
> "container of" macro to get it from 'dev'.
Will fix this.
>
>> +}
>> +
>> +struct thermal_cooling_device *
>> +of_pd_warming_register(struct device *parent, int pd_id)
>> +{
>> + struct pd_warming_device *pd_wdev;
>> + struct of_phandle_args pd_args;
>> + char cdev_name[THERMAL_NAME_LENGTH];
>> + int ret;
>> +
>> + pd_wdev = kzalloc(sizeof(*pd_wdev), GFP_KERNEL);
>> + if (!pd_wdev)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + dev_set_name(&pd_wdev->dev, "%s_%d_warming_dev",
>> + dev_name(parent), pd_id);
>> + pd_wdev->dev.parent = parent;
>> + pd_wdev->dev.release = pd_warming_release;
>> +
>> + ret = device_register(&pd_wdev->dev);
>> + if (ret) {
>> + put_device(&pd_wdev->dev);
>> + goto free_pd_wdev;
>> + }
>> +
>> + ret = ida_simple_get(&pd_ida, 0, 0, GFP_KERNEL);
>> + if (ret < 0)
>> + goto unregister_device;
>
> If you use and ida, you might as well use it as a part of the
> dev_set_name() above.
>
> That should give you a unique name, similar to how you use it for the
> cdev_name below.
dev_set_name above already has a unique name with the power controller
name and the power domain id. cdev on the other hand creates a virtual
thermal device and needs a unique name.
>
>> +
>> + pd_wdev->id = ret;
>> +
>> + pd_args.np = parent->of_node;
>> + pd_args.args[0] = pd_id;
>> + pd_args.args_count = 1;
>> +
>> + ret = of_genpd_add_device(&pd_args, &pd_wdev->dev);
>> +
>> + if (ret)
>> + goto remove_ida;
>> +
>> + ret = dev_pm_genpd_performance_state_count(&pd_wdev->dev);
>> + if (ret < 0)
>> + goto out_genpd;
>> +
>> + pd_wdev->max_state = ret - 1;
>> + pm_runtime_enable(&pd_wdev->dev);
>> + pd_wdev->runtime_resumed = false;
>> +
>> + snprintf(cdev_name, sizeof(cdev_name), "thermal-pd-%d", pd_wdev->id);
>> + pd_wdev->cdev = thermal_of_cooling_device_register
>> + (NULL, cdev_name, pd_wdev,
>> + &pd_warming_device_ops);
>> + if (IS_ERR(pd_wdev->cdev)) {
>> + pr_err("unable to register %s cooling device\n", cdev_name);
>> + ret = PTR_ERR(pd_wdev->cdev);
>> + goto out_runtime_disable;
>> + }
>> +
>> + return pd_wdev->cdev;
>> +
>> +out_runtime_disable:
>> + pm_runtime_disable(&pd_wdev->dev);
>> +out_genpd:
>> + pm_genpd_remove_device(&pd_wdev->dev);
>> +remove_ida:
>> + ida_simple_remove(&pd_ida, pd_wdev->id);
>> +unregister_device:
>> + device_unregister(&pd_wdev->dev);
>> + pd_warming_release(&pd_wdev->dev);
>
> This is wrong, drop this.
Oops . sorry . Will do. Will fix rest of the comments below as well.
>
>> +free_pd_wdev:
>> + kfree(pd_wdev);
>
> Since you should free this from the ->release() callback, there is no
> need to do this here.
>
>> + return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(of_pd_warming_register);
>> +
>> +void pd_warming_unregister(struct thermal_cooling_device *cdev)
>> +{
>> + struct pd_warming_device *pd_wdev = cdev->devdata;
>> + struct device *dev = &pd_wdev->dev;
>> +
>> + if (pd_wdev->runtime_resumed) {
>> + dev_pm_genpd_set_performance_state(dev, 0);
>> + pm_runtime_put(dev);
>> + pd_wdev->runtime_resumed = false;
>> + }
>> + pm_runtime_disable(dev);
>> + pm_genpd_remove_device(dev);
>> + ida_simple_remove(&pd_ida, pd_wdev->id);
>> + thermal_cooling_device_unregister(cdev);
>> + kfree(pd_wdev);
>
> Don't use kfree here, but instead device_unregister(dev);
>
>> +}
>> +EXPORT_SYMBOL_GPL(pd_warming_unregister);
>> diff --git a/include/linux/pd_warming.h b/include/linux/pd_warming.h
>> new file mode 100644
>> index 000000000000..550a5683b56d
>> --- /dev/null
>> +++ b/include/linux/pd_warming.h
>> @@ -0,0 +1,29 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019, Linaro Ltd.
>> + */
>> +#ifndef __PWR_DOMAIN_WARMING_H__
>> +#define __PWR_DOMAIN_WARMING_H__
>> +
>> +#include <linux/pm_domain.h>
>> +#include <linux/thermal.h>
>> +
>> +#ifdef CONFIG_PWR_DOMAIN_WARMING_THERMAL
>> +struct thermal_cooling_device *
>> +of_pd_warming_register(struct device *parent, int pd_id);
>> +
>> +void pd_warming_unregister(struct thermal_cooling_device *cdev);
>> +
>> +#else
>> +static inline struct thermal_cooling_device *
>> +of_pd_warming_register(struct device *parent, int pd_id)
>> +{
>> + return ERR_PTR(-ENOSYS);
>> +}
>> +
>> +static inline void
>> +pd_warming_unregister(struct thermal_cooling_device *cdev)
>> +{
>> +}
>> +#endif /* CONFIG_PWR_DOMAIN_WARMING_THERMAL */
>> +#endif /* __PWR_DOMAIN_WARMING_H__ */
>> --
>> 2.20.1
>>
>
> Besides the few things above, this looks good to me.
>
> Kind regards
> Uffe
>
--
Warm Regards
Thara
On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
> Populate .get_performace_state_count in genpd ops to retrieve the count of
> performance states supported by a rpmh power domain.
>
> Reviewed-by: Ulf Hansson <[email protected]>
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
> drivers/soc/qcom/rpmhpd.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index 4d264d0672c4..7142409a3b77 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -341,6 +341,13 @@ static unsigned int rpmhpd_get_performance_state(struct generic_pm_domain *genpd
> return dev_pm_opp_get_level(opp);
> }
>
> +static int rpmhpd_performance_states_count(struct generic_pm_domain *domain)
> +{
> + struct rpmhpd *pd = domain_to_rpmhpd(domain);
> +
> + return pd->level_count;
> +}
> +
> static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
> {
> int i;
> @@ -429,6 +436,8 @@ static int rpmhpd_probe(struct platform_device *pdev)
> rpmhpds[i]->pd.power_on = rpmhpd_power_on;
> rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance_state;
> rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance_state;
> + rpmhpds[i]->pd.get_performance_state_count =
> + rpmhpd_performance_states_count;
I would prefer if you ignore the 80-char limit here and leave the line
unwrapped.
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> pm_genpd_init(&rpmhpds[i]->pd, NULL, true);
>
> data->domains[i] = &rpmhpds[i]->pd;
> --
> 2.20.1
>
On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
> Add two new APIs in the genpd framework, dev_pm_genpd_get_performance_state
> to return the current performance state of a power domain and
> dev_pm_genpd_performance_state_count to return the total number of
> performance states supported by a power domain. Since the genpd framework
> does not maintain a count of number of performance states supported by a
> power domain, introduce a new callback(.get_performance_state_count) that
> can be used to retrieve this information from power domain drivers.
>
> These APIs are added to aid the implementation of a power domain as a
> warming device. Linux kernel cooling device framework(into which warming
> device can be plugged in) requires during initialization to be provided
> with the maximum number of states that can be supported. When a power
> domain acts as a warming device, the max state is the max number of
> perfomrance states supported by the power domain. The cooling device
> framework implements API to retrieve the current state of the cooling
> device. This in turn translates to the current performance state of the
> power domain.
>
> Reviewed-by: Ulf Hansson <[email protected]>
> Signed-off-by: Thara Gopinath <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
> ---
> drivers/base/power/domain.c | 37 +++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 13 +++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 959d6d5eb000..d0297c48fa79 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -408,6 +408,43 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state)
> }
> EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state);
>
> +int dev_pm_genpd_get_performance_state(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> + unsigned int state;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (IS_ERR(genpd))
> + return -ENODEV;
> +
> + genpd_lock(genpd);
> + state = genpd->performance_state;
> + genpd_unlock(genpd);
> +
> + return state;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_get_performance_state);
> +
> +int dev_pm_genpd_performance_state_count(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> + int count;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (IS_ERR(genpd))
> + return -ENODEV;
> +
> + if (unlikely(!genpd->get_performance_state_count))
> + return -EINVAL;
> +
> + genpd_lock(genpd);
> + count = genpd->get_performance_state_count(genpd);
> + genpd_unlock(genpd);
> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_performance_state_count);
> +
> static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
> {
> unsigned int state_idx = genpd->state_idx;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 9ec78ee53652..7d415350380f 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -117,6 +117,7 @@ struct generic_pm_domain {
> struct dev_pm_opp *opp);
> int (*set_performance_state)(struct generic_pm_domain *genpd,
> unsigned int state);
> + int (*get_performance_state_count)(struct generic_pm_domain *genpd);
> struct gpd_dev_ops dev_ops;
> s64 max_off_time_ns; /* Maximum allowed "suspended" time. */
> bool max_off_time_changed;
> @@ -204,6 +205,8 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
> struct dev_power_governor *gov, bool is_off);
> int pm_genpd_remove(struct generic_pm_domain *genpd);
> int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
> +int dev_pm_genpd_get_performance_state(struct device *dev);
> +int dev_pm_genpd_performance_state_count(struct device *dev);
>
> extern struct dev_power_governor simple_qos_governor;
> extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -251,6 +254,16 @@ static inline int dev_pm_genpd_set_performance_state(struct device *dev,
> return -ENOTSUPP;
> }
>
> +static inline int dev_pm_genpd_get_performance_state(struct device *dev)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int dev_pm_genpd_performance_state_count(struct device *dev)
> +{
> + return -ENOTSUPP;
> +}
> +
> #define simple_qos_governor (*(struct dev_power_governor *)(NULL))
> #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
> #endif
> --
> 2.20.1
>
On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
> RPMh power control hosts power domains that can be used as
> thermal warming devices. Register these power domains
> with the generic power domain warming device thermal framework.
>
> Reviewed-by: Ulf Hansson <[email protected]>
> Signed-off-by: Thara Gopinath <[email protected]>
> ---
>
> v3->v4:
> - Introduce a boolean value is_warming_dev in rpmhpd structure to
> indicate if a generic power domain can be used as a warming
> device or not.With this change, device tree no longer has to
> specify which power domain inside the rpmh power domain provider
> is a warming device.
> - Move registering of warming devices into a late initcall to
> ensure that warming devices are registered after thermal
> framework is initialized.
This information is lost when we merge patches, as such I would like
such design decisions to be described in the commit message itself.
But...
>
> drivers/soc/qcom/rpmhpd.c | 37 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index 7142409a3b77..4e9c0bbb8826 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -11,6 +11,7 @@
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/pm_opp.h>
> +#include <linux/pd_warming.h>
> #include <soc/qcom/cmd-db.h>
> #include <soc/qcom/rpmh.h>
> #include <dt-bindings/power/qcom-rpmpd.h>
> @@ -48,6 +49,7 @@ struct rpmhpd {
> bool enabled;
> const char *res_name;
> u32 addr;
> + bool is_warming_dev;
> };
>
> struct rpmhpd_desc {
> @@ -55,6 +57,8 @@ struct rpmhpd_desc {
> size_t num_pds;
> };
>
> +const struct rpmhpd_desc *global_desc;
> +
> static DEFINE_MUTEX(rpmhpd_lock);
>
> /* SDM845 RPMH powerdomains */
> @@ -89,6 +93,7 @@ static struct rpmhpd sdm845_mx = {
> .pd = { .name = "mx", },
> .peer = &sdm845_mx_ao,
> .res_name = "mx.lvl",
> + .is_warming_dev = true,
> };
>
> static struct rpmhpd sdm845_mx_ao = {
> @@ -452,7 +457,14 @@ static int rpmhpd_probe(struct platform_device *pdev)
> &rpmhpds[i]->pd);
> }
>
> - return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
> + ret = of_genpd_add_provider_onecell(pdev->dev.of_node, data);
> +
> + if (ret)
> + return ret;
> +
> + global_desc = desc;
> +
> + return 0;
> }
>
> static struct platform_driver rpmhpd_driver = {
> @@ -469,3 +481,26 @@ static int __init rpmhpd_init(void)
> return platform_driver_register(&rpmhpd_driver);
> }
> core_initcall(rpmhpd_init);
> +
> +static int __init rpmhpd_init_warming_device(void)
> +{
> + size_t num_pds;
> + struct rpmhpd **rpmhpds;
> + int i;
> +
> + if (!global_desc)
> + return -EINVAL;
> +
> + rpmhpds = global_desc->rpmhpds;
> + num_pds = global_desc->num_pds;
> +
> + if (!of_find_property(rpmhpds[0]->dev->of_node, "#cooling-cells", NULL))
> + return 0;
> +
> + for (i = 0; i < num_pds; i++)
> + if (rpmhpds[i]->is_warming_dev)
> + of_pd_warming_register(rpmhpds[i]->dev, i);
> +
> + return 0;
> +}
> +late_initcall(rpmhpd_init_warming_device);
...why can't this be done in rpmhpd_probe()?
In particular with the recent patches from John Stultz to allow rpmhpd
to be built as a module I don't think there's any guarantees that
rpmh_probe() will have succeeded before rpmhpd_init_warming_device()
executes.
Regards,
Bjorn
On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
> RPMh hosts mx power domain that can be used to warm up the SoC. Indicate
> this by using #cooling-cells property.
>
> Reviewed-by: Ulf Hansson <[email protected]>
> Signed-off-by: Thara Gopinath <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
> ---
>
> v3->v4:
> - Removed subnode to indicate that mx power domain is a warming
> device. Instead #cooling-cells is used as a power domain
> provider property to indicate if the provider hosts a power
> domain that can be used as a warming device.
>
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index fe35d37a11cc..0d878b2ca351 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -3703,6 +3703,7 @@
> rpmhpd: power-controller {
> compatible = "qcom,sdm845-rpmhpd";
> #power-domain-cells = <1>;
> + #cooling-cells = <2>;
> operating-points-v2 = <&rpmhpd_opp_table>;
>
> rpmhpd_opp_table: opp-table {
> --
> 2.20.1
>
On 3/27/20 6:15 PM, Bjorn Andersson wrote:
> On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
>
>> Populate .get_performace_state_count in genpd ops to retrieve the count of
>> performance states supported by a rpmh power domain.
>>
>> Reviewed-by: Ulf Hansson <[email protected]>
>> Signed-off-by: Thara Gopinath <[email protected]>
>> ---
>> drivers/soc/qcom/rpmhpd.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
>> index 4d264d0672c4..7142409a3b77 100644
>> --- a/drivers/soc/qcom/rpmhpd.c
>> +++ b/drivers/soc/qcom/rpmhpd.c
>> @@ -341,6 +341,13 @@ static unsigned int rpmhpd_get_performance_state(struct generic_pm_domain *genpd
>> return dev_pm_opp_get_level(opp);
>> }
>>
>> +static int rpmhpd_performance_states_count(struct generic_pm_domain *domain)
>> +{
>> + struct rpmhpd *pd = domain_to_rpmhpd(domain);
>> +
>> + return pd->level_count;
>> +}
>> +
>> static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
>> {
>> int i;
>> @@ -429,6 +436,8 @@ static int rpmhpd_probe(struct platform_device *pdev)
>> rpmhpds[i]->pd.power_on = rpmhpd_power_on;
>> rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance_state;
>> rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance_state;
>> + rpmhpds[i]->pd.get_performance_state_count =
>> + rpmhpd_performance_states_count;
>
> I would prefer if you ignore the 80-char limit here and leave the line
> unwrapped.
Hi Bjorn,
Thanks for the reviews. I will fix this in the next version.
>
> Reviewed-by: Bjorn Andersson <[email protected]>
>
> Regards,
> Bjorn
>
>> pm_genpd_init(&rpmhpds[i]->pd, NULL, true);
>>
>> data->domains[i] = &rpmhpds[i]->pd;
>> --
>> 2.20.1
>>
--
Warm Regards
Thara
On 3/27/20 6:53 PM, Bjorn Andersson wrote:
> On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
>
>> RPMh power control hosts power domains that can be used as
>> thermal warming devices. Register these power domains
>> with the generic power domain warming device thermal framework.
>>
>> Reviewed-by: Ulf Hansson <[email protected]>
>> Signed-off-by: Thara Gopinath <[email protected]>
>> ---
>>
>> v3->v4:
>> - Introduce a boolean value is_warming_dev in rpmhpd structure to
>> indicate if a generic power domain can be used as a warming
>> device or not.With this change, device tree no longer has to
>> specify which power domain inside the rpmh power domain provider
>> is a warming device.
>> - Move registering of warming devices into a late initcall to
>> ensure that warming devices are registered after thermal
>> framework is initialized.
>
> This information is lost when we merge patches, as such I would like
> such design decisions to be described in the commit message itself.
> But...
>
>>
>> drivers/soc/qcom/rpmhpd.c | 37 ++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
>> index 7142409a3b77..4e9c0bbb8826 100644
>> --- a/drivers/soc/qcom/rpmhpd.c
>> +++ b/drivers/soc/qcom/rpmhpd.c
>> @@ -11,6 +11,7 @@
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_opp.h>
>> +#include <linux/pd_warming.h>
>> #include <soc/qcom/cmd-db.h>
>> #include <soc/qcom/rpmh.h>
>> #include <dt-bindings/power/qcom-rpmpd.h>
>> @@ -48,6 +49,7 @@ struct rpmhpd {
>> bool enabled;
>> const char *res_name;
>> u32 addr;
>> + bool is_warming_dev;
>> };
>>
>> struct rpmhpd_desc {
>> @@ -55,6 +57,8 @@ struct rpmhpd_desc {
>> size_t num_pds;
>> };
>>
>> +const struct rpmhpd_desc *global_desc;
>> +
>> static DEFINE_MUTEX(rpmhpd_lock);
>>
>> /* SDM845 RPMH powerdomains */
>> @@ -89,6 +93,7 @@ static struct rpmhpd sdm845_mx = {
>> .pd = { .name = "mx", },
>> .peer = &sdm845_mx_ao,
>> .res_name = "mx.lvl",
>> + .is_warming_dev = true,
>> };
>>
>> static struct rpmhpd sdm845_mx_ao = {
>> @@ -452,7 +457,14 @@ static int rpmhpd_probe(struct platform_device *pdev)
>> &rpmhpds[i]->pd);
>> }
>>
>> - return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
>> + ret = of_genpd_add_provider_onecell(pdev->dev.of_node, data);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + global_desc = desc;
>> +
>> + return 0;
>> }
>>
>> static struct platform_driver rpmhpd_driver = {
>> @@ -469,3 +481,26 @@ static int __init rpmhpd_init(void)
>> return platform_driver_register(&rpmhpd_driver);
>> }
>> core_initcall(rpmhpd_init);
>> +
>> +static int __init rpmhpd_init_warming_device(void)
>> +{
>> + size_t num_pds;
>> + struct rpmhpd **rpmhpds;
>> + int i;
>> +
>> + if (!global_desc)
>> + return -EINVAL;
>> +
>> + rpmhpds = global_desc->rpmhpds;
>> + num_pds = global_desc->num_pds;
>> +
>> + if (!of_find_property(rpmhpds[0]->dev->of_node, "#cooling-cells", NULL))
>> + return 0;
>> +
>> + for (i = 0; i < num_pds; i++)
>> + if (rpmhpds[i]->is_warming_dev)
>> + of_pd_warming_register(rpmhpds[i]->dev, i);
>> +
>> + return 0;
>> +}
>> +late_initcall(rpmhpd_init_warming_device);
>
> ...why can't this be done in rpmhpd_probe()?
>
> In particular with the recent patches from John Stultz to allow rpmhpd
> to be built as a module I don't think there's any guarantees that
> rpmh_probe() will have succeeded before rpmhpd_init_warming_device()
> executes.
It is to take care of boot order.
So this has to happen after the thermal framework is initialized.
Thermal framework is initialized with core_initcall. Can I move the
rpmhpd init as a postcore_initcall ? Then I can get rid of this separate
function and keep it as part of probe.
>
> Regards,
> Bjorn
>
--
Warm Regards
Thara
On Mon 30 Mar 07:53 PDT 2020, Thara Gopinath wrote:
> On 3/27/20 6:53 PM, Bjorn Andersson wrote:
> > On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote:
[..]
> > > +static int __init rpmhpd_init_warming_device(void)
> > > +{
> > > + size_t num_pds;
> > > + struct rpmhpd **rpmhpds;
> > > + int i;
> > > +
> > > + if (!global_desc)
> > > + return -EINVAL;
> > > +
> > > + rpmhpds = global_desc->rpmhpds;
> > > + num_pds = global_desc->num_pds;
> > > +
> > > + if (!of_find_property(rpmhpds[0]->dev->of_node, "#cooling-cells", NULL))
> > > + return 0;
> > > +
> > > + for (i = 0; i < num_pds; i++)
> > > + if (rpmhpds[i]->is_warming_dev)
> > > + of_pd_warming_register(rpmhpds[i]->dev, i);
> > > +
> > > + return 0;
> > > +}
> > > +late_initcall(rpmhpd_init_warming_device);
> >
> > ...why can't this be done in rpmhpd_probe()?
> >
> > In particular with the recent patches from John Stultz to allow rpmhpd
> > to be built as a module I don't think there's any guarantees that
> > rpmh_probe() will have succeeded before rpmhpd_init_warming_device()
> > executes.
>
> It is to take care of boot order.
Understood.
> So this has to happen after the thermal framework is initialized. Thermal
> framework is initialized with core_initcall. Can I move the rpmhpd init as a
> postcore_initcall ? Then I can get rid of this separate function and keep it
> as part of probe.
>
So I presume the problem is that if this is called from probe, you might
of_pd_warming_register(), which ends up in
__thermal_cooling_device_register() before thermal_init() has been
invoked?
Which is bad because e.g. thermal_class is not yet initialized.
I don't want to rely on the order of initcalls for things to work, so
could we make this more robust by having
thermal_of_cooling_device_register() return -EPROBE_DEFER is
thermal_init() isn't done?
Regards,
Bjorn