Re: [PATCH v5 2/2] PM / Domains: use device's next wakeup to determine domain idle state

[Lina Iyer <ilina@codeaurora.org>]

On Thu, Nov 19 2020 at 02:57 -0700, Ulf Hansson wrote:
>On Mon, 16 Nov 2020 at 16:57, Lina Iyer <ilina@codeaurora.org> wrote:
>>
>> On Fri, Nov 13 2020 at 03:34 -0700, Ulf Hansson wrote:
>> >On Wed, 11 Nov 2020 at 17:51, Lina Iyer <ilina@codeaurora.org> wrote:
>> >>
>> >> On Tue, Nov 10 2020 at 03:02 -0700, Ulf Hansson wrote:
>> >> >On Mon, 9 Nov 2020 at 18:41, Lina Iyer <ilina@codeaurora.org> wrote:
>> >> >>
>> >> >> On Mon, Nov 09 2020 at 08:27 -0700, Ulf Hansson wrote:
>> >> >> >On Fri, 6 Nov 2020 at 17:48, Lina Iyer <ilina@codeaurora.org> wrote:
>> [...]
>>
>> >> >> >For example, there's no point doing the above, if the domain doesn't
>> >> >> >specify residency values for its idle states.
>> >> >> >
>> >> >> We would still need to ensure that the next wakeup is after the
>> >> >> power_off_latency, if specified.
>> >> >
>> >> >Good point! Although, I would rather avoid adding the overhead, unless
>> >> >the residency is specified. Do you see a problem with this approach?
>> >> >
>> >> Hmmm, no strong objections. However, we still need to run through the
>> >> states to make sure the residency is not set and have a variable track
>> >> that.
>> >
>> >Right.
>> >
>> >The important part is that we can do that once and not for every call
>> >to the governor.
>> >
>> >> The devices wouldn't know that and would still continue to set the
>> >> next wakeup, unless we find a way to let them know we are not using this
>> >> feature for the domain.
>> >
>> >Right.
>> >
>> >To allow the driver to know, we could respond with an error code from
>> >the new dev_pm_genpd_set_performance_state() API (from patch1), in
>> >case the genpd+governor doesn't support it.
>> >
>> It would an unnecessary work everytime a next wakeup is being set to
>> iterate through the available states and figure out if the residency has
>> been set or not. We could probably hold that result in a variable when
>> we setup the genpd states. Expect the next_wake to be set from a
>> critical path or an interrupt handler, so we have to be quick.
>
>Yes, that's the idea I had in mind.
>
>Maybe it's not feasible, let's see. However, for sure I am looking at
>decreasing overhead, not to increase. :-)
>
Wondering what do you think about a genpd flag for this purpose? The
flag may be set when the genpd is initialized with idle states that have
residency specified. In the governor, we could skip this path
completely, if the flag is not set.

--Lina

>>
>> >Would that be okay? Otherwise we will have to add a separate genpd
>> >API, asking explicitly if the "next wakeup" feature is supported.
>> >
>> Would like to avoid that as much as possible.
>
>Okay, good.
>
>Kind regards
>Uffe