Add RSC power domain support

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

Signed-off-by Ulf Hansson <[email protected]>

[applied to new path, resolved conflicts]

Signed-off-by Maulik Shah <[email protected]>

---
 drivers/cpuidle/cpuidle-psci-domain.c | 137 +++++++++++++++++++++++++++++-----
 drivers/cpuidle/cpuidle-psci.c        |  41 +++++-----
 drivers/cpuidle/cpuidle-psci.h        |  11 +++
 3 files changed, 153 insertions(+), 36 deletions(-)

drivers/cpuidle/cpuidle-psci-domain.c +1 -1

index 423f03b..3c417f7 100644
@@ -26,13 +26,17 @@ struct psci_pd_provider {
 };
 
 static LIST_HEAD(psci_pd_providers);
-static bool osi_mode_enabled __initdata;
+static bool osi_mode_enabled;

drivers/cpuidle/cpuidle-psci-domain.c +77 -0

 	if (!state->data)
 		return 0;
 

@@ -101,6 +105,105 @@ static void psci_pd_free_states(struct genpd_power_state *states,
 	kfree(states);
 }
 
+static void psci_pd_convert_states(struct cpuidle_state *idle_state,
+			u32 *psci_state, struct genpd_power_state *state)
+{
+	u32 *state_data = state->data;
+	u64 target_residency_us = state->residency_ns;
+	u64 exit_latency_us = state->power_on_latency_ns +
+			state->power_off_latency_ns;
+
+	*psci_state = *state_data;
+	do_div(target_residency_us, 1000);
+	idle_state->target_residency = target_residency_us;
+	do_div(exit_latency_us, 1000);
+	idle_state->exit_latency = exit_latency_us;
+	idle_state->enter = &psci_enter_domain_idle_state;
+	idle_state->flags |= CPUIDLE_FLAG_TIMER_STOP;
+
+	strncpy(idle_state->name, to_of_node(state->fwnode)->name,
+		CPUIDLE_NAME_LEN - 1);
+	strncpy(idle_state->desc, to_of_node(state->fwnode)->name,
+		CPUIDLE_NAME_LEN - 1);
+}
+
+static bool psci_pd_is_provider(struct device_node *np)
+{
+	struct psci_pd_provider *pd_prov, *it;
+
+	list_for_each_entry_safe(pd_prov, it, &psci_pd_providers, link) {
+		if (pd_prov->node == np)
+			return true;
+	}
+
+	return false;
+}
+
+int __init psci_dt_pm_domains_parse_states(struct cpuidle_driver *drv,
+			struct device_node *cpu_node, u32 *psci_states)
+{
+	struct genpd_power_state *pd_states;
+	struct of_phandle_args args;
+	int ret, pd_state_count, i, state_idx, psci_idx;
+	u32 cpu_psci_state = psci_states[drv->state_count - 1];
+	struct device_node *np = of_node_get(cpu_node);
+
+	/* Walk the CPU topology to find compatible domain idle states. */
+	while (np) {
+		ret = of_parse_phandle_with_args(np, "power-domains",
+					"#power-domain-cells", 0, &args);
+		of_node_put(np);
+		if (ret)
+			return 0;
+
+		np = args.np;
+
+		/* Verify that the node represents a psci pd provider. */
+		if (!psci_pd_is_provider(np)) {
+			of_node_put(np);
+			return 0;
+		}
+
+		/* Parse for compatible domain idle states. */
+		ret = psci_pd_parse_states(np, &pd_states, &pd_state_count);
+		if (ret) {
+			of_node_put(np);
+			return ret;
+		}
+
+		i = 0;
+		while (i < pd_state_count) {
+
+			state_idx = drv->state_count;
+			if (state_idx >= CPUIDLE_STATE_MAX) {
+				pr_warn("exceeding max cpuidle states\n");
+				of_node_put(np);
+				return 0;
+			}
+
+			psci_idx = state_idx + i;

drivers/cpuidle/cpuidle-psci-domain.c +27 -17

+			pr_debug("psci-power-state %#x index %d\n",
+				psci_states[psci_idx], psci_idx);
+
+			drv->state_count++;
+			i++;
+		}
+		psci_pd_free_states(pd_states, pd_state_count);
+	}
+
+	return 0;
+}
+
 static int __init psci_pd_init(struct device_node *np)
 {
 	struct generic_pm_domain *pd;

@@ -125,11 +228,14 @@ static int __init psci_pd_init(struct device_node *np)
 	 * Parse the domain idle states and let genpd manage the state selection
 	 * for those being compatible with "domain-idle-state".
 	 */
-	ret = psci_pd_parse_states(np, &states, &state_count);
-	if (ret)
-		goto free_name;
 
-	pd->free_states = psci_pd_free_states;
+	if (psci_has_osi_support()) {
+		ret = psci_pd_parse_states(np, &states, &state_count);
+		if (ret)
+			goto free_name;
+		pd->free_states = psci_pd_free_states;
+	}
+
 	pd->name = kbasename(pd->name);
 	pd->power_off = psci_pd_power_off;
 	pd->states = states;

@@ -236,10 +342,6 @@ static int __init psci_idle_init_domains(void)
 	if (!np)
 		return -ENODEV;
 
-	/* Currently limit the hierarchical topology to be used in OSI mode. */
-	if (!psci_has_osi_support())
-		goto out;
-
 	/*
 	 * Parse child nodes for the "#power-domain-cells" property and
 	 * initialize a genpd/genpd-of-provider pair when it's found.

@@ -265,14 +367,16 @@ static int __init psci_idle_init_domains(void)
 		goto remove_pd;
 
 	/* Try to enable OSI mode. */
-	ret = psci_set_osi_mode();
-	if (ret) {
-		pr_warn("failed to enable OSI mode: %d\n", ret);
-		psci_pd_remove_topology(np);
-		goto remove_pd;
+	if (psci_has_osi_support()) {
+		ret = psci_set_osi_mode();
+		if (ret) {
+			pr_warn("failed to enable OSI mode: %d\n", ret);
+			psci_pd_remove_topology(np);
+			goto remove_pd;
+		} else
+			osi_mode_enabled = true;
 	}
 
-	osi_mode_enabled = true;
 	of_node_put(np);
 	pr_info("Initialized CPU PM domain topology\n");
 	return pd_count;

@@ -293,9 +397,6 @@ struct device __init *psci_dt_attach_cpu(int cpu)
 {
 	struct device *dev;
 
-	if (!osi_mode_enabled)
-		return NULL;
-
 	dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci");
 	if (IS_ERR_OR_NULL(dev))
 		return dev;

drivers/cpuidle/cpuidle-psci.c +16 -18

index edd7a54..3fa2aee 100644
@@ -49,7 +49,7 @@ static inline int psci_enter_state(int idx, u32 state)
 	return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, idx, state);
 }
 
-static int psci_enter_domain_idle_state(struct cpuidle_device *dev,
+int psci_enter_domain_idle_state(struct cpuidle_device *dev,
 					struct cpuidle_driver *drv, int idx)
 {
 	struct psci_cpuidle_data *data = this_cpu_ptr(&psci_cpuidle_data);

@@ -193,24 +193,29 @@ static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv,
 		goto free_mem;
 	}
 
-	/* Currently limit the hierarchical topology to be used in OSI mode. */
-	if (psci_has_osi_support()) {
-		data->dev = psci_dt_attach_cpu(cpu);
-		if (IS_ERR(data->dev)) {
-			ret = PTR_ERR(data->dev);
+	if (!psci_has_osi_support()) {
+		ret = psci_dt_pm_domains_parse_states(drv, cpu_node,
+					      psci_states);
+		if (ret)
 			goto free_mem;
-		}
-
-		/*
-		 * Using the deepest state for the CPU to trigger a potential
-		 * selection of a shared state for the domain, assumes the
-		 * domain states are all deeper states.
-		 */
-		if (data->dev) {
-			drv->states[state_count - 1].enter =
-				psci_enter_domain_idle_state;
-			psci_cpuidle_use_cpuhp = true;
-		}
+	}
+
+	data->dev = psci_dt_attach_cpu(cpu);
+	if (IS_ERR(data->dev)) {
+		ret = PTR_ERR(data->dev);
+		goto free_mem;
+	}
+
+	/*
+	 * Using the deepest state for the CPU to trigger a potential
+	 * selection of a shared state for the domain, assumes the

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 25 lines

Signed-off-by Ulf Hansson <[email protected]>

[applied to new path, resolved conflicts]

Signed-off-by Maulik Shah <[email protected]>

---
  drivers/cpuidle/cpuidle-psci-domain.c | 137 +++++++++++++++++++++++++++++-----
  drivers/cpuidle/cpuidle-psci.c        |  41 +++++-----
  drivers/cpuidle/cpuidle-psci.h        |  11 +++
  3 files changed, 153 insertions(+), 36 deletions(-)

drivers/cpuidle/cpuidle-psci-domain.c +1 -1

index 423f03b..3c417f7 100644
@@ -26,13 +26,17 @@ struct psci_pd_provider {
  };
 
  static LIST_HEAD(psci_pd_providers);
-static bool osi_mode_enabled __initdata;
+static bool osi_mode_enabled;
 
  static int psci_pd_power_off(struct generic_pm_domain *pd)
  {
  	struct genpd_power_state *state = &pd->states[pd->state_idx];
  	u32 *pd_state;
 

drivers/cpuidle/cpuidle-psci-domain.c struct psci_pd_provider { · line 39

Quoted 4 lines

drivers/cpuidle/cpuidle-psci-domain.c +4 -0

+	/* If we have failed to enable OSI mode, then abort power off. */
+	if ((psci_has_osi_support()) && !osi_mode_enabled)
+		return -EBUSY;
+

Why is this needed ? IIUC we don't create genpd domains if OSI is not enabled.

Quoted 87 lines

drivers/cpuidle/cpuidle-psci-domain.c +80 -0

  	if (!state->data)
  		return 0;
 

@@ -101,6 +105,105 @@ static void psci_pd_free_states(struct genpd_power_state *states,
  	kfree(states);
  }
 
+static void psci_pd_convert_states(struct cpuidle_state *idle_state,
+			u32 *psci_state, struct genpd_power_state *state)
+{
+	u32 *state_data = state->data;
+	u64 target_residency_us = state->residency_ns;
+	u64 exit_latency_us = state->power_on_latency_ns +
+			state->power_off_latency_ns;
+
+	*psci_state = *state_data;
+	do_div(target_residency_us, 1000);
+	idle_state->target_residency = target_residency_us;
+	do_div(exit_latency_us, 1000);
+	idle_state->exit_latency = exit_latency_us;
+	idle_state->enter = &psci_enter_domain_idle_state;
+	idle_state->flags |= CPUIDLE_FLAG_TIMER_STOP;
+
+	strncpy(idle_state->name, to_of_node(state->fwnode)->name,
+		CPUIDLE_NAME_LEN - 1);
+	strncpy(idle_state->desc, to_of_node(state->fwnode)->name,
+		CPUIDLE_NAME_LEN - 1);
+}
+
+static bool psci_pd_is_provider(struct device_node *np)
+{
+	struct psci_pd_provider *pd_prov, *it;
+
+	list_for_each_entry_safe(pd_prov, it, &psci_pd_providers, link) {
+		if (pd_prov->node == np)
+			return true;
+	}
+
+	return false;
+}
+
+int __init psci_dt_pm_domains_parse_states(struct cpuidle_driver *drv,
+			struct device_node *cpu_node, u32 *psci_states)
+{
+	struct genpd_power_state *pd_states;
+	struct of_phandle_args args;
+	int ret, pd_state_count, i, state_idx, psci_idx;
+	u32 cpu_psci_state = psci_states[drv->state_count - 1];
+	struct device_node *np = of_node_get(cpu_node);
+
+	/* Walk the CPU topology to find compatible domain idle states. */
+	while (np) {
+		ret = of_parse_phandle_with_args(np, "power-domains",
+					"#power-domain-cells", 0, &args);
+		of_node_put(np);
+		if (ret)
+			return 0;
+
+		np = args.np;
+
+		/* Verify that the node represents a psci pd provider. */
+		if (!psci_pd_is_provider(np)) {
+			of_node_put(np);
+			return 0;
+		}
+
+		/* Parse for compatible domain idle states. */
+		ret = psci_pd_parse_states(np, &pd_states, &pd_state_count);
+		if (ret) {
+			of_node_put(np);
+			return ret;
+		}
+
+		i = 0;
+		while (i < pd_state_count) {
+
+			state_idx = drv->state_count;
+			if (state_idx >= CPUIDLE_STATE_MAX) {
+				pr_warn("exceeding max cpuidle states\n");
+				of_node_put(np);
+				return 0;
+			}
+
+			psci_idx = state_idx + i;
+			psci_pd_convert_states(&drv->states[state_idx + i],
+					&psci_states[psci_idx], &pd_states[i]);
+

drivers/cpuidle/cpuidle-psci-domain.c static void psci_pd_free_states(struct genpd_power_state *states, · line 194

Quoted 7 lines

drivers/cpuidle/cpuidle-psci-domain.c +7 -0

+			/*
+			 * In the hierarchical CPU topology the master PM domain
+			 * idle state's DT property, "arm,psci-suspend-param",
+			 * don't contain the bits for the idle state of the CPU,
+			 * let's add those here.
+			 */
+			psci_states[psci_idx] |= cpu_psci_state;

No we can't do that. Refer previous discussions around that.

Quoted 129 lines

drivers/cpuidle/cpuidle-psci-domain.c +27 -17

+			pr_debug("psci-power-state %#x index %d\n",
+				psci_states[psci_idx], psci_idx);
+
+			drv->state_count++;
+			i++;
+		}
+		psci_pd_free_states(pd_states, pd_state_count);
+	}
+
+	return 0;
+}
+
  static int __init psci_pd_init(struct device_node *np)
  {
  	struct generic_pm_domain *pd;

@@ -125,11 +228,14 @@ static int __init psci_pd_init(struct device_node *np)
  	 * Parse the domain idle states and let genpd manage the state selection
  	 * for those being compatible with "domain-idle-state".
  	 */
-	ret = psci_pd_parse_states(np, &states, &state_count);
-	if (ret)
-		goto free_name;
 
-	pd->free_states = psci_pd_free_states;
+	if (psci_has_osi_support()) {
+		ret = psci_pd_parse_states(np, &states, &state_count);
+		if (ret)
+			goto free_name;
+		pd->free_states = psci_pd_free_states;
+	}
+
  	pd->name = kbasename(pd->name);
  	pd->power_off = psci_pd_power_off;
  	pd->states = states;

@@ -236,10 +342,6 @@ static int __init psci_idle_init_domains(void)
  	if (!np)
  		return -ENODEV;
 
-	/* Currently limit the hierarchical topology to be used in OSI mode. */
-	if (!psci_has_osi_support())
-		goto out;
-
  	/*
  	 * Parse child nodes for the "#power-domain-cells" property and
  	 * initialize a genpd/genpd-of-provider pair when it's found.

@@ -265,14 +367,16 @@ static int __init psci_idle_init_domains(void)
  		goto remove_pd;
 
  	/* Try to enable OSI mode. */
-	ret = psci_set_osi_mode();
-	if (ret) {
-		pr_warn("failed to enable OSI mode: %d\n", ret);
-		psci_pd_remove_topology(np);
-		goto remove_pd;
+	if (psci_has_osi_support()) {
+		ret = psci_set_osi_mode();
+		if (ret) {
+			pr_warn("failed to enable OSI mode: %d\n", ret);
+			psci_pd_remove_topology(np);
+			goto remove_pd;
+		} else
+			osi_mode_enabled = true;
  	}
 
-	osi_mode_enabled = true;
  	of_node_put(np);
  	pr_info("Initialized CPU PM domain topology\n");
  	return pd_count;

@@ -293,9 +397,6 @@ struct device __init *psci_dt_attach_cpu(int cpu)
  {
  	struct device *dev;
 
-	if (!osi_mode_enabled)
-		return NULL;
-
  	dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci");
  	if (IS_ERR_OR_NULL(dev))
  		return dev;

drivers/cpuidle/cpuidle-psci.c +16 -18

index edd7a54..3fa2aee 100644
@@ -49,7 +49,7 @@ static inline int psci_enter_state(int idx, u32 state)
  	return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, idx, state);
  }
 
-static int psci_enter_domain_idle_state(struct cpuidle_device *dev,
+int psci_enter_domain_idle_state(struct cpuidle_device *dev,
  					struct cpuidle_driver *drv, int idx)
  {
  	struct psci_cpuidle_data *data = this_cpu_ptr(&psci_cpuidle_data);

@@ -193,24 +193,29 @@ static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv,
  		goto free_mem;
  	}
 
-	/* Currently limit the hierarchical topology to be used in OSI mode. */
-	if (psci_has_osi_support()) {
-		data->dev = psci_dt_attach_cpu(cpu);
-		if (IS_ERR(data->dev)) {
-			ret = PTR_ERR(data->dev);
+	if (!psci_has_osi_support()) {
+		ret = psci_dt_pm_domains_parse_states(drv, cpu_node,
+					      psci_states);
+		if (ret)
  			goto free_mem;
-		}
-
-		/*
-		 * Using the deepest state for the CPU to trigger a potential
-		 * selection of a shared state for the domain, assumes the
-		 * domain states are all deeper states.
-		 */
-		if (data->dev) {
-			drv->states[state_count - 1].enter =
-				psci_enter_domain_idle_state;
-			psci_cpuidle_use_cpuhp = true;
-		}
+	}
+
+	data->dev = psci_dt_attach_cpu(cpu);
+	if (IS_ERR(data->dev)) {
+		ret = PTR_ERR(data->dev);
+		goto free_mem;
+	}
+
+	/*
+	 * Using the deepest state for the CPU to trigger a potential
+	 * selection of a shared state for the domain, assumes the

drivers/cpuidle/cpuidle-psci.c static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv, · line 221

Quoted 10 lines

drivers/cpuidle/cpuidle-psci.c +7 -0

+	 * domain states are all deeper states.
+	 */
+
+	if (data->dev) {
+		drv->states[state_count - 1].enter =
+			psci_enter_domain_idle_state;
+		psci_cpuidle_use_cpuhp = true;
  	}
 
  	/* Idle states parsed correctly, store them in the per-cpu struct. */

--
Regards,
Sudeep

Quoted 14 lines · Sudeep Holla

Quoted 13 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

Quoted 2 lines

And what's the main motivation for this if OSI is not supported in the firmware ?

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Quoted 25 lines

Quoted 25 lines

Signed-off-by Ulf Hansson <[email protected]>

[applied to new path, resolved conflicts]

Signed-off-by Maulik Shah <[email protected]>

---
  drivers/cpuidle/cpuidle-psci-domain.c | 137 +++++++++++++++++++++++++++++-----
  drivers/cpuidle/cpuidle-psci.c        |  41 +++++-----
  drivers/cpuidle/cpuidle-psci.h        |  11 +++
  3 files changed, 153 insertions(+), 36 deletions(-)

drivers/cpuidle/cpuidle-psci-domain.c +1 -1

index 423f03b..3c417f7 100644
@@ -26,13 +26,17 @@ struct psci_pd_provider {
  };
 
  static LIST_HEAD(psci_pd_providers);
-static bool osi_mode_enabled __initdata;
+static bool osi_mode_enabled;
 
  static int psci_pd_power_off(struct generic_pm_domain *pd)
  {
  	struct genpd_power_state *state = &pd->states[pd->state_idx];
  	u32 *pd_state;
 

Quoted 6 lines

drivers/cpuidle/cpuidle-psci-domain.c struct psci_pd_provider { · line 39

Quoted 4 lines

drivers/cpuidle/cpuidle-psci-domain.c +4 -0

+	/* If we have failed to enable OSI mode, then abort power off. */
+	if ((psci_has_osi_support()) && !osi_mode_enabled)
+		return -EBUSY;
+

Why is this needed ? IIUC we don't create genpd domains if OSI is not enabled.

we do create genpd domains, for cpu domains, we just abort power off here since idle states are converted into regular flattened mode.

however genpd poweroff will be used by parent domain (rsc in this case) which is kept in hireachy in DTSI with cluster domain to do last man activities.

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Quoted 25 lines

Quoted 25 lines

Quoted 25 lines

Signed-off-by Ulf Hansson <[email protected]>

[applied to new path, resolved conflicts]

Signed-off-by Maulik Shah <[email protected]>

---
   drivers/cpuidle/cpuidle-psci-domain.c | 137 +++++++++++++++++++++++++++++-----
   drivers/cpuidle/cpuidle-psci.c        |  41 +++++-----
   drivers/cpuidle/cpuidle-psci.h        |  11 +++
   3 files changed, 153 insertions(+), 36 deletions(-)

drivers/cpuidle/cpuidle-psci-domain.c +1 -1

index 423f03b..3c417f7 100644
@@ -26,13 +26,17 @@ struct psci_pd_provider {
   };
 
   static LIST_HEAD(psci_pd_providers);
-static bool osi_mode_enabled __initdata;
+static bool osi_mode_enabled;
 
   static int psci_pd_power_off(struct generic_pm_domain *pd)
   {
   	struct genpd_power_state *state = &pd->states[pd->state_idx];
   	u32 *pd_state;
 

Quoted 9 lines

Quoted 6 lines

drivers/cpuidle/cpuidle-psci-domain.c struct psci_pd_provider { · line 39

Quoted 4 lines

drivers/cpuidle/cpuidle-psci-domain.c +4 -0

+	/* If we have failed to enable OSI mode, then abort power off. */
+	if ((psci_has_osi_support()) && !osi_mode_enabled)
+		return -EBUSY;
+

Why is this needed ? IIUC we don't create genpd domains if OSI is not enabled.

we do create genpd domains, for cpu domains, we just abort power off here since idle states are converted into regular flattened mode.

OK, IIRC the OSI patches from Ulf didn't add the genpd or rather removed them in case of any failure to enable OSI. Has that been changed ? If so, why ?

Quoted 4 lines

however genpd poweroff will be used by parent domain (rsc in this case) which is kept in hireachy in DTSI with cluster domain to do last man activities.

I am bit confused here. Either we do OSI or PC and what you are describing sounds like a mix-n-match to me and I am totally against it.

--
Regards,
Sudeep

Quoted 18 lines · Sudeep Holla

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 7 lines

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Hi Sudeep,

there are cases like,

Last cpu going to deepest idle mode need to lower various resoruce requirements (for eg DDR freq).

Quoted 32 lines · Maulik Shah

Quoted 18 lines · Sudeep Holla

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 7 lines

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Hi Sudeep,

there are cases like,

Quoted 3 lines

Last cpu going to deepest idle mode need to lower various resoruce requirements (for eg DDR freq).

In PC mode, only PSCI implementation knows the last man and there shouldn't be any notion of it in OS. If you need it, you may need OSI. You are still mixing up the things. NACK for any such approach, sorry.

Quoted 38 lines · Sudeep Holla

Quoted 35 lines · Maulik Shah

Quoted 18 lines · Sudeep Holla

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 7 lines

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Hi Sudeep,

there are cases like,

Last cpu going to deepest idle mode need to lower various resoruce requirements (for eg DDR freq).

Quoted 3 lines

In PC mode, only PSCI implementation knows the last man and there shouldn't be any notion of it in OS. If you need it, you may need OSI. You are still mixing up the things. NACK for any such approach, sorry.

Sudeep, I don't quite agree with your NACK to this. At least not yet. :-)

I do agree that the best suited solution seems to be OSI, as to support this kind of SoC requirements.

However, if for some reason the PC mode is being used, we could still allow Linux to control "last-man activities" as it knows what each CPU has voted for when going idle. Yes, the PSCI FW decides in the end, but that doesn't really matter. Or is there another technical reason to why you object?

As a matter of fact, if we allow support for PC mode with "last-man-activities", it would allow us to make a fair performance/energy comparison between the two PSCI CPU suspend modes, for the same SoC. I would be thrilled about looking into doing such tests, I bet you are as well!?

Quoted 39 lines · Ulf Hansson

Quoted 38 lines · Sudeep Holla

Quoted 35 lines · Maulik Shah

Quoted 18 lines · Sudeep Holla

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 7 lines

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Hi Sudeep,

there are cases like,

Last cpu going to deepest idle mode need to lower various resoruce requirements (for eg DDR freq).

Quoted 6 lines

Quoted 3 lines

In PC mode, only PSCI implementation knows the last man and there shouldn't be any notion of it in OS. If you need it, you may need OSI. You are still mixing up the things. NACK for any such approach, sorry.

Sudeep, I don't quite agree with your NACK to this. At least not yet. :-)

OK, I am not surprised :-)

Quoted 4 lines

I do agree that the best suited solution seems to be OSI, as to support this kind of SoC requirements.

That's the main point. We need to draw some line as what we want to do with PC and OSI mode. If we plan to take up all last man responsibility in the kernel, what's the point in not supporting OSI in the firmware then ? I can't buy it yet.

Quoted 6 lines

However, if for some reason the PC mode is being used, we could still allow Linux to control "last-man activities" as it knows what each CPU has voted for when going idle. Yes, the PSCI FW decides in the end, but that doesn't really matter. Or is there another technical reason to why you object?

Precisely, FW decides and let it. Just because we can do in the kernel doesn't mean we must do it. It's clear in the spec and doing it in the kernel will be sub-optimal if PSCI f/w aborted entering the deeper state that required some action in the first place.

Quoted 7 lines

As a matter of fact, if we allow support for PC mode with "last-man-activities", it would allow us to make a fair performance/energy comparison between the two PSCI CPU suspend modes, for the same SoC. I would be thrilled about looking into doing such tests, I bet you are as well!?

I was, but not anymore, especially if we want such changes in the kernel to do so.

Just use OSI as that was the point of adding all these after years of discussion claiming it's more optimal compared to PC. Now telling that you need more changes to compare it with PC just doesn't make any sense at all to me.

Quoted 12 lines · Sudeep Holla

Quoted 10 lines · Ulf Hansson

Quoted 9 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

Quoted 5 lines · Sudeep Holla

Quoted 4 lines · Maulik Shah

Quoted 3 lines · Sudeep Holla

Quoted 2 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

Quoted 7 lines

I was, but not anymore, especially if we want such changes in the kernel to do so.

Just use OSI as that was the point of adding all these after years of discussion claiming it's more optimal compared to PC. Now telling that you need more changes to compare it with PC just doesn't make any sense at all to me.

Fair enough.

I was just pondering over if there are other reasons to why we may want this.

One other thing that could be problematic to support, is when are other resources, I/O controllers for example, sharing the same power rail as a cluster. When such controller is in use, idle states of the cluster must be prevented. Without using genpd to model the CPU topology, it may be difficult to deal with this.

Of course, using PC mode when trying to deal with this platform/board-requirement would also be suboptimal. In other words, your argument about when using OSI vs PC mode, still stands.

Quoted 31 lines · Ulf Hansson

Quoted 12 lines · Sudeep Holla

Quoted 10 lines · Ulf Hansson

Quoted 9 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

Quoted 5 lines · Sudeep Holla

Quoted 4 lines · Maulik Shah

Quoted 3 lines · Sudeep Holla

Quoted 2 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

Quoted 7 lines

I was, but not anymore, especially if we want such changes in the kernel to do so.

Just use OSI as that was the point of adding all these after years of discussion claiming it's more optimal compared to PC. Now telling that you need more changes to compare it with PC just doesn't make any sense at all to me.

Fair enough.

I was just pondering over if there are other reasons to why we may want this.

One other thing that could be problematic to support, is when are other resources, I/O controllers for example, sharing the same power rail as a cluster. When such controller is in use, idle states of the cluster must be prevented. Without using genpd to model the CPU topology, it may be difficult to deal with this.

Quoted 4 lines

Of course, using PC mode when trying to deal with this platform/board-requirement would also be suboptimal. In other words, your argument about when using OSI vs PC mode, still stands.

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

[...]

Quoted 18 lines

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 5 lines

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

Just to make it clear, I fully agree with you in regards to overhead for PC-mode. This is especially critical for ARM SoCs with lots of cores, I assume.

However, the overhead you refer to, is *only* going to be present in case when the DTS has the hierarchical CPU topology description with "power-domains". Because, that is *optional* to use, I am expecting only those SoC/platforms that needs to manage last-man activities to use this layout, the others will remain unaffected.

Quoted 30 lines · Ulf Hansson

[...]

Quoted 18 lines

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 5 lines

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

Just to make it clear, I fully agree with you in regards to overhead for PC-mode. This is especially critical for ARM SoCs with lots of cores, I assume.

Quoted 5 lines

However, the overhead you refer to, is *only* going to be present in case when the DTS has the hierarchical CPU topology description with "power-domains". Because, that is *optional* to use, I am expecting only those SoC/platforms that needs to manage last-man activities to use this layout, the others will remain unaffected.

In PC mode not only there is no need but it is wrong to manage any last-man activity in the kernel. I wonder why we are still talking about this to be honest.

Code to handle PSCI platform coordinated mode has been/is in the kernel today and that's all is needed according to the PSCI specifications.

Quoted 38 lines · Lorenzo Pieralisi

Quoted 35 lines · Ulf Hansson

[...]

Quoted 18 lines

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 5 lines

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

Just to make it clear, I fully agree with you in regards to overhead for PC-mode. This is especially critical for ARM SoCs with lots of cores, I assume.

However, the overhead you refer to, is *only* going to be present in case when the DTS has the hierarchical CPU topology description with "power-domains". Because, that is *optional* to use, I am expecting only those SoC/platforms that needs to manage last-man activities to use this layout, the others will remain unaffected.

Quoted 3 lines

In PC mode not only there is no need but it is wrong to manage any last-man activity in the kernel. I wonder why we are still talking about this to be honest.

I guess the discussion is here because there is a use case to consider now.

For sure, we agree on what is the best solution. But this is rather about what can we do to improve the current situation, if we should do anything.

Quoted 4 lines

Code to handle PSCI platform coordinated mode has been/is in the kernel today and that's all is needed according to the PSCI specifications.

PSCI specifies CPU power management, not SoC power management. If these things were completely decoupled, I would agree with you, but that's not the case. Maybe SCMI, etc, helps with this in future.

Anyway, my fear is that not many ARM vendors implements OSI support, but still they have "last-man-activities" to deal with. This is not only QCOM.

I guess an option would be to add OSI support to the public ARM Trusted Firmware, then we could more easily point to that - rather than trying to mitigate the problem on the kernel side.

Quoted 40 lines · Ulf Hansson

Quoted 38 lines · Lorenzo Pieralisi

Quoted 35 lines · Ulf Hansson

[...]

Quoted 18 lines

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 5 lines

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

Just to make it clear, I fully agree with you in regards to overhead for PC-mode. This is especially critical for ARM SoCs with lots of cores, I assume.

However, the overhead you refer to, is *only* going to be present in case when the DTS has the hierarchical CPU topology description with "power-domains". Because, that is *optional* to use, I am expecting only those SoC/platforms that needs to manage last-man activities to use this layout, the others will remain unaffected.

Quoted 6 lines

Quoted 3 lines

In PC mode not only there is no need but it is wrong to manage any last-man activity in the kernel. I wonder why we are still talking about this to be honest.

I guess the discussion is here because there is a use case to consider now.

If this is what Bjorn presented in his email, I have responded to that. If it's any different, please let us know the complete details.

Quoted 4 lines

For sure, we agree on what is the best solution. But this is rather about what can we do to improve the current situation, if we should do anything.

Sure, and I haven't found a reason to do that in OSPM yet(as part of the discussion in this thread)

Quoted 9 lines

Quoted 4 lines

Code to handle PSCI platform coordinated mode has been/is in the kernel today and that's all is needed according to the PSCI specifications.

PSCI specifies CPU power management, not SoC power management. If these things were completely decoupled, I would agree with you, but that's not the case. Maybe SCMI, etc, helps with this in future.

Why does that not work even if they are not decoupled. The IO/device that share with CPU votes from OSPM and the CPU/Cluster from PSCI in PC mode. There is no argument there, but why it needs to be done in OSPM is the objection here.

Quoted 5 lines

Anyway, my fear is that not many ARM vendors implements OSI support, but still they have "last-man-activities" to deal with. This is not only QCOM.

I am interested to hear from them. And the same question to same too as above.

Quoted 5 lines

I guess an option would be to add OSI support to the public ARM Trusted Firmware, then we could more easily point to that - rather than trying to mitigate the problem on the kernel side.

I would say go for it. But don't mix responsibility of OSPM in PC vs OSI. We have discussed this for years and I hope this discussion ends ASAP. I don't see any point in dragging this any further.

Quoted 70 lines · Sudeep Holla

Quoted 40 lines · Ulf Hansson

Quoted 38 lines · Lorenzo Pieralisi

Quoted 35 lines · Ulf Hansson

[...]

Quoted 18 lines

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 5 lines

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

Just to make it clear, I fully agree with you in regards to overhead for PC-mode. This is especially critical for ARM SoCs with lots of cores, I assume.

However, the overhead you refer to, is *only* going to be present in case when the DTS has the hierarchical CPU topology description with "power-domains". Because, that is *optional* to use, I am expecting only those SoC/platforms that needs to manage last-man activities to use this layout, the others will remain unaffected.

Quoted 6 lines

Quoted 3 lines

In PC mode not only there is no need but it is wrong to manage any last-man activity in the kernel. I wonder why we are still talking about this to be honest.

I guess the discussion is here because there is a use case to consider now.

If this is what Bjorn presented in his email, I have responded to that. If it's any different, please let us know the complete details.

Quoted 4 lines

For sure, we agree on what is the best solution. But this is rather about what can we do to improve the current situation, if we should do anything.

Sure, and I haven't found a reason to do that in OSPM yet(as part of the discussion in this thread)

Quoted 9 lines

Quoted 4 lines

Code to handle PSCI platform coordinated mode has been/is in the kernel today and that's all is needed according to the PSCI specifications.

PSCI specifies CPU power management, not SoC power management. If these things were completely decoupled, I would agree with you, but that's not the case. Maybe SCMI, etc, helps with this in future.

Quoted 4 lines

Why does that not work even if they are not decoupled. The IO/device that share with CPU votes from OSPM and the CPU/Cluster from PSCI in PC mode. There is no argument there, but why it needs to be done in OSPM is the objection here.

That implies the votes from I/O devices needs to reach the FW immediately when the vote is done. No caching or other optimizations can be done at OSPM.

In principle, the FW needs to have an always up to date view of the votes, etc. That sounds highly inefficient, both from energy and latency point of view, at least in my opinion.

Quoted 8 lines

Quoted 5 lines

Anyway, my fear is that not many ARM vendors implements OSI support, but still they have "last-man-activities" to deal with. This is not only QCOM.

I am interested to hear from them. And the same question to same too as above.

I have been talking to some of them. But, yes, we need to hear more from them.

Quoted 6 lines

Quoted 5 lines

I guess an option would be to add OSI support to the public ARM Trusted Firmware, then we could more easily point to that - rather than trying to mitigate the problem on the kernel side.

Quoted 3 lines

I would say go for it. But don't mix responsibility of OSPM in PC vs OSI. We have discussed this for years and I hope this discussion ends ASAP. I don't see any point in dragging this any further.

Okay.

[...]

Quoted 18 lines

Quoted 3 lines

I understand the arguments for using PC vs OSI and agree with it. But what in PSCI is against Linux knowing when the last core is powering down when the PSCI is configured to do only Platform Cordinated.

Nothing :D. But knowing the evolution and reasons for adding OSI in the PSCI specification and having argued about benefits of OSI over PC for years and finally when we have it in mainline, this argument of using PC for exact reasons why OSI evolved is something I can't understand and I am confused.

Quoted 7 lines

There should not be any objection to drivers knowing when all the cores are powered down, be it reference counting CPU PM notifications or using a cleaner approach like this where GendPD framwork does everything cleanly and gives a nice callback. ARM architecture allows for different aspects of CPU access be handled at different levels. I see this as an extension of that approach.

Quoted 5 lines

One thing that was repeatedly pointed out during OSI patch review was no extra overhead for PC mode where firmware can make decisions. So, just use OSI now and let us be done with this discussion of OSI vs PC. If PC is what you think you need for future, we can revert all OSI changes and start discussing again :-)

Just to make it clear, I fully agree with you in regards to overhead for PC-mode. This is especially critical for ARM SoCs with lots of cores, I assume.

However, the overhead you refer to, is *only* going to be present in case when the DTS has the hierarchical CPU topology description with "power-domains". Because, that is *optional* to use, I am expecting only those SoC/platforms that needs to manage last-man activities to use this layout, the others will remain unaffected.

That said, does that address your concern?

Quoted 32 lines · Maulik Shah

Quoted 18 lines · Sudeep Holla

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 7 lines

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Hi Sudeep,

there are cases like,

Quoted 3 lines

Last cpu going to deepest idle mode need to lower various resoruce requirements (for eg DDR freq).

In PC mode, only PSCI implementation knows the last man and there shouldn't be any notion of it in OS. If you need it, you may need OSI. You are still mixing up the things. NACK for any such approach, sorry.

Quoted 37 lines · Sudeep Holla

Quoted 35 lines · Maulik Shah

Quoted 18 lines · Sudeep Holla

Quoted 17 lines · Maulik Shah

Quoted 16 lines · Sudeep Holla

Quoted 7 lines · Maulik Shah

From: Ulf Hansson <[email protected]>

If the hierarchical CPU topology is used, but the OS initiated mode isn't supported, we need to rely solely on the regular cpuidle framework to manage the idle state selection, rather than using genpd and its governor.

Quoted 6 lines

For this reason, introduce a new PSCI DT helper function, psci_dt_pm_domains_parse_states(), which parses and converts the hierarchically described domain idle states from DT, into regular flattened cpuidle states. The converted states are added to the existing cpuidle driver's array of idle states, which make them available for cpuidle.

And what's the main motivation for this if OSI is not supported in the firmware ?

Quoted 7 lines

Quoted 5 lines

Hi Sudeep,

Main motivation is to do last-man activities before the CPU cluster can enter a deep idle state.

Details on those last-man activities will help the discussion. Basically I am wondering what they are and why they need to done in OSPM ?

Hi Sudeep,

there are cases like,

Last cpu going to deepest idle mode need to lower various resoruce requirements (for eg DDR freq).

Quoted 4 lines

In PC mode, only PSCI implementation knows the last man and there shouldn't be any notion of it in OS. If you need it, you may need OSI. You are still mixing up the things. NACK for any such approach, sorry.

Forgive me if I'm misunderstanding PSCI's role here, but doesn't it deal with the power management of the "processor subsystem" in the SoC?

In the Qualcomm platforms most resources (voltage rails, clocks, etc) are controlled through a power controller that provides controls for a state when the CPU subsystem is running and when it's asleep. This allows non-CPU-related device to control if resources that are shared with the CPU subsystem should be kept on when the last CPU/cluster goes down.

An example of this would be the display controller voting to keep a voltage rail on after the CPU subsystem collapses, because the display is still on.

But as long as the CPU subsystem is running it will keep these resources available and there's no need to change these votes (e.g. if the display is turned on and off while the CPU is active the sleep-requests cancels out), so they are simply cached/batched up in the RPMh driver and what Maulik's series is attempting to do is to flush the cached values when Linux believes that the firmware might decide to enter a lower power state.

Add device bindings for cpuidle states for cpu devices.

Cc [email protected]

Signed-off-by Maulik Shah <[email protected]>

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

arch/arm64/boot/dts/qcom/sc7180.dtsi +0 -0

index 8011c5f..0aa0ced 100644

Quoted 8 lines · Maulik Shah

Add device bindings for cpuidle states for cpu devices.

Cc [email protected]

Signed-off-by Maulik Shah <[email protected]>

---
  arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++
  1 file changed, 78 insertions(+)

arch/arm64/boot/dts/qcom/sc7180.dtsi line 91

Quoted 11 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi +3 -0

index 8011c5f..0aa0ced 100644
@@ -86,6 +86,9 @@
  			compatible = "arm,armv8";
  			reg = <0x0 0x0>;
  			enable-method = "psci";
+			cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+					   &LITTLE_CPU_SLEEP_1
+					   &CLUSTER_SLEEP_0>;

These entries are deleted again by the next patch in this series ('arm64:
dts: qcom: sc7180: Convert to the hierarchical CPU topology layout'). What is the point in adding them in the first place?