mbox series

[v2,00/48] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs

Message ID 20201217180638.22748-1-digetx@gmail.com
Headers show
Series Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs | expand

Message

Dmitry Osipenko Dec. 17, 2020, 6:05 p.m. UTC
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

Comments

Dmitry Osipenko Dec. 17, 2020, 6:45 p.m. UTC | #1
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.
Dmitry Osipenko Dec. 17, 2020, 8:58 p.m. UTC | #2
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.
Viresh Kumar Dec. 18, 2020, 7:14 a.m. UTC | #3
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.

-- 
viresh
Dmitry Osipenko Dec. 18, 2020, 1:51 p.m. UTC | #4
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!
Viresh Kumar Dec. 22, 2020, 9:15 a.m. UTC | #5
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).
Dmitry Osipenko Dec. 22, 2020, 7:14 p.m. UTC | #6
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
Krzysztof Kozlowski Jan. 5, 2021, 5:11 p.m. UTC | #7
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
Dmitry Osipenko Jan. 7, 2021, 7:39 p.m. UTC | #8
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.