| Message ID | 20201217180638.22748-1-digetx@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs | expand |
On 17/12/2020 19:06, Dmitry Osipenko wrote: > Enable CPU voltage scaling and thermal throttling on Tegra20 Ventana board. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > arch/arm/boot/dts/tegra20-ventana.dts | 40 ++++++++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts > index 14ace2ef749c..c2d9f38960bc 100644 > --- a/arch/arm/boot/dts/tegra20-ventana.dts > +++ b/arch/arm/boot/dts/tegra20-ventana.dts > @@ -2,8 +2,10 @@ > /dts-v1/; > > #include <dt-bindings/input/input.h> > +#include <dt-bindings/thermal/thermal.h> > #include "tegra20.dtsi" > #include "tegra20-cpu-opp.dtsi" > +#include "tegra20-cpu-opp-microvolt.dtsi" > > / { > model = "NVIDIA Tegra20 Ventana evaluation board"; > @@ -527,9 +529,10 @@ ldo_rtc { > }; > }; > > - temperature-sensor@4c { > + nct1008: temperature-sensor@4c { > compatible = "onnn,nct1008"; > reg = <0x4c>; > + #thermal-sensor-cells = <1>; > }; > }; > > @@ -615,10 +618,13 @@ clk32k_in: clock@0 { > > cpus { > cpu0: cpu@0 { > + cpu-supply = <&vdd_cpu>; > operating-points-v2 = <&cpu0_opp_table>; > + #cooling-cells = <2>; > }; > > cpu@1 { > + cpu-supply = <&vdd_cpu>; > operating-points-v2 = <&cpu0_opp_table>; > }; > }; > @@ -717,4 +723,36 @@ sound { > <&tegra_car TEGRA20_CLK_CDEV1>; > clock-names = "pll_a", "pll_a_out0", "mclk"; > }; > + > + thermal-zones { > + cpu-thermal { > + polling-delay-passive = <1000>; /* milliseconds */ > + polling-delay = <5000>; /* milliseconds */ > + > + thermal-sensors = <&nct1008 1>; > + > + trips { > + trip0: cpu-alert0 { > + /* start throttling at 50C */ > + temperature = <50000>; > + hysteresis = <200>; Did you mean <2000> ? > + type = "passive"; > + }; > + > + trip1: cpu-crit { > + /* shut down at 60C */ > + temperature = <60000>; > + hysteresis = <2000>; I think you can drop the hysteresis here, when the critical temperature is reached, there is an emergency shutdown. 50°C and 60°C sound very low values, no ? > + type = "critical"; > + }; > + }; > + > + cooling-maps { > + map0 { > + trip = <&trip0>; > + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; You should add all CPUs here. > + }; > + }; > + }; > + }; > }; >
On 17/12/2020 19:06, Dmitry Osipenko wrote: > Enable CPU voltage scaling and thermal throttling on Tegra30 Cardhu board. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- Same comments as 47/48 > arch/arm/boot/dts/tegra30-cardhu.dtsi | 61 ++++++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi > index d74c9ca78a7f..08c0ea4e6228 100644 > --- a/arch/arm/boot/dts/tegra30-cardhu.dtsi > +++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi > @@ -1,6 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <dt-bindings/input/input.h> > +#include <dt-bindings/thermal/thermal.h> > #include "tegra30.dtsi" > +#include "tegra30-cpu-opp.dtsi" > +#include "tegra30-cpu-opp-microvolt.dtsi" > > /** > * This file contains common DT entry for all fab version of Cardhu. > @@ -339,12 +342,13 @@ ldo8_reg: ldo8 { > }; > }; > > - temperature-sensor@4c { > + nct1008: temperature-sensor@4c { > compatible = "onnn,nct1008"; > reg = <0x4c>; > vcc-supply = <&sys_3v3_reg>; > interrupt-parent = <&gpio>; > interrupts = <TEGRA_GPIO(CC, 2) IRQ_TYPE_LEVEL_LOW>; > + #thermal-sensor-cells = <1>; > }; > > vdd_core: tps62361@60 { > @@ -438,6 +442,29 @@ clk32k_in: clock@0 { > #clock-cells = <0>; > }; > > + cpus { > + cpu0: cpu@0 { > + cpu-supply = <&vddctrl_reg>; > + operating-points-v2 = <&cpu0_opp_table>; > + #cooling-cells = <2>; > + }; > + > + cpu@1 { > + cpu-supply = <&vddctrl_reg>; > + operating-points-v2 = <&cpu0_opp_table>; > + }; > + > + cpu@2 { > + cpu-supply = <&vddctrl_reg>; > + operating-points-v2 = <&cpu0_opp_table>; > + }; > + > + cpu@3 { > + cpu-supply = <&vddctrl_reg>; > + operating-points-v2 = <&cpu0_opp_table>; > + }; > + }; > + > panel: panel { > compatible = "chunghwa,claa101wb01"; > ddc-i2c-bus = <&panelddc>; > @@ -617,6 +644,38 @@ sound { > <&tegra_car TEGRA30_CLK_EXTERN1>; > }; > > + thermal-zones { > + cpu-thermal { > + polling-delay-passive = <1000>; /* milliseconds */ > + polling-delay = <5000>; /* milliseconds */ > + > + thermal-sensors = <&nct1008 1>; > + > + trips { > + trip0: cpu-alert0 { > + /* throttle at 57C until temperature drops to 56.8C */ > + temperature = <57000>; > + hysteresis = <200>; > + type = "passive"; > + }; > + > + trip1: cpu-crit { > + /* shut down at 60C */ > + temperature = <60000>; > + hysteresis = <2000>; > + type = "critical"; > + }; > + }; > + > + cooling-maps { > + map0 { > + trip = <&trip0>; > + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; > + }; > + }; > + }; > + }; > + > gpio-keys { > compatible = "gpio-keys"; > >
17.12.2020 21:21, Mikko Perttunen пишет: > On 12/17/20 8:06 PM, Dmitry Osipenko wrote: >> Add suspend/resume and generic power domain support to the Host1x driver. >> This is required for enabling system-wide DVFS and supporting dynamic >> power management using a generic power domain. >> >> Tested-by: Peter Geis <pgwipeout@gmail.com> >> Tested-by: Nicolas Chauvet <kwizart@gmail.com> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/gpu/host1x/dev.c | 102 ++++++++++++++++++++++++++++++++++----- >> 1 file changed, 91 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c >> index d0ebb70e2fdd..c1525cffe7b1 100644 >> --- a/drivers/gpu/host1x/dev.c >> +++ b/drivers/gpu/host1x/dev.c >> @@ -12,6 +12,7 @@ >> #include <linux/module.h> >> #include <linux/of_device.h> >> #include <linux/of.h> >> +#include <linux/pm_runtime.h> >> #include <linux/slab.h> >> #define CREATE_TRACE_POINTS >> @@ -417,7 +418,7 @@ static int host1x_probe(struct platform_device *pdev) >> return err; >> } >> - host->rst = devm_reset_control_get(&pdev->dev, "host1x"); >> + host->rst = devm_reset_control_get_exclusive_released(&pdev->dev, >> "host1x"); >> if (IS_ERR(host->rst)) { >> err = PTR_ERR(host->rst); >> dev_err(&pdev->dev, "failed to get reset: %d\n", err); >> @@ -437,16 +438,15 @@ static int host1x_probe(struct platform_device >> *pdev) >> goto iommu_exit; >> } >> - err = clk_prepare_enable(host->clk); >> - if (err < 0) { >> - dev_err(&pdev->dev, "failed to enable clock\n"); >> - goto free_channels; >> - } >> + pm_runtime_enable(&pdev->dev); >> + err = pm_runtime_get_sync(&pdev->dev); >> + if (err < 0) >> + goto rpm_disable; >> err = reset_control_deassert(host->rst); >> if (err < 0) { >> dev_err(&pdev->dev, "failed to deassert reset: %d\n", err); >> - goto unprepare_disable; >> + goto rpm_disable; >> } >> err = host1x_syncpt_init(host); >> @@ -485,9 +485,10 @@ static int host1x_probe(struct platform_device >> *pdev) >> host1x_syncpt_deinit(host); >> reset_assert: >> reset_control_assert(host->rst); >> -unprepare_disable: >> - clk_disable_unprepare(host->clk); >> -free_channels: >> +rpm_disable: >> + pm_runtime_put(&pdev->dev); >> + pm_runtime_disable(&pdev->dev); >> + >> host1x_channel_list_free(&host->channel_list); >> iommu_exit: >> host1x_iommu_exit(host); >> @@ -504,16 +505,95 @@ static int host1x_remove(struct platform_device >> *pdev) >> host1x_intr_deinit(host); >> host1x_syncpt_deinit(host); >> reset_control_assert(host->rst); >> - clk_disable_unprepare(host->clk); >> + pm_runtime_put(&pdev->dev); >> + pm_runtime_disable(&pdev->dev); >> host1x_iommu_exit(host); >> return 0; >> } >> +static int __maybe_unused host1x_runtime_suspend(struct device *dev) >> +{ >> + struct host1x *host = dev_get_drvdata(dev); >> + >> + clk_disable_unprepare(host->clk); >> + reset_control_release(host->rst); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused host1x_runtime_resume(struct device *dev) >> +{ >> + struct host1x *host = dev_get_drvdata(dev); >> + int err; >> + >> + err = reset_control_acquire(host->rst); >> + if (err) { >> + dev_err(dev, "failed to acquire reset: %d\n", err); >> + return err; >> + } >> + >> + err = clk_prepare_enable(host->clk); >> + if (err) { >> + dev_err(dev, "failed to enable clock: %d\n", err); >> + goto release_reset; >> + } >> + >> + return 0; >> + >> +release_reset: >> + reset_control_release(host->rst); >> + >> + return err; >> +} >> + >> +static __maybe_unused int host1x_suspend(struct device *dev) >> +{ >> + struct host1x *host = dev_get_drvdata(dev); >> + int err; >> + >> + host1x_syncpt_save(host); >> + >> + err = pm_runtime_force_suspend(dev); >> + if (err < 0) >> + return err; >> + >> + return 0; >> +} >> + >> +static __maybe_unused int host1x_resume(struct device *dev) >> +{ >> + struct host1x *host = dev_get_drvdata(dev); >> + struct host1x_channel *channel; >> + unsigned int index; >> + int err; >> + >> + err = pm_runtime_force_resume(dev); >> + if (err < 0) >> + return err; >> + >> + host1x_syncpt_restore(host); > > We also need to execute 'host1x_setup_sid_table' upon resume. Indeed, thanks. I'll correct it in the next revision. Perhaps the actual save/restore needs to be moved to the runtime callbacks. At least I can't remember right now why this wasn't done in the first place.
17.12.2020 21:28, Daniel Lezcano пишет: > On 17/12/2020 19:06, Dmitry Osipenko wrote: >> Enable CPU voltage scaling and thermal throttling on Tegra20 Ventana board. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> arch/arm/boot/dts/tegra20-ventana.dts | 40 ++++++++++++++++++++++++++- >> 1 file changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts >> index 14ace2ef749c..c2d9f38960bc 100644 >> --- a/arch/arm/boot/dts/tegra20-ventana.dts >> +++ b/arch/arm/boot/dts/tegra20-ventana.dts >> @@ -2,8 +2,10 @@ >> /dts-v1/; >> >> #include <dt-bindings/input/input.h> >> +#include <dt-bindings/thermal/thermal.h> >> #include "tegra20.dtsi" >> #include "tegra20-cpu-opp.dtsi" >> +#include "tegra20-cpu-opp-microvolt.dtsi" >> >> / { >> model = "NVIDIA Tegra20 Ventana evaluation board"; >> @@ -527,9 +529,10 @@ ldo_rtc { >> }; >> }; >> >> - temperature-sensor@4c { >> + nct1008: temperature-sensor@4c { >> compatible = "onnn,nct1008"; >> reg = <0x4c>; >> + #thermal-sensor-cells = <1>; >> }; >> }; >> >> @@ -615,10 +618,13 @@ clk32k_in: clock@0 { >> >> cpus { >> cpu0: cpu@0 { >> + cpu-supply = <&vdd_cpu>; >> operating-points-v2 = <&cpu0_opp_table>; >> + #cooling-cells = <2>; >> }; >> >> cpu@1 { >> + cpu-supply = <&vdd_cpu>; >> operating-points-v2 = <&cpu0_opp_table>; >> }; >> }; >> @@ -717,4 +723,36 @@ sound { >> <&tegra_car TEGRA20_CLK_CDEV1>; >> clock-names = "pll_a", "pll_a_out0", "mclk"; >> }; >> + >> + thermal-zones { >> + cpu-thermal { >> + polling-delay-passive = <1000>; /* milliseconds */ >> + polling-delay = <5000>; /* milliseconds */ >> + >> + thermal-sensors = <&nct1008 1>; >> + >> + trips { >> + trip0: cpu-alert0 { >> + /* start throttling at 50C */ >> + temperature = <50000>; >> + hysteresis = <200>; > > Did you mean <2000> ? The <200> is correct. Please see this commit for example: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2b22393b27670b45a2c870bce3df6579efd9a86a >> + type = "passive"; >> + }; >> + >> + trip1: cpu-crit { >> + /* shut down at 60C */ >> + temperature = <60000>; >> + hysteresis = <2000>; > > I think you can drop the hysteresis here, when the critical temperature > is reached, there is an emergency shutdown. Yes, perhaps you're right. Hysteresis doesn't make sense for a critical trip. I'll improve it in the next revision, thanks. > 50°C and 60°C sound very low values, no ? Tegra20 CPU is very cold in comparison to later generations, it should stay under 50°C even at a full load. >> + type = "critical"; >> + }; >> + }; >> + >> + cooling-maps { >> + map0 { >> + trip = <&trip0>; >> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; > > You should add all CPUs here. All CPU cores are coupled on Tegra in regards to CPUFreq, hence I think it won't make any difference if secondary CPU cores will be added here, isn't it?
On 17/12/2020 20:01, Dmitry Osipenko wrote: > 17.12.2020 21:28, Daniel Lezcano пишет: >> On 17/12/2020 19:06, Dmitry Osipenko wrote: >>> Enable CPU voltage scaling and thermal throttling on Tegra20 Ventana board. >>> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> arch/arm/boot/dts/tegra20-ventana.dts | 40 ++++++++++++++++++++++++++- >>> 1 file changed, 39 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts >>> index 14ace2ef749c..c2d9f38960bc 100644 >>> --- a/arch/arm/boot/dts/tegra20-ventana.dts >>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts >>> @@ -2,8 +2,10 @@ >>> /dts-v1/; >>> >>> #include <dt-bindings/input/input.h> >>> +#include <dt-bindings/thermal/thermal.h> >>> #include "tegra20.dtsi" >>> #include "tegra20-cpu-opp.dtsi" >>> +#include "tegra20-cpu-opp-microvolt.dtsi" >>> >>> / { >>> model = "NVIDIA Tegra20 Ventana evaluation board"; >>> @@ -527,9 +529,10 @@ ldo_rtc { >>> }; >>> }; >>> >>> - temperature-sensor@4c { >>> + nct1008: temperature-sensor@4c { >>> compatible = "onnn,nct1008"; >>> reg = <0x4c>; >>> + #thermal-sensor-cells = <1>; >>> }; >>> }; >>> >>> @@ -615,10 +618,13 @@ clk32k_in: clock@0 { >>> >>> cpus { >>> cpu0: cpu@0 { >>> + cpu-supply = <&vdd_cpu>; >>> operating-points-v2 = <&cpu0_opp_table>; >>> + #cooling-cells = <2>; >>> }; >>> >>> cpu@1 { >>> + cpu-supply = <&vdd_cpu>; >>> operating-points-v2 = <&cpu0_opp_table>; >>> }; >>> }; >>> @@ -717,4 +723,36 @@ sound { >>> <&tegra_car TEGRA20_CLK_CDEV1>; >>> clock-names = "pll_a", "pll_a_out0", "mclk"; >>> }; >>> + >>> + thermal-zones { >>> + cpu-thermal { >>> + polling-delay-passive = <1000>; /* milliseconds */ >>> + polling-delay = <5000>; /* milliseconds */ >>> + >>> + thermal-sensors = <&nct1008 1>; >>> + >>> + trips { >>> + trip0: cpu-alert0 { >>> + /* start throttling at 50C */ >>> + temperature = <50000>; >>> + hysteresis = <200>; >> >> Did you mean <2000> ? > > The <200> is correct. > > Please see this commit for example: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2b22393b27670b45a2c870bce3df6579efd9a86a Ok, thanks for the pointer. >>> + type = "passive"; >>> + }; >>> + >>> + trip1: cpu-crit { >>> + /* shut down at 60C */ >>> + temperature = <60000>; >>> + hysteresis = <2000>; >> >> I think you can drop the hysteresis here, when the critical temperature >> is reached, there is an emergency shutdown. > > Yes, perhaps you're right. Hysteresis doesn't make sense for a critical > trip. I'll improve it in the next revision, thanks. > >> 50°C and 60°C sound very low values, no ? > > Tegra20 CPU is very cold in comparison to later generations, it should > stay under 50°C even at a full load. Interesting, thanks. >>> + type = "critical"; >>> + }; >>> + }; >>> + >>> + cooling-maps { >>> + map0 { >>> + trip = <&trip0>; >>> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; >> >> You should add all CPUs here. > > > All CPU cores are coupled on Tegra in regards to CPUFreq, hence I think > it won't make any difference if secondary CPU cores will be added here, > isn't it? The explanation is in the description of commit ef4734500407ce4d
17.12.2020 21:45, Dmitry Osipenko пишет: > 17.12.2020 21:21, Mikko Perttunen пишет: >> On 12/17/20 8:06 PM, Dmitry Osipenko wrote: >>> Add suspend/resume and generic power domain support to the Host1x driver. >>> This is required for enabling system-wide DVFS and supporting dynamic >>> power management using a generic power domain. >>> >>> Tested-by: Peter Geis <pgwipeout@gmail.com> >>> Tested-by: Nicolas Chauvet <kwizart@gmail.com> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> drivers/gpu/host1x/dev.c | 102 ++++++++++++++++++++++++++++++++++----- >>> 1 file changed, 91 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c >>> index d0ebb70e2fdd..c1525cffe7b1 100644 >>> --- a/drivers/gpu/host1x/dev.c >>> +++ b/drivers/gpu/host1x/dev.c >>> @@ -12,6 +12,7 @@ >>> #include <linux/module.h> >>> #include <linux/of_device.h> >>> #include <linux/of.h> >>> +#include <linux/pm_runtime.h> >>> #include <linux/slab.h> >>> #define CREATE_TRACE_POINTS >>> @@ -417,7 +418,7 @@ static int host1x_probe(struct platform_device *pdev) >>> return err; >>> } >>> - host->rst = devm_reset_control_get(&pdev->dev, "host1x"); >>> + host->rst = devm_reset_control_get_exclusive_released(&pdev->dev, >>> "host1x"); >>> if (IS_ERR(host->rst)) { >>> err = PTR_ERR(host->rst); >>> dev_err(&pdev->dev, "failed to get reset: %d\n", err); >>> @@ -437,16 +438,15 @@ static int host1x_probe(struct platform_device >>> *pdev) >>> goto iommu_exit; >>> } >>> - err = clk_prepare_enable(host->clk); >>> - if (err < 0) { >>> - dev_err(&pdev->dev, "failed to enable clock\n"); >>> - goto free_channels; >>> - } >>> + pm_runtime_enable(&pdev->dev); >>> + err = pm_runtime_get_sync(&pdev->dev); >>> + if (err < 0) >>> + goto rpm_disable; >>> err = reset_control_deassert(host->rst); >>> if (err < 0) { >>> dev_err(&pdev->dev, "failed to deassert reset: %d\n", err); >>> - goto unprepare_disable; >>> + goto rpm_disable; >>> } >>> err = host1x_syncpt_init(host); >>> @@ -485,9 +485,10 @@ static int host1x_probe(struct platform_device >>> *pdev) >>> host1x_syncpt_deinit(host); >>> reset_assert: >>> reset_control_assert(host->rst); >>> -unprepare_disable: >>> - clk_disable_unprepare(host->clk); >>> -free_channels: >>> +rpm_disable: >>> + pm_runtime_put(&pdev->dev); >>> + pm_runtime_disable(&pdev->dev); >>> + >>> host1x_channel_list_free(&host->channel_list); >>> iommu_exit: >>> host1x_iommu_exit(host); >>> @@ -504,16 +505,95 @@ static int host1x_remove(struct platform_device >>> *pdev) >>> host1x_intr_deinit(host); >>> host1x_syncpt_deinit(host); >>> reset_control_assert(host->rst); >>> - clk_disable_unprepare(host->clk); >>> + pm_runtime_put(&pdev->dev); >>> + pm_runtime_disable(&pdev->dev); >>> host1x_iommu_exit(host); >>> return 0; >>> } >>> +static int __maybe_unused host1x_runtime_suspend(struct device *dev) >>> +{ >>> + struct host1x *host = dev_get_drvdata(dev); >>> + >>> + clk_disable_unprepare(host->clk); >>> + reset_control_release(host->rst); >>> + >>> + return 0; >>> +} >>> + >>> +static int __maybe_unused host1x_runtime_resume(struct device *dev) >>> +{ >>> + struct host1x *host = dev_get_drvdata(dev); >>> + int err; >>> + >>> + err = reset_control_acquire(host->rst); >>> + if (err) { >>> + dev_err(dev, "failed to acquire reset: %d\n", err); >>> + return err; >>> + } >>> + >>> + err = clk_prepare_enable(host->clk); >>> + if (err) { >>> + dev_err(dev, "failed to enable clock: %d\n", err); >>> + goto release_reset; >>> + } >>> + >>> + return 0; >>> + >>> +release_reset: >>> + reset_control_release(host->rst); >>> + >>> + return err; >>> +} >>> + >>> +static __maybe_unused int host1x_suspend(struct device *dev) >>> +{ >>> + struct host1x *host = dev_get_drvdata(dev); >>> + int err; >>> + >>> + host1x_syncpt_save(host); >>> + >>> + err = pm_runtime_force_suspend(dev); >>> + if (err < 0) >>> + return err; >>> + >>> + return 0; >>> +} >>> + >>> +static __maybe_unused int host1x_resume(struct device *dev) >>> +{ >>> + struct host1x *host = dev_get_drvdata(dev); >>> + struct host1x_channel *channel; >>> + unsigned int index; >>> + int err; >>> + >>> + err = pm_runtime_force_resume(dev); >>> + if (err < 0) >>> + return err; >>> + >>> + host1x_syncpt_restore(host); >> >> We also need to execute 'host1x_setup_sid_table' upon resume. > > Indeed, thanks. I'll correct it in the next revision. > > Perhaps the actual save/restore needs to be moved to the runtime > callbacks. At least I can't remember right now why this wasn't done in > the first place. > I looked at the save/restore once again and recalled why it's done so. The reason is that the host1x touches hardware during the driver probe, and thus, RPM needs to be resumed first. It will be a bigger change to properly decouple the hardware initialization so that it all could be put it into the RPM callback. I'll try to do it in v3.
On 17/12/2020 21:28, Dmitry Osipenko wrote: > 17.12.2020 22:36, Daniel Lezcano пишет: >>>>> + type = "critical"; >>>>> + }; >>>>> + }; >>>>> + >>>>> + cooling-maps { >>>>> + map0 { >>>>> + trip = <&trip0>; >>>>> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; >>>> You should add all CPUs here. >>> >>> All CPU cores are coupled on Tegra in regards to CPUFreq, hence I think >>> it won't make any difference if secondary CPU cores will be added here, >>> isn't it? >> The explanation is in the description of commit ef4734500407ce4d > > I think that really only makes sense if CPU cores have independent clock > rate management. ATM I did not see any ARM platform having a clock line per CPU but I may be wrong. > IIRC, I actually made some research about this in the > past and intentionally removed the secondary cores from the > cooling-device since they didn't make any difference for a coupled CPU > cores. > > That commit also says: > > "But as soon as this CPU ordering changes and any other CPU is used to > bring up the cooling device, we will start seeing failures." > > I don't quite understand to what "failures" that commit referrers. I > tried to change the cpu0 to cpu1 in the cooling-device and don't see any > failures. Could you please clarify this? > > In general it should be fine to add all the cores to the cooling-device > and I'll do it in v3, but I want to make it clear why this is needed. AFAIR, if CPU0 is unplugged the cooling device can not rebind to CPU1. And if CPU0 is plugged in again, the cooling device fails to initialize. And, if the CPUs are mapped with the physical CPU0 to Linux numbering CPU1, the cooling device mapping will fail.
18.12.2020 00:19, Daniel Lezcano пишет: > On 17/12/2020 21:28, Dmitry Osipenko wrote: >> 17.12.2020 22:36, Daniel Lezcano пишет: >>>>>> + type = "critical"; >>>>>> + }; >>>>>> + }; >>>>>> + >>>>>> + cooling-maps { >>>>>> + map0 { >>>>>> + trip = <&trip0>; >>>>>> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; >>>>> You should add all CPUs here. >>>> >>>> All CPU cores are coupled on Tegra in regards to CPUFreq, hence I think >>>> it won't make any difference if secondary CPU cores will be added here, >>>> isn't it? >>> The explanation is in the description of commit ef4734500407ce4d >> >> I think that really only makes sense if CPU cores have independent clock >> rate management. > > ATM I did not see any ARM platform having a clock line per CPU but I may > be wrong. > >> IIRC, I actually made some research about this in the >> past and intentionally removed the secondary cores from the >> cooling-device since they didn't make any difference for a coupled CPU >> cores. >> >> That commit also says: >> >> "But as soon as this CPU ordering changes and any other CPU is used to >> bring up the cooling device, we will start seeing failures." >> >> I don't quite understand to what "failures" that commit referrers. I >> tried to change the cpu0 to cpu1 in the cooling-device and don't see any >> failures. Could you please clarify this? >> >> In general it should be fine to add all the cores to the cooling-device >> and I'll do it in v3, but I want to make it clear why this is needed. > > AFAIR, if CPU0 is unplugged the cooling device can not rebind to CPU1. > And if CPU0 is plugged in again, the cooling device fails to initialize. > > And, if the CPUs are mapped with the physical CPU0 to Linux numbering > CPU1, the cooling device mapping will fail. Alright, thank you.
On 17-12-20, 21:05, Dmitry Osipenko wrote: > Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces > power consumption and heating of the Tegra chips. Tegra SoC has multiple > hardware units which belong to a core power domain of the SoC and share > the core voltage. The voltage must be selected in accordance to a minimum > requirement of every core hardware unit. Please submit the OPP changes separately (alone), it will never get merged this way. Send fixes at the top, any features you want later in the series. It is fine for you to base your series of patches which are under review, you just need to mention that in your cover letter for your platform's patchset.
18.12.2020 10:14, Viresh Kumar пишет: > On 17-12-20, 21:05, Dmitry Osipenko wrote: >> Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces >> power consumption and heating of the Tegra chips. Tegra SoC has multiple >> hardware units which belong to a core power domain of the SoC and share >> the core voltage. The voltage must be selected in accordance to a minimum >> requirement of every core hardware unit. > > Please submit the OPP changes separately (alone), it will never get > merged this way. Send fixes at the top, any features you want later in > the series. It is fine for you to base your series of patches which > are under review, you just need to mention that in your cover letter > for your platform's patchset. > Alright, although I haven't pretended that v2 patches should be merged right away since they are fundamentally different from v1, and thus, all patches need to be reviewed first. If the current OPP changes look good to you, then please give yours r-b to the patches. Thanks in advance!
On 18-12-20, 16:51, Dmitry Osipenko wrote: > Alright, although I haven't pretended that v2 patches should be merged > right away since they are fundamentally different from v1, and thus, all > patches need to be reviewed first. I agree. I have done some basic review for the stuff. > If the current OPP changes look good to you, then please give yours r-b > to the patches. Thanks in advance! r-b-y isn't required as they will go through my tree itself. So if everyone is happy with the idea, please submit the patches separately (fixes, improvements, devm_*, etc).
22.12.2020 12:15, Viresh Kumar пишет: > On 18-12-20, 16:51, Dmitry Osipenko wrote: >> Alright, although I haven't pretended that v2 patches should be merged >> right away since they are fundamentally different from v1, and thus, all >> patches need to be reviewed first. > > I agree. I have done some basic review for the stuff. Thank you for the review. >> If the current OPP changes look good to you, then please give yours r-b >> to the patches. Thanks in advance! > > r-b-y isn't required as they will go through my tree itself. So if everyone is > happy with the idea, please submit the patches separately (fixes, improvements, > devm_*, etc). okay
On Thu, Dec 17, 2020 at 09:05:50PM +0300, Dmitry Osipenko wrote: > Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces > power consumption and heating of the Tegra chips. Tegra SoC has multiple > hardware units which belong to a core power domain of the SoC and share > the core voltage. The voltage must be selected in accordance to a minimum > requirement of every core hardware unit. > > The minimum core voltage requirement depends on: > > 1. Clock enable state of a hardware unit. > 2. Clock frequency. > 3. Unit's internal idling/active state. > > This series is tested on Acer A500 (T20), AC100 (T20), Nexus 7 (T30), > Ouya (T30), TK1 (T124) and some others. I also added voltage scaling to > the Ventana (T20) and Cardhu (T30) boards which are tested by NVIDIA's CI > farm. Tegra30 is now couple degrees cooler on Nexus 7 and stays cool on > Ouya (instead of becoming burning hot) while system is idling. It should > be possible to improve this further by implementing a more advanced power > management features for the kernel drivers. > > The DVFS support is opt-in for all boards, meaning that older DTBs will > continue to work like they did it before this series. It should be possible > to easily add the core voltage scaling support for Tegra114+ SoCs based on > this grounding work later on, if anyone will want to implement it. The same comment as for your interconnect work: for sets touching multiple systems please mention the dependencies between patches in the cover letter. Not as a reply to such remark like I make here, but as a separate entry in the cover letter. Best regards, Krzysztof
05.01.2021 20:11, Krzysztof Kozlowski пишет: > On Thu, Dec 17, 2020 at 09:05:50PM +0300, Dmitry Osipenko wrote: >> Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces >> power consumption and heating of the Tegra chips. Tegra SoC has multiple >> hardware units which belong to a core power domain of the SoC and share >> the core voltage. The voltage must be selected in accordance to a minimum >> requirement of every core hardware unit. >> >> The minimum core voltage requirement depends on: >> >> 1. Clock enable state of a hardware unit. >> 2. Clock frequency. >> 3. Unit's internal idling/active state. >> >> This series is tested on Acer A500 (T20), AC100 (T20), Nexus 7 (T30), >> Ouya (T30), TK1 (T124) and some others. I also added voltage scaling to >> the Ventana (T20) and Cardhu (T30) boards which are tested by NVIDIA's CI >> farm. Tegra30 is now couple degrees cooler on Nexus 7 and stays cool on >> Ouya (instead of becoming burning hot) while system is idling. It should >> be possible to improve this further by implementing a more advanced power >> management features for the kernel drivers. >> >> The DVFS support is opt-in for all boards, meaning that older DTBs will >> continue to work like they did it before this series. It should be possible >> to easily add the core voltage scaling support for Tegra114+ SoCs based on >> this grounding work later on, if anyone will want to implement it. > > The same comment as for your interconnect work: for sets touching > multiple systems please mention the dependencies between patches in the > cover letter. Not as a reply to such remark like I make here, but as a > separate entry in the cover letter. I'll describe all the dependencies in the next revision, thanks.
Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces power consumption and heating of the Tegra chips. Tegra SoC has multiple hardware units which belong to a core power domain of the SoC and share the core voltage. The voltage must be selected in accordance to a minimum requirement of every core hardware unit. The minimum core voltage requirement depends on: 1. Clock enable state of a hardware unit. 2. Clock frequency. 3. Unit's internal idling/active state. This series is tested on Acer A500 (T20), AC100 (T20), Nexus 7 (T30), Ouya (T30), TK1 (T124) and some others. I also added voltage scaling to the Ventana (T20) and Cardhu (T30) boards which are tested by NVIDIA's CI farm. Tegra30 is now couple degrees cooler on Nexus 7 and stays cool on Ouya (instead of becoming burning hot) while system is idling. It should be possible to improve this further by implementing a more advanced power management features for the kernel drivers. The DVFS support is opt-in for all boards, meaning that older DTBs will continue to work like they did it before this series. It should be possible to easily add the core voltage scaling support for Tegra114+ SoCs based on this grounding work later on, if anyone will want to implement it. Changelog: v2: - Replaced Core voltage regulator with a Core power domain. The voltage control is now done using GENPD API. This was suggested by Ulf Hansson. - Added basic runtime PM and GENPD support to 2d, 3d, host1x and clk drivers. - Added new core-power-domain and clk-device drivers. Some high-freq PLLs and clocks require a higher minimum core voltage and the new clk-device driver manages the voltage for these clocks based on the clock state. - Moved voltage scaling entirely to the new clk-device driver for devices which don't require advanced power management, like PWM for example. - Added devm_tegra_core_dev_init_opp_table() common helper which sets up OPP table for Tegra drivers. - Added resource-managed version for OPP API functions, as it was discussed previously in the comments to v1. - Added new APIs, features and fixed various bugs related to voltage scaling and power management done via GENPD API. Dmitry Osipenko (48): dt-bindings: memory: tegra20: emc: Replace core regulator with power domain dt-bindings: memory: tegra30: emc: Replace core regulator with power domain dt-bindings: memory: tegra124: emc: Replace core regulator with power domain dt-bindings: host1x: Document OPP and power domain properties media: dt: bindings: tegra-vde: Document OPP and power domain properties dt-bindings: clock: tegra: Document clocks sub-node dt-bindings: arm: tegra: Add binding for core power domain regulator: Make regulator_sync_voltage() usable by coupled regulators opp: Add dev_pm_opp_sync_regulators() opp: Add dev_pm_opp_set_voltage() opp: Add dev_pm_opp_find_level_ceil() opp: Add dev_pm_opp_get_required_pstate() opp: Add resource-managed versions of OPP API functions opp: Filter out OPPs based on availability of a required-OPP opp: Support set_opp() customization without requiring to use regulators opp: Handle missing OPP table in dev_pm_opp_xlate_performance_state() opp: Correct debug message in _opp_add_static_v2() opp: Print OPP level in debug message of _opp_add_static_v2() opp: Fix adding OPP entries in a wrong order if rate is unavailable PM: domains: Make set_performance_state() callback optional PM: domains: Add "performance" column to debug summary soc/tegra: pmc: Fix imbalanced clock disabling in error code path soc/tegra: pmc: Pulse resets after removing power clamp soc/tegra: pmc: Ensure that clock rates aren't too high soc/tegra: pmc: Print out domain name when reset fails to acquire soc/tegra: Add devm_tegra_core_dev_init_opp_table() soc/tegra: Add CONFIG_SOC_TEGRA_COMMON and select PM_OPP by default soc/tegra: Introduce core power domain driver soc/tegra: pmc: Link domains to the parent Core domain soc/tegra: regulators: Fix locking up when voltage-spread is out of range soc/tegra: regulators: Support Core domain state syncing clk: tegra: Support runtime PM, power domain and OPP gpu: host1x: Add host1x_channel_stop() gpu: host1x: Support power management drm/tegra: dc: Support OPP and SoC core voltage scaling drm/tegra: gr2d: Correct swapped device-tree compatibles drm/tegra: gr2d: Support OPP and power management drm/tegra: g3d: Support OPP and power management drm/tegra: vic: Stop channel before suspending media: staging: tegra-vde: Support OPP and generic power domain memory: tegra20-emc: Use devm_tegra_core_dev_init_opp_table() memory: tegra30-emc: Use devm_tegra_core_dev_init_opp_table() ARM: tegra: Add OPP tables and power domains to Tegra20 device-tree ARM: tegra: Add OPP tables and power domains to Tegra30 device-tree ARM: tegra: acer-a500: Enable core voltage scaling ARM: tegra: ventana: Enable core voltage scaling ARM: tegra: ventana: Support CPU voltage scaling and thermal throttling ARM: tegra: cardhu: Support CPU voltage scaling and thermal throttling .../arm/tegra/nvidia,tegra20-core-domain.yaml | 48 + .../bindings/clock/nvidia,tegra20-car.txt | 26 + .../bindings/clock/nvidia,tegra30-car.txt | 26 + .../display/tegra/nvidia,tegra20-host1x.txt | 49 + .../bindings/media/nvidia,tegra-vde.txt | 12 + .../nvidia,tegra124-emc.yaml | 6 +- .../memory-controllers/nvidia,tegra20-emc.txt | 4 +- .../nvidia,tegra30-emc.yaml | 6 +- .../boot/dts/tegra20-acer-a500-picasso.dts | 8 +- arch/arm/boot/dts/tegra20-colibri.dtsi | 6 +- arch/arm/boot/dts/tegra20-harmony.dts | 6 +- arch/arm/boot/dts/tegra20-paz00.dts | 46 +- .../arm/boot/dts/tegra20-peripherals-opp.dtsi | 941 +++++++++++ arch/arm/boot/dts/tegra20-seaboard.dts | 6 +- arch/arm/boot/dts/tegra20-tamonten.dtsi | 6 +- arch/arm/boot/dts/tegra20-trimslice.dts | 12 + arch/arm/boot/dts/tegra20-ventana.dts | 78 +- arch/arm/boot/dts/tegra20.dtsi | 220 +++ .../tegra30-asus-nexus7-grouper-common.dtsi | 4 + arch/arm/boot/dts/tegra30-beaver.dts | 4 + arch/arm/boot/dts/tegra30-cardhu.dtsi | 81 +- arch/arm/boot/dts/tegra30-colibri.dtsi | 20 +- arch/arm/boot/dts/tegra30-ouya.dts | 4 + .../arm/boot/dts/tegra30-peripherals-opp.dtsi | 1412 +++++++++++++++++ arch/arm/boot/dts/tegra30.dtsi | 358 +++++ drivers/base/power/domain.c | 33 +- drivers/clk/tegra/Makefile | 1 + drivers/clk/tegra/clk-device.c | 222 +++ drivers/clk/tegra/clk-divider.c | 2 +- drivers/clk/tegra/clk-periph-gate.c | 2 +- drivers/clk/tegra/clk-periph.c | 2 +- drivers/clk/tegra/clk-pll.c | 2 +- drivers/clk/tegra/clk-super.c | 4 +- drivers/clk/tegra/clk-tegra-periph.c | 140 +- drivers/clk/tegra/clk-tegra114.c | 1 + drivers/clk/tegra/clk-tegra124.c | 1 + drivers/clk/tegra/clk-tegra20-emc.c | 2 +- drivers/clk/tegra/clk-tegra20.c | 123 +- drivers/clk/tegra/clk-tegra210.c | 1 + drivers/clk/tegra/clk-tegra30.c | 133 +- drivers/clk/tegra/clk.c | 89 ++ drivers/clk/tegra/clk.h | 7 + drivers/gpu/drm/tegra/dc.c | 66 +- drivers/gpu/drm/tegra/gr2d.c | 77 +- drivers/gpu/drm/tegra/gr3d.c | 264 ++- drivers/gpu/drm/tegra/vic.c | 15 + drivers/gpu/host1x/channel.c | 8 + drivers/gpu/host1x/dev.c | 102 +- drivers/memory/tegra/tegra20-emc.c | 57 +- drivers/memory/tegra/tegra30-emc.c | 57 +- drivers/opp/core.c | 390 ++++- drivers/opp/of.c | 34 +- drivers/opp/opp.h | 2 +- drivers/regulator/core.c | 6 + drivers/soc/tegra/Kconfig | 19 + drivers/soc/tegra/Makefile | 1 + drivers/soc/tegra/common.c | 137 ++ drivers/soc/tegra/core-power-domain.c | 125 ++ drivers/soc/tegra/pmc.c | 122 +- drivers/soc/tegra/regulators-tegra20.c | 19 +- drivers/soc/tegra/regulators-tegra30.c | 20 +- drivers/staging/media/tegra-vde/vde.c | 63 +- include/linux/host1x.h | 1 + include/linux/pm_opp.h | 81 + include/soc/tegra/common.h | 41 + 65 files changed, 5458 insertions(+), 403 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-core-domain.yaml create mode 100644 drivers/clk/tegra/clk-device.c create mode 100644 drivers/soc/tegra/core-power-domain.c