[PATCH v2] pmdomain: core: Add tracepoints for domain state changes

[PATCH v2] pmdomain: core: Add tracepoints for domain state changes

[Abel Vesa]

Power domain state changes are difficult to correlate with device activity
using snapshots of the genpd debugfs state alone.

Add tracepoints for domain initialization, power-on and power-off
operations, and performance-state changes. Record the domain name and
initial power and performance states at initialization. For power
operations, record the idle-state index and return value, and for
performance-state changes, record the requested state and return value.

Emit the operation tracepoints on both success and failure paths,
including power notifier failures and performance-state rollback paths,
to make failed transitions visible alongside successful ones.

Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
Prepared with help from an LLM-based assistant.

Changes in v2:
- Initialize ret to 0 at declaration instead of assigning it before the
  trace labels in _genpd_set_performance_state(), _genpd_power_on() and
  _genpd_power_off() (Ulf)
- Link to v1: https://patch.msgid.link/20260924-add-pmdomain-tracepoints-v1-1-d89f3f0a6998@oss.qualcomm.com
---
 drivers/pmdomain/core.c      | 30 +++++++++-----
 include/trace/events/genpd.h | 94 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+), 10 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index b99c2dcf3d8f..58c6fd8ef52f 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -28,6 +28,9 @@
 
 #include <trace/events/ipi.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/genpd.h>
+
 /* Provides a unique ID for each genpd device */
 static DEFINE_IDA(genpd_ida);
 
@@ -466,7 +469,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 					unsigned int state, int depth)
 {
 	struct gpd_link *link = NULL;
-	int ret;
+	int ret = 0;
 
 	if (state == genpd->performance_state)
 		return 0;
@@ -485,7 +488,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 		if (ret) {
 			if (link)
 				goto rollback_parents_up;
-			return ret;
+			goto out;
 		}
 	}
 
@@ -499,15 +502,17 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 	}
 
 	genpd->performance_state = state;
-	return 0;
+	goto out;
 
 rollback_parents_up:
 	list_for_each_entry_continue_reverse(link, &genpd->child_links, child_node)
 		_genpd_rollback_parent_state(link, depth);
-	return ret;
+	goto out;
 rollback_parents_down:
 	list_for_each_entry_continue(link, &genpd->child_links, child_node)
 		_genpd_rollback_parent_state(link, depth);
+out:
+	trace_genpd_set_performance_state(dev_name(&genpd->dev), state, ret);
 	return ret;
 }
 
@@ -834,7 +839,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 	unsigned int state_idx = genpd->state_idx;
 	ktime_t time_start;
 	s64 elapsed_ns;
-	int ret;
+	int ret = 0;
 
 	/* Notify consumers that we are about to power on. */
 	ret = raw_notifier_call_chain_robust(&genpd->power_notifiers,
@@ -842,7 +847,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 					     GENPD_NOTIFY_OFF, NULL);
 	ret = notifier_to_errno(ret);
 	if (ret)
-		return ret;
+		goto trace;
 
 	if (!genpd->power_on)
 		goto out;
@@ -873,10 +878,12 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 out:
 	raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL);
 	genpd->synced_poweroff = false;
-	return 0;
+	goto trace;
 err:
 	raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF,
 				NULL);
+trace:
+	trace_genpd_power_on(dev_name(&genpd->dev), state_idx, ret);
 	return ret;
 }
 
@@ -885,7 +892,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
 	unsigned int state_idx = genpd->state_idx;
 	ktime_t time_start;
 	s64 elapsed_ns;
-	int ret;
+	int ret = 0;
 
 	/* Notify consumers that we are about to power off. */
 	ret = raw_notifier_call_chain_robust(&genpd->power_notifiers,
@@ -893,7 +900,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
 					     GENPD_NOTIFY_ON, NULL);
 	ret = notifier_to_errno(ret);
 	if (ret)
-		return ret;
+		goto trace;
 
 	if (!genpd->power_off)
 		goto out;
@@ -924,9 +931,11 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
 out:
 	raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF,
 				NULL);
-	return 0;
+	goto trace;
 busy:
 	raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL);
+trace:
+	trace_genpd_power_off(dev_name(&genpd->dev), state_idx, ret);
 	return ret;
 }
 
@@ -2529,6 +2538,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 		return ret;
 
 	mutex_lock(&gpd_list_lock);
+	trace_genpd_init(dev_name(&genpd->dev), !is_off, genpd->performance_state);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);
 	genpd_debug_add(genpd);
diff --git a/include/trace/events/genpd.h b/include/trace/events/genpd.h
new file mode 100644
index 000000000000..e2970be0796a
--- /dev/null
+++ b/include/trace/events/genpd.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM genpd
+
+#if !defined(_TRACE_GENPD_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_GENPD_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(genpd_init,
+
+	TP_PROTO(const char *name, bool on, unsigned int performance_state),
+
+	TP_ARGS(name, on, performance_state),
+
+	TP_STRUCT__entry(
+		__string(name, name)
+		__field(bool, on)
+		__field(unsigned int, performance_state)
+	),
+
+	TP_fast_assign(
+		__assign_str(name);
+		__entry->on = on;
+		__entry->performance_state = performance_state;
+	),
+
+	TP_printk("%s on=%d performance_state=%u", __get_str(name),
+		  __entry->on, __entry->performance_state)
+);
+
+/* state_idx identifies the idle state entered or exited. */
+DECLARE_EVENT_CLASS(genpd_power,
+
+	TP_PROTO(const char *name, unsigned int state_idx, int ret),
+
+	TP_ARGS(name, state_idx, ret),
+
+	TP_STRUCT__entry(
+		__string(name, name)
+		__field(unsigned int, state_idx)
+		__field(int, ret)
+	),
+
+	TP_fast_assign(
+		__assign_str(name);
+		__entry->state_idx = state_idx;
+		__entry->ret = ret;
+	),
+
+	TP_printk("%s state_idx=%u ret=%d", __get_str(name),
+		  __entry->state_idx, __entry->ret)
+);
+
+DEFINE_EVENT(genpd_power, genpd_power_on,
+
+	TP_PROTO(const char *name, unsigned int state_idx, int ret),
+
+	TP_ARGS(name, state_idx, ret)
+);
+
+DEFINE_EVENT(genpd_power, genpd_power_off,
+
+	TP_PROTO(const char *name, unsigned int state_idx, int ret),
+
+	TP_ARGS(name, state_idx, ret)
+);
+
+TRACE_EVENT(genpd_set_performance_state,
+
+	TP_PROTO(const char *name, unsigned int state, int ret),
+
+	TP_ARGS(name, state, ret),
+
+	TP_STRUCT__entry(
+		__string(name, name)
+		__field(unsigned int, state)
+		__field(int, ret)
+	),
+
+	TP_fast_assign(
+		__assign_str(name);
+		__entry->state = state;
+		__entry->ret = ret;
+	),
+
+	TP_printk("%s state=%u ret=%d", __get_str(name),
+		  __entry->state, __entry->ret)
+);
+
+#endif /* _TRACE_GENPD_H */
+
+/* This part must be outside protection. */
+#include <trace/define_trace.h>

---
base-commit: 3d7783543c2646af69ad65825e810060494bea21
change-id: 20260924-add-pmdomain-tracepoints-15254f2cd40f

Best regards,
--  

Abel

Re: [PATCH v2] pmdomain: core: Add tracepoints for domain state changes

[Steven Rostedt]

On Thu, 05 Nov 2026 18:18:49 +0200
Abel Vesa <abel.vesa@oss.qualcomm.com> wrote:

> diff --git a/include/trace/events/genpd.h b/include/trace/events/genpd.h
> new file mode 100644
> index 000000000000..e2970be0796a
> --- /dev/null
> +++ b/include/trace/events/genpd.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM genpd
> +
> +#if !defined(_TRACE_GENPD_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_GENPD_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(genpd_init,
> +
> +	TP_PROTO(const char *name, bool on, unsigned int performance_state),
> +
> +	TP_ARGS(name, on, performance_state),
> +
> +	TP_STRUCT__entry(
> +		__string(name, name)
> +		__field(bool, on)
> +		__field(unsigned int, performance_state)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(name);
> +		__entry->on = on;
> +		__entry->performance_state = performance_state;
> +	),
> +
> +	TP_printk("%s on=%d performance_state=%u", __get_str(name),
> +		  __entry->on, __entry->performance_state)
> +);
> +
> +/* state_idx identifies the idle state entered or exited. */
> +DECLARE_EVENT_CLASS(genpd_power,
> +
> +	TP_PROTO(const char *name, unsigned int state_idx, int ret),
> +
> +	TP_ARGS(name, state_idx, ret),
> +
> +	TP_STRUCT__entry(
> +		__string(name, name)
> +		__field(unsigned int, state_idx)
> +		__field(int, ret)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(name);
> +		__entry->state_idx = state_idx;
> +		__entry->ret = ret;
> +	),
> +
> +	TP_printk("%s state_idx=%u ret=%d", __get_str(name),
> +		  __entry->state_idx, __entry->ret)
> +);
> +
> +DEFINE_EVENT(genpd_power, genpd_power_on,
> +
> +	TP_PROTO(const char *name, unsigned int state_idx, int ret),
> +
> +	TP_ARGS(name, state_idx, ret)
> +);
> +
> +DEFINE_EVENT(genpd_power, genpd_power_off,
> +
> +	TP_PROTO(const char *name, unsigned int state_idx, int ret),
> +
> +	TP_ARGS(name, state_idx, ret)
> +);
> +
> +TRACE_EVENT(genpd_set_performance_state,
> +
> +	TP_PROTO(const char *name, unsigned int state, int ret),
> +
> +	TP_ARGS(name, state, ret),
> +
> +	TP_STRUCT__entry(
> +		__string(name, name)
> +		__field(unsigned int, state)
> +		__field(int, ret)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(name);
> +		__entry->state = state;
> +		__entry->ret = ret;
> +	),
> +
> +	TP_printk("%s state=%u ret=%d", __get_str(name),
> +		  __entry->state, __entry->ret)

This is pretty much the same as the above trace events except that it
has state instead of state_idx. If you really need this to say "state"
instead of "state_idx", you can use:

DEFINE_EVENT_PRINT(genpd_power, genpd_set_performance_state,
	TP_PROTO(const char *name, unsigned int state_idx, int ret),
	TP_ARGS(name, state_idx, ret),
	TP_printk("%s state=%u ret=%d", __get_str(name),
		  __entry->state, __entry->ret)
);

Which will make that trace event the same as the above except that it
will print "state" instead of "state_idx", and save some memory.

-- Steve



> +);
> +
> +#endif /* _TRACE_GENPD_H */
> +
> +/* This part must be outside protection. */
> +#include <trace/define_trace.h>
> 
> ---
> base-commit: 3d7783543c2646af69ad65825e810060494bea21
> change-id: 20260924-add-pmdomain-tracepoints-15254f2cd40f
> 
> Best regards,
> --  
> 
> Abel