Re-sending as it went out as HTML instead of plain text.
On 3/15/21 11:13 AM, Sowjanya Komatineni wrote:
Hi Sudeep,
I see you are one of the maintainer of PSCI driver. Please add any
other right persons if you think should also agree/comment.
Can you please comment on below 2 items based on your feedback?
1. Can you please suggest on proper way of generalizing to pass state
residency time run-time along with state during state enter?
Not sure if any other drivers need this but for Tegra as MCE firmware
is in-charge of states enter and decisions, passing run-time state
residency from kernel to ATF is required and agree on not using
power_state value for this which is against PSCI spec.
2. Regarding state thresholds, although state thresholds are policy
related in Tegra cpu idle perspective these thresholds are platform
specific based on use case and mainly for MCE firmware usage to decide
on state transitions for core and core clusters as well.
As these are Tegra platform specific, Please comment if any other
concerns in having this configured by Tegra CPU Idle kernel driver.
Based on my understanding only above issue-1 is PSCI compliant
related. Please confirm.
Thanks
Sowjanya
On 3/12/21 2:27 PM, Sowjanya Komatineni wrote:
Hi Sudeep,
To make our driver PSCI compliant below are few updates to be done
1. Standardize passing residency time run-time thru PSCI to ATF.
From PSCI specification I only see PSCI_STAT_RESIDENCY and
PSCI_STAT_COUNT optional functions for PSCI1.0/PSCI1.1
Can you please help add right people to explore on possible ways
to add PSCI function for passing corresponding state residency time
to ATF?
2. Tegra CPU Idle support definitely need to pass deepest cluster
state and threshold to MCE firmware from DT and looks like we can
move this implementation to ATF
With both of the above implementation changes Tegra194 driver
will be PSCI compliant.
Thanks
Sowjanya
On 3/11/21 1:11 PM, Sowjanya Komatineni wrote:
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
