| Message ID | 1571254641-13626-5-git-send-email-thara.gopinath@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce Power domain based warming device driver | expand |
On 10/17/2019 04:47 AM, Ulf Hansson wrote: > On Wed, 16 Oct 2019 at 21:37, Thara Gopinath <thara.gopinath@linaro.org> wrote: >> >> Resources modeled as power domains in linux kenrel >> 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> >> --- >> drivers/thermal/Kconfig | 10 +++ >> drivers/thermal/Makefile | 2 + >> drivers/thermal/pwr_domain_warming.c | 136 +++++++++++++++++++++++++++++++++++ >> include/linux/pwr_domain_warming.h | 31 ++++++++ >> 4 files changed, 179 insertions(+) >> create mode 100644 drivers/thermal/pwr_domain_warming.c >> create mode 100644 include/linux/pwr_domain_warming.h >> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >> index 001a21a..0c5c93e 100644 >> --- a/drivers/thermal/Kconfig >> +++ b/drivers/thermal/Kconfig >> @@ -187,6 +187,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 74a37c7..382c64a 100644 >> --- a/drivers/thermal/Makefile >> +++ b/drivers/thermal/Makefile >> @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o >> # devfreq cooling >> thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o >> >> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o >> + >> # platform thermal drivers >> obj-y += broadcom/ >> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o >> diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c >> new file mode 100644 >> index 0000000..60fae3e >> --- /dev/null >> +++ b/drivers/thermal/pwr_domain_warming.c >> @@ -0,0 +1,136 @@ >> +// 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/pwr_domain_warming.h> >> + >> +struct pd_warming_device { >> + struct thermal_cooling_device *cdev; >> + struct generic_pm_domain *gpd; > > No, this isn't a genpd provider and thus we should not need to carry > the above pointer in the struct pd_warming_device. I store this to attach the device in late_init. More about this approach below. > >> + struct device *dev; >> + int max_state; >> + int cur_state; >> + bool runtime_resumed; >> +}; >> + >> +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 int pd_wdev_late_init(struct thermal_cooling_device *cdev) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + struct device *dev = &cdev->device; >> + int state_count, ret; >> + >> + ret = pm_genpd_add_device(pd_wdev->gpd, dev); > > The pm_genpd_add_device() is a legacy interface and we are striving to > remove it. I think there are two better options for you to use to deal > with the attach thingy. I was not aware of this. Apologies. > > 1. The easiest one is probably just to convert into using > of_genpd_add_device() instead. I think you already have the > information that you need in the ->cdev pointer to do that. However, > that also means you need to add the ->late_init() callback to the > struct thermal_cooling_device_ops, like what you do here. > > 2. Maybe the most correct solution is, rather than attaching > &cdev->device to the PM domain, to register a separate new device > (device_register()) and assign it the corresponding OF node as the > genpd provider's subnode and then attach this one instead. If > "power-domains" can be specified in the subnode, you can even use > dev_pm_domain_attach() to attach the device to the PM domain, else > of_genpd_add_device() should work. In the second step, when > registering the cooling device, the new device above should be > assigned as parent to the device that is about to be registered via > thermal_of_cooling_device_register(). In other words, the > thermal_of_cooling_device_register() needs to be extended to cope with > receiving a parent device as an in-parameter, but that should be > doable I think. In this way, you don't need to add the ->late_init() > callback at all, but you can instead just use the parent device when > operating on the PM domain. I did toy with registering a separate device vs reusing cdev device. My rational was, the power domain is actually controlled/needed by the cdev and hence should be attached to it. For me either solution is acceptable . It is a trade off between creating a new device and registering it as a parent of cooling device vs introducing a late init. With the second approach I should be able to do away with the generic_pm_domain pointer in pd_warming_device. To register a parent for a cooling device, I will have to introduce a new API in the thermal framework. Like thermal_of_cooling_device_parent_register. I am ok with this as well. I would like to hear on what some of the thermal maintainers/reviewers have to say about both the approaches and which is better. I will wait a few days for others to review and if there are no major comments, I will send across the series after updating it to the second approach. -- Warm Regards Thara
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 001a21a..0c5c93e 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -187,6 +187,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 74a37c7..382c64a 100644 --- a/drivers/thermal/Makefile +++ b/drivers/thermal/Makefile @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o # devfreq cooling thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o + # platform thermal drivers obj-y += broadcom/ obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c new file mode 100644 index 0000000..60fae3e --- /dev/null +++ b/drivers/thermal/pwr_domain_warming.c @@ -0,0 +1,136 @@ +// 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/pwr_domain_warming.h> + +struct pd_warming_device { + struct thermal_cooling_device *cdev; + struct generic_pm_domain *gpd; + struct device *dev; + int max_state; + int cur_state; + bool runtime_resumed; +}; + +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 int pd_wdev_late_init(struct thermal_cooling_device *cdev) +{ + struct pd_warming_device *pd_wdev = cdev->devdata; + struct device *dev = &cdev->device; + int state_count, ret; + + ret = pm_genpd_add_device(pd_wdev->gpd, dev); + + if (ret) + return ret; + + state_count = dev_pm_genpd_performance_state_count(dev); + if (state_count < 0) + return state_count; + + pm_runtime_enable(dev); + + pd_wdev->dev = dev; + pd_wdev->max_state = state_count - 1; + + return 0; +} + +static struct thermal_cooling_device_ops pd_warming_device_ops = { + .late_init = pd_wdev_late_init, + .get_max_state = pd_wdev_get_max_state, + .get_cur_state = pd_wdev_get_cur_state, + .set_cur_state = pd_wdev_set_cur_state, +}; + +struct thermal_cooling_device * +pwr_domain_warming_register(struct device_node *node, + struct generic_pm_domain *gpd) +{ + struct pd_warming_device *pd_wdev; + + pd_wdev = kzalloc(sizeof(*pd_wdev), GFP_KERNEL); + if (!pd_wdev) + return ERR_PTR(-ENOMEM); + + pd_wdev->runtime_resumed = false; + pd_wdev->gpd = gpd; + + pd_wdev->cdev = thermal_of_cooling_device_register + (node, gpd->name, pd_wdev, + &pd_warming_device_ops); + if (IS_ERR(pd_wdev->cdev)) { + pr_err("unable to register %s cooling device\n", gpd->name); + if (pd_wdev->dev) + pm_runtime_disable(pd_wdev->dev); + } + + return pd_wdev->cdev; +} +EXPORT_SYMBOL_GPL(pwr_domain_warming_register); + +void pwr_domain_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(pd_wdev->dev); + thermal_cooling_device_unregister(cdev); + kfree(pd_wdev); +} +EXPORT_SYMBOL_GPL(pwr_domain_warming_unregister); diff --git a/include/linux/pwr_domain_warming.h b/include/linux/pwr_domain_warming.h new file mode 100644 index 0000000..ed9942b --- /dev/null +++ b/include/linux/pwr_domain_warming.h @@ -0,0 +1,31 @@ +// 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 * +pwr_domain_warming_register(struct device_node *node, + struct generic_pm_domain *gpd); + +void pwr_domain_warming_unregister(struct thermal_cooling_device *cdev); + +#else +static inline struct thermal_cooling_device * +pwr_domain_warming_register(struct device_node *node, + struct generic_pm_domain *gpd) +{ + return ERR_PTR(-ENOSYS); +} + +static inline void +pwr_domain_warming_unregister(struct thermal_cooling_device *cdev) +{ +} +#endif /* CONFIG_PWR_DOMAIN_WARMING_THERMAL */ +#endif /* __PWR_DOMAIN_WARMING_H__ */
Resources modeled as power domains in linux kenrel 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> --- drivers/thermal/Kconfig | 10 +++ drivers/thermal/Makefile | 2 + drivers/thermal/pwr_domain_warming.c | 136 +++++++++++++++++++++++++++++++++++ include/linux/pwr_domain_warming.h | 31 ++++++++ 4 files changed, 179 insertions(+) create mode 100644 drivers/thermal/pwr_domain_warming.c create mode 100644 include/linux/pwr_domain_warming.h -- 2.1.4