Re: [PATCH v2 11/48] opp: Add dev_pm_opp_find_level_ceil()

[Dmitry Osipenko <digetx@gmail.com>]

28.12.2020 09:22, Viresh Kumar пишет:
> On 24-12-20, 16:00, Dmitry Osipenko wrote:
>> In a device driver I want to set PD to the lowest performance state by
>> removing the performance vote when dev_pm_opp_set_rate(dev, 0) is
>> invoked by the driver.
>>
>> The OPP core already does this, but if OPP levels don't start from 0 in
>> a device-tree for PD, then it currently doesn't work since there is a
>> need to get a rounded-up performance state because
>> dev_pm_opp_set_voltage() takes OPP entry for the argument (patches 9 and
>> 28).
>>
>> The PD powering off and performance-changes are separate from each other
>> in the GENPD core. The GENPD core automatically turns off domain when
>> all devices within the domain are suspended by system-suspend or RPM.
>>
>> The performance state of a power domain is controlled solely by a device
>> driver. GENPD core only aggregates the performance requests, it doesn't
>> change the performance state of a domain by itself when device is
>> suspended or resumed, IIUC this is intentional. And I want to put domain
>> into lowest performance state when device is suspended.
> 
> Right, so if you really want to just drop the performance vote, then with a
> value of 0 for the performance state the call will reach to your genpd's
> callback ->set_performance_state(). Just as dev_pm_opp_set_rate() accepts the
> frequency to be 0, I would expect dev_pm_opp_set_rate() to accept opp argument
> as NULL and in that case set voltage to 0 and do regulator_disable() as well.
> Won't that work better than going for the lowest voltage ?
> 

We can make dev_pm_opp_set_voltage() to accept OPP=NULL in order to
disable the regulator, like it's done for dev_pm_opp_set_rate(dev, 0).
Although, I don't need this kind of behaviour for the Tegra PD driver,
and thus, would prefer to leave this for somebody else to implement in
the future, once it will be really needed.

Still we need the dev_pm_opp_find_level_ceil() because level=0 means
that we want to set PD to the lowest (minimal) performance state, i.e.
it doesn't necessarily mean that we want to set the voltage to 0 and
disable the PD entirely. GENPD has a separate controls for on/off.