| Message ID | 20201217180638.22748-29-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs | expand |
22.12.2020 09:40, Viresh Kumar пишет: > On 17-12-20, 21:06, Dmitry Osipenko wrote: >> +++ b/drivers/soc/tegra/core-power-domain.c >> @@ -0,0 +1,125 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * NVIDIA Tegra SoC Core Power Domain Driver >> + */ >> + >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_domain.h> >> +#include <linux/pm_opp.h> >> +#include <linux/slab.h> >> + >> +#include <soc/tegra/common.h> >> + >> +static struct lock_class_key tegra_core_domain_lock_class; >> +static bool tegra_core_domain_state_synced; >> + >> +static int tegra_genpd_set_performance_state(struct generic_pm_domain *genpd, >> + unsigned int level) >> +{ >> + struct dev_pm_opp *opp; >> + int err; >> + >> + opp = dev_pm_opp_find_level_ceil(&genpd->dev, &level); > > We don't need ceil or floor versions for level, but rather _exact() version. Or > maybe just call it dev_pm_opp_find_level(). The _exact() version won't find OPP for level=0 if levels don't start with 0. >> + if (IS_ERR(opp)) { >> + dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n", >> + level, opp); >> + return PTR_ERR(opp); >> + } >> + >> + err = dev_pm_opp_set_voltage(&genpd->dev, opp); > > IIUC, you implemented this callback because you want to use the voltage triplet > present in the OPP table ? > > And so you are setting the regulator ("power") later in this patch ? yes > I am not in favor of implementing this routine, as it just adds a wrapper above > the regulator API. What you should be doing rather is get the regulator by > yourself here (instead of depending on the OPP core). And then you can do > dev_pm_opp_get_voltage() here and set the voltage yourself. You may want to > implement a version supporting triplet here though for the same. > > And you won't require the sync version of the API as well then. > That's what I initially did for this driver. I don't mind to revert back to the initial variant in v3, it appeared to me that it will be nicer and cleaner to have OPP API managing everything here.
On 22-12-20, 22:39, Dmitry Osipenko wrote: > 22.12.2020 22:21, Dmitry Osipenko пишет: > >>> + if (IS_ERR(opp)) { > >>> + dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n", > >>> + level, opp); > >>> + return PTR_ERR(opp); > >>> + } > >>> + > >>> + err = dev_pm_opp_set_voltage(&genpd->dev, opp); > >> IIUC, you implemented this callback because you want to use the voltage triplet > >> present in the OPP table ? > >> > >> And so you are setting the regulator ("power") later in this patch ? > > yes > > > >> I am not in favor of implementing this routine, as it just adds a wrapper above > >> the regulator API. What you should be doing rather is get the regulator by > >> yourself here (instead of depending on the OPP core). And then you can do > >> dev_pm_opp_get_voltage() here and set the voltage yourself. You may want to > >> implement a version supporting triplet here though for the same. > >> > >> And you won't require the sync version of the API as well then. > >> > > That's what I initially did for this driver. I don't mind to revert back > > to the initial variant in v3, it appeared to me that it will be nicer > > and cleaner to have OPP API managing everything here. > > I forgot one important detail (why the initial variant wasn't good).. > OPP entries that have unsupportable voltages should be filtered out and > OPP core performs the filtering only if regulator is assigned to the OPP > table. > > If regulator is assigned to the OPP table, then we need to use OPP API > for driving the regulator, hence that's why I added > dev_pm_opp_sync_regulators() and dev_pm_opp_set_voltage(). > > Perhaps it should be possible to add dev_pm_opp_get_regulator() that What's wrong with getting the regulator in the driver as well ? Apart from the OPP core ? > will return the OPP table regulator in order to allow driver to use the > regulator directly. But I'm not sure whether this is a much better > option than the opp_sync_regulators() and opp_set_voltage() APIs. set_voltage() is still fine as there is some data that the OPP core has, but sync_regulator() has nothing to do with OPP core. And this may lead to more wrapper helpers in the OPP core, which I am afraid of. And so even if it is not the best, I would like the OPP core to provide the data and not get into this. Ofcourse there is an exception to this, opp_set_rate. -- viresh
23.12.2020 08:57, Viresh Kumar пишет: > On 22-12-20, 22:39, Dmitry Osipenko wrote: >> 22.12.2020 22:21, Dmitry Osipenko пишет: >>>>> + if (IS_ERR(opp)) { >>>>> + dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n", >>>>> + level, opp); >>>>> + return PTR_ERR(opp); >>>>> + } >>>>> + >>>>> + err = dev_pm_opp_set_voltage(&genpd->dev, opp); >>>> IIUC, you implemented this callback because you want to use the voltage triplet >>>> present in the OPP table ? >>>> >>>> And so you are setting the regulator ("power") later in this patch ? >>> yes >>> >>>> I am not in favor of implementing this routine, as it just adds a wrapper above >>>> the regulator API. What you should be doing rather is get the regulator by >>>> yourself here (instead of depending on the OPP core). And then you can do >>>> dev_pm_opp_get_voltage() here and set the voltage yourself. You may want to >>>> implement a version supporting triplet here though for the same. >>>> >>>> And you won't require the sync version of the API as well then. >>>> >>> That's what I initially did for this driver. I don't mind to revert back >>> to the initial variant in v3, it appeared to me that it will be nicer >>> and cleaner to have OPP API managing everything here. >> >> I forgot one important detail (why the initial variant wasn't good).. >> OPP entries that have unsupportable voltages should be filtered out and >> OPP core performs the filtering only if regulator is assigned to the OPP >> table. >> >> If regulator is assigned to the OPP table, then we need to use OPP API >> for driving the regulator, hence that's why I added >> dev_pm_opp_sync_regulators() and dev_pm_opp_set_voltage(). >> >> Perhaps it should be possible to add dev_pm_opp_get_regulator() that > > What's wrong with getting the regulator in the driver as well ? Apart from the > OPP core ? The voltage syncing should be done for each consumer regulator individually [1]. Secondly, regulator core doesn't work well today if the same regulator is requested more than one time for the same device. >> will return the OPP table regulator in order to allow driver to use the >> regulator directly. But I'm not sure whether this is a much better >> option than the opp_sync_regulators() and opp_set_voltage() APIs. > > set_voltage() is still fine as there is some data that the OPP core has, but > sync_regulator() has nothing to do with OPP core. > > And this may lead to more wrapper helpers in the OPP core, which I am afraid of. > And so even if it is not the best, I would like the OPP core to provide the data > and not get into this. Ofcourse there is an exception to this, opp_set_rate. > The regulator_sync_voltage() should be invoked only if voltage was changed previously [1]. The OPP core already has the info about whether voltage was changed and it provides the necessary locking for both set_voltage() and sync_regulator(). Perhaps I'll need to duplicate that functionality in the PD driver, instead of making it all generic and re-usable by other drivers. [1] https://elixir.bootlin.com/linux/v5.10.2/source/drivers/regulator/core.c#L4107
23.12.2020 23:37, Dmitry Osipenko пишет: > 23.12.2020 08:57, Viresh Kumar пишет: >> On 22-12-20, 22:39, Dmitry Osipenko wrote: >>> 22.12.2020 22:21, Dmitry Osipenko пишет: >>>>>> + if (IS_ERR(opp)) { >>>>>> + dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n", >>>>>> + level, opp); >>>>>> + return PTR_ERR(opp); >>>>>> + } >>>>>> + >>>>>> + err = dev_pm_opp_set_voltage(&genpd->dev, opp); >>>>> IIUC, you implemented this callback because you want to use the voltage triplet >>>>> present in the OPP table ? >>>>> >>>>> And so you are setting the regulator ("power") later in this patch ? >>>> yes >>>> >>>>> I am not in favor of implementing this routine, as it just adds a wrapper above >>>>> the regulator API. What you should be doing rather is get the regulator by >>>>> yourself here (instead of depending on the OPP core). And then you can do >>>>> dev_pm_opp_get_voltage() here and set the voltage yourself. You may want to >>>>> implement a version supporting triplet here though for the same. >>>>> >>>>> And you won't require the sync version of the API as well then. >>>>> >>>> That's what I initially did for this driver. I don't mind to revert back >>>> to the initial variant in v3, it appeared to me that it will be nicer >>>> and cleaner to have OPP API managing everything here. >>> >>> I forgot one important detail (why the initial variant wasn't good).. >>> OPP entries that have unsupportable voltages should be filtered out and >>> OPP core performs the filtering only if regulator is assigned to the OPP >>> table. >>> >>> If regulator is assigned to the OPP table, then we need to use OPP API >>> for driving the regulator, hence that's why I added >>> dev_pm_opp_sync_regulators() and dev_pm_opp_set_voltage(). >>> >>> Perhaps it should be possible to add dev_pm_opp_get_regulator() that >> >> What's wrong with getting the regulator in the driver as well ? Apart from the >> OPP core ? > > The voltage syncing should be done for each consumer regulator > individually [1]. > > Secondly, regulator core doesn't work well today if the same regulator > is requested more than one time for the same device. > >>> will return the OPP table regulator in order to allow driver to use the >>> regulator directly. But I'm not sure whether this is a much better >>> option than the opp_sync_regulators() and opp_set_voltage() APIs. >> >> set_voltage() is still fine as there is some data that the OPP core has, but >> sync_regulator() has nothing to do with OPP core. >> >> And this may lead to more wrapper helpers in the OPP core, which I am afraid of. >> And so even if it is not the best, I would like the OPP core to provide the data >> and not get into this. Ofcourse there is an exception to this, opp_set_rate. >> > > The regulator_sync_voltage() should be invoked only if voltage was > changed previously [1]. > > The OPP core already has the info about whether voltage was changed and > it provides the necessary locking for both set_voltage() and > sync_regulator(). Perhaps I'll need to duplicate that functionality in > the PD driver, instead of making it all generic and re-usable by other > drivers. > > [1] > https://elixir.bootlin.com/linux/v5.10.2/source/drivers/regulator/core.c#L4107 > I just realized that the locking is missing in the v2 patches, something to fix in the next revision :) Still I'm in favor of extending the OPP API with the new common functions. But if you have a strong opinion about that, then I'll try to work around it in the PD driver in v3.
On 23-12-20, 23:37, Dmitry Osipenko wrote: > 23.12.2020 08:57, Viresh Kumar пишет: > > What's wrong with getting the regulator in the driver as well ? Apart from the > > OPP core ? > > The voltage syncing should be done for each consumer regulator > individually [1]. > > Secondly, regulator core doesn't work well today if the same regulator > is requested more than one time for the same device. Hmm... > >> will return the OPP table regulator in order to allow driver to use the > >> regulator directly. But I'm not sure whether this is a much better > >> option than the opp_sync_regulators() and opp_set_voltage() APIs. > > > > set_voltage() is still fine as there is some data that the OPP core has, but > > sync_regulator() has nothing to do with OPP core. > > > > And this may lead to more wrapper helpers in the OPP core, which I am afraid of. > > And so even if it is not the best, I would like the OPP core to provide the data > > and not get into this. Ofcourse there is an exception to this, opp_set_rate. > > > > The regulator_sync_voltage() should be invoked only if voltage was > changed previously [1]. > > The OPP core already has the info about whether voltage was changed and > it provides the necessary locking for both set_voltage() and > sync_regulator(). Perhaps I'll need to duplicate that functionality in > the PD driver, instead of making it all generic and re-usable by other > drivers. > > [1] > https://elixir.bootlin.com/linux/v5.10.2/source/drivers/regulator/core.c#L4107 Lets do it in the OPP core and see where we go. -- viresh
24.12.2020 09:51, Viresh Kumar пишет: > On 23-12-20, 23:37, Dmitry Osipenko wrote: >> 23.12.2020 08:57, Viresh Kumar пишет: >>> What's wrong with getting the regulator in the driver as well ? Apart from the >>> OPP core ? >> >> The voltage syncing should be done for each consumer regulator >> individually [1]. >> >> Secondly, regulator core doesn't work well today if the same regulator >> is requested more than one time for the same device. > > Hmm... > >>>> will return the OPP table regulator in order to allow driver to use the >>>> regulator directly. But I'm not sure whether this is a much better >>>> option than the opp_sync_regulators() and opp_set_voltage() APIs. >>> >>> set_voltage() is still fine as there is some data that the OPP core has, but >>> sync_regulator() has nothing to do with OPP core. >>> >>> And this may lead to more wrapper helpers in the OPP core, which I am afraid of. >>> And so even if it is not the best, I would like the OPP core to provide the data >>> and not get into this. Ofcourse there is an exception to this, opp_set_rate. >>> >> >> The regulator_sync_voltage() should be invoked only if voltage was >> changed previously [1]. >> >> The OPP core already has the info about whether voltage was changed and >> it provides the necessary locking for both set_voltage() and >> sync_regulator(). Perhaps I'll need to duplicate that functionality in >> the PD driver, instead of making it all generic and re-usable by other >> drivers. >> >> [1] >> https://elixir.bootlin.com/linux/v5.10.2/source/drivers/regulator/core.c#L4107 > > Lets do it in the OPP core and see where we go. > Alright, thank you.
On Thu, 17 Dec 2020 at 19:07, Dmitry Osipenko <digetx@gmail.com> wrote: > > NVIDIA Tegra SoCs have multiple power domains, each domain corresponds > to an external SoC power rail. Core power domain covers vast majority of > hardware blocks within a Tegra SoC. The voltage of a power domain should > be set to a value which satisfies all devices within a power domain. Add > driver for the core power domain in order to manage the voltage state of > the domain. This allows us to support a system-wide DVFS on Tegra. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> FYI: from a genpd provider driver point of view, this looks good to me. However, withholding my ack for the next version, just to make sure I take a second look. Kind regards Uffe > --- > drivers/soc/tegra/Kconfig | 6 ++ > drivers/soc/tegra/Makefile | 1 + > drivers/soc/tegra/core-power-domain.c | 125 ++++++++++++++++++++++++++ > include/soc/tegra/common.h | 6 ++ > 4 files changed, 138 insertions(+) > create mode 100644 drivers/soc/tegra/core-power-domain.c > > diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig > index bcd61ae59ba3..fccbc168dd87 100644 > --- a/drivers/soc/tegra/Kconfig > +++ b/drivers/soc/tegra/Kconfig > @@ -16,6 +16,7 @@ config ARCH_TEGRA_2x_SOC > select SOC_TEGRA_COMMON > select SOC_TEGRA_FLOWCTRL > select SOC_TEGRA_PMC > + select SOC_TEGRA_CORE_POWER_DOMAIN > select SOC_TEGRA20_VOLTAGE_COUPLER > select TEGRA_TIMER > help > @@ -31,6 +32,7 @@ config ARCH_TEGRA_3x_SOC > select SOC_TEGRA_COMMON > select SOC_TEGRA_FLOWCTRL > select SOC_TEGRA_PMC > + select SOC_TEGRA_CORE_POWER_DOMAIN > select SOC_TEGRA30_VOLTAGE_COUPLER > select TEGRA_TIMER > help > @@ -170,3 +172,7 @@ config SOC_TEGRA20_VOLTAGE_COUPLER > config SOC_TEGRA30_VOLTAGE_COUPLER > bool "Voltage scaling support for Tegra30 SoCs" > depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST > + > +config SOC_TEGRA_CORE_POWER_DOMAIN > + bool > + select PM_GENERIC_DOMAINS > diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile > index 9c809c1814bd..8f1294f954b4 100644 > --- a/drivers/soc/tegra/Makefile > +++ b/drivers/soc/tegra/Makefile > @@ -7,3 +7,4 @@ obj-$(CONFIG_SOC_TEGRA_PMC) += pmc.o > obj-$(CONFIG_SOC_TEGRA_POWERGATE_BPMP) += powergate-bpmp.o > obj-$(CONFIG_SOC_TEGRA20_VOLTAGE_COUPLER) += regulators-tegra20.o > obj-$(CONFIG_SOC_TEGRA30_VOLTAGE_COUPLER) += regulators-tegra30.o > +obj-$(CONFIG_SOC_TEGRA_CORE_POWER_DOMAIN) += core-power-domain.o > diff --git a/drivers/soc/tegra/core-power-domain.c b/drivers/soc/tegra/core-power-domain.c > new file mode 100644 > index 000000000000..7c0cec8c79fd > --- /dev/null > +++ b/drivers/soc/tegra/core-power-domain.c > @@ -0,0 +1,125 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * NVIDIA Tegra SoC Core Power Domain Driver > + */ > + > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_opp.h> > +#include <linux/slab.h> > + > +#include <soc/tegra/common.h> > + > +static struct lock_class_key tegra_core_domain_lock_class; > +static bool tegra_core_domain_state_synced; > + > +static int tegra_genpd_set_performance_state(struct generic_pm_domain *genpd, > + unsigned int level) > +{ > + struct dev_pm_opp *opp; > + int err; > + > + opp = dev_pm_opp_find_level_ceil(&genpd->dev, &level); > + if (IS_ERR(opp)) { > + dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n", > + level, opp); > + return PTR_ERR(opp); > + } > + > + err = dev_pm_opp_set_voltage(&genpd->dev, opp); > + dev_pm_opp_put(opp); > + > + if (err) { > + dev_err(&genpd->dev, "failed to set voltage to %duV: %d\n", > + level, err); > + return err; > + } > + > + return 0; > +} > + > +static unsigned int > +tegra_genpd_opp_to_performance_state(struct generic_pm_domain *genpd, > + struct dev_pm_opp *opp) > +{ > + return dev_pm_opp_get_level(opp); > +} > + > +static int tegra_core_domain_probe(struct platform_device *pdev) > +{ > + struct generic_pm_domain *genpd; > + struct opp_table *opp_table; > + const char *rname = "power"; > + int err; > + > + genpd = devm_kzalloc(&pdev->dev, sizeof(*genpd), GFP_KERNEL); > + if (!genpd) > + return -ENOMEM; > + > + genpd->name = pdev->dev.of_node->name; > + genpd->set_performance_state = tegra_genpd_set_performance_state; > + genpd->opp_to_performance_state = tegra_genpd_opp_to_performance_state; > + > + opp_table = devm_pm_opp_set_regulators(&pdev->dev, &rname, 1); > + if (IS_ERR(opp_table)) > + return dev_err_probe(&pdev->dev, PTR_ERR(opp_table), > + "failed to set OPP regulator\n"); > + > + err = pm_genpd_init(genpd, NULL, false); > + if (err) { > + dev_err(&pdev->dev, "failed to init genpd: %d\n", err); > + return err; > + } > + > + /* > + * We have a "PMC -> Core" hierarchy of the power domains where > + * PMC needs to resume and change performance (voltage) of the > + * Core domain from the PMC GENPD on/off callbacks, hence we need > + * to annotate the lock in order to remove confusion from the > + * lockdep checker when a nested access happens. > + */ > + lockdep_set_class(&genpd->mlock, &tegra_core_domain_lock_class); > + > + err = of_genpd_add_provider_simple(pdev->dev.of_node, genpd); > + if (err) { > + dev_err(&pdev->dev, "failed to add genpd: %d\n", err); > + goto remove_genpd; > + } > + > + return 0; > + > +remove_genpd: > + pm_genpd_remove(genpd); > + > + return err; > +} > + > +bool tegra_soc_core_domain_state_synced(void) > +{ > + return tegra_core_domain_state_synced; > +} > + > +static void tegra_core_domain_sync_state(struct device *dev) > +{ > + tegra_core_domain_state_synced = true; > + > + dev_pm_opp_sync_regulators(dev); > +} > + > +static const struct of_device_id tegra_core_domain_match[] = { > + { .compatible = "nvidia,tegra20-core-domain", }, > + { .compatible = "nvidia,tegra30-core-domain", }, > + { } > +}; > + > +static struct platform_driver tegra_core_domain_driver = { > + .driver = { > + .name = "tegra-core-power", > + .of_match_table = tegra_core_domain_match, > + .suppress_bind_attrs = true, > + .sync_state = tegra_core_domain_sync_state, > + }, > + .probe = tegra_core_domain_probe, > +}; > +builtin_platform_driver(tegra_core_domain_driver); > diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h > index 57b56793a9e5..6c2ccbbbf073 100644 > --- a/include/soc/tegra/common.h > +++ b/include/soc/tegra/common.h > @@ -27,6 +27,7 @@ struct tegra_core_opp_params { > > #ifdef CONFIG_ARCH_TEGRA > bool soc_is_tegra(void); > +bool tegra_soc_core_domain_state_synced(void); > int devm_tegra_core_dev_init_opp_table(struct device *dev, > struct tegra_core_opp_params *cfg); > #else > @@ -35,6 +36,11 @@ static inline bool soc_is_tegra(void) > return false; > } > > +static inline bool tegra_soc_core_domain_state_synced(void) > +{ > + return false; > +} > + > static inline int > devm_tegra_core_dev_init_opp_table(struct device *dev, > struct tegra_core_opp_params *cfg) > -- > 2.29.2 >
diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig index bcd61ae59ba3..fccbc168dd87 100644 --- a/drivers/soc/tegra/Kconfig +++ b/drivers/soc/tegra/Kconfig @@ -16,6 +16,7 @@ config ARCH_TEGRA_2x_SOC select SOC_TEGRA_COMMON select SOC_TEGRA_FLOWCTRL select SOC_TEGRA_PMC + select SOC_TEGRA_CORE_POWER_DOMAIN select SOC_TEGRA20_VOLTAGE_COUPLER select TEGRA_TIMER help @@ -31,6 +32,7 @@ config ARCH_TEGRA_3x_SOC select SOC_TEGRA_COMMON select SOC_TEGRA_FLOWCTRL select SOC_TEGRA_PMC + select SOC_TEGRA_CORE_POWER_DOMAIN select SOC_TEGRA30_VOLTAGE_COUPLER select TEGRA_TIMER help @@ -170,3 +172,7 @@ config SOC_TEGRA20_VOLTAGE_COUPLER config SOC_TEGRA30_VOLTAGE_COUPLER bool "Voltage scaling support for Tegra30 SoCs" depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST + +config SOC_TEGRA_CORE_POWER_DOMAIN + bool + select PM_GENERIC_DOMAINS diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile index 9c809c1814bd..8f1294f954b4 100644 --- a/drivers/soc/tegra/Makefile +++ b/drivers/soc/tegra/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_SOC_TEGRA_PMC) += pmc.o obj-$(CONFIG_SOC_TEGRA_POWERGATE_BPMP) += powergate-bpmp.o obj-$(CONFIG_SOC_TEGRA20_VOLTAGE_COUPLER) += regulators-tegra20.o obj-$(CONFIG_SOC_TEGRA30_VOLTAGE_COUPLER) += regulators-tegra30.o +obj-$(CONFIG_SOC_TEGRA_CORE_POWER_DOMAIN) += core-power-domain.o diff --git a/drivers/soc/tegra/core-power-domain.c b/drivers/soc/tegra/core-power-domain.c new file mode 100644 index 000000000000..7c0cec8c79fd --- /dev/null +++ b/drivers/soc/tegra/core-power-domain.c @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * NVIDIA Tegra SoC Core Power Domain Driver + */ + +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <linux/pm_opp.h> +#include <linux/slab.h> + +#include <soc/tegra/common.h> + +static struct lock_class_key tegra_core_domain_lock_class; +static bool tegra_core_domain_state_synced; + +static int tegra_genpd_set_performance_state(struct generic_pm_domain *genpd, + unsigned int level) +{ + struct dev_pm_opp *opp; + int err; + + opp = dev_pm_opp_find_level_ceil(&genpd->dev, &level); + if (IS_ERR(opp)) { + dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n", + level, opp); + return PTR_ERR(opp); + } + + err = dev_pm_opp_set_voltage(&genpd->dev, opp); + dev_pm_opp_put(opp); + + if (err) { + dev_err(&genpd->dev, "failed to set voltage to %duV: %d\n", + level, err); + return err; + } + + return 0; +} + +static unsigned int +tegra_genpd_opp_to_performance_state(struct generic_pm_domain *genpd, + struct dev_pm_opp *opp) +{ + return dev_pm_opp_get_level(opp); +} + +static int tegra_core_domain_probe(struct platform_device *pdev) +{ + struct generic_pm_domain *genpd; + struct opp_table *opp_table; + const char *rname = "power"; + int err; + + genpd = devm_kzalloc(&pdev->dev, sizeof(*genpd), GFP_KERNEL); + if (!genpd) + return -ENOMEM; + + genpd->name = pdev->dev.of_node->name; + genpd->set_performance_state = tegra_genpd_set_performance_state; + genpd->opp_to_performance_state = tegra_genpd_opp_to_performance_state; + + opp_table = devm_pm_opp_set_regulators(&pdev->dev, &rname, 1); + if (IS_ERR(opp_table)) + return dev_err_probe(&pdev->dev, PTR_ERR(opp_table), + "failed to set OPP regulator\n"); + + err = pm_genpd_init(genpd, NULL, false); + if (err) { + dev_err(&pdev->dev, "failed to init genpd: %d\n", err); + return err; + } + + /* + * We have a "PMC -> Core" hierarchy of the power domains where + * PMC needs to resume and change performance (voltage) of the + * Core domain from the PMC GENPD on/off callbacks, hence we need + * to annotate the lock in order to remove confusion from the + * lockdep checker when a nested access happens. + */ + lockdep_set_class(&genpd->mlock, &tegra_core_domain_lock_class); + + err = of_genpd_add_provider_simple(pdev->dev.of_node, genpd); + if (err) { + dev_err(&pdev->dev, "failed to add genpd: %d\n", err); + goto remove_genpd; + } + + return 0; + +remove_genpd: + pm_genpd_remove(genpd); + + return err; +} + +bool tegra_soc_core_domain_state_synced(void) +{ + return tegra_core_domain_state_synced; +} + +static void tegra_core_domain_sync_state(struct device *dev) +{ + tegra_core_domain_state_synced = true; + + dev_pm_opp_sync_regulators(dev); +} + +static const struct of_device_id tegra_core_domain_match[] = { + { .compatible = "nvidia,tegra20-core-domain", }, + { .compatible = "nvidia,tegra30-core-domain", }, + { } +}; + +static struct platform_driver tegra_core_domain_driver = { + .driver = { + .name = "tegra-core-power", + .of_match_table = tegra_core_domain_match, + .suppress_bind_attrs = true, + .sync_state = tegra_core_domain_sync_state, + }, + .probe = tegra_core_domain_probe, +}; +builtin_platform_driver(tegra_core_domain_driver); diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h index 57b56793a9e5..6c2ccbbbf073 100644 --- a/include/soc/tegra/common.h +++ b/include/soc/tegra/common.h @@ -27,6 +27,7 @@ struct tegra_core_opp_params { #ifdef CONFIG_ARCH_TEGRA bool soc_is_tegra(void); +bool tegra_soc_core_domain_state_synced(void); int devm_tegra_core_dev_init_opp_table(struct device *dev, struct tegra_core_opp_params *cfg); #else @@ -35,6 +36,11 @@ static inline bool soc_is_tegra(void) return false; } +static inline bool tegra_soc_core_domain_state_synced(void) +{ + return false; +} + static inline int devm_tegra_core_dev_init_opp_table(struct device *dev, struct tegra_core_opp_params *cfg)
NVIDIA Tegra SoCs have multiple power domains, each domain corresponds to an external SoC power rail. Core power domain covers vast majority of hardware blocks within a Tegra SoC. The voltage of a power domain should be set to a value which satisfies all devices within a power domain. Add driver for the core power domain in order to manage the voltage state of the domain. This allows us to support a system-wide DVFS on Tegra. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/soc/tegra/Kconfig | 6 ++ drivers/soc/tegra/Makefile | 1 + drivers/soc/tegra/core-power-domain.c | 125 ++++++++++++++++++++++++++ include/soc/tegra/common.h | 6 ++ 4 files changed, 138 insertions(+) create mode 100644 drivers/soc/tegra/core-power-domain.c