Skip to contentSkip to conversation

PM / Domains: enable domain idle state accounting

PATCH Superseded Lina Iyer 2 participants 3 messages Oct 3, 2020 – Oct 7, 2020

Subsystems (6)
Arm Qualcomm mailing list Driver core Suspend to RAM Hibernation +2 more subsystems

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Lina Iyer

author
#0 lore raw

PATCH v2 1/1 PM / Domains: enable domain idle state accounting

To enable better debug of PM domains, let's keep a track of the success and rejections in entering each domain idle state.

This statistics is exported in debugfs when reading the idle_states node, associated with each PM domain.

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

---
Changes in v2:
- Renamed 'failed' to 'rejected'

This patch depends-on: https://lkml.org/lkml/2020/9/24/465

---
 drivers/base/power/domain.c | 7 +++++--
 include/linux/pm_domain.h   | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/base/power/domain.c +5 -2
index f001ac6326fb..dbe89454f594 100644
@@ -564,6 +564,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 
 	genpd->status = GENPD_STATE_OFF;
 	genpd_update_accounting(genpd);
+	genpd->states[genpd->state_idx].usage++;
 
 	list_for_each_entry(link, &genpd->child_links, child_node) {
 		genpd_sd_counter_dec(link->parent);

@@ -574,6 +575,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 
 	return 0;
 busy:
+	genpd->states[genpd->state_idx].rejected++;
 	if (nr_calls)
 		__raw_notifier_call_chain(&genpd->power_notifiers,
 					  GENPD_STATE_ON, NULL,

@@ -3053,7 +3055,7 @@ static int idle_states_show(struct seq_file *s, void *data)
 	if (ret)
 		return -ERESTARTSYS;
 
-	seq_puts(s, "State          Time Spent(ms)\n");
+	seq_puts(s, "State          Time Spent(ms) Usage          Rejected\n");
 
 	for (i = 0; i < genpd->state_count; i++) {
 		ktime_t delta = 0;

@@ -3065,7 +3067,8 @@ static int idle_states_show(struct seq_file *s, void *data)
 
 		msecs = ktime_to_ms(
 			ktime_add(genpd->states[i].idle_time, delta));
-		seq_printf(s, "S%-13i %lld\n", i, msecs);
+		seq_printf(s, "S%-13i %-14lld %-14llu %llu\n", i, msecs,
+			      genpd->states[i].usage, genpd->states[i].rejected);
 	}
 
 	genpd_unlock(genpd);
include/linux/pm_domain.h +2 -0
index 3b2b561ce846..239647f2d27f 100644
@@ -82,6 +82,8 @@ struct genpd_power_state {
 	s64 power_off_latency_ns;
 	s64 power_on_latency_ns;
 	s64 residency_ns;
+	u64 usage;
+	u64 rejected;
 	struct fwnode_handle *fwnode;
 	ktime_t idle_time;
 	void *data;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#1 lore raw
Quoted 23 lines · Lina Iyer

To enable better debug of PM domains, let's keep a track of the success and rejections in entering each domain idle state.

This statistics is exported in debugfs when reading the idle_states node, associated with each PM domain.

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

---
Changes in v2:
- Renamed 'failed' to 'rejected'

This patch depends-on: https://lkml.org/lkml/2020/9/24/465

---
 drivers/base/power/domain.c | 7 +++++--
 include/linux/pm_domain.h   | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/base/power/domain.c +0 -0
index f001ac6326fb..dbe89454f594 100644

drivers/base/power/domain.c static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, · line 567

Quoted 4 lines
drivers/base/power/domain.c +1 -0
@@ -564,6 +564,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 
        genpd->status = GENPD_STATE_OFF;
        genpd_update_accounting(genpd);
+       genpd->states[genpd->state_idx].usage++;

Why not to do this in genpd_update_accounting()?

drivers/base/power/domain.c static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, · line 581

Quoted 11 lines
drivers/base/power/domain.c +1 -0
        list_for_each_entry(link, &genpd->child_links, child_node) {
                genpd_sd_counter_dec(link->parent);

@@ -574,6 +575,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 
        return 0;
 busy:
+       genpd->states[genpd->state_idx].rejected++;
        if (nr_calls)
                __raw_notifier_call_chain(&genpd->power_notifiers,
                                          GENPD_STATE_ON, NULL,

This doesn't apply to the current code, please rebase.

Quoted 36 lines
drivers/base/power/domain.c +3 -2
@@ -3053,7 +3055,7 @@ static int idle_states_show(struct seq_file *s, void *data)
        if (ret)
                return -ERESTARTSYS;
 
-       seq_puts(s, "State          Time Spent(ms)\n");
+       seq_puts(s, "State          Time Spent(ms) Usage          Rejected\n");
 
        for (i = 0; i < genpd->state_count; i++) {
                ktime_t delta = 0;

@@ -3065,7 +3067,8 @@ static int idle_states_show(struct seq_file *s, void *data)
 
                msecs = ktime_to_ms(
                        ktime_add(genpd->states[i].idle_time, delta));
-               seq_printf(s, "S%-13i %lld\n", i, msecs);
+               seq_printf(s, "S%-13i %-14lld %-14llu %llu\n", i, msecs,
+                             genpd->states[i].usage, genpd->states[i].rejected);
        }
 
        genpd_unlock(genpd);
include/linux/pm_domain.h +2 -1
index 3b2b561ce846..239647f2d27f 100644
@@ -82,6 +82,8 @@ struct genpd_power_state {
        s64 power_off_latency_ns;
        s64 power_on_latency_ns;
        s64 residency_ns;
+       u64 usage;
+       u64 rejected;
        struct fwnode_handle *fwnode;
        ktime_t idle_time;
        void *data;
--

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

author
#2 lore raw
Quoted 25 lines · Rafael J. Wysocki
Quoted 24 lines · Lina Iyer

To enable better debug of PM domains, let's keep a track of the success and rejections in entering each domain idle state.

This statistics is exported in debugfs when reading the idle_states node, associated with each PM domain.

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

---
Changes in v2:
- Renamed 'failed' to 'rejected'

This patch depends-on: https://lkml.org/lkml/2020/9/24/465

---
 drivers/base/power/domain.c | 7 +++++--
 include/linux/pm_domain.h   | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/base/power/domain.c +0 -0
index f001ac6326fb..dbe89454f594 100644
@@ -564,6 +564,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
Quoted 6 lines

drivers/base/power/domain.c static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, · line 567

Quoted 3 lines
drivers/base/power/domain.c +1 -0
        genpd->status = GENPD_STATE_OFF;
        genpd_update_accounting(genpd);
+       genpd->states[genpd->state_idx].usage++;

Why not to do this in genpd_update_accounting()?

That function is clubbed with debugfs and does heavy tracking using timers. This accounting is fairly basic and still quite useful for debugging.

Quoted 5 lines
Quoted 5 lines
drivers/base/power/domain.c +0 -0
        list_for_each_entry(link, &genpd->child_links, child_node) {
                genpd_sd_counter_dec(link->parent);

@@ -574,6 +575,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
Quoted 9 lines

drivers/base/power/domain.c static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, · line 581

Quoted 6 lines
drivers/base/power/domain.c +1 -0
        return 0;
 busy:
+       genpd->states[genpd->state_idx].rejected++;
        if (nr_calls)
                __raw_notifier_call_chain(&genpd->power_notifiers,
                                          GENPD_STATE_ON, NULL,

This doesn't apply to the current code, please rebase.

I believe it applies cleanly on top of
https://lkml.org/lkml/2020/9/24/465, which I believe you are applied to your tree. Let me rebase and re-post. Sorry about that.

Thanks,
Lina

Quoted 36 lines
Quoted 36 lines
drivers/base/power/domain.c +3 -2
@@ -3053,7 +3055,7 @@ static int idle_states_show(struct seq_file *s, void *data)
        if (ret)
                return -ERESTARTSYS;
 
-       seq_puts(s, "State          Time Spent(ms)\n");
+       seq_puts(s, "State          Time Spent(ms) Usage          Rejected\n");
 
        for (i = 0; i < genpd->state_count; i++) {
                ktime_t delta = 0;

@@ -3065,7 +3067,8 @@ static int idle_states_show(struct seq_file *s, void *data)
 
                msecs = ktime_to_ms(
                        ktime_add(genpd->states[i].idle_time, delta));
-               seq_printf(s, "S%-13i %lld\n", i, msecs);
+               seq_printf(s, "S%-13i %-14lld %-14llu %llu\n", i, msecs,
+                             genpd->states[i].usage, genpd->states[i].rejected);
        }
 
        genpd_unlock(genpd);
include/linux/pm_domain.h +2 -1
index 3b2b561ce846..239647f2d27f 100644
@@ -82,6 +82,8 @@ struct genpd_power_state {
        s64 power_off_latency_ns;
        s64 power_on_latency_ns;
        s64 residency_ns;
+       u64 usage;
+       u64 rejected;
        struct fwnode_handle *fwnode;
        ktime_t idle_time;
        void *data;
--

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