Re: [PATCH v8 01/34] opp: Add dev_pm_opp_sync() helper

[Dmitry Osipenko <digetx@gmail.com>]

18.08.2021 13:08, Ulf Hansson пишет:
> On Wed, 18 Aug 2021 at 11:50, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> On 18-08-21, 11:41, Ulf Hansson wrote:
>>> On Wed, 18 Aug 2021 at 11:14, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>> What we need here is just configure. So something like this then:
>>>>
>>>> - genpd->get_performance_state()
>>>>   -> dev_pm_opp_get_current_opp() //New API
>>>>   -> dev_pm_genpd_set_performance_state(dev, current_opp->pstate);
>>>>
>>>> This can be done just once from probe() then.
>>>
>>> How would dev_pm_opp_get_current_opp() work? Do you have a suggestion?
>>
>> The opp core already has a way of finding current OPP, that's what
>> Dmitry is trying to use here. It finds it using clk_get_rate(), if
>> that is zero, it picks the lowest freq possible.
>>
>>> I am sure I understand the problem. When a device is getting probed,
>>> it needs to consume power, how else can the corresponding driver
>>> successfully probe it?
>>
>> Dmitry can answer that better, but a device doesn't necessarily need
>> to consume energy in probe. It can consume bus clock, like APB we
>> have, but the more energy consuming stuff can be left disabled until
>> the time a user comes up. Probe will just end up registering the
>> driver and initializing it.
> 
> That's perfectly fine, as then it's likely that it won't vote for an
> OPP, but can postpone that as well.
> 
> Perhaps the problem is rather that the HW may already carry a non-zero
> vote made from a bootloader. If the consumer driver tries to clear
> that vote (calling dev_pm_opp_set_rate(dev, 0), for example), it would
> still not lead to any updates of the performance state in genpd,
> because genpd internally has initialized the performance-state to
> zero.

We don't need to discover internal SoC devices because we use
device-tree on ARM. For most devices power isn't required at a probe
time because probe function doesn't touch h/w at all, thus devices are
left in suspended state after probe.

We have three components comprising PM on Tegra:

1. Power gate
2. Clock state
3. Voltage state

GENPD on/off represents the 'power gate'.

Clock and reset are controlled by device drivers using clk and rst APIs.

Voltage state is represented by GENPD's performance level.

GENPD core assumes that at a first rpm-resume of a consumer device, its
genpd_performance=0. Not true for Tegra because h/w of the device is
preconfigured to a non-zero perf level initially, h/w may not support
zero level at all.

GENPD core assumes that consumer devices can work at any performance
level. Not true for Tegra because voltage needs to be set in accordance
to the clock rate before clock is enabled, otherwise h/w won't work
properly, perhaps clock may be unstable or h/w won't be latching.

Performance level should be set to 0 while device is suspended.
Performance level needs to be bumped on rpm-resume of a device in
accordance to h/w state before hardware is enabled.