| Message ID | 20200320014107.26087-1-thara.gopinath@linaro.org |
|---|---|
| Headers | show |
| Series | Introduce Power domain based warming device driver | expand |
On Fri, 20 Mar 2020 at 02:41, 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> > --- > > v3->v4: > - Removed late_init hook pd_warming_device_ops. > - Use of_genpd_add_device instead of pm_genpd_add_device to attach > device to the generic power domain. > - Use thermal_of_cooling_device_parent_register to register the > cooling device so that the device with genpd attached can be > made parent of the cooling device. > - With above changes, remove reference to generic_pm_domain in > pd_warming_device. > > v4->v5: > - All the below changes are as per Ulf's review comments. > - Renamed pwr_domain_warming.c and pwr_domain_warming.h to > pd_warming.c and pd_warming.h. > - Renamed pwr_domain_warming_register API to > of_pd_warming_register. > - Dropped in-param pd_name to of_pd_warming_register. > - Introduced ID allocator to uniquely identify each power domain > warming device. > - Introduced pd_warming_release to handle device kfree for > pd_warming_device. > - Introduced pm_genpd_remove_device in the error exit path > of of_pd_warming_register. > > drivers/thermal/Kconfig | 10 +++ > drivers/thermal/Makefile | 2 + > drivers/thermal/pd_warming.c | 168 +++++++++++++++++++++++++++++++++++ > include/linux/pd_warming.h | 29 ++++++ > 4 files changed, 209 insertions(+) > create mode 100644 drivers/thermal/pd_warming.c > create mode 100644 include/linux/pd_warming.h > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 4d6753f2b18f..92522d541d0e 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -206,6 +206,16 @@ config DEVFREQ_THERMAL > > If you want this support, you should say Y here. > > +config PWR_DOMAIN_WARMING_THERMAL > + bool "Power Domain based warming device" > + depends on PM_GENERIC_DOMAINS_OF > + help > + This implements the generic power domain based warming > + mechanism through increasing the performance state of > + a power domain. > + > + If you want this support, you should say Y here. > + > config THERMAL_EMULATION > bool "Thermal emulation mode support" > help > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 8c8ed7b79915..7db87a779126 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -28,6 +28,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o > # devfreq cooling > thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > > +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pd_warming.o > + > # platform thermal drivers > obj-y += broadcom/ > obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > diff --git a/drivers/thermal/pd_warming.c b/drivers/thermal/pd_warming.c > new file mode 100644 > index 000000000000..c0854d2e4b92 > --- /dev/null > +++ b/drivers/thermal/pd_warming.c > @@ -0,0 +1,168 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019, Linaro Ltd > + */ > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > +#include <linux/slab.h> > +#include <linux/pd_warming.h> > + > +struct pd_warming_device { > + struct thermal_cooling_device *cdev; > + struct device dev; > + int id; > + int max_state; > + int cur_state; > + bool runtime_resumed; > +}; > + > +static DEFINE_IDA(pd_ida); > + > +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, > + unsigned long *state) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + > + *state = pd_wdev->max_state; > + return 0; > +} > + > +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, > + unsigned long *state) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + > + *state = dev_pm_genpd_get_performance_state(&pd_wdev->dev); > + > + return 0; > +} > + > +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, > + unsigned long state) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + struct device *dev = &pd_wdev->dev; > + int ret; > + > + ret = dev_pm_genpd_set_performance_state(dev, state); > + > + if (ret) > + return ret; > + > + if (state && !pd_wdev->runtime_resumed) { > + ret = pm_runtime_get_sync(dev); > + pd_wdev->runtime_resumed = true; > + } else if (!state && pd_wdev->runtime_resumed) { > + ret = pm_runtime_put(dev); > + pd_wdev->runtime_resumed = false; > + } > + > + return ret; > +} > + > +static struct thermal_cooling_device_ops pd_warming_device_ops = { > + .get_max_state = pd_wdev_get_max_state, > + .get_cur_state = pd_wdev_get_cur_state, > + .set_cur_state = pd_wdev_set_cur_state, > +}; > + > +static void pd_warming_release(struct device *dev) > +{ > + kfree(dev); This is wrong, you should free a "struct pd_warming_device *". Use the "container of" macro to get it from 'dev'. > +} > + > +struct thermal_cooling_device * > +of_pd_warming_register(struct device *parent, int pd_id) > +{ > + struct pd_warming_device *pd_wdev; > + struct of_phandle_args pd_args; > + char cdev_name[THERMAL_NAME_LENGTH]; > + int ret; > + > + pd_wdev = kzalloc(sizeof(*pd_wdev), GFP_KERNEL); > + if (!pd_wdev) > + return ERR_PTR(-ENOMEM); > + > + dev_set_name(&pd_wdev->dev, "%s_%d_warming_dev", > + dev_name(parent), pd_id); > + pd_wdev->dev.parent = parent; > + pd_wdev->dev.release = pd_warming_release; > + > + ret = device_register(&pd_wdev->dev); > + if (ret) { > + put_device(&pd_wdev->dev); > + goto free_pd_wdev; > + } > + > + ret = ida_simple_get(&pd_ida, 0, 0, GFP_KERNEL); > + if (ret < 0) > + goto unregister_device; If you use and ida, you might as well use it as a part of the dev_set_name() above. That should give you a unique name, similar to how you use it for the cdev_name below. > + > + pd_wdev->id = ret; > + > + pd_args.np = parent->of_node; > + pd_args.args[0] = pd_id; > + pd_args.args_count = 1; > + > + ret = of_genpd_add_device(&pd_args, &pd_wdev->dev); > + > + if (ret) > + goto remove_ida; > + > + ret = dev_pm_genpd_performance_state_count(&pd_wdev->dev); > + if (ret < 0) > + goto out_genpd; > + > + pd_wdev->max_state = ret - 1; > + pm_runtime_enable(&pd_wdev->dev); > + pd_wdev->runtime_resumed = false; > + > + snprintf(cdev_name, sizeof(cdev_name), "thermal-pd-%d", pd_wdev->id); > + pd_wdev->cdev = thermal_of_cooling_device_register > + (NULL, cdev_name, pd_wdev, > + &pd_warming_device_ops); > + if (IS_ERR(pd_wdev->cdev)) { > + pr_err("unable to register %s cooling device\n", cdev_name); > + ret = PTR_ERR(pd_wdev->cdev); > + goto out_runtime_disable; > + } > + > + return pd_wdev->cdev; > + > +out_runtime_disable: > + pm_runtime_disable(&pd_wdev->dev); > +out_genpd: > + pm_genpd_remove_device(&pd_wdev->dev); > +remove_ida: > + ida_simple_remove(&pd_ida, pd_wdev->id); > +unregister_device: > + device_unregister(&pd_wdev->dev); > + pd_warming_release(&pd_wdev->dev); This is wrong, drop this. > +free_pd_wdev: > + kfree(pd_wdev); Since you should free this from the ->release() callback, there is no need to do this here. > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(of_pd_warming_register); > + > +void pd_warming_unregister(struct thermal_cooling_device *cdev) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + struct device *dev = &pd_wdev->dev; > + > + if (pd_wdev->runtime_resumed) { > + dev_pm_genpd_set_performance_state(dev, 0); > + pm_runtime_put(dev); > + pd_wdev->runtime_resumed = false; > + } > + pm_runtime_disable(dev); > + pm_genpd_remove_device(dev); > + ida_simple_remove(&pd_ida, pd_wdev->id); > + thermal_cooling_device_unregister(cdev); > + kfree(pd_wdev); Don't use kfree here, but instead device_unregister(dev); > +} > +EXPORT_SYMBOL_GPL(pd_warming_unregister); > diff --git a/include/linux/pd_warming.h b/include/linux/pd_warming.h > new file mode 100644 > index 000000000000..550a5683b56d > --- /dev/null > +++ b/include/linux/pd_warming.h > @@ -0,0 +1,29 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019, Linaro Ltd. > + */ > +#ifndef __PWR_DOMAIN_WARMING_H__ > +#define __PWR_DOMAIN_WARMING_H__ > + > +#include <linux/pm_domain.h> > +#include <linux/thermal.h> > + > +#ifdef CONFIG_PWR_DOMAIN_WARMING_THERMAL > +struct thermal_cooling_device * > +of_pd_warming_register(struct device *parent, int pd_id); > + > +void pd_warming_unregister(struct thermal_cooling_device *cdev); > + > +#else > +static inline struct thermal_cooling_device * > +of_pd_warming_register(struct device *parent, int pd_id) > +{ > + return ERR_PTR(-ENOSYS); > +} > + > +static inline void > +pd_warming_unregister(struct thermal_cooling_device *cdev) > +{ > +} > +#endif /* CONFIG_PWR_DOMAIN_WARMING_THERMAL */ > +#endif /* __PWR_DOMAIN_WARMING_H__ */ > -- > 2.20.1 > Besides the few things above, this looks good to me. Kind regards Uffe
On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote: > Populate .get_performace_state_count in genpd ops to retrieve the count of > performance states supported by a rpmh power domain. > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > --- > drivers/soc/qcom/rpmhpd.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c > index 4d264d0672c4..7142409a3b77 100644 > --- a/drivers/soc/qcom/rpmhpd.c > +++ b/drivers/soc/qcom/rpmhpd.c > @@ -341,6 +341,13 @@ static unsigned int rpmhpd_get_performance_state(struct generic_pm_domain *genpd > return dev_pm_opp_get_level(opp); > } > > +static int rpmhpd_performance_states_count(struct generic_pm_domain *domain) > +{ > + struct rpmhpd *pd = domain_to_rpmhpd(domain); > + > + return pd->level_count; > +} > + > static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd) > { > int i; > @@ -429,6 +436,8 @@ static int rpmhpd_probe(struct platform_device *pdev) > rpmhpds[i]->pd.power_on = rpmhpd_power_on; > rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance_state; > rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance_state; > + rpmhpds[i]->pd.get_performance_state_count = > + rpmhpd_performance_states_count; I would prefer if you ignore the 80-char limit here and leave the line unwrapped. Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > pm_genpd_init(&rpmhpds[i]->pd, NULL, true); > > data->domains[i] = &rpmhpds[i]->pd; > -- > 2.20.1 >
On 3/27/20 6:15 PM, Bjorn Andersson wrote: > On Thu 19 Mar 18:41 PDT 2020, Thara Gopinath wrote: > >> Populate .get_performace_state_count in genpd ops to retrieve the count of >> performance states supported by a rpmh power domain. >> >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >> --- >> drivers/soc/qcom/rpmhpd.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c >> index 4d264d0672c4..7142409a3b77 100644 >> --- a/drivers/soc/qcom/rpmhpd.c >> +++ b/drivers/soc/qcom/rpmhpd.c >> @@ -341,6 +341,13 @@ static unsigned int rpmhpd_get_performance_state(struct generic_pm_domain *genpd >> return dev_pm_opp_get_level(opp); >> } >> >> +static int rpmhpd_performance_states_count(struct generic_pm_domain *domain) >> +{ >> + struct rpmhpd *pd = domain_to_rpmhpd(domain); >> + >> + return pd->level_count; >> +} >> + >> static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd) >> { >> int i; >> @@ -429,6 +436,8 @@ static int rpmhpd_probe(struct platform_device *pdev) >> rpmhpds[i]->pd.power_on = rpmhpd_power_on; >> rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance_state; >> rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance_state; >> + rpmhpds[i]->pd.get_performance_state_count = >> + rpmhpd_performance_states_count; > > I would prefer if you ignore the 80-char limit here and leave the line > unwrapped. Hi Bjorn, Thanks for the reviews. I will fix this in the next version. > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Regards, > Bjorn > >> pm_genpd_init(&rpmhpds[i]->pd, NULL, true); >> >> data->domains[i] = &rpmhpds[i]->pd; >> -- >> 2.20.1 >>
Certain resources modeled as a generic power domain in linux kernel can be used to warm up the SoC (mx power domain on sdm845) if the temperature falls below certain threshold. These power domains can be considered as thermal warming devices. (opposite of thermal cooling devices). In kernel, these warming devices can be modeled as a thermal cooling device. Since linux kernel today has no instance of a resource modeled as a power domain acting as a thermal warming device, a generic power domain based thermal warming device driver that can be used pan-Socs is the approach taken in this patch series. Since thermal warming devices can be thought of as the mirror opposite of thermal cooling devices, this patch series re-uses thermal cooling device framework. To use these power domains as warming devices require further tweaks in the thermal framework which are out of scope of this patch series. These tweaks have been posted as a separate series[1]. The first patch in this series extends the genpd framework to export out the performance states of a power domain so that when a power domain is modeled as a cooling device, the number of possible states and current state of the cooling device can be retrieved from the genpd framework. The second patch implements the newly added genpd callback for Qualcomm RPMH power domain driver which hosts the mx power domain. The third patch introduces a new cooling device register API that allows a parent to be specified for the cooling device. The fourth patch introduces the generic power domain warming device driver. The fifth patch extends Qualcomm RPMh power controller driver to register mx power domain as a thermal warming device in the kernel. The sixth patch describes the dt binding extensions for mx power domain to be a thermal warming device. The seventh patch introduces the DT entreis for sdm845 to register mx power domain as a thermal warming device. v1->v2: - Rename the patch series from "qcom: Model RPMH power domains as thermal cooling devices" to "Introduce Power domain based thermal warming devices" as it is more appropriate. - Introduce a new patch(patch 3) describing the dt-bindings for generic power domain warming device. - Patch specific changes mentioned in respective patches. v2->v3: - Changed power domain warming device from a virtual device node entry in DT to being a subnode of power domain controller binding following Rob's review comments. - Implemented Ulf's review comments. - The changes above introduced two new patches (patch 3 and 4) v3->v4: - Dropped late_init hook in cooling device ops. Instead introduced a new cooling device register API that allows to define a parent for the cooling device. - Patch specific changes mentioned in respective patches. v4->v5: - Dropped the patch that introduced the cooling device register API with parent as per review comments from Ulf. - Patch specific changes mentioned in respective patches. 1. https://lkml.org/lkml/2019/9/18/1180 Thara Gopinath (6): PM/Domains: Add support for retrieving genpd performance states information soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count thermal: Add generic power domain warming device driver. soc: qcom: Extend RPMh power controller driver to register warming devices. dt-bindings: power: Extend RPMh power controller binding to describe thermal warming device arm64: dts: qcom: Indicate rpmhpd hosts a power domain that can be used as a warming device. .../devicetree/bindings/power/qcom,rpmpd.yaml | 3 + arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + drivers/base/power/domain.c | 37 ++++ drivers/soc/qcom/rpmhpd.c | 46 ++++- drivers/thermal/Kconfig | 10 ++ drivers/thermal/Makefile | 2 + drivers/thermal/pd_warming.c | 168 ++++++++++++++++++ include/linux/pd_warming.h | 29 +++ include/linux/pm_domain.h | 13 ++ 9 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 drivers/thermal/pd_warming.c create mode 100644 include/linux/pd_warming.h