diff mbox series

pmdomain: qcom: rpmhpd: Skip retention level for Power Domains

Message ID 20240531114148.8550-1-quic_tdas@quicinc.com
State Accepted
Commit ddab91f4b2de5c5b46e312a90107d9353087d8ea
Headers show
Series pmdomain: qcom: rpmhpd: Skip retention level for Power Domains | expand

Commit Message

Taniya Das May 31, 2024, 11:41 a.m. UTC
In the cases where the power domain connected to logics is allowed to
transition from a level(L)-->power collapse(0)-->retention(1) or
vice versa retention(1)-->power collapse(0)-->level(L)  will cause the
logic to lose the configurations. The ARC does not support retention
to collapse transition on MxC rails.

The targets from SM8450 onwards the PLL logics of clock controllers are
connected to MxC rails and the recommended configurations are carried
out during the clock controller probes. The MxC transition as mentioned
above should be skipped to ensure the PLL settings are intact across
clock controller power on & off.

On older generation of targets which supports only Mx the logic is never
collapsed and it is parked always at RETENTION, thus this issue is never
observed on those targets.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/pmdomain/qcom/rpmhpd.c | 7 +++++++
 1 file changed, 7 insertions(+)
---
 drivers/pmdomain/qcom/rpmhpd.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Bjorn Andersson May 31, 2024, 10:52 p.m. UTC | #1
On Fri, May 31, 2024 at 05:11:48PM GMT, Taniya Das wrote:
> In the cases where the power domain connected to logics is allowed to
> transition from a level(L)-->power collapse(0)-->retention(1) or
> vice versa retention(1)-->power collapse(0)-->level(L)  will cause the
> logic to lose the configurations. The ARC does not support retention
> to collapse transition on MxC rails.
> 
> The targets from SM8450 onwards the PLL logics of clock controllers are
> connected to MxC rails and the recommended configurations are carried
> out during the clock controller probes. The MxC transition as mentioned
> above should be skipped to ensure the PLL settings are intact across
> clock controller power on & off.
> 
> On older generation of targets which supports only Mx the logic is never
> collapsed and it is parked always at RETENTION, thus this issue is never
> observed on those targets.
> 

Cc: stable@vger.kernel.org # v5.17
Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/pmdomain/qcom/rpmhpd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> ---
>  drivers/pmdomain/qcom/rpmhpd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pmdomain/qcom/rpmhpd.c b/drivers/pmdomain/qcom/rpmhpd.c
> index de9121ef4216..d2cb4271a1ca 100644
> --- a/drivers/pmdomain/qcom/rpmhpd.c
> +++ b/drivers/pmdomain/qcom/rpmhpd.c
> @@ -40,6 +40,7 @@
>   * @addr:		Resource address as looped up using resource name from
>   *			cmd-db
>   * @state_synced:	Indicator that sync_state has been invoked for the rpmhpd resource
> + * @skip_retention_level: Indicate that retention level should not be used for the power domain
>   */
>  struct rpmhpd {
>  	struct device	*dev;
> @@ -56,6 +57,7 @@ struct rpmhpd {
>  	const char	*res_name;
>  	u32		addr;
>  	bool		state_synced;
> +	bool            skip_retention_level;
>  };
>  
>  struct rpmhpd_desc {
> @@ -173,6 +175,7 @@ static struct rpmhpd mxc = {
>  	.pd = { .name = "mxc", },
>  	.peer = &mxc_ao,
>  	.res_name = "mxc.lvl",
> +	.skip_retention_level = true,
>  };
>  
>  static struct rpmhpd mxc_ao = {
> @@ -180,6 +183,7 @@ static struct rpmhpd mxc_ao = {
>  	.active_only = true,
>  	.peer = &mxc,
>  	.res_name = "mxc.lvl",
> +	.skip_retention_level = true,
>  };
>  
>  static struct rpmhpd nsp = {
> @@ -819,6 +823,9 @@ static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
>  		return -EINVAL;
>  
>  	for (i = 0; i < rpmhpd->level_count; i++) {
> +		if (rpmhpd->skip_retention_level && buf[i] == RPMH_REGULATOR_LEVEL_RETENTION)
> +			continue;
> +
>  		rpmhpd->level[i] = buf[i];
>  
>  		/* Remember the first corner with non-zero level */
> -- 
> 2.17.1
>
Konrad Dybcio June 4, 2024, 12:16 p.m. UTC | #2
On 5/31/24 13:41, Taniya Das wrote:
> In the cases where the power domain connected to logics is allowed to
> transition from a level(L)-->power collapse(0)-->retention(1) or
> vice versa retention(1)-->power collapse(0)-->level(L)  will cause the
> logic to lose the configurations. The ARC does not support retention
> to collapse transition on MxC rails.

This is not very legible. Are you saying that:

a) transitioning to/from LEVEL_RETENTION causes the resource to be powered
off internally for some short time and lose state

or

b) the linux implementation of rpmhpd handling causes that transition to
include a power collapse step that makes it lose the state

?

> 
> The targets from SM8450 onwards the PLL logics of clock controllers are
> connected to MxC rails and the recommended configurations are carried
> out during the clock controller probes. The MxC transition as mentioned
> above should be skipped to ensure the PLL settings are intact across
> clock controller power on & off.

So is this a workaround for clock controller drivers specifically, or should
MXC never enter retention, and only poweroff? (the latter sounds like it makes
more sense given MXC's purpose)

> 
> On older generation of targets which supports only Mx the logic is never
> collapsed and it is parked always at RETENTION, thus this issue is never
> observed on those targets.

"On older targets that do not split MX into MXA and MXC..."

Konrad
Taniya Das June 10, 2024, 9:42 a.m. UTC | #3
On 6/4/2024 5:46 PM, Konrad Dybcio wrote:
> 
> 
> On 5/31/24 13:41, Taniya Das wrote:
>> In the cases where the power domain connected to logics is allowed to
>> transition from a level(L)-->power collapse(0)-->retention(1) or
>> vice versa retention(1)-->power collapse(0)-->level(L)  will cause the
>> logic to lose the configurations. The ARC does not support retention
>> to collapse transition on MxC rails.
> 
> This is not very legible. Are you saying that:
> 
> a) transitioning to/from LEVEL_RETENTION causes the resource to be powered
> off internally for some short time and lose state
> 

If there is a logic connected to MxC and the vote on that logic(MXC) 
from a subsystem is initially to LEVEL_RETENTION and then to power 
collapse [0], then the PLL connected to MxC will loose the contents. 
This the transition I am referring here.

> or
> 
> b) the linux implementation of rpmhpd handling causes that transition to
> include a power collapse step that makes it lose the state

No, this is not the case of SW implementation, it is more from the HW 
ARC implementation.
> 
> ?
> 
>>
>> The targets from SM8450 onwards the PLL logics of clock controllers are
>> connected to MxC rails and the recommended configurations are carried
>> out during the clock controller probes. The MxC transition as mentioned
>> above should be skipped to ensure the PLL settings are intact across
>> clock controller power on & off.
> 
> So is this a workaround for clock controller drivers specifically, or 
> should
> MXC never enter retention, and only poweroff? (the latter sounds like it 
> makes
> more sense given MXC's purpose)
> 

This is avoid MxC to not enter retention to OFF state.

>>
>> On older generation of targets which supports only Mx the logic is never
>> collapsed and it is parked always at RETENTION, thus this issue is never
>> observed on those targets.
> 
> "On older targets that do not split MX into MXA and MXC..."

Yes, but that is only Mx :).
> 
> Konrad
Konrad Dybcio June 18, 2024, 1:17 p.m. UTC | #4
On 6/10/24 11:42, Taniya Das wrote:
> 
> 
> On 6/4/2024 5:46 PM, Konrad Dybcio wrote:
>>
>>
>> On 5/31/24 13:41, Taniya Das wrote:
>>> In the cases where the power domain connected to logics is allowed to
>>> transition from a level(L)-->power collapse(0)-->retention(1) or
>>> vice versa retention(1)-->power collapse(0)-->level(L)  will cause the
>>> logic to lose the configurations. The ARC does not support retention
>>> to collapse transition on MxC rails.
>>
>> This is not very legible. Are you saying that:
>>
>> a) transitioning to/from LEVEL_RETENTION causes the resource to be powered
>> off internally for some short time and lose state
>>
> 
> If there is a logic connected to MxC and the vote on that logic(MXC) from a subsystem is initially to LEVEL_RETENTION and then to power collapse [0], then the PLL connected to MxC will loose the contents. This the transition I am referring here.

Okay, is there *always* some logic connected to MxC?

> 
>> or
>>
>> b) the linux implementation of rpmhpd handling causes that transition to
>> include a power collapse step that makes it lose the state
> 
> No, this is not the case of SW implementation, it is more from the HW ARC implementation.
>>
>> ?
>>
>>>
>>> The targets from SM8450 onwards the PLL logics of clock controllers are
>>> connected to MxC rails and the recommended configurations are carried
>>> out during the clock controller probes. The MxC transition as mentioned
>>> above should be skipped to ensure the PLL settings are intact across
>>> clock controller power on & off.
>>
>> So is this a workaround for clock controller drivers specifically, or should
>> MXC never enter retention, and only poweroff? (the latter sounds like it makes
>> more sense given MXC's purpose)
>>
> 
> This is avoid MxC to not enter retention to OFF state.

I'm still not sure I understand. Is this to prevent MxC being switched off
while Linux is voting for LEVEL_RETENTION, as RPMh sees no other user and
decides it's okay to cut the power?

> 
>>>
>>> On older generation of targets which supports only Mx the logic is never
>>> collapsed and it is parked always at RETENTION, thus this issue is never
>>> observed on those targets.
>>
>> "On older targets that do not split MX into MXA and MXC..."
> 
> Yes, but that is only Mx :).

My point is about the wording.. There is no such thing as "supporting Mx".

Konrad
Taniya Das June 21, 2024, 4:20 a.m. UTC | #5
On 6/18/2024 6:47 PM, Konrad Dybcio wrote:
> 
> 
> On 6/10/24 11:42, Taniya Das wrote:
>>
>>
>> On 6/4/2024 5:46 PM, Konrad Dybcio wrote:
>>>
>>>
>>> On 5/31/24 13:41, Taniya Das wrote:
>>>> In the cases where the power domain connected to logics is allowed to
>>>> transition from a level(L)-->power collapse(0)-->retention(1) or
>>>> vice versa retention(1)-->power collapse(0)-->level(L)  will cause the
>>>> logic to lose the configurations. The ARC does not support retention
>>>> to collapse transition on MxC rails.
>>>
>>> This is not very legible. Are you saying that:
>>>
>>> a) transitioning to/from LEVEL_RETENTION causes the resource to be 
>>> powered
>>> off internally for some short time and lose state
>>>
>>
>> If there is a logic connected to MxC and the vote on that logic(MXC) 
>> from a subsystem is initially to LEVEL_RETENTION and then to power 
>> collapse [0], then the PLL connected to MxC will loose the contents. 
>> This the transition I am referring here.
> 
> Okay, is there *always* some logic connected to MxC?
> 

 From the clock controller PoV PLLs logic (majorly Multimedia clock 
controller PLLs) would be connected to MxC.
There can be logics from the other subsystems (other than APSS) 
connected to MxC.

>>
>>> or
>>>
>>> b) the linux implementation of rpmhpd handling causes that transition to
>>> include a power collapse step that makes it lose the state
>>
>> No, this is not the case of SW implementation, it is more from the HW 
>> ARC implementation.
>>>
>>> ?
>>>
>>>>
>>>> The targets from SM8450 onwards the PLL logics of clock controllers are
>>>> connected to MxC rails and the recommended configurations are carried
>>>> out during the clock controller probes. The MxC transition as mentioned
>>>> above should be skipped to ensure the PLL settings are intact across
>>>> clock controller power on & off.
>>>
>>> So is this a workaround for clock controller drivers specifically, or 
>>> should
>>> MXC never enter retention, and only poweroff? (the latter sounds like 
>>> it makes
>>> more sense given MXC's purpose)
>>>
>>
>> This is avoid MxC to not enter retention to OFF state.
> 
> I'm still not sure I understand. Is this to prevent MxC being switched off
> while Linux is voting for LEVEL_RETENTION, as RPMh sees no other user and
> decides it's okay to cut the power?
> 

It is to prevent Linux from voting for LEVEL_RETENTION and then voting 
for an OFF state or vice versa. By avoiding this transition we ensure 
that the PLL configurations are not lost.

Other users(subsystems) also avoid voting for RETENTION.

>>
>>>>
>>>> On older generation of targets which supports only Mx the logic is 
>>>> never
>>>> collapsed and it is parked always at RETENTION, thus this issue is 
>>>> never
>>>> observed on those targets.
>>>
>>> "On older targets that do not split MX into MXA and MXC..."
>>
>> Yes, but that is only Mx :).
> 
> My point is about the wording.. There is no such thing as "supporting Mx".
> 

Sure, I can update the commit text in the next patch.

> Konrad
diff mbox series

Patch

diff --git a/drivers/pmdomain/qcom/rpmhpd.c b/drivers/pmdomain/qcom/rpmhpd.c
index de9121ef4216..d2cb4271a1ca 100644
--- a/drivers/pmdomain/qcom/rpmhpd.c
+++ b/drivers/pmdomain/qcom/rpmhpd.c
@@ -40,6 +40,7 @@ 
  * @addr:		Resource address as looped up using resource name from
  *			cmd-db
  * @state_synced:	Indicator that sync_state has been invoked for the rpmhpd resource
+ * @skip_retention_level: Indicate that retention level should not be used for the power domain
  */
 struct rpmhpd {
 	struct device	*dev;
@@ -56,6 +57,7 @@  struct rpmhpd {
 	const char	*res_name;
 	u32		addr;
 	bool		state_synced;
+	bool            skip_retention_level;
 };
 
 struct rpmhpd_desc {
@@ -173,6 +175,7 @@  static struct rpmhpd mxc = {
 	.pd = { .name = "mxc", },
 	.peer = &mxc_ao,
 	.res_name = "mxc.lvl",
+	.skip_retention_level = true,
 };
 
 static struct rpmhpd mxc_ao = {
@@ -180,6 +183,7 @@  static struct rpmhpd mxc_ao = {
 	.active_only = true,
 	.peer = &mxc,
 	.res_name = "mxc.lvl",
+	.skip_retention_level = true,
 };
 
 static struct rpmhpd nsp = {
@@ -819,6 +823,9 @@  static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
 		return -EINVAL;
 
 	for (i = 0; i < rpmhpd->level_count; i++) {
+		if (rpmhpd->skip_retention_level && buf[i] == RPMH_REGULATOR_LEVEL_RETENTION)
+			continue;
+
 		rpmhpd->level[i] = buf[i];
 
 		/* Remember the first corner with non-zero level */