2020-06-04 01:58:57

by Thara Gopinath

[permalink] [raw]
Subject: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.

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


2020-06-16 09:24:33

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.

On Thu, 4 Jun 2020 at 03:53, 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]>

Reviewed-by: Ulf Hansson <[email protected]>

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
>

2020-07-03 10:02:45

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.


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

2020-07-07 10:59:32

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v6 3/6] thermal: Add generic power domain warming device driver.

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__ */
>>
>
>