PM: SoC idle support using PM domains

drivers/base/power/domain.c +19 -0

index 3ddd05d..8204615 100644
@@ -40,6 +40,46 @@
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
+struct genpd_lock_fns {
+	void (*lock)(struct generic_pm_domain *genpd);
+	void (*lock_nested)(struct generic_pm_domain *genpd, int depth);
+	int (*lock_interruptible)(struct generic_pm_domain *genpd);
+	void (*unlock)(struct generic_pm_domain *genpd);
+};
+
+static void genpd_lock_irq(struct generic_pm_domain *genpd)
+{
+	mutex_lock(&genpd->mlock);
+}
+
+static void genpd_lock_irq_nested(struct generic_pm_domain *genpd,
+					int depth)
+{
+	mutex_lock_nested(&genpd->mlock, depth);
+}
+
+static int genpd_lock_interruptible_irq(struct generic_pm_domain *genpd)

Quoted 32 lines · Lina Iyer

drivers/base/power/domain.c +24 -0

index 3ddd05d..8204615 100644
@@ -40,6 +40,46 @@
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
+struct genpd_lock_fns {
+	void (*lock)(struct generic_pm_domain *genpd);
+	void (*lock_nested)(struct generic_pm_domain *genpd, int depth);
+	int (*lock_interruptible)(struct generic_pm_domain *genpd);
+	void (*unlock)(struct generic_pm_domain *genpd);
+};
+
+static void genpd_lock_irq(struct generic_pm_domain *genpd)
+{
+	mutex_lock(&genpd->mlock);
+}
+
+static void genpd_lock_irq_nested(struct generic_pm_domain *genpd,
+					int depth)
+{
+	mutex_lock_nested(&genpd->mlock, depth);
+}
+
+static int genpd_lock_interruptible_irq(struct generic_pm_domain *genpd)
+{
+	return mutex_lock_interruptible(&genpd->mlock);
+}
+
+static void genpd_unlock_irq(struct generic_pm_domain *genpd)

drivers/base/power/domain.c line 71

Quoted 5 lines

drivers/base/power/domain.c +5 -0

+{
+	return mutex_unlock(&genpd->mlock);
+}
+
+static struct genpd_lock_fns irq_lock = {

Can this be const? Also, why is this called irq_lock when the lock functions are mutex based?

Quoted 5 lines

drivers/base/power/domain.c +5 -0

+	.lock = genpd_lock_irq,
+	.lock_nested = genpd_lock_irq_nested,
+	.lock_interruptible = genpd_lock_interruptible_irq,
+	.unlock = genpd_unlock_irq,
+};

drivers/base/power/domain.c struct generic_pm_domain { · line 78

Quoted 6 lines

drivers/base/power/domain.c +2 -0

+

@@ -74,6 +75,8 @@ struct generic_pm_domain {
 	struct genpd_power_state *states;
 	unsigned int state_count; /* number of states */
 	unsigned int state_idx; /* state that genpd will go to when off */
+	struct genpd_lock_fns *lock_fns;

const?

Documentation/power/devices.txt +0 -0

index 8ba6625..c06f0b6 100644

Documentation/power/devices.txt +1 -0

+subdomain to another IRQ safe domain.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,

drivers/base/power/domain.c +17 -0

index 8204615..3c4f675 100644
@@ -75,11 +75,59 @@ static struct genpd_lock_fns irq_lock = {
 	.unlock = genpd_unlock_irq,
 };
 
+static void genpd_lock_nosleep(struct generic_pm_domain *genpd)
+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&genpd->slock, flags);
+	genpd->lock_flags = flags;
+}
+
+static void genpd_lock_nosleep_nested(struct generic_pm_domain *genpd,
+					int depth)
+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave_nested(&genpd->slock, flags, depth);
+	genpd->lock_flags = flags;

drivers/base/power/domain.c +12 -0

+	.lock = genpd_lock_nosleep,
+	.lock_nested = genpd_lock_nosleep_nested,
+	.lock_interruptible = genpd_lock_nosleep_interruptible,
+	.unlock = genpd_unlock_nosleep,
+};
+
 #define genpd_lock(p)			p->lock_fns->lock(p)
 #define genpd_lock_nested(p, d)		p->lock_fns->lock_nested(p, d)
 #define genpd_lock_interruptible(p)	p->lock_fns->lock_interruptible(p)
 #define genpd_unlock(p)			p->lock_fns->unlock(p)
 
+static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
+		struct generic_pm_domain *genpd)
+{
+	return dev->power.irq_safe && !genpd->irq_safe;
+}
+
 /*
  * Get the generic PM domain for a particular struct device.
  * This validates the struct device pointer, the PM domain pointer,

drivers/base/power/domain.c +4 -0

+			genpd->name);
+		return -EINVAL;
+	}
+
 	gpd_data = genpd_alloc_dev_data(dev, genpd, td);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);

drivers/base/power/domain.c +4 -0

+				genpd->name, subdomain->name);
+		return -EINVAL;
+	}
+
 	link = kzalloc(sizeof(*link), GFP_KERNEL);
 	if (!link)
 		return -ENOMEM;

Quoted 11 lines · Lina Iyer

Documentation/power/devices.txt +2 -1

index 8ba6625..c06f0b6 100644
@@ -607,7 +607,16 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain.

Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 614

Quoted 3 lines

Documentation/power/devices.txt +3 -0

+
+Devices, by default, operate in process context and if a device can operate in
+IRQ safe context, has to be explicitly set as IRQ safe. Power domains by

Devices, by default, operate in process context. If a device can operate in IRQ safe context that has to be explicitly indicated by setting the irq_safe boolean inside struct generic_pm_domain to true. Power domains typically operate in process context...

Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 617

Quoted 3 lines

Documentation/power/devices.txt +3 -0

+default, operate in process context but could have devices that are IRQ safe.
+Such power domains cannot be powered on/off during runtime PM. On the other
+hand, an IRQ safe PM domains that have IRQ safe devices may be powered off

On the other hand, IRQ safe PM domains that have ..

Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 618

Quoted 1 line

Documentation/power/devices.txt +1 -0

+when all the devices are in idle. An IRQ safe domain may only be attached as a

all the devices in the domain?

Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 619

Quoted 1 line

Documentation/power/devices.txt +1 -0

+static struct genpd_lock_fns no_sleep_lock = {

const?

Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 621

Quoted 2 lines

Documentation/power/devices.txt +2 -0

+	/*
+	 * As we dont power off a non IRQ safe domain, which holds

s/dont/don't/

Documentation/power/devices.txt individually. Instead, a set of devices sharing a power resource can be put · line 622

Quoted 1 line

Documentation/power/devices.txt +1 -0

+	 * an IRQ safe device, we dont need to restore power to it.

s/dont/don't/

Quoted 9 lines

Documentation/power/devices.txt +2 -0

+	 */
+	if (dev->power.irq_safe && !genpd->irq_safe) {
 		timed = false;
 		goto out;
 	}

@@ -1296,6 +1359,13 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 

Documentation/power/devices.txt int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, · line 1364

Quoted 3 lines

Documentation/power/devices.txt +3 -0

+	if (genpd->irq_safe && !dev->power.irq_safe) {
+		dev_err(dev,
+			"PM Domain %s is IRQ safe; device has to IRQ safe.\n",

has to be?

Quoted 11 lines

Documentation/power/devices.txt +4 -0

+			genpd->name);
+		return -EINVAL;
+	}
+
 	gpd_data = genpd_alloc_dev_data(dev, genpd, td);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);

@@ -1394,6 +1464,17 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 	    || genpd == subdomain)
 		return -EINVAL;
 

Documentation/power/devices.txt int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, · line 1473

Quoted 7 lines

Documentation/power/devices.txt +7 -0

+	/*
+	 * If the domain can be powered on/off in an IRQ safe
+	 * context, ensure that the subdomain can also be
+	 * powered on/off in that context.
+	 */
+	if (!genpd->irq_safe && subdomain->irq_safe) {
+		WARN("Parent %s of subdomain %s must be IRQ-safe\n",

Nitpick! IRQ-safe or IRQ safe? Use one consistently please.

Check for NULL platform callback before calling.

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/base/power/cpu_domains.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

drivers/base/power/cpu_domains.c +0 -0

index 7069411..bcaa474 100644

Quoted 12 lines · Lina Iyer

Check for NULL platform callback before calling.

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/base/power/cpu_domains.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

drivers/base/power/cpu_domains.c +0 -0

index 7069411..bcaa474 100644

drivers/base/power/cpu_domains.c static int cpu_pd_power_on(struct generic_pm_domain *genpd) · line 164

Quoted 9 lines

drivers/base/power/cpu_domains.c +4 -1

@@ -157,16 +157,22 @@ static int cpu_pd_power_on(struct generic_pm_domain *genpd)
 {
        struct cpu_pm_domain *pd = to_cpu_pd(genpd);
 
-       return pd->ops.power_on();
+       if (pd->ops.power_on)
+               return pd->ops.power_on();
+
+       return 0;
 }

I usually write things like that as

return pd->ops.power_on ? pd->ops.power_on() : 0;

That gets the job done in just one line of code instead of 4 and in one statement instead of 3.

drivers/base/power/cpu_domains.c new file +10 -0

+
+/**
+ * of_init_cpu_pm_domain() - Initialize a CPU PM domain from a device node
+ *
+ * @dn: The domain provider's device node
+ * @ops: The power_on/_off callbacks for the domain
+ *
+ * Returns the generic_pm_domain (genpd) pointer to the domain on success
+ */
+static struct generic_pm_domain *of_init_cpu_pm_domain(struct device_node *dn,

drivers/base/power/cpu_domains.c new file +15 -0

+	if (!genpd)
+		goto fail;
+
+	genpd->name = kstrndup(dn->full_name, CPU_PD_NAME_MAX, GFP_KERNEL);
+	if (!genpd->name)
+		goto fail;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		goto fail;
+
+	pd->genpd = genpd;
+	pd->genpd->power_off = cpu_pd_power_off;
+	pd->genpd->power_on = cpu_pd_power_on;
+	pd->genpd->flags |= GENPD_FLAG_IRQ_SAFE;

drivers/base/power/cpu_domains.c new file +6 -0

+	ret = of_pm_genpd_init(dn, pd->genpd, &simple_qos_governor, false);
+	if (ret) {
+		pr_err("Unable to initialize domain %s\n", dn->full_name);
+		goto fail;
+	}
+

drivers/base/power/cpu_domains.c new file +8 -0

+	kfree(pd);
+	return ERR_PTR(ret);
+}
+
+static struct generic_pm_domain *of_get_cpu_domain(struct device_node *dn,
+		const struct cpu_pd_ops *ops, int cpu)
+{
+	struct of_phandle_args args;

drivers/base/power/cpu_domains.c new file +28 -0

+		 */
+		cpu_pd = to_cpu_pd(genpd);
+		parent_cpu_pd = to_cpu_pd(parent);
+
+		if (cpu_pd && parent_cpu_pd)
+			cpu_pd->parent = parent_cpu_pd;
+	}
+
+skip_parent:
+	return genpd;
+}
+
+/**
+ * of_setup_cpu_pd_single() - Setup the PM domains for a CPU
+ *
+ * @cpu: The CPU for which the PM domain is to be set up.
+ * @ops: The PM domain suspend/resume ops for the CPU's domain
+ *
+ * If the CPU PM domain exists already, then the CPU is attached to
+ * that CPU PD. If it doesn't, the domain is created, the @ops are
+ * set for power_on/power_off callbacks and then the CPU is attached
+ * to that domain. If the domain was created outside this framework,
+ * then we do not attach the CPU to the domain.
+ */
+int of_setup_cpu_pd_single(int cpu, const struct cpu_pd_ops *ops)
+{
+
+	struct device_node *dn;

drivers/base/power/cpu_domains.c new file +9 -0

+
+	/* Find the genpd for this CPU, create if not found */
+	genpd = of_get_cpu_domain(dn, ops, cpu);
+	if (IS_ERR(genpd))
+		return PTR_ERR(genpd);
+
+	return cpu_pd_attach_cpu(cpu);
+}
+EXPORT_SYMBOL(of_setup_cpu_pd_single);

drivers/base/power/cpu_domains.c line 4

Quoted 10 lines · Lina Iyer

drivers/base/power/cpu_domains.c new file +4 -0

new file mode 100644
index 0000000..981592f
@@ -0,0 +1,267 @@
+
+/* List of CPU PM domains we care about */
+static LIST_HEAD(of_cpu_pd_list);
+static DEFINE_SPINLOCK(cpu_pd_list_lock);

Can this be a mutex?

drivers/base/power/cpu_domains.c line 109

Quoted 1 line

drivers/base/power/cpu_domains.c new file +1 -0

+	genpd = kzalloc(sizeof(*(genpd)), GFP_KERNEL);

Drop extra parenthesis

drivers/base/power/cpu_domains.c line 134

Quoted 2 lines

drivers/base/power/cpu_domains.c new file +2 -0

+	/* Register the CPU genpd */
+	pr_debug("adding %s as CPU PM domain.\n", pd->genpd->name);

Drop the full stop?

drivers/base/power/cpu_domains.c line 150

Quoted 2 lines

drivers/base/power/cpu_domains.c new file +2 -0

+	kfree(genpd);
+	kfree(genpd->name);

Switch order so that name is freed first to avoid junk deref here.

Quoted 7 lines

drivers/base/power/cpu_domains.c new file +7 -0

+	kfree(pd);
+	return ERR_PTR(ret);
+}
+
+static struct generic_pm_domain *of_get_cpu_domain(struct device_node *dn,
+		const struct cpu_pd_ops *ops, int cpu)
+{

drivers/base/power/cpu_domains.c line 168

Quoted 3 lines

drivers/base/power/cpu_domains.c new file +3 -0

+	genpd = of_genpd_get_from_provider(&args);
+	if (!IS_ERR(genpd))
+		goto skip_parent;

Why not just return genpd and drop the goto?

drivers/base/power/cpu_domains.c line 177

Quoted 8 lines

drivers/base/power/cpu_domains.c new file +8 -0

+	genpd = of_init_cpu_pm_domain(dn, ops);
+	if (IS_ERR(genpd))
+		return genpd;
+
+	/* Is there a domain provider for this domain? */
+	ret = of_parse_phandle_with_args(dn, "power-domains",
+			"#power-domain-cells", 0, &args);
+	of_node_put(dn);

Shouldn't this be of_node_put(args.np)? I suppose it's the same so this isn't too important.

drivers/base/power/cpu_domains.c line 182

Quoted 5 lines

drivers/base/power/cpu_domains.c new file +5 -0

+	if (ret < 0)
+		goto skip_parent;
+
+	/* Find its parent and attach this domain to it, recursively */
+	parent = of_get_cpu_domain(args.np, ops, cpu);

Except that we use the np here. So perhaps move the of_node_put() down to the skip_parent goto?

drivers/base/power/cpu_domains.c line 186

Quoted 4 lines

drivers/base/power/cpu_domains.c new file +4 -0

+	if (IS_ERR(parent)) {
+		struct cpu_pm_domain *cpu_pd, *parent_cpu_pd;
+
+		ret = pm_genpd_add_subdomain(genpd, parent);

parent is an error pointer here... isn't this always going to fail? Maybe that should be if (!IS_ERR(parent)) up there?

drivers/base/power/cpu_domains.c line 197

Quoted 4 lines

drivers/base/power/cpu_domains.c new file +4 -0

+		/*
+		 * Reference parent domain for easy access.
+		 * Note: We could be attached to a domain that is not a
+		 * CPU PM domain in that case dont reference the parent.

s/dont/don't/

drivers/base/power/cpu_domains.c line 235

Quoted 8 lines

drivers/base/power/cpu_domains.c new file +8 -0

+	dn = of_get_cpu_node(cpu, NULL);
+	if (!dn)
+		return -ENODEV;
+
+	dn = of_parse_phandle(dn, "power-domains", 0);
+	if (!dn)
+		return -ENODEV;
+	of_node_put(dn);

This should be put after of_get_cpu_domain().

Quoted 9 lines

drivers/base/power/cpu_domains.c new file +9 -0

+
+	/* Find the genpd for this CPU, create if not found */
+	genpd = of_get_cpu_domain(dn, ops, cpu);
+	if (IS_ERR(genpd))
+		return PTR_ERR(genpd);
+
+	return cpu_pd_attach_cpu(cpu);
+}
+EXPORT_SYMBOL(of_setup_cpu_pd_single);

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

drivers/cpuidle/cpuidle-arm.c +2 -0

+
 		/*
 		 * Pass idle state index to cpu_suspend which in turn will
 		 * call the CPU ops suspend protocol with idle index as a

@@ -52,6 +57,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
 		 */
 		arm_cpuidle_suspend(idx);
 
+		RCU_NONIDLE(pm_runtime_get_sync(cpu_dev));
 		cpu_pm_exit();
 	}
 

drivers/cpuidle/cpuidle-arm.c +14 -0

+		break;
+	case CPU_STARTING:
+	case CPU_STARTING_FROZEN:
+		RCU_NONIDLE(pm_runtime_get_sync(cpu_dev));
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+#endif
+
 /*
  * arm_idle_init
  *

@@ -96,6 +126,7 @@ static int __init arm_idle_init(void)
 	int cpu, ret;
 	struct cpuidle_driver *drv = &arm_idle_driver;
 	struct cpuidle_device *dev;
+	struct device *cpu_dev;
 
 	/*
 	 * Initialize idle states data, starting at index 1.

drivers/cpuidle/cpuidle-arm.c static int arm_enter_idle_state(struct cpuidle_device *dev, · line 51

Quoted 5 lines · Lina Iyer

drivers/cpuidle/cpuidle-arm.c +1 -0

@@ -45,6 +48,8 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
 
 	ret = cpu_pm_enter();
 	if (!ret) {
+		RCU_NONIDLE(pm_runtime_put_sync_suspend(cpu_dev));

Can you add a comment on why we need to use RCU_NONIDLE here? It's not super obvious.

Quoted 16 lines

drivers/cpuidle/cpuidle-arm.c +2 -0

+
 		/*
 		 * Pass idle state index to cpu_suspend which in turn will
 		 * call the CPU ops suspend protocol with idle index as a

@@ -52,6 +57,7 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
 		 */
 		arm_cpuidle_suspend(idx);
 
+		RCU_NONIDLE(pm_runtime_get_sync(cpu_dev));
 		cpu_pm_exit();
 	}
 

@@ -84,6 +90,30 @@ static const struct of_device_id arm_idle_state_match[] __initconst = {
 	{ },
 };
 

drivers/cpuidle/cpuidle-arm.c static const struct of_device_id arm_idle_state_match[] __initconst = { · line 94

Quoted 2 lines

drivers/cpuidle/cpuidle-arm.c +2 -0

+#ifdef CONFIG_HOTPLUG_CPU
+static int cpu_hotplug(struct notifier_block *nb,

This function is pretty generically named. Maybe something more runtime PM specific or cpu idle specific?

drivers/cpuidle/cpuidle-arm.c static const struct of_device_id arm_idle_state_match[] __initconst = { · line 100

Quoted 6 lines

drivers/cpuidle/cpuidle-arm.c +6 -0

+			unsigned long action, void *data)
+{
+	struct device *cpu_dev = get_cpu_device(smp_processor_id());
+
+	/* Execute CPU runtime PM on that CPU */
+	switch (action) {

We could do the & ~CPU_TASKS_FROZEN trick here to save a few cases.

drivers/cpuidle/cpuidle-arm.c static const struct of_device_id arm_idle_state_match[] __initconst = { · line 103

Quoted 3 lines

drivers/cpuidle/cpuidle-arm.c +3 -0

+	case CPU_DYING:
+	case CPU_DYING_FROZEN:
+		RCU_NONIDLE(pm_runtime_put_sync_suspend(cpu_dev));

And do we actually need to use it for hotplug path? These notifiers don't run from idle context do they?

+

Quoted 15 lines

drivers/cpuidle/cpuidle-arm.c +1 -0

 /*
  * arm_idle_init
  *

@@ -96,6 +126,7 @@ static int __init arm_idle_init(void)
 	int cpu, ret;
 	struct cpuidle_driver *drv = &arm_idle_driver;
 	struct cpuidle_device *dev;
+	struct device *cpu_dev;
 
 	/*
 	 * Initialize idle states data, starting at index 1.

@@ -148,10 +189,17 @@ static int __init arm_idle_init(void)
 		}
 	}
 

drivers/cpuidle/cpuidle-arm.c static int __init arm_idle_init(void) · line 194

Quoted 3 lines

drivers/cpuidle/cpuidle-arm.c +3 -0

+#ifdef CONFIG_HOTPLUG_CPU
+	/* Register for hotplug notifications for runtime PM */
+	hotcpu_notifier(cpu_hotplug, 0);

Define an empty cpu_hotplug() function for !CONFIG_HOTPLUG_CPU and then always call this without the ifdef?

include/linux/cpu_domains.h +2 -1

+
 struct cpu_pd_ops {
-	int (*power_off)(u32 state_idx, u32 param);
+	int (*power_off)(u32 state_idx, u32 param, const struct cpumask *mask);
 	int (*power_on)(void);
 };

Quoted 8 lines · Lina Iyer

include/linux/cpu_domains.h +0 -0

index bab4846..0c539f0 100644
@@ -11,8 +11,10 @@
 #ifndef __CPU_DOMAINS_H__
 #define __CPU_DOMAINS_H__
 

include/linux/cpu_domains.h line 14

Quoted 1 line

include/linux/cpu_domains.h +1 -0

+#include <linux/cpumask.h>

Just forward declare struct cpumask instead?

Quoted 6 lines

include/linux/cpu_domains.h +2 -1

+
 struct cpu_pd_ops {
-	int (*power_off)(u32 state_idx, u32 param);
+	int (*power_off)(u32 state_idx, u32 param, const struct cpumask *mask);
 	int (*power_on)(void);
 };

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

drivers/base/power/cpu_domains.c +15 -0

@@ -52,6 +55,76 @@ struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d)
 	return res;
 }
 
+static bool cpu_pd_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	struct cpu_pm_domain *cpu_pd = to_cpu_pd(genpd);
+	int qos = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
+	u64 sleep_ns;
+	ktime_t earliest, next_wakeup;
+	int cpu;
+	int i;
+
+	/* Reset the last set genpd state, default to index 0 */
+	genpd->state_idx = 0;
+
+	/* We dont want to power down, if QoS is 0 */
+	if (!qos)

drivers/base/power/cpu_domains.c +10 -0

+			earliest = next_wakeup;
+	}
+
+	sleep_ns = ktime_to_ns(ktime_sub(earliest, ktime_get()));
+	if (sleep_ns <= 0)
+		return false;
+
+	/*
+	 * Find the deepest sleep state that satisfies the residency
+	 * requirement and the QoS constraint

Quoted 24 lines · Lina Iyer

drivers/base/power/cpu_domains.c +20 -0

@@ -52,6 +55,76 @@ struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d)
 	return res;
 }
 
+static bool cpu_pd_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	struct cpu_pm_domain *cpu_pd = to_cpu_pd(genpd);
+	int qos = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
+	u64 sleep_ns;
+	ktime_t earliest, next_wakeup;
+	int cpu;
+	int i;
+
+	/* Reset the last set genpd state, default to index 0 */
+	genpd->state_idx = 0;
+
+	/* We dont want to power down, if QoS is 0 */
+	if (!qos)
+		return false;
+
+	/*
+	 * Find the sleep time for the cluster.
+	 * The time between now and the first wake up of any CPU that

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 82

Quoted 5 lines

drivers/base/power/cpu_domains.c +5 -0

+	 * are in this domain hierarchy is the time available for the
+	 * domain to be idle.
+	 */
+	earliest = ktime_set(KTIME_SEC_MAX, 0);
+	for_each_cpu_and(cpu, cpu_pd->cpus, cpu_online_mask) {

We're not worried about hotplug happening in parallel because preemption is disabled here?

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 84

Quoted 2 lines

drivers/base/power/cpu_domains.c +2 -0

+		next_wakeup = tick_nohz_get_next_wakeup(cpu);
+		if (earliest.tv64 > next_wakeup.tv64)

if (ktime_before(next_wakeup, earliest))

Quoted 14 lines

drivers/base/power/cpu_domains.c +14 -0

+			earliest = next_wakeup;
+	}
+
+	sleep_ns = ktime_to_ns(ktime_sub(earliest, ktime_get()));
+	if (sleep_ns <= 0)
+		return false;
+
+	/*
+	 * Find the deepest sleep state that satisfies the residency
+	 * requirement and the QoS constraint
+	 */
+	for (i = genpd->state_count - 1; i >= 0; i--) {
+		u64 state_sleep_ns;
+

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 104

Quoted 6 lines

drivers/base/power/cpu_domains.c +6 -0

+		state_sleep_ns = genpd->states[i].power_off_latency_ns +
+			genpd->states[i].power_on_latency_ns +
+			genpd->states[i].residency_ns;
+
+		/*
+		 * If we cant sleep to save power in the state, move on

s/cant/can't/

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 111

Quoted 7 lines

drivers/base/power/cpu_domains.c +7 -0

+		 * to the next lower idle state.
+		 */
+		if (state_sleep_ns > sleep_ns)
+			continue;
+
+		/*
+		 * We also dont want to sleep more than we should to

s/dont/don't/

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 114

Quoted 3 lines

drivers/base/power/cpu_domains.c +3 -0

+		 * gaurantee QoS.
+		 */
+		if (state_sleep_ns < (qos * NSEC_PER_USEC))

Maybe we should make qos into qos_ns? Presumably the compiler would hoist out the multiplication here, but it doesn't hurt to do it explicitly.

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 121

Quoted 7 lines

drivers/base/power/cpu_domains.c +7 -0

+			break;
+	}
+
+	if (i >= 0)
+		genpd->state_idx = i;
+
+	return  (i >= 0) ? true : false;

Just return i >= 0?

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Quoted 26 lines · Stephen Boyd

Quoted 25 lines · Lina Iyer

drivers/base/power/cpu_domains.c +20 -0

@@ -52,6 +55,76 @@ struct cpu_pm_domain *to_cpu_pd(struct
 generic_pm_domain *d)
     return res;
 }
 
+static bool cpu_pd_down_ok(struct dev_pm_domain *pd)
+{
+    struct generic_pm_domain *genpd = pd_to_genpd(pd);
+    struct cpu_pm_domain *cpu_pd = to_cpu_pd(genpd);
+    int qos = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
+    u64 sleep_ns;
+    ktime_t earliest, next_wakeup;
+    int cpu;
+    int i;
+
+    /* Reset the last set genpd state, default to index 0 */
+    genpd->state_idx = 0;
+
+    /* We dont want to power down, if QoS is 0 */
+    if (!qos)
+        return false;
+
+    /*
+     * Find the sleep time for the cluster.
+     * The time between now and the first wake up of any CPU that

Quoted 9 lines

drivers/base/power/cpu_domains.c struct cpu_pm_domain *to_cpu_pd(struct generic_pm_domain *d) · line 82

Quoted 5 lines

drivers/base/power/cpu_domains.c +5 -0

+     * are in this domain hierarchy is the time available for the
+     * domain to be idle.
+     */
+    earliest = ktime_set(KTIME_SEC_MAX, 0);
+    for_each_cpu_and(cpu, cpu_pd->cpus, cpu_online_mask) {

We're not worried about hotplug happening in parallel because preemption is disabled here?

Nope. Hotplug on the same domain or in its hierarchy will be waiting on the domain lock to released before becoming online. Any other domain is not of concern for this domain governor.

If a core was hotplugged out while this is happening, then we may risk making an premature wake up decision, which would happen either way if we lock hotplug here.

Documentation/power/cpu_domains.txt new file +8 -0

+resources when any of the CPUs are active. When the CPUs are in idle, some of
+these cluster components may also idle. A cluster may also be nested inside
+another cluster that provides common coherency interfaces to share data
+between the clusters. The organization of such clusters and CPU may be
+descibed in DT, since they are SoC specific.
+
+CPUIdle framework enables the CPUs to determine the sleep time and enter low
+power state to save power during periods of idle. CPUs in a cluster may enter

Documentation/power/cpu_domains.txt new file +20 -0

+device may have a domain provider and a domain provider may support multiple
+domain consumers. Domains like clusters, may also be nested inside one
+another. A domain that has no active consumer, may be powered off and any
+resuming consumer would trigger the domain back to active. Parent domains may
+be powered off when the child domains are powered off. The CPU cluster can be
+fashioned as a PM domain. When the CPU devices are powered off, the PM domain
+may be powered off.
+
+Device idle is reference counted by runtime PM. When there is no active need
+for the device, runtime PM invokes callbacks to suspend the parent domain.
+Generic PM domain (genpd) handles the hierarchy of devices, domains and the
+reference counting of objects leading to last man down and first man up in the
+domain. The CPU domains helper functions defines PM domains for each CPU
+cluster and attaches the CPU devices to the respective PM domains.
+
+Platform drivers may use the following API to register their CPU PM domains.
+
+of_setup_cpu_pd() -
+Provides a single step registration of the CPU PM domain and attach CPUs to
+the genpd. Platform drivers may additionally register callbacks for power_on

Documentation/power/cpu_domains.txt new file +5 -0

+for known timer interrupts and unknown interrupts from idle. Prediction
+algorithms and heuristic based algorithms like the Menu governor for cpuidle
+can determine the next wakeup of the CPU. However, determining the wakeup
+across a group of CPUs is a tough problem to solve.
+

Documentation/power/cpu_domains.txt line 5

Quoted 11 lines · Lina Iyer

Documentation/power/cpu_domains.txt new file +5 -0

new file mode 100644
index 0000000..5fdc66d
@@ -0,0 +1,79 @@
+CPU PM domains
+==============
+
+Newer CPUs are grouped in SoCs as clusters. A cluster in addition to the CPUs
+may have caches, VFP and architecture specific power controller that share the

caches, floating point units, and other architecture specific hardware that share resources when any of the CPUs are active.

Quoted 8 lines

Documentation/power/cpu_domains.txt new file +8 -0

+resources when any of the CPUs are active. When the CPUs are in idle, some of
+these cluster components may also idle. A cluster may also be nested inside
+another cluster that provides common coherency interfaces to share data
+between the clusters. The organization of such clusters and CPU may be
+descibed in DT, since they are SoC specific.
+
+CPUIdle framework enables the CPUs to determine the sleep time and enter low
+power state to save power during periods of idle. CPUs in a cluster may enter

Documentation/power/cpu_domains.txt line 23

Quoted 10 lines

Documentation/power/cpu_domains.txt new file +10 -0

+and exit idle state independently of each other. During the time when all the
+CPUs are in idle state, the cluster may safely put some of the shared
+resources in their idle state. The time between the last CPU to enter idle and
+the first CPU to wake up is the time available for the cluster to enter its
+idle state.
+
+When SoCs power down the CPU during cpuidle, they generally have supplemental
+hardware that can handshake with the CPU with a signal that indicates that the
+CPU has stopped execution. The hardware is also responsible for warm booting
+the CPU on receiving an interrupt. With cluster architecture, common resources

In a cluster architecture,

Documentation/power/cpu_domains.txt line 24

Quoted 1 line

Documentation/power/cpu_domains.txt new file +1 -0

+that are shared by the cluster may also be powered down by an external

shared by a cluster

Documentation/power/cpu_domains.txt line 32

Quoted 8 lines

Documentation/power/cpu_domains.txt new file +8 -0

+microcontroller or a processor. The microcontroller may be programmed in
+advance to put the hardware blocks in a low power state, when the last active
+CPU sends the idle signal. When the signal is received, the microcontroller
+may trigger the hardware blocks to enter their low power state. When an
+interrupt to wakeup the processor is received, the microcontroller is
+responsible for bringing up the hardware blocks to its active state, before
+waking up the CPU. The timelines for such operations should be in the
+acceptable range for the for CPU idle to get power benefits.

acceptable range for CPU idle to get power benefits.

Documentation/power/cpu_domains.txt line 37

Quoted 5 lines

Documentation/power/cpu_domains.txt new file +5 -0

+
+CPU PM Domain Setup
+-------------------
+
+PM domains  are represented in the DT as domain consumers and providers. A

^ extra space here

Quoted 20 lines

Documentation/power/cpu_domains.txt new file +20 -0

+device may have a domain provider and a domain provider may support multiple
+domain consumers. Domains like clusters, may also be nested inside one
+another. A domain that has no active consumer, may be powered off and any
+resuming consumer would trigger the domain back to active. Parent domains may
+be powered off when the child domains are powered off. The CPU cluster can be
+fashioned as a PM domain. When the CPU devices are powered off, the PM domain
+may be powered off.
+
+Device idle is reference counted by runtime PM. When there is no active need
+for the device, runtime PM invokes callbacks to suspend the parent domain.
+Generic PM domain (genpd) handles the hierarchy of devices, domains and the
+reference counting of objects leading to last man down and first man up in the
+domain. The CPU domains helper functions defines PM domains for each CPU
+cluster and attaches the CPU devices to the respective PM domains.
+
+Platform drivers may use the following API to register their CPU PM domains.
+
+of_setup_cpu_pd() -
+Provides a single step registration of the CPU PM domain and attach CPUs to
+the genpd. Platform drivers may additionally register callbacks for power_on

Documentation/power/cpu_domains.txt line 67

Quoted 10 lines

Documentation/power/cpu_domains.txt new file +10 -0

+and power_off operations for the PM domain.
+
+of_setup_cpu_pd_single() -
+Define PM domain for a single CPU and attach the CPU to its domain.
+
+
+CPU PM Domain governor
+----------------------
+
+CPUs have an unique ability to determine their next wakeup. CPUs may wake up

a unique

Quoted 5 lines

Documentation/power/cpu_domains.txt new file +5 -0

+for known timer interrupts and unknown interrupts from idle. Prediction
+algorithms and heuristic based algorithms like the Menu governor for cpuidle
+can determine the next wakeup of the CPU. However, determining the wakeup
+across a group of CPUs is a tough problem to solve.
+

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

arch/arm64/boot/dts/qcom/msm8916.dtsi +8 -0

+	};
+
+	pd-power-states {
+		CLUSTER_RET: power-state at 1 {
+			state-param = <0x1000010>;
+			entry-latency-us = <500>;
+			exit-latency-us = <500>;
+			residency-us = <2000>;

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

 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";

Quoted 4 lines · Lina Iyer

(quoted diff) +0 -0

@@ -101,6 +105,27 @@
 		};
 	};
 

arch/arm64/boot/dts/qcom/msm8916.dtsi line 110

Quoted 3 lines

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

+	CPU_PD: cpu-pd at 0 {
+		#power-domain-cells = <0>;
+		power-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;

Why isn't this part of the psci node? PSCI is the node that's providing the code/logic for the power domain.

Quoted 13 lines

arch/arm64/boot/dts/qcom/msm8916.dtsi +13 -0

+	};
+
+	pd-power-states {
+		CLUSTER_RET: power-state at 1 {
+			state-param = <0x1000010>;
+			entry-latency-us = <500>;
+			exit-latency-us = <500>;
+			residency-us = <2000>;
+		 };
+
+		CLUSTER_PWR_DWN: power-state at 2 {
+			state-param = <0x1000030>;
+			entry-latency-us = <2000>;

arch/arm64/boot/dts/qcom/msm8916.dtsi line 128

Quoted 5 lines

arch/arm64/boot/dts/qcom/msm8916.dtsi +5 -0

+			exit-latency-us = <2000>;
+			residency-us = <6000>;
+		};
+	};
+

And I would expect these to be put somewhere inside the power domain provider as well? Is this documented somewhere?

Quoted 3 lines

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

 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project