| Message ID | 20190414175939.12368-7-digetx@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Introduce machine-specific regulators coupling API | expand |
On Sun, Apr 14, 2019 at 08:59:39PM +0300, Dmitry Osipenko wrote: > Add regulators coupler for Tegra30 SoC's that performs voltage balancing > of a coupled regulators and thus provides voltage scaling functionality. Same here, what are the requirements this is implementing?
08.05.2019 10:58, Mark Brown пишет: > On Sun, Apr 14, 2019 at 08:59:39PM +0300, Dmitry Osipenko wrote: >> Add regulators coupler for Tegra30 SoC's that performs voltage balancing >> of a coupled regulators and thus provides voltage scaling functionality. > > Same here, what are the requirements this is implementing? > There are two coupled regulators: CPU and CORE. Constraints: 1) The max-spread voltage is 300mV. 2) CORE voltage must be higher than the CPU by at least N mV, where N varies depending on the CPU voltage. 3) There is a constraint on the maximum CORE voltage depending on hardware model/revision (cpu_speedo_id) where a higher voltages apparently may cause physical damage, so it's better to hardcode the limitation in the code rather than to rely on a board's device-tree description. This constraint is quite vaguely defined in the downstream kernel, I'm not really sure if it's solely about the hardware safety.
On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote: > Constraints: > 1) The max-spread voltage is 300mV. > 2) CORE voltage must be higher than the CPU by at least N mV, where N > varies depending on the CPU voltage. Those seem like they should be doable in generic code, though the fact that the constraint is variable makes it annoying to specify - otherwise it'd just be a minimum and maximum spread. I'm not really coming up with any great ideas right now, it's getting into OPP type territory but it sounds like there's more flexibility for ramping the core voltage so you'd end up with silly numbers of OPPs. > 3) There is a constraint on the maximum CORE voltage depending on > hardware model/revision (cpu_speedo_id) where a higher voltages > apparently may cause physical damage, so it's better to hardcode the > limitation in the code rather than to rely on a board's device-tree > description. This constraint is quite vaguely defined in the downstream > kernel, I'm not really sure if it's solely about the hardware safety. I'd expect this to be enforced by the cpufreq driver just not selecting higher voltages on affected parts.
12.05.2019 12:04, Mark Brown пишет: > On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote: > >> Constraints: > >> 1) The max-spread voltage is 300mV. > >> 2) CORE voltage must be higher than the CPU by at least N mV, where N >> varies depending on the CPU voltage. > > Those seem like they should be doable in generic code, though the fact > that the constraint is variable makes it annoying to specify - otherwise > it'd just be a minimum and maximum spread. I'm not really coming up > with any great ideas right now, it's getting into OPP type territory but > it sounds like there's more flexibility for ramping the core voltage so > you'd end up with silly numbers of OPPs. The OPP shouldn't have to do anything in regards to the regulators coupling. The whole idea of the regulators coupling is to make device drivers to not churn with the coupling. The coupling in this case is specific to SoC and not to a particular board. I think the current approach with the customized regulators coupler is the best solution for the time being. We may consider something more generic if there will be other users with a similar coupling requirements, otherwise it's quite difficult to judge what is "generic". Do you agree? >> 3) There is a constraint on the maximum CORE voltage depending on >> hardware model/revision (cpu_speedo_id) where a higher voltages >> apparently may cause physical damage, so it's better to hardcode the >> limitation in the code rather than to rely on a board's device-tree >> description. This constraint is quite vaguely defined in the downstream >> kernel, I'm not really sure if it's solely about the hardware safety. > > I'd expect this to be enforced by the cpufreq driver just not selecting > higher voltages on affected parts. > CPUFreq driver will only handle the CPU regulator and it won't know anything about the CORE. Anyway, please scratch the third constraint, I messed up it with the other *minimum* CORE voltage constraint detail which makes the minimum voltage to depend on the hardware version in addition to the CPU voltage.
On Sun, May 12, 2019 at 09:29:54PM +0300, Dmitry Osipenko wrote: > 12.05.2019 12:04, Mark Brown пишет: > > On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote: > > Those seem like they should be doable in generic code, though the fact > > that the constraint is variable makes it annoying to specify - otherwise > > it'd just be a minimum and maximum spread. I'm not really coming up > > with any great ideas right now, it's getting into OPP type territory but > > it sounds like there's more flexibility for ramping the core voltage so > > you'd end up with silly numbers of OPPs. > The OPP shouldn't have to do anything in regards to the regulators > coupling. The whole idea of the regulators coupling is to make device > drivers to not churn with the coupling. The coupling in this case is > specific to SoC and not to a particular board. The thing with OPPs is that they specify a whole table of values that work together including regulator settings, the result being that you have many fewer options but don't need to think about constraints. > I think the current approach with the customized regulators coupler is > the best solution for the time being. We may consider something more > generic if there will be other users with a similar coupling > requirements, otherwise it's quite difficult to judge what is "generic". > Do you agree? Some of the constraints (like having drivers loaded) are kind of fun...
13.05.2019 20:40, Mark Brown пишет: > On Sun, May 12, 2019 at 09:29:54PM +0300, Dmitry Osipenko wrote: >> 12.05.2019 12:04, Mark Brown пишет: >>> On Wed, May 08, 2019 at 04:27:42PM +0300, Dmitry Osipenko wrote: > >>> Those seem like they should be doable in generic code, though the fact >>> that the constraint is variable makes it annoying to specify - otherwise >>> it'd just be a minimum and maximum spread. I'm not really coming up >>> with any great ideas right now, it's getting into OPP type territory but >>> it sounds like there's more flexibility for ramping the core voltage so >>> you'd end up with silly numbers of OPPs. > >> The OPP shouldn't have to do anything in regards to the regulators >> coupling. The whole idea of the regulators coupling is to make device >> drivers to not churn with the coupling. The coupling in this case is >> specific to SoC and not to a particular board. > > The thing with OPPs is that they specify a whole table of values that > work together including regulator settings, the result being that you > have many fewer options but don't need to think about constraints. I'm afraid this is just a way of abusing the OPP's. I actually already had variant of the CPUFreq driver where it was managing all of the coupled regulators and gave up on it because it's just not very practical and adds a lot of unnecessary churning into the code. Note that it's just the CPUFreq driver, there are quite a lot of other (CORE) drivers as well and there are a lot of voltage combinations because OPP entries are also specific to a range of hardware versions. >> I think the current approach with the customized regulators coupler is >> the best solution for the time being. We may consider something more >> generic if there will be other users with a similar coupling >> requirements, otherwise it's quite difficult to judge what is "generic". >> Do you agree? > > Some of the constraints (like having drivers loaded) are kind of fun... > AFAIK, there is no good solution in upstream kernel for that problem yet. Maybe it will be possible to reset hardware into a some more predictable state early during kernel's boot for the start, will see.
On Tue, May 14, 2019 at 09:30:05PM +0300, Dmitry Osipenko wrote: > 13.05.2019 20:40, Mark Brown пишет: > > The thing with OPPs is that they specify a whole table of values that > > work together including regulator settings, the result being that you > > have many fewer options but don't need to think about constraints. > I'm afraid this is just a way of abusing the OPP's. I actually already There's nothing wrong with handling regulators in an OPP, that's a totally normal thing.
15.05.2019 12:05, Mark Brown пишет: > On Tue, May 14, 2019 at 09:30:05PM +0300, Dmitry Osipenko wrote: >> 13.05.2019 20:40, Mark Brown пишет: > >>> The thing with OPPs is that they specify a whole table of values that >>> work together including regulator settings, the result being that you >>> have many fewer options but don't need to think about constraints. > >> I'm afraid this is just a way of abusing the OPP's. I actually already > > There's nothing wrong with handling regulators in an OPP, that's a > totally normal thing. > Only if those regulators are directly related to the hardware unit, which is not the case here. Regulators coupling is the right abstraction that glues things together, there is absolutely no need in trying to make workarounds using OPP.
On Wed, May 15, 2019 at 02:44:33PM +0300, Dmitry Osipenko wrote: > 15.05.2019 12:05, Mark Brown пишет: > > On Tue, May 14, 2019 at 09:30:05PM +0300, Dmitry Osipenko wrote: > >> I'm afraid this is just a way of abusing the OPP's. I actually already > > There's nothing wrong with handling regulators in an OPP, that's a > > totally normal thing. > Only if those regulators are directly related to the hardware unit, > which is not the case here. Regulators coupling is the right abstraction > that glues things together, there is absolutely no need in trying to > make workarounds using OPP. The thing with OPPs is that they are often system level rather than related to one specific parrt of the device - one of the reasons people use them is that they eliminate the needs to think about dynamic combinations of things and instead pick a suitable configuration off a relatively short menu. This makes both validation and runtime easier.
diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig index 545c0da2e069..a5235af27644 100644 --- a/drivers/soc/tegra/Kconfig +++ b/drivers/soc/tegra/Kconfig @@ -139,3 +139,9 @@ config SOC_TEGRA20_VOLTAGE_COUPLER def_bool y depends on ARCH_TEGRA_2x_SOC depends on REGULATOR + +config SOC_TEGRA30_VOLTAGE_COUPLER + bool "Voltage scaling support for Tegra30 SoC's" + def_bool y + depends on ARCH_TEGRA_3x_SOC + depends on REGULATOR diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile index 9f0bdd53bef8..9c809c1814bd 100644 --- a/drivers/soc/tegra/Makefile +++ b/drivers/soc/tegra/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_SOC_TEGRA_FLOWCTRL) += flowctrl.o 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 diff --git a/drivers/soc/tegra/regulators-tegra30.c b/drivers/soc/tegra/regulators-tegra30.c new file mode 100644 index 000000000000..ecd6a984e045 --- /dev/null +++ b/drivers/soc/tegra/regulators-tegra30.c @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Voltage regulators coupling resolver for NVIDIA Tegra30 + * + * Author: Dmitry Osipenko <digetx@gmail.com> + */ + +#define pr_fmt(fmt) "tegra voltage-coupler: " fmt + +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/of.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> + +#include <soc/tegra/fuse.h> + +struct tegra_regulators_coupler { + struct regulators_coupler coupler; + int core_min_uV; +}; + +static const char * const cpu_names[] = { + "vdd_cpu,vdd_sys", + "+V1.0_VDD_CPU", + "vdd_cpu", +}; + +static const char * const core_names[] = { + "tps62361-vout", + "tps62362-vout", + "vdd_core", +}; + +static inline struct tegra_regulators_coupler * +to_tegra_coupler(struct regulators_coupler *coupler) +{ + return container_of(coupler, struct tegra_regulators_coupler, coupler); +} + +static int tegra30_core_limit(struct tegra_regulators_coupler *tegra, + struct regulator_dev *core_rdev) +{ + if (tegra->core_min_uV > 0) + return tegra->core_min_uV; + + tegra->core_min_uV = regulator_get_voltage_rdev(core_rdev); + if (tegra->core_min_uV > 0) + pr_info("core minimum voltage limited to %duV\n", + tegra->core_min_uV); + + return tegra->core_min_uV; +} + +static int tegra30_core_cpu_limit(int cpu_uV) +{ + if (cpu_uV < 800000) + return 950000; + + if (cpu_uV < 900000) + return 1000000; + + if (cpu_uV < 1000000) + return 1100000; + + if (cpu_uV < 1100000) + return 1200000; + + if (cpu_uV < 1250000) { + switch (tegra_sku_info.cpu_speedo_id) { + case 0 ... 1: + case 4: + case 7: + case 8: + return 1200000; + + default: + return 1300000; + } + } + + return -EINVAL; +} + +static int tegra30_voltage_update(struct tegra_regulators_coupler *tegra, + struct regulator_dev *cpu_rdev, + struct regulator_dev *core_rdev) +{ + int cpu_min_uV, cpu_max_uV = INT_MAX; + int core_min_uV, core_max_uV = INT_MAX; + int core_min_limited_uV; + int core_target_uV; + int cpu_target_uV; + int core_uV; + int cpu_uV; + int err; + + /* + * The CORE voltage scaling is currently not hooked up in drivers, + * hence we will limit the minimum CORE voltage to the initial value. + * This should be good enough for the time being. + */ + core_min_uV = tegra30_core_limit(tegra, core_rdev); + if (core_min_uV < 0) + return core_min_uV; + + err = regulator_check_consumers(core_rdev, &core_min_uV, &core_max_uV, + PM_SUSPEND_ON); + if (err) + return err; + + cpu_min_uV = core_min_uV - 300000; + + err = regulator_check_consumers(cpu_rdev, &cpu_min_uV, &cpu_max_uV, + PM_SUSPEND_ON); + if (err) + return err; + + err = regulator_check_voltage(cpu_rdev, &cpu_min_uV, &cpu_max_uV); + if (err) + return err; + + cpu_uV = regulator_get_voltage_rdev(cpu_rdev); + if (cpu_uV < 0) + return cpu_uV; + + core_uV = regulator_get_voltage_rdev(core_rdev); + if (core_uV < 0) + return core_uV; + + /* + * Bootloader shall set up voltages correctly, but if it + * happens that there is a violation, then try to fix it + * at first. + */ + core_min_limited_uV = tegra30_core_cpu_limit(cpu_uV); + if (core_min_limited_uV < 0) + return core_min_limited_uV; + + core_min_uV = max(core_min_uV, tegra30_core_cpu_limit(cpu_min_uV)); + + err = regulator_check_voltage(core_rdev, &core_min_uV, &core_max_uV); + if (err) + return err; + + if (core_min_limited_uV > core_uV) { + pr_err("core voltage constraint violated: %d %d %d\n", + core_uV, core_min_limited_uV, cpu_uV); + goto update_core; + } + + while (cpu_uV != cpu_min_uV || core_uV != core_min_uV) { + if (cpu_uV < cpu_min_uV) { + cpu_target_uV = min(cpu_uV + 100000, cpu_min_uV); + } else { + cpu_target_uV = max(cpu_uV - 100000, cpu_min_uV); + cpu_target_uV = max(core_uV - 300000, cpu_target_uV); + } + + err = regulator_set_voltage_rdev(cpu_rdev, + cpu_target_uV, + cpu_max_uV, + PM_SUSPEND_ON); + if (err) + return err; + + cpu_uV = cpu_target_uV; +update_core: + core_min_limited_uV = tegra30_core_cpu_limit(cpu_uV); + if (core_min_limited_uV < 0) + return core_min_limited_uV; + + core_target_uV = max(core_min_limited_uV, core_min_uV); + + if (core_uV < core_target_uV) { + core_target_uV = min(core_target_uV, core_uV + 100000); + core_target_uV = min(core_target_uV, cpu_uV + 300000); + } else { + core_target_uV = max(core_target_uV, core_uV - 100000); + } + + err = regulator_set_voltage_rdev(core_rdev, + core_target_uV, + core_max_uV, + PM_SUSPEND_ON); + if (err) + return err; + + core_uV = core_target_uV; + } + + return 0; +} + +static struct regulator_dev *lookup_rdev(struct regulator_dev *rdev, + const char * const *names, + unsigned int num_names) +{ + struct coupling_desc *c_desc = &rdev->coupling_desc; + unsigned int i, k; + + for (i = 0; i < num_names; i++) { + if (!strcmp(names[i], rdev_get_name(rdev))) + return rdev; + } + + for (k = 0; k < c_desc->n_coupled; k++) { + rdev = c_desc->coupled_rdevs[k]; + + for (i = 0; i < num_names; i++) { + if (!strcmp(names[i], rdev_get_name(rdev))) + return rdev; + } + } + + pr_err_once("%s: failed for %s\n", __func__, rdev_get_name(rdev)); + + for (i = 0; i < num_names; i++) + pr_err_once("%s: entry%u: %s\n", __func__, i, names[i]); + + return NULL; +} + +static int tegra30_regulator_balance_voltage(struct regulators_coupler *coupler, + struct regulator_dev *rdev, + suspend_state_t state) +{ + struct tegra_regulators_coupler *tegra = to_tegra_coupler(coupler); + struct regulator_dev *core_rdev; + struct regulator_dev *cpu_rdev; + + core_rdev = lookup_rdev(rdev, core_names, ARRAY_SIZE(core_names)); + cpu_rdev = lookup_rdev(rdev, cpu_names, ARRAY_SIZE(cpu_names)); + + if (!core_rdev || !cpu_rdev || state != PM_SUSPEND_ON) { + pr_err("regulators are not coupled properly\n"); + return -EINVAL; + } + + return tegra30_voltage_update(tegra, cpu_rdev, core_rdev); +} + +static struct tegra_regulators_coupler tegra30_coupler = { + .coupler = { + .balance_voltage = tegra30_regulator_balance_voltage, + }, +}; + +static int __init tegra_regulators_coupler_init(void) +{ + if (!of_machine_is_compatible("nvidia,tegra30")) + return 0; + + return regulators_coupler_register(&tegra30_coupler.coupler); +} +arch_initcall(tegra_regulators_coupler_init);
Add regulators coupler for Tegra30 SoC's that performs voltage balancing of a coupled regulators and thus provides voltage scaling functionality. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/soc/tegra/Kconfig | 6 + drivers/soc/tegra/Makefile | 1 + drivers/soc/tegra/regulators-tegra30.c | 256 +++++++++++++++++++++++++ 3 files changed, 263 insertions(+) create mode 100644 drivers/soc/tegra/regulators-tegra30.c