diff mbox series

[2/2] PM / Domains: use device's next wakeup to determine domain idle state

Message ID 20201012223400.23609-3-ilina@codeaurora.org
State Superseded
Headers show
Series Better domain idle from device wakeup patterns | expand

Commit Message

Lina Iyer Oct. 12, 2020, 10:34 p.m. UTC
If the device's next event is known, determine if it is worthwhile
entering a domain idle state. To find the next wakeup, traverse a domain
for all child devices and find out the earliest wakeup value. A parent
domain's next wakeup is the earliest of all its child devices and
domains. The next wakeup is specified by devices in the domains before
they devices are suspended.

Update the domain governor logic to determine if it is worthwhile to
enter an idle state based on the next wakeup for the domain along with
other existing constraints.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/base/power/domain_governor.c | 83 ++++++++++++++++++++++++++--
 include/linux/pm_domain.h            |  1 +
 2 files changed, 79 insertions(+), 5 deletions(-)

Comments

kernel test robot Oct. 13, 2020, 1:46 p.m. UTC | #1
Hi Lina,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pm/linux-next]
[also build test WARNING on linus/master v5.9 next-20201013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lina-Iyer/Better-domain-idle-from-device-wakeup-patterns/20201013-063543
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-m021-20201013 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
drivers/base/power/domain_governor.c:269 default_power_down_ok() warn: always true condition '(state_idx >= 0) => (0-u32max >= 0)'
drivers/base/power/domain_governor.c:269 default_power_down_ok() warn: always true condition '(state_idx >= 0) => (0-u32max >= 0)'
drivers/base/power/domain_governor.c:277 default_power_down_ok() warn: unsigned 'state_idx' is never less than zero.

vim +269 drivers/base/power/domain_governor.c

   245	
   246	/**
   247	 * default_power_down_ok - Default generic PM domain power off governor routine.
   248	 * @pd: PM domain to check.
   249	 *
   250	 * This routine must be executed under the PM domain's lock.
   251	 */
   252	static bool default_power_down_ok(struct dev_pm_domain *pd)
   253	{
   254		struct generic_pm_domain *genpd = pd_to_genpd(pd);
   255		struct gpd_link *link;
   256		unsigned int state_idx;
   257		ktime_t now = ktime_get();
   258	
   259		/*
   260		 * Find the next wakeup from devices that can determine their own wakeup
   261		 * to find when the domain would wakeup and do it for every device down
   262		 * the hierarchy. It is not worth while to sleep if the state's residency
   263		 * cannot be met.
   264		 */
   265		update_domain_next_wakeup(genpd, now);
   266		state_idx = genpd->state_count - 1;
   267		if (genpd->next_wakeup != KTIME_MAX) {
   268			/* Let's find out the deepest domain idle state, the devices prefer */
 > 269			while (state_idx >= 0) {

   270				if (next_wakeup_allows_state(genpd, state_idx, now)) {
   271					genpd->max_off_time_changed = true;
   272					break;
   273				}
   274				state_idx--;
   275			}
   276	
 > 277			if (state_idx < 0) {

   278				state_idx = 0;
   279				genpd->cached_power_down_ok = false;
   280				goto done;
   281			}
   282		}
   283	
   284		if (!genpd->max_off_time_changed) {
   285			genpd->state_idx = genpd->cached_power_down_state_idx;
   286			return genpd->cached_power_down_ok;
   287		}
   288	
   289		/*
   290		 * We have to invalidate the cached results for the parents, so
   291		 * use the observation that default_power_down_ok() is not
   292		 * going to be called for any parent until this instance
   293		 * returns.
   294		 */
   295		list_for_each_entry(link, &genpd->child_links, child_node)
   296			link->parent->max_off_time_changed = true;
   297	
   298		genpd->max_off_time_ns = -1;
   299		genpd->max_off_time_changed = false;
   300		genpd->cached_power_down_ok = true;
   301	
   302		/* Find a state to power down to, starting from the state
   303		 * determined by the next wakeup.
   304		 */
   305		while (!__default_power_down_ok(pd, state_idx)) {
   306			if (state_idx == 0) {
   307				genpd->cached_power_down_ok = false;
   308				break;
   309			}
   310			state_idx--;
   311		}
   312	
   313	done:
   314		genpd->state_idx = state_idx;
   315		genpd->cached_power_down_state_idx = genpd->state_idx;
   316		return genpd->cached_power_down_ok;
   317	}
   318	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index 490ed7deb99a..77b4928aa389 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -117,6 +117,49 @@  static bool default_suspend_ok(struct device *dev)
 	return td->cached_suspend_ok;
 }
 
+static void update_domain_next_wakeup(struct generic_pm_domain *genpd, ktime_t now)
+{
+	ktime_t domain_wakeup = KTIME_MAX;
+	ktime_t next_wakeup;
+	struct pm_domain_data *pdd;
+	struct gpd_link *link;
+
+	/* Find the earliest wakeup for all devices in the domain */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		next_wakeup = READ_ONCE(pdd->dev->power.next_event);
+		if (next_wakeup != KTIME_MAX && !ktime_before(next_wakeup, now))
+			if (ktime_before(next_wakeup, domain_wakeup))
+				domain_wakeup = next_wakeup;
+	}
+
+	/* Then find the earliest wakeup of from all the child domains */
+	list_for_each_entry(link, &genpd->parent_links, parent_node) {
+		next_wakeup = link->child->next_wakeup;
+		if (next_wakeup != KTIME_MAX && !ktime_before(next_wakeup, now))
+			if (ktime_before(next_wakeup, domain_wakeup))
+				domain_wakeup = next_wakeup;
+	}
+
+	genpd->next_wakeup = domain_wakeup;
+}
+
+static bool next_wakeup_allows_state(struct generic_pm_domain *genpd,
+				     unsigned int state, ktime_t now)
+{
+	s64 idle_time_ns, min_sleep_ns;
+	ktime_t domain_wakeup = genpd->next_wakeup;
+
+	min_sleep_ns = genpd->states[state].power_off_latency_ns +
+		       genpd->states[state].power_on_latency_ns +
+		       genpd->states[state].residency_ns;
+
+	idle_time_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
+	if (idle_time_ns < min_sleep_ns)
+		return false;
+
+	return true;
+}
+
 static bool __default_power_down_ok(struct dev_pm_domain *pd,
 				     unsigned int state)
 {
@@ -210,6 +253,33 @@  static bool default_power_down_ok(struct dev_pm_domain *pd)
 {
 	struct generic_pm_domain *genpd = pd_to_genpd(pd);
 	struct gpd_link *link;
+	unsigned int state_idx;
+	ktime_t now = ktime_get();
+
+	/*
+	 * Find the next wakeup from devices that can determine their own wakeup
+	 * to find when the domain would wakeup and do it for every device down
+	 * the hierarchy. It is not worth while to sleep if the state's residency
+	 * cannot be met.
+	 */
+	update_domain_next_wakeup(genpd, now);
+	state_idx = genpd->state_count - 1;
+	if (genpd->next_wakeup != KTIME_MAX) {
+		/* Let's find out the deepest domain idle state, the devices prefer */
+		while (state_idx >= 0) {
+			if (next_wakeup_allows_state(genpd, state_idx, now)) {
+				genpd->max_off_time_changed = true;
+				break;
+			}
+			state_idx--;
+		}
+
+		if (state_idx < 0) {
+			state_idx = 0;
+			genpd->cached_power_down_ok = false;
+			goto done;
+		}
+	}
 
 	if (!genpd->max_off_time_changed) {
 		genpd->state_idx = genpd->cached_power_down_state_idx;
@@ -228,17 +298,20 @@  static bool default_power_down_ok(struct dev_pm_domain *pd)
 	genpd->max_off_time_ns = -1;
 	genpd->max_off_time_changed = false;
 	genpd->cached_power_down_ok = true;
-	genpd->state_idx = genpd->state_count - 1;
 
-	/* Find a state to power down to, starting from the deepest. */
-	while (!__default_power_down_ok(pd, genpd->state_idx)) {
-		if (genpd->state_idx == 0) {
+	/* Find a state to power down to, starting from the state
+	 * determined by the next wakeup.
+	 */
+	while (!__default_power_down_ok(pd, state_idx)) {
+		if (state_idx == 0) {
 			genpd->cached_power_down_ok = false;
 			break;
 		}
-		genpd->state_idx--;
+		state_idx--;
 	}
 
+done:
+	genpd->state_idx = state_idx;
 	genpd->cached_power_down_state_idx = genpd->state_idx;
 	return genpd->cached_power_down_ok;
 }
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index ee11502a575b..9ea6f666967b 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -119,6 +119,7 @@  struct generic_pm_domain {
 				     unsigned int state);
 	struct gpd_dev_ops dev_ops;
 	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
+	ktime_t next_wakeup;
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	bool cached_power_down_state_idx;