Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sowjanya Komatineni <skomatineni@nvidia.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: <thierry.reding@gmail.com>, <jonathanh@nvidia.com>,
	<daniel.lezcano@linaro.org>, <robh+dt@kernel.org>,
	<ksitaraman@nvidia.com>, <sanjayc@nvidia.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	<linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-pm@vger.kernel.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes
Date: Tue, 16 Mar 2021 04:24:44 -0700	[thread overview]
Message-ID: <f1d49d3b-0d6f-dda0-ea44-1bff21b52fb4@nvidia.com> (raw)
In-Reply-To: <20210316071811.5mqcatmmbvrask2p@bogus>

On 3/16/21 12:18 AM, Sudeep Holla wrote:
> On Mon, Mar 15, 2021 at 11:13:24AM -0700, 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.
>>
> Yes, I prefer you need to get this added in the PSCI specification.
> I have passed this thread to the author of the specification.
Thanks Sudeep.
>
>> 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.
>>
>  From previous emails, I gather these can be moved to firmware and need not be
> there in DT ?

Yes we can move state thresholds programming from kernel driver to ATF 
but we still need to use DT properties for this to allow users to tweak 
for their use-cases.

With DT access in ATF this can be done. But checking internally on 
having DTB access in ATF as currently we don't support that.

>
>> As these are Tegra platform specific, Please comment if any other concerns
>> in having this configured by Tegra CPU Idle kernel driver.
>>
> I prefer not to have Tegra specific idle driver if we can get the necessary
> changes in PSCI spec. We must then have just one PSCI idle driver in the
> kernel.

Agree by adding state residency run-time propagation along with power 
state to PSCI specification and moving state thresholds update to MCE 
from kernel driver to AT looks like we can use same PSCI cpu idle driver 
although we will be using state thresholds additional DT properties 
under cpu nodes which will be used by ATF firmware once we decide on no 
concerns to allow DTB access in ATF.

>> Based on my understanding only above issue-1 is PSCI compliant related.
>> Please confirm.
>>
> Correct.
>
> --
> Regards,
> Sudeep