On 3/10/21 6:52 PM, Sudeep Holla wrote:
On Mon, Mar 08, 2021 at 10:32:17AM -0800, Sowjanya Komatineni wrote:
On 3/7/21 8:37 PM, Sudeep Holla wrote:
On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.
MCE firmware is in charge of state transition for Tegra194 carmel CPUs.
Sure, but I assume only TF-A talks to MCE and not any OSPM/Linux kernel.
No. Tegra194 CPU idle driver works with MCE firmware running in
background so cpuidle kernel driver also talks to MCE firmware directly
on state information.
For run-time state transitions, need to provide state request along with its
residency time to MCE firmware which is running in the background.
Sounds similar to x86 mwait, perhaps we need to extend PSCI if we need
to make this firmware PSCI compliant or just say it is not and implement
completely independent implementation. I am not saying that is acceptable
ATM but I prefer not to mix some implementation to make it look like
PSCI compliant.
State min residency is updated into power_state value along with state id
that is passed to psci_cpu_suspend_enter
Sounds like a hack/workaround. I would prefer to standardise that. IIUC
the power_state is more static and derived from DT. I don't like to
overload that TBH. Need to check with authors of that binding.
Passing state idle time to ATF along with state to enter is Tegra
specific as ATF firmware updates idle time to Tegra MCE firmware which
will be used for deciding on state transition along with other
information and background load.
Not sure if this need to be standardized but will try to find alternate
way to update idle time without misusing power-state value.
Will discuss on this internally and get back.
Also states cross-over idle times need to be provided to MCE firmware.
New requirements if this has to be PSCI compliant.
Updating cross-over idle times from DT to MCE firmware directly from
cpuidle kernel driver with corresponding MCE ARI commands is again Tegra
specific.
MCE firmware decides on state transition based on these inputs along with
its background work load.
So, Tegra specific CPU idle driver is required mainly to provide cross-over
thresholds from DT and run time idle state information to MCE firmware
through Tegra MCE communication APIs.
I am worried if different vendors will come up with different custom
solution for this. We need to either standardise this is Linux/DT or
in PSCI.
Allowing cross-over threshold through DT allows users to vary idle time
thresholds for state transitions based on different use-cases.
Sounds like policy and not platform specific to be in DT, but I will leave
that to DT maintainers.
cross-over idle times are based on supported CPU core and cluster states
and updating these from DT to Tegra MCE firmware running in the
background is Tegra specific.
--
Regards,
Sudeep
