linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/6] Introduce Power domain based warming device driver
@ 2020-06-04  1:53 Thara Gopinath
  2020-06-04  1:53 ` [PATCH v6 1/6] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

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.

v5->v6:
	- Rebased to latest kernel
	- Few other fixes identified in the review process 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                     |  25 ++-
 drivers/thermal/Kconfig                       |  10 ++
 drivers/thermal/Makefile                      |   4 +
 drivers/thermal/pd_warming.c                  | 169 ++++++++++++++++++
 include/linux/pd_warming.h                    |  29 +++
 include/linux/pm_domain.h                     |  13 ++
 9 files changed, 290 insertions(+), 1 deletion(-)
 create mode 100644 drivers/thermal/pd_warming.c
 create mode 100644 include/linux/pd_warming.h

-- 
2.20.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v6 1/6] PM/Domains: Add support for retrieving genpd performance states information
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
@ 2020-06-04  1:53 ` Thara Gopinath
  2020-06-04  1:53 ` [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

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.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 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 0a01df608849..88f8773cabe7 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
  2020-06-04  1:53 ` [PATCH v6 1/6] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath
@ 2020-06-04  1:53 ` Thara Gopinath
  2020-06-16  9:21   ` Ulf Hansson
  2020-06-04  1:53 ` [PATCH v6 3/6] thermal: Add generic power domain warming device driver Thara Gopinath
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

Populate .get_performance_state_count in genpd ops to retrieve the count of
performance states supported by a rpmh power domain.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/rpmhpd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index e72426221a69..a9c597143525 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -362,6 +362,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;
@@ -450,6 +457,7 @@ 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 3/6] thermal: Add generic power domain warming device driver.
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
  2020-06-04  1:53 ` [PATCH v6 1/6] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath
  2020-06-04  1:53 ` [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath
@ 2020-06-04  1:53 ` Thara Gopinath
  2020-06-16  9:21   ` Ulf Hansson
  2020-07-03 10:02   ` Daniel Lezcano
  2020-06-04  1:53 ` [PATCH v6 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices Thara Gopinath
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

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 <thara.gopinath@linaro.org>
---

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.
v5->v6:
	- Fixed issues with ->release() and kfree(dev) as pointed
	  out by Ulf.

 drivers/thermal/Kconfig      |  10 +++
 drivers/thermal/Makefile     |   4 +
 drivers/thermal/pd_warming.c | 169 +++++++++++++++++++++++++++++++++++
 include/linux/pd_warming.h   |  29 ++++++
 4 files changed, 212 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 e53314ea9e25..3a0bcf3e8bd9 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 86c506410cc0..14fa696a08bd 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -28,7 +28,11 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
 # devfreq cooling
 thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
 
+#pwr domain warming
+thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL)	+= pd_warming.o
+
 obj-$(CONFIG_K3_THERMAL)	+= k3_bandgap.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..1ea93481c79b
--- /dev/null
+++ b/drivers/thermal/pd_warming.c
@@ -0,0 +1,169 @@
+// 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)
+{
+	struct pd_warming_device *pd_wdev;
+
+	pd_wdev = container_of(dev, struct pd_warming_device, dev);
+	kfree(pd_wdev);
+}
+
+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 out;
+	}
+
+	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);
+out:
+	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);
+	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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices.
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
                   ` (2 preceding siblings ...)
  2020-06-04  1:53 ` [PATCH v6 3/6] thermal: Add generic power domain warming device driver Thara Gopinath
@ 2020-06-04  1:53 ` Thara Gopinath
  2020-06-16  9:21   ` Ulf Hansson
  2020-06-04  1:53 ` [PATCH v6 5/6] dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device Thara Gopinath
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

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.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---

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.

v5->v6:
	- Moved back registering of warming devices into probe since
	  Bjorn pointed out that now the driver can be initialized as
	  as a module, late_initcall will not work. Thermal framework
	  takes care of binding a cooling device to a thermal zone even
	  if the cooling device is registered before the thermal framework
	  is initialized.

 drivers/soc/qcom/rpmhpd.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index a9c597143525..29e1eb4d11af 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -12,6 +12,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>
@@ -49,6 +50,7 @@ struct rpmhpd {
 	bool		enabled;
 	const char	*res_name;
 	u32		addr;
+	bool		is_warming_dev;
 };
 
 struct rpmhpd_desc {
@@ -90,6 +92,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 = {
@@ -472,7 +475,19 @@ 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;
+
+	if (!of_find_property(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;
 }
 
 static struct platform_driver rpmhpd_driver = {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 5/6] dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
                   ` (3 preceding siblings ...)
  2020-06-04  1:53 ` [PATCH v6 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices Thara Gopinath
@ 2020-06-04  1:53 ` Thara Gopinath
  2020-06-16  9:21   ` Ulf Hansson
  2020-06-04  1:53 ` [PATCH v6 6/6] arm64: dts: qcom: Indicate rpmhpd hosts a power domain that can be used as a " Thara Gopinath
  2020-06-16 10:53 ` [PATCH v6 0/6] Introduce Power domain based warming device driver Pavel Machek
  6 siblings, 1 reply; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

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 <thara.gopinath@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---

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 8058955fb3b9..a4fbbd88ce18 100644
--- a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
@@ -28,6 +28,9 @@ properties:
   '#power-domain-cells':
     const: 1
 
+  '#cooling-cells':
+    const: 2
+
   operating-points-v2: true
 
   opp-table:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v6 6/6] arm64: dts: qcom: Indicate rpmhpd hosts a power domain that can be used as a warming device.
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
                   ` (4 preceding siblings ...)
  2020-06-04  1:53 ` [PATCH v6 5/6] dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device Thara Gopinath
@ 2020-06-04  1:53 ` Thara Gopinath
  2020-06-16 10:53 ` [PATCH v6 0/6] Introduce Power domain based warming device driver Pavel Machek
  6 siblings, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2020-06-04  1:53 UTC (permalink / raw)
  To: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

RPMh hosts mx power domain that can be used to warm up the SoC.  Indicate
this by using #cooling-cells property.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

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 8eb5a31346d2..dcc3bcd16b68 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3854,6 +3854,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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count
  2020-06-04  1:53 ` [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath
@ 2020-06-16  9:21   ` Ulf Hansson
  2020-06-16 17:19     ` Thara Gopinath
  0 siblings, 1 reply; 17+ messages in thread
From: Ulf Hansson @ 2020-06-16  9:21 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Zhang Rui, Daniel Lezcano, Bjorn Andersson, Andy Gross,
	Rob Herring, Amit Kucheria, Mark Rutland, Rafael J. Wysocki,
	Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List

On Thu, 4 Jun 2020 at 03:53, Thara Gopinath <thara.gopinath@linaro.org> wrote:
>
> Populate .get_performance_state_count in genpd ops to retrieve the count of
> performance states supported by a rpmh power domain.
>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>  drivers/soc/qcom/rpmhpd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index e72426221a69..a9c597143525 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -362,6 +362,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;
> @@ -450,6 +457,7 @@ 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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.
  2020-06-04  1:53 ` [PATCH v6 3/6] thermal: Add generic power domain warming device driver Thara Gopinath
@ 2020-06-16  9:21   ` Ulf Hansson
  2020-07-03 10:02   ` Daniel Lezcano
  1 sibling, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2020-06-16  9:21 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Zhang Rui, Daniel Lezcano, Bjorn Andersson, Andy Gross,
	Rob Herring, Amit Kucheria, Mark Rutland, Rafael J. Wysocki,
	Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List

On Thu, 4 Jun 2020 at 03:53, Thara Gopinath <thara.gopinath@linaro.org> 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 <thara.gopinath@linaro.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe



> ---
>
> 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.
> v5->v6:
>         - Fixed issues with ->release() and kfree(dev) as pointed
>           out by Ulf.
>
>  drivers/thermal/Kconfig      |  10 +++
>  drivers/thermal/Makefile     |   4 +
>  drivers/thermal/pd_warming.c | 169 +++++++++++++++++++++++++++++++++++
>  include/linux/pd_warming.h   |  29 ++++++
>  4 files changed, 212 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 e53314ea9e25..3a0bcf3e8bd9 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 86c506410cc0..14fa696a08bd 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -28,7 +28,11 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)  += clock_cooling.o
>  # devfreq cooling
>  thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>
> +#pwr domain warming
> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL)       += pd_warming.o
> +
>  obj-$(CONFIG_K3_THERMAL)       += k3_bandgap.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..1ea93481c79b
> --- /dev/null
> +++ b/drivers/thermal/pd_warming.c
> @@ -0,0 +1,169 @@
> +// 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)
> +{
> +       struct pd_warming_device *pd_wdev;
> +
> +       pd_wdev = container_of(dev, struct pd_warming_device, dev);
> +       kfree(pd_wdev);
> +}
> +
> +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 out;
> +       }
> +
> +       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);
> +out:
> +       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);
> +       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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices.
  2020-06-04  1:53 ` [PATCH v6 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices Thara Gopinath
@ 2020-06-16  9:21   ` Ulf Hansson
  0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2020-06-16  9:21 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Zhang Rui, Daniel Lezcano, Bjorn Andersson, Andy Gross,
	Rob Herring, Amit Kucheria, Mark Rutland, Rafael J. Wysocki,
	Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List

On Thu, 4 Jun 2020 at 03:53, Thara Gopinath <thara.gopinath@linaro.org> 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.
>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>
> 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.
>
> v5->v6:
>         - Moved back registering of warming devices into probe since
>           Bjorn pointed out that now the driver can be initialized as
>           as a module, late_initcall will not work. Thermal framework
>           takes care of binding a cooling device to a thermal zone even
>           if the cooling device is registered before the thermal framework
>           is initialized.
>
>  drivers/soc/qcom/rpmhpd.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index a9c597143525..29e1eb4d11af 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -12,6 +12,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>
> @@ -49,6 +50,7 @@ struct rpmhpd {
>         bool            enabled;
>         const char      *res_name;
>         u32             addr;
> +       bool            is_warming_dev;
>  };
>
>  struct rpmhpd_desc {
> @@ -90,6 +92,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 = {
> @@ -472,7 +475,19 @@ 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;
> +
> +       if (!of_find_property(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;
>  }
>
>  static struct platform_driver rpmhpd_driver = {
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 5/6] dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device
  2020-06-04  1:53 ` [PATCH v6 5/6] dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device Thara Gopinath
@ 2020-06-16  9:21   ` Ulf Hansson
  0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2020-06-16  9:21 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Zhang Rui, Daniel Lezcano, Bjorn Andersson, Andy Gross,
	Rob Herring, Amit Kucheria, Mark Rutland, Rafael J. Wysocki,
	Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List

On Thu, 4 Jun 2020 at 03:53, Thara Gopinath <thara.gopinath@linaro.org> 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 <thara.gopinath@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>
> 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 8058955fb3b9..a4fbbd88ce18 100644
> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
> @@ -28,6 +28,9 @@ properties:
>    '#power-domain-cells':
>      const: 1
>
> +  '#cooling-cells':
> +    const: 2
> +
>    operating-points-v2: true
>
>    opp-table:
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 0/6] Introduce Power domain based warming device driver
  2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
                   ` (5 preceding siblings ...)
  2020-06-04  1:53 ` [PATCH v6 6/6] arm64: dts: qcom: Indicate rpmhpd hosts a power domain that can be used as a " Thara Gopinath
@ 2020-06-16 10:53 ` Pavel Machek
  2020-06-16 17:24   ` Thara Gopinath
  6 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2020-06-16 10:53 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross,
	robh, amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

Hi!

> 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).

Would you explain when this is needed?

I'd normally expect "too low" temperature to be a problem during power-on, but at
that time Linux is not running so it can not provide the heating...

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count
  2020-06-16  9:21   ` Ulf Hansson
@ 2020-06-16 17:19     ` Thara Gopinath
  0 siblings, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2020-06-16 17:19 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Zhang Rui, Daniel Lezcano, Bjorn Andersson, Andy Gross,
	Rob Herring, Amit Kucheria, Mark Rutland, Rafael J. Wysocki,
	Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List



On 6/16/20 5:21 AM, Ulf Hansson wrote:
> On Thu, 4 Jun 2020 at 03:53, Thara Gopinath <thara.gopinath@linaro.org> wrote:
>>
>> Populate .get_performance_state_count in genpd ops to retrieve the count of
>> performance states supported by a rpmh power domain.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks Ulf for all the reviews.
> 
> Kind regards
> Uffe
> 


-- 
Warm Regards
Thara

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 0/6] Introduce Power domain based warming device driver
  2020-06-16 10:53 ` [PATCH v6 0/6] Introduce Power domain based warming device driver Pavel Machek
@ 2020-06-16 17:24   ` Thara Gopinath
  2020-06-17 21:14     ` Pavel Machek
  0 siblings, 1 reply; 17+ messages in thread
From: Thara Gopinath @ 2020-06-16 17:24 UTC (permalink / raw)
  To: Pavel Machek
  Cc: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross,
	robh, amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel



On 6/16/20 6:53 AM, Pavel Machek wrote:
> Hi!
> 
>> 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).
> 
> Would you explain when this is needed?
> 
> I'd normally expect "too low" temperature to be a problem during power-on, but at
> that time Linux is not running so it can not provide the heating...
Hi Pavel,

This is more in the scenario if the system in on and temperature is 
dipping (I have been told in colder climates). Idea is to turn on 
resources so as to prevent further dipping of temperature if possible.

> 
> Best regards,
> 
> 									Pavel
> 

-- 
Warm Regards
Thara

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 0/6] Introduce Power domain based warming device driver
  2020-06-16 17:24   ` Thara Gopinath
@ 2020-06-17 21:14     ` Pavel Machek
  0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2020-06-17 21:14 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross,
	robh, amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

Hi!

> >>falls below certain threshold. These power domains can be considered as
> >>thermal warming devices.  (opposite of thermal cooling devices).
> >
> >Would you explain when this is needed?
> >
> >I'd normally expect "too low" temperature to be a problem during power-on, but at
> >that time Linux is not running so it can not provide the heating...
> Hi Pavel,
> 
> This is more in the scenario if the system in on and temperature is dipping
> (I have been told in colder climates). Idea is to turn on resources so as to
> prevent further dipping of temperature if possible.

I guess even that makes sense...

But, out of curiosity, do you know which kind of device is that and in what
kind of environment? I mean, theoretically it may make sense on a cellphone,
but... I guess you have some fun device and would like to know what it is :-).

Hmm. And we can make this quite generic.

while (too_cold())
	barrier();

Wasting power is really easy :-).
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.
  2020-06-04  1:53 ` [PATCH v6 3/6] thermal: Add generic power domain warming device driver Thara Gopinath
  2020-06-16  9:21   ` Ulf Hansson
@ 2020-07-03 10:02   ` Daniel Lezcano
  2020-07-07 10:58     ` Lukasz Luba
  1 sibling, 1 reply; 17+ messages in thread
From: Daniel Lezcano @ 2020-07-03 10:02 UTC (permalink / raw)
  To: Thara Gopinath, rui.zhang, ulf.hansson, bjorn.andersson, agross, robh
  Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel, Lukasz Luba


Hi Thara,

sorry for the delay.

Added Lukasz.

On 04/06/2020 03:53, Thara Gopinath 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).

The patch itself looks fine but I'm not very convinced about using a
specific driver as a warming device.

It is all about changing the performance state of a device and the power
domain is a way to access the associated callback.

It could be used as a cooling device as well.

The cpufreq cooling device could be used as a warming device and the way
we access the performance state is the freq qos.

We end up with different ways to do the same thing : change the
performance state.

On the other side, the energy model is being moved to the struct device,
so we will have gpu and cpu using it to retrieve a performance state
given a power value.

My opinion is instead of multiplying the ways to do the same think, we
should find a way to unify all the passive cooling devices into a single
generic performance state based mitigation device.

It does not imply to make all the passive cooling device changes but
provide a generic one first.

The ideal would be to register a struct device as a performance state
capable device and use the energy model stored in it.

In the meantime, the energy model should embed a couple of callbacks
get_power / set_power to set the performance state.

That implies a bit of more work, but IMHO it is worth to do.

Does it make sense ?


> ---
> 
> 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.
> v5->v6:
> 	- Fixed issues with ->release() and kfree(dev) as pointed
> 	  out by Ulf.
> 
>  drivers/thermal/Kconfig      |  10 +++
>  drivers/thermal/Makefile     |   4 +
>  drivers/thermal/pd_warming.c | 169 +++++++++++++++++++++++++++++++++++
>  include/linux/pd_warming.h   |  29 ++++++
>  4 files changed, 212 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 e53314ea9e25..3a0bcf3e8bd9 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 86c506410cc0..14fa696a08bd 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -28,7 +28,11 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
>  # devfreq cooling
>  thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>  
> +#pwr domain warming
> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL)	+= pd_warming.o
> +
>  obj-$(CONFIG_K3_THERMAL)	+= k3_bandgap.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..1ea93481c79b
> --- /dev/null
> +++ b/drivers/thermal/pd_warming.c
> @@ -0,0 +1,169 @@
> +// 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)
> +{
> +	struct pd_warming_device *pd_wdev;
> +
> +	pd_wdev = container_of(dev, struct pd_warming_device, dev);
> +	kfree(pd_wdev);
> +}
> +
> +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 out;
> +	}
> +
> +	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);
> +out:
> +	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);
> +	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__ */
> 


-- 
<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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.
  2020-07-03 10:02   ` Daniel Lezcano
@ 2020-07-07 10:58     ` Lukasz Luba
  0 siblings, 0 replies; 17+ messages in thread
From: Lukasz Luba @ 2020-07-07 10:58 UTC (permalink / raw)
  To: Daniel Lezcano, Thara Gopinath
  Cc: rui.zhang, ulf.hansson, bjorn.andersson, agross, robh,
	amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
	linux-arm-msm, linux-kernel

Hi Daniel and Thara

On 7/3/20 11:02 AM, Daniel Lezcano wrote:
> 
> Hi Thara,
> 
> sorry for the delay.
> 
> Added Lukasz.

Thank you for adding me.
I will go through the patches to understand them and build
the context. I can see some interesting idea described below.

Regards,
Lukasz

> 
> On 04/06/2020 03:53, Thara Gopinath 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).
> 
> The patch itself looks fine but I'm not very convinced about using a
> specific driver as a warming device.
> 
> It is all about changing the performance state of a device and the power
> domain is a way to access the associated callback.
> 
> It could be used as a cooling device as well.
> 
> The cpufreq cooling device could be used as a warming device and the way
> we access the performance state is the freq qos.
> 
> We end up with different ways to do the same thing : change the
> performance state.
> 
> On the other side, the energy model is being moved to the struct device,
> so we will have gpu and cpu using it to retrieve a performance state
> given a power value.
> 
> My opinion is instead of multiplying the ways to do the same think, we
> should find a way to unify all the passive cooling devices into a single
> generic performance state based mitigation device.
> 
> It does not imply to make all the passive cooling device changes but
> provide a generic one first.
> 
> The ideal would be to register a struct device as a performance state
> capable device and use the energy model stored in it.
> 
> In the meantime, the energy model should embed a couple of callbacks
> get_power / set_power to set the performance state.
> 
> That implies a bit of more work, but IMHO it is worth to do.
> 
> Does it make sense ?
> 
> 
>> ---
>>
>> 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.
>> v5->v6:
>> 	- Fixed issues with ->release() and kfree(dev) as pointed
>> 	  out by Ulf.
>>
>>   drivers/thermal/Kconfig      |  10 +++
>>   drivers/thermal/Makefile     |   4 +
>>   drivers/thermal/pd_warming.c | 169 +++++++++++++++++++++++++++++++++++
>>   include/linux/pd_warming.h   |  29 ++++++
>>   4 files changed, 212 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 e53314ea9e25..3a0bcf3e8bd9 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 86c506410cc0..14fa696a08bd 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -28,7 +28,11 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
>>   # devfreq cooling
>>   thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>>   
>> +#pwr domain warming
>> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL)	+= pd_warming.o
>> +
>>   obj-$(CONFIG_K3_THERMAL)	+= k3_bandgap.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..1ea93481c79b
>> --- /dev/null
>> +++ b/drivers/thermal/pd_warming.c
>> @@ -0,0 +1,169 @@
>> +// 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)
>> +{
>> +	struct pd_warming_device *pd_wdev;
>> +
>> +	pd_wdev = container_of(dev, struct pd_warming_device, dev);
>> +	kfree(pd_wdev);
>> +}
>> +
>> +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 out;
>> +	}
>> +
>> +	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);
>> +out:
>> +	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);
>> +	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__ */
>>
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-07-07 10:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04  1:53 [PATCH v6 0/6] Introduce Power domain based warming device driver Thara Gopinath
2020-06-04  1:53 ` [PATCH v6 1/6] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath
2020-06-04  1:53 ` [PATCH v6 2/6] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath
2020-06-16  9:21   ` Ulf Hansson
2020-06-16 17:19     ` Thara Gopinath
2020-06-04  1:53 ` [PATCH v6 3/6] thermal: Add generic power domain warming device driver Thara Gopinath
2020-06-16  9:21   ` Ulf Hansson
2020-07-03 10:02   ` Daniel Lezcano
2020-07-07 10:58     ` Lukasz Luba
2020-06-04  1:53 ` [PATCH v6 4/6] soc: qcom: Extend RPMh power controller driver to register warming devices Thara Gopinath
2020-06-16  9:21   ` Ulf Hansson
2020-06-04  1:53 ` [PATCH v6 5/6] dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device Thara Gopinath
2020-06-16  9:21   ` Ulf Hansson
2020-06-04  1:53 ` [PATCH v6 6/6] arm64: dts: qcom: Indicate rpmhpd hosts a power domain that can be used as a " Thara Gopinath
2020-06-16 10:53 ` [PATCH v6 0/6] Introduce Power domain based warming device driver Pavel Machek
2020-06-16 17:24   ` Thara Gopinath
2020-06-17 21:14     ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).