linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Add power/gpu_frequency tracepoint.
@ 2020-08-13 21:03 Peiyong Lin
  2020-08-13 21:22 ` Steven Rostedt
       [not found] ` <159741850487.10342.14268227307882225260@build.alporthouse.com>
  0 siblings, 2 replies; 21+ messages in thread
From: Peiyong Lin @ 2020-08-13 21:03 UTC (permalink / raw)
  To: Steven Rostedt, Ingo Molnar, Greg Kroah-Hartman, Masahiro Yamada,
	Paul Walmsley, Rafael J. Wysocki, Amit Kucheria, Ulf Hansson,
	Pavel Machek, linux-kernel, Peiyong Lin
  Cc: prahladk, android-kernel

Historically there is no common trace event for GPU frequency, in
downstream Android each different hardware vendor implements their own
way to expose GPU frequency, for example as a debugfs node.  This patch
standardize it as a common trace event in upstream linux kernel to help
the ecosystem have a common implementation across hardware vendors.
Toolings in the Linux ecosystem will benefit from this especially in the
downstream Android, where this information is critical to graphics
developers.

Signed-off-by: Peiyong Lin <lpy@google.com>
---
 drivers/gpu/Makefile                    |  1 +
 drivers/gpu/trace/Kconfig               |  3 +++
 drivers/gpu/trace/Makefile              |  1 +
 drivers/gpu/trace/trace_gpu_frequency.c | 13 +++++++++++++
 include/trace/events/power.h            | 26 +++++++++++++++++++++++++
 5 files changed, 44 insertions(+)
 create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..f289a47eb031 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)	+= host1x/
 obj-y			+= drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_TRACE_GPU_FREQUENCY)		+= trace/
diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
index c24e9edd022e..ac4aec8d5845 100644
--- a/drivers/gpu/trace/Kconfig
+++ b/drivers/gpu/trace/Kconfig
@@ -2,3 +2,6 @@
 
 config TRACE_GPU_MEM
 	bool
+
+config TRACE_GPU_FREQUENCY
+	bool
diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
index b70fbdc5847f..2b7ae69327d6 100644
--- a/drivers/gpu/trace/Makefile
+++ b/drivers/gpu/trace/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
+obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
new file mode 100644
index 000000000000..f5af5800b52d
--- /dev/null
+++ b/drivers/gpu/trace/trace_gpu_frequency.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPU frequency trace points
+ *
+ * Copyright (C) 2020 Google, Inc.
+ */
+
+#include <linux/module.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/power.h>
+
+EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index af5018aa9517..befc0157131e 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -500,6 +500,32 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
 
 	TP_ARGS(name, type, new_value)
 );
+
+/**
+ * gpu_frequency - Reports frequency changes in GPU clock domains
+ * @state:  New frequency (in KHz)
+ * @gpu_id: GPU clock domain
+ */
+TRACE_EVENT(gpu_frequency,
+
+	TP_PROTO(unsigned int state, unsigned int gpu_id),
+
+	TP_ARGS(state, gpu_id),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, state)
+		__field(unsigned int, gpu_id)
+	),
+
+	TP_fast_assign(
+		__entry->state = state;
+		__entry->gpu_id = gpu_id;
+	),
+
+	TP_printk("state=%lu gpu_id=%lu",
+		(unsigned long)__entry->state,
+		(unsigned long)__entry->gpu_id)
+);
 #endif /* _TRACE_POWER_H */
 
 /* This part must be outside protection */
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add power/gpu_frequency tracepoint.
  2020-08-13 21:03 [PATCH] Add power/gpu_frequency tracepoint Peiyong Lin
@ 2020-08-13 21:22 ` Steven Rostedt
  2020-08-13 21:37   ` [PATCH v2] " Peiyong Lin
       [not found] ` <159741850487.10342.14268227307882225260@build.alporthouse.com>
  1 sibling, 1 reply; 21+ messages in thread
From: Steven Rostedt @ 2020-08-13 21:22 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: Ingo Molnar, Greg Kroah-Hartman, Masahiro Yamada, Paul Walmsley,
	Rafael J. Wysocki, Amit Kucheria, Ulf Hansson, Pavel Machek,
	linux-kernel, prahladk, android-kernel

On Thu, 13 Aug 2020 14:03:57 -0700
Peiyong Lin <lpy@google.com> wrote:

> +/**
> + * gpu_frequency - Reports frequency changes in GPU clock domains
> + * @state:  New frequency (in KHz)
> + * @gpu_id: GPU clock domain
> + */
> +TRACE_EVENT(gpu_frequency,
> +
> +	TP_PROTO(unsigned int state, unsigned int gpu_id),
> +
> +	TP_ARGS(state, gpu_id),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned int, state)
> +		__field(unsigned int, gpu_id)

Both of the above entries are unsigned int.

> +	),
> +
> +	TP_fast_assign(
> +		__entry->state = state;
> +		__entry->gpu_id = gpu_id;
> +	),
> +
> +	TP_printk("state=%lu gpu_id=%lu",
> +		(unsigned long)__entry->state,
> +		(unsigned long)__entry->gpu_id)

Why typecast to (unsigned long) to use it in %lu? Why not just have:

	TP_printk("state=%u gpu_id=%u",
		__entry->state, __entry->gpu_id)

?

-- Steve


> +);
>  #endif /* _TRACE_POWER_H */
>  
>  /* This part must be outside protection */


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2] Add power/gpu_frequency tracepoint.
  2020-08-13 21:22 ` Steven Rostedt
@ 2020-08-13 21:37   ` Peiyong Lin
  2020-08-13 21:50     ` Steven Rostedt
  0 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-08-13 21:37 UTC (permalink / raw)
  To: rostedt
  Cc: amit.kucheria, android-kernel, gregkh, linux-kernel, lpy, mingo,
	paul.walmsley, pavel, prahladk, rafael.j.wysocki, ulf.hansson,
	yamada.masahiro

Historically there is no common trace event for GPU frequency, in
downstream Android each different hardware vendor implements their own
way to expose GPU frequency, for example as a debugfs node.  This patch
standardize it as a common trace event in upstream linux kernel to help
the ecosystem have a common implementation across hardware vendors.
Toolings in the Linux ecosystem will benefit from this especially in the
downstream Android, where this information is critical to graphics
developers.

Signed-off-by: Peiyong Lin <lpy@google.com>
---

Changelog since v1:
 - Use %u in TP_printk

 drivers/gpu/Makefile                    |  1 +
 drivers/gpu/trace/Kconfig               |  3 +++
 drivers/gpu/trace/Makefile              |  1 +
 drivers/gpu/trace/trace_gpu_frequency.c | 13 +++++++++++++
 include/trace/events/power.h            | 25 +++++++++++++++++++++++++
 5 files changed, 43 insertions(+)
 create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..f289a47eb031 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)	+= host1x/
 obj-y			+= drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_TRACE_GPU_FREQUENCY)		+= trace/
diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
index c24e9edd022e..ac4aec8d5845 100644
--- a/drivers/gpu/trace/Kconfig
+++ b/drivers/gpu/trace/Kconfig
@@ -2,3 +2,6 @@
 
 config TRACE_GPU_MEM
 	bool
+
+config TRACE_GPU_FREQUENCY
+	bool
diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
index b70fbdc5847f..2b7ae69327d6 100644
--- a/drivers/gpu/trace/Makefile
+++ b/drivers/gpu/trace/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
+obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
new file mode 100644
index 000000000000..f5af5800b52d
--- /dev/null
+++ b/drivers/gpu/trace/trace_gpu_frequency.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPU frequency trace points
+ *
+ * Copyright (C) 2020 Google, Inc.
+ */
+
+#include <linux/module.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/power.h>
+
+EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index af5018aa9517..c81dd2157567 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -500,6 +500,31 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
 
 	TP_ARGS(name, type, new_value)
 );
+
+/**
+ * gpu_frequency - Reports frequency changes in GPU clock domains
+ * @state:  New frequency (in KHz)
+ * @gpu_id: GPU clock domain
+ */
+TRACE_EVENT(gpu_frequency,
+
+	TP_PROTO(unsigned int state, unsigned int gpu_id),
+
+	TP_ARGS(state, gpu_id),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, state)
+		__field(unsigned int, gpu_id)
+	),
+
+	TP_fast_assign(
+		__entry->state = state;
+		__entry->gpu_id = gpu_id;
+	),
+
+	TP_printk("state=%u gpu_id=%u",
+		__entry->state, __entry->gpu_id)
+);
 #endif /* _TRACE_POWER_H */
 
 /* This part must be outside protection */
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] Add power/gpu_frequency tracepoint.
  2020-08-13 21:37   ` [PATCH v2] " Peiyong Lin
@ 2020-08-13 21:50     ` Steven Rostedt
  2020-08-20 19:41       ` [PATCH v3] " Peiyong Lin
  0 siblings, 1 reply; 21+ messages in thread
From: Steven Rostedt @ 2020-08-13 21:50 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: amit.kucheria, android-kernel, gregkh, linux-kernel, mingo,
	paul.walmsley, pavel, prahladk, rafael.j.wysocki, ulf.hansson,
	yamada.masahiro

On Thu, 13 Aug 2020 14:37:03 -0700
Peiyong Lin <lpy@google.com> wrote:

> Historically there is no common trace event for GPU frequency, in
> downstream Android each different hardware vendor implements their own
> way to expose GPU frequency, for example as a debugfs node.  This patch
> standardize it as a common trace event in upstream linux kernel to help
> the ecosystem have a common implementation across hardware vendors.
> Toolings in the Linux ecosystem will benefit from this especially in the
> downstream Android, where this information is critical to graphics
> developers.
> 
> Signed-off-by: Peiyong Lin <lpy@google.com>
> ---
> 
> Changelog since v1:
>  - Use %u in TP_printk
> 

For just the tracing side of this, but the use case is up for others to do:

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add power/gpu_frequency tracepoint.
       [not found] ` <159741850487.10342.14268227307882225260@build.alporthouse.com>
@ 2020-08-14 19:25   ` Peiyong Lin
  0 siblings, 0 replies; 21+ messages in thread
From: Peiyong Lin @ 2020-08-14 19:25 UTC (permalink / raw)
  To: Chris Wilson, Sidath Senanayake, zzyiwei
  Cc: Amit Kucheria, Greg Kroah-Hartman, Ingo Molnar, Masahiro Yamada,
	Paul Walmsley, Pavel Machek, Rafael J. Wysocki, Steven Rostedt,
	Ulf Hansson, linux-kernel, Prahlad Kilambi, android-kernel

Hi Chris, please see my comments inline.


On Fri, Aug 14, 2020 at 8:22 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Peiyong Lin (2020-08-13 22:03:57)
> > Historically there is no common trace event for GPU frequency, in
> > downstream Android each different hardware vendor implements their own
> > way to expose GPU frequency, for example as a debugfs node.  This patch
> > standardize it as a common trace event in upstream linux kernel to help
> > the ecosystem have a common implementation across hardware vendors.
> > Toolings in the Linux ecosystem will benefit from this especially in the
> > downstream Android, where this information is critical to graphics
> > developers.
> >
> > Signed-off-by: Peiyong Lin <lpy@google.com>
> > ---
> >  drivers/gpu/Makefile                    |  1 +
> >  drivers/gpu/trace/Kconfig               |  3 +++
> >  drivers/gpu/trace/Makefile              |  1 +
> >  drivers/gpu/trace/trace_gpu_frequency.c | 13 +++++++++++++
> >  include/trace/events/power.h            | 26 +++++++++++++++++++++++++
> >  5 files changed, 44 insertions(+)
> >  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
> >
> > diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> > index 835c88318cec..f289a47eb031 100644
> > --- a/drivers/gpu/Makefile
> > +++ b/drivers/gpu/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
> >  obj-y                  += drm/ vga/
> >  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
> >  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> > +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> > diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> > index c24e9edd022e..ac4aec8d5845 100644
> > --- a/drivers/gpu/trace/Kconfig
> > +++ b/drivers/gpu/trace/Kconfig
> > @@ -2,3 +2,6 @@
> >
> >  config TRACE_GPU_MEM
> >         bool
> > +
> > +config TRACE_GPU_FREQUENCY
> > +       bool
> > diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> > index b70fbdc5847f..2b7ae69327d6 100644
> > --- a/drivers/gpu/trace/Makefile
> > +++ b/drivers/gpu/trace/Makefile
> > @@ -1,3 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >
> >  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> > +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> > diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> > new file mode 100644
> > index 000000000000..f5af5800b52d
> > --- /dev/null
> > +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * GPU frequency trace points
> > + *
> > + * Copyright (C) 2020 Google, Inc.
> > + */
> > +
> > +#include <linux/module.h>
> > +
> > +#define CREATE_TRACE_POINTS
> > +#include <trace/events/power.h>
> > +
> > +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> > diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> > index af5018aa9517..befc0157131e 100644
> > --- a/include/trace/events/power.h
> > +++ b/include/trace/events/power.h
> > @@ -500,6 +500,32 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
> >
> >         TP_ARGS(name, type, new_value)
> >  );
> > +
> > +/**
> > + * gpu_frequency - Reports frequency changes in GPU clock domains
> > + * @state:  New frequency (in KHz)
> > + * @gpu_id: GPU clock domain
> > + */
> > +TRACE_EVENT(gpu_frequency,
> > +
> > +       TP_PROTO(unsigned int state, unsigned int gpu_id),
> > +
> > +       TP_ARGS(state, gpu_id),
> > +
> > +       TP_STRUCT__entry(
> > +               __field(unsigned int, state)
> > +               __field(unsigned int, gpu_id)
>
> What is a gpu-id and how are we supposed to create one?

gpu_id is the id for per GPU clock domain, it should be created for
per GPU clock domain. It's not necessarily tied to a physical GPU.

>
> 'state' is quite non-descript, and since this is not an event template
> you could be a little more specific.
>
> So when should this tracepoint fire? For the frequency change we request,
> or the frequency change of the black box of the pcu?

If an implementation implements this, the GPU frequency tracepoint
should be fired whenever there's a frequency change. That is to say,
when gpu goes from idle to active or vice versa, or when the gpu
frequency goes up and down.

>
> We have found that a tracepoint is not a useful monitor of the actual GPU
> frequencies, for that we use provide sampling via perf-events.
> -Chris

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3] Add power/gpu_frequency tracepoint.
  2020-08-13 21:50     ` Steven Rostedt
@ 2020-08-20 19:41       ` Peiyong Lin
  2020-08-20 20:27         ` Steven Rostedt
  0 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-08-20 19:41 UTC (permalink / raw)
  To: rostedt
  Cc: amit.kucheria, android-kernel, gregkh, linux-kernel, lpy, mingo,
	paul.walmsley, pavel, prahladk, rafael.j.wysocki, ulf.hansson,
	yamada.masahiro, zzyiwei, sidaths

Historically there is no common trace event for GPU frequency, in
downstream Android each different hardware vendor implements their own
way to expose GPU frequency, for example as a debugfs node.  This patch
standardize it as a common trace event in upstream linux kernel to help
the ecosystem have a common implementation across hardware vendors.
Toolings in the Linux ecosystem will benefit from this especially in the
downstream Android, where this information is critical to graphics
developers.

Signed-off-by: Peiyong Lin <lpy@google.com>
---

Changelog sice v2:
 - Add more comments to indicate when the event should be emitted.
 - Change state to frequency.

Changelog since v1:
 - Use %u in TP_printk

 drivers/gpu/Makefile                    |  1 +
 drivers/gpu/trace/Kconfig               |  3 +++
 drivers/gpu/trace/Makefile              |  1 +
 drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
 include/trace/events/power.h            | 33 +++++++++++++++++++++++++
 5 files changed, 51 insertions(+)
 create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..f289a47eb031 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)	+= host1x/
 obj-y			+= drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_TRACE_GPU_FREQUENCY)		+= trace/
diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
index c24e9edd022e..ac4aec8d5845 100644
--- a/drivers/gpu/trace/Kconfig
+++ b/drivers/gpu/trace/Kconfig
@@ -2,3 +2,6 @@
 
 config TRACE_GPU_MEM
 	bool
+
+config TRACE_GPU_FREQUENCY
+	bool
diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
index b70fbdc5847f..2b7ae69327d6 100644
--- a/drivers/gpu/trace/Makefile
+++ b/drivers/gpu/trace/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
+obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
new file mode 100644
index 000000000000..f5af5800b52d
--- /dev/null
+++ b/drivers/gpu/trace/trace_gpu_frequency.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPU frequency trace points
+ *
+ * Copyright (C) 2020 Google, Inc.
+ */
+
+#include <linux/module.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/power.h>
+
+EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index af5018aa9517..343825a76953 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
 
 	TP_ARGS(name, type, new_value)
 );
+
+/**
+ * gpu_frequency - Reports the GPU frequency in GPU clock domains.
+ *
+ * This event should be emitted whenever there's a GPU frequency change happens,
+ * or a GPU goes from idle state to active state, or vice versa.
+ *
+ * When the GPU goes from idle state to active state, this event should report
+ * the GPU frequency of the active state. When the GPU goes from active state to
+ * idle state, this event should report a zero frequency value.
+ *
+ * @frequency:  New frequency (in KHz)
+ * @gpu_id: Id for each GPU clock domain
+ */
+TRACE_EVENT(gpu_frequency,
+
+	TP_PROTO(unsigned int frequency, unsigned int gpu_id),
+
+	TP_ARGS(frequency, gpu_id),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, frequency)
+		__field(unsigned int, gpu_id)
+	),
+
+	TP_fast_assign(
+		__entry->frequency = frequency;
+		__entry->gpu_id = gpu_id;
+	),
+
+	TP_printk("frequency=%u gpu_id=%u",
+		__entry->frequency, __entry->gpu_id)
+);
 #endif /* _TRACE_POWER_H */
 
 /* This part must be outside protection */
-- 
2.28.0.297.g1956fa8f8d-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3] Add power/gpu_frequency tracepoint.
  2020-08-20 19:41       ` [PATCH v3] " Peiyong Lin
@ 2020-08-20 20:27         ` Steven Rostedt
  2020-10-22 16:59           ` [PATCH] " Peiyong Lin
  2020-10-22 17:34           ` [PATCH v4] " Peiyong Lin
  0 siblings, 2 replies; 21+ messages in thread
From: Steven Rostedt @ 2020-08-20 20:27 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: amit.kucheria, android-kernel, gregkh, linux-kernel, mingo,
	paul.walmsley, pavel, prahladk, rafael.j.wysocki, ulf.hansson,
	yamada.masahiro, zzyiwei, sidaths

On Thu, 20 Aug 2020 12:41:34 -0700
Peiyong Lin <lpy@google.com> wrote:

> Historically there is no common trace event for GPU frequency, in
> downstream Android each different hardware vendor implements their own
> way to expose GPU frequency, for example as a debugfs node.  This patch
> standardize it as a common trace event in upstream linux kernel to help
> the ecosystem have a common implementation across hardware vendors.
> Toolings in the Linux ecosystem will benefit from this especially in the
> downstream Android, where this information is critical to graphics
> developers.
> 
> Signed-off-by: Peiyong Lin <lpy@google.com>

Just from the tracing point of view (not from the content of the trace
point of view).

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] Add power/gpu_frequency tracepoint.
  2020-08-20 20:27         ` Steven Rostedt
@ 2020-10-22 16:59           ` Peiyong Lin
  2020-10-22 17:34           ` [PATCH v4] " Peiyong Lin
  1 sibling, 0 replies; 21+ messages in thread
From: Peiyong Lin @ 2020-10-22 16:59 UTC (permalink / raw)
  To: rostedt
  Cc: amit.kucheria, android-kernel, gregkh, linux-kernel, lpy, mingo,
	paul.walmsley, pavel, prahladk, rafael.j.wysocki, ulf.hansson,
	yamada.masahiro, zzyiwei, sidaths

Historically there is no common trace event for GPU frequency, in
downstream Android each different hardware vendor implements their own
way to expose GPU frequency, for example as a debugfs node.  This patch
standardize it as a common trace event in upstream linux kernel to help
the ecosystem have a common implementation across hardware vendors.
Toolings in the Linux ecosystem will benefit from this especially in the
downstream Android, where this information is critical to graphics
developers.

Signed-off-by: Peiyong Lin <lpy@google.com>
---

Changelog since v3:
 - Correct copyright title.

Changelog since v2:
 - Add more comments to indicate when the event should be emitted.
 - Change state to frequency.

Changelog since v1:
 - Use %u in TP_printk

 drivers/gpu/Makefile                    |  1 +
 drivers/gpu/trace/Kconfig               |  3 +++
 drivers/gpu/trace/Makefile              |  1 +
 drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
 include/trace/events/power.h            | 33 +++++++++++++++++++++++++
 5 files changed, 51 insertions(+)
 create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..f289a47eb031 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)	+= host1x/
 obj-y			+= drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_TRACE_GPU_FREQUENCY)		+= trace/
diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
index c24e9edd022e..ac4aec8d5845 100644
--- a/drivers/gpu/trace/Kconfig
+++ b/drivers/gpu/trace/Kconfig
@@ -2,3 +2,6 @@
 
 config TRACE_GPU_MEM
 	bool
+
+config TRACE_GPU_FREQUENCY
+	bool
diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
index b70fbdc5847f..2b7ae69327d6 100644
--- a/drivers/gpu/trace/Makefile
+++ b/drivers/gpu/trace/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
+obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
new file mode 100644
index 000000000000..668fabd6b77a
--- /dev/null
+++ b/drivers/gpu/trace/trace_gpu_frequency.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPU frequency trace points
+ *
+ * Copyright (C) 2020 Google LLC
+ */
+
+#include <linux/module.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/power.h>
+
+EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index af5018aa9517..343825a76953 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
 
 	TP_ARGS(name, type, new_value)
 );
+
+/**
+ * gpu_frequency - Reports the GPU frequency in GPU clock domains.
+ *
+ * This event should be emitted whenever there's a GPU frequency change happens,
+ * or a GPU goes from idle state to active state, or vice versa.
+ *
+ * When the GPU goes from idle state to active state, this event should report
+ * the GPU frequency of the active state. When the GPU goes from active state to
+ * idle state, this event should report a zero frequency value.
+ *
+ * @frequency:  New frequency (in KHz)
+ * @gpu_id: Id for each GPU clock domain
+ */
+TRACE_EVENT(gpu_frequency,
+
+	TP_PROTO(unsigned int frequency, unsigned int gpu_id),
+
+	TP_ARGS(frequency, gpu_id),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, frequency)
+		__field(unsigned int, gpu_id)
+	),
+
+	TP_fast_assign(
+		__entry->frequency = frequency;
+		__entry->gpu_id = gpu_id;
+	),
+
+	TP_printk("frequency=%u gpu_id=%u",
+		__entry->frequency, __entry->gpu_id)
+);
 #endif /* _TRACE_POWER_H */
 
 /* This part must be outside protection */
-- 
2.29.0.rc1.297.gfa9743e501-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-08-20 20:27         ` Steven Rostedt
  2020-10-22 16:59           ` [PATCH] " Peiyong Lin
@ 2020-10-22 17:34           ` Peiyong Lin
  2020-10-26 22:25             ` Peiyong Lin
                               ` (2 more replies)
  1 sibling, 3 replies; 21+ messages in thread
From: Peiyong Lin @ 2020-10-22 17:34 UTC (permalink / raw)
  To: rostedt
  Cc: amit.kucheria, android-kernel, gregkh, linux-kernel, lpy, mingo,
	paul.walmsley, pavel, prahladk, rafael.j.wysocki, ulf.hansson,
	yamada.masahiro, zzyiwei, sidaths

Historically there is no common trace event for GPU frequency, in
downstream Android each different hardware vendor implements their own
way to expose GPU frequency, for example as a debugfs node.  This patch
standardize it as a common trace event in upstream linux kernel to help
the ecosystem have a common implementation across hardware vendors.
Toolings in the Linux ecosystem will benefit from this especially in the
downstream Android, where this information is critical to graphics
developers.

Signed-off-by: Peiyong Lin <lpy@google.com>
---

Changelog since v3:
 - Correct copyright title.

Changelog since v2:
 - Add more comments to indicate when the event should be emitted.
 - Change state to frequency.

Changelog since v1:
 - Use %u in TP_printk

 drivers/gpu/Makefile                    |  1 +
 drivers/gpu/trace/Kconfig               |  3 +++
 drivers/gpu/trace/Makefile              |  1 +
 drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
 include/trace/events/power.h            | 33 +++++++++++++++++++++++++
 5 files changed, 51 insertions(+)
 create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..f289a47eb031 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)	+= host1x/
 obj-y			+= drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_TRACE_GPU_FREQUENCY)		+= trace/
diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
index c24e9edd022e..ac4aec8d5845 100644
--- a/drivers/gpu/trace/Kconfig
+++ b/drivers/gpu/trace/Kconfig
@@ -2,3 +2,6 @@
 
 config TRACE_GPU_MEM
 	bool
+
+config TRACE_GPU_FREQUENCY
+	bool
diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
index b70fbdc5847f..2b7ae69327d6 100644
--- a/drivers/gpu/trace/Makefile
+++ b/drivers/gpu/trace/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
+obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
new file mode 100644
index 000000000000..668fabd6b77a
--- /dev/null
+++ b/drivers/gpu/trace/trace_gpu_frequency.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPU frequency trace points
+ *
+ * Copyright (C) 2020 Google LLC
+ */
+
+#include <linux/module.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/power.h>
+
+EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index af5018aa9517..343825a76953 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
 
 	TP_ARGS(name, type, new_value)
 );
+
+/**
+ * gpu_frequency - Reports the GPU frequency in GPU clock domains.
+ *
+ * This event should be emitted whenever there's a GPU frequency change happens,
+ * or a GPU goes from idle state to active state, or vice versa.
+ *
+ * When the GPU goes from idle state to active state, this event should report
+ * the GPU frequency of the active state. When the GPU goes from active state to
+ * idle state, this event should report a zero frequency value.
+ *
+ * @frequency:  New frequency (in KHz)
+ * @gpu_id: Id for each GPU clock domain
+ */
+TRACE_EVENT(gpu_frequency,
+
+	TP_PROTO(unsigned int frequency, unsigned int gpu_id),
+
+	TP_ARGS(frequency, gpu_id),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, frequency)
+		__field(unsigned int, gpu_id)
+	),
+
+	TP_fast_assign(
+		__entry->frequency = frequency;
+		__entry->gpu_id = gpu_id;
+	),
+
+	TP_printk("frequency=%u gpu_id=%u",
+		__entry->frequency, __entry->gpu_id)
+);
 #endif /* _TRACE_POWER_H */
 
 /* This part must be outside protection */
-- 
2.29.0.rc1.297.gfa9743e501-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-10-22 17:34           ` [PATCH v4] " Peiyong Lin
@ 2020-10-26 22:25             ` Peiyong Lin
  2020-10-27  6:16               ` Greg Kroah-Hartman
  2020-11-16 20:55             ` Peiyong Lin
  2020-11-17 21:31             ` Peiyong Lin
  2 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-10-26 22:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Amit Kucheria, android-kernel, Greg Kroah-Hartman, linux-kernel,
	Ingo Molnar, Paul Walmsley, Pavel Machek, Prahlad Kilambi,
	Rafael J. Wysocki, Ulf Hansson, Masahiro Yamada, zzyiwei,
	Sidath Senanayake

Hi there,

May I ask for a review please?

Thanks,
Peiyong


On Thu, Oct 22, 2020 at 10:34 AM Peiyong Lin <lpy@google.com> wrote:
>
> Historically there is no common trace event for GPU frequency, in
> downstream Android each different hardware vendor implements their own
> way to expose GPU frequency, for example as a debugfs node.  This patch
> standardize it as a common trace event in upstream linux kernel to help
> the ecosystem have a common implementation across hardware vendors.
> Toolings in the Linux ecosystem will benefit from this especially in the
> downstream Android, where this information is critical to graphics
> developers.
>
> Signed-off-by: Peiyong Lin <lpy@google.com>
> ---
>
> Changelog since v3:
>  - Correct copyright title.
>
> Changelog since v2:
>  - Add more comments to indicate when the event should be emitted.
>  - Change state to frequency.
>
> Changelog since v1:
>  - Use %u in TP_printk
>
>  drivers/gpu/Makefile                    |  1 +
>  drivers/gpu/trace/Kconfig               |  3 +++
>  drivers/gpu/trace/Makefile              |  1 +
>  drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
>  include/trace/events/power.h            | 33 +++++++++++++++++++++++++
>  5 files changed, 51 insertions(+)
>  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
>
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 835c88318cec..f289a47eb031 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
>  obj-y                  += drm/ vga/
>  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
>  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> index c24e9edd022e..ac4aec8d5845 100644
> --- a/drivers/gpu/trace/Kconfig
> +++ b/drivers/gpu/trace/Kconfig
> @@ -2,3 +2,6 @@
>
>  config TRACE_GPU_MEM
>         bool
> +
> +config TRACE_GPU_FREQUENCY
> +       bool
> diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> index b70fbdc5847f..2b7ae69327d6 100644
> --- a/drivers/gpu/trace/Makefile
> +++ b/drivers/gpu/trace/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> new file mode 100644
> index 000000000000..668fabd6b77a
> --- /dev/null
> +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * GPU frequency trace points
> + *
> + * Copyright (C) 2020 Google LLC
> + */
> +
> +#include <linux/module.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/power.h>
> +
> +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index af5018aa9517..343825a76953 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
>
>         TP_ARGS(name, type, new_value)
>  );
> +
> +/**
> + * gpu_frequency - Reports the GPU frequency in GPU clock domains.
> + *
> + * This event should be emitted whenever there's a GPU frequency change happens,
> + * or a GPU goes from idle state to active state, or vice versa.
> + *
> + * When the GPU goes from idle state to active state, this event should report
> + * the GPU frequency of the active state. When the GPU goes from active state to
> + * idle state, this event should report a zero frequency value.
> + *
> + * @frequency:  New frequency (in KHz)
> + * @gpu_id: Id for each GPU clock domain
> + */
> +TRACE_EVENT(gpu_frequency,
> +
> +       TP_PROTO(unsigned int frequency, unsigned int gpu_id),
> +
> +       TP_ARGS(frequency, gpu_id),
> +
> +       TP_STRUCT__entry(
> +               __field(unsigned int, frequency)
> +               __field(unsigned int, gpu_id)
> +       ),
> +
> +       TP_fast_assign(
> +               __entry->frequency = frequency;
> +               __entry->gpu_id = gpu_id;
> +       ),
> +
> +       TP_printk("frequency=%u gpu_id=%u",
> +               __entry->frequency, __entry->gpu_id)
> +);
>  #endif /* _TRACE_POWER_H */
>
>  /* This part must be outside protection */
> --
> 2.29.0.rc1.297.gfa9743e501-goog
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-10-26 22:25             ` Peiyong Lin
@ 2020-10-27  6:16               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-27  6:16 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: Steven Rostedt, Amit Kucheria, android-kernel, linux-kernel,
	Ingo Molnar, Paul Walmsley, Pavel Machek, Prahlad Kilambi,
	Rafael J. Wysocki, Ulf Hansson, Masahiro Yamada, zzyiwei,
	Sidath Senanayake

On Mon, Oct 26, 2020 at 03:25:22PM -0700, Peiyong Lin wrote:
> Hi there,
> 
> May I ask for a review please?

Please do not top-post, and you just sent this a few days ago, in the
middle of the merge window when we could not do anything.

Give maintainers time to catch up, and they will get to it...

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-10-22 17:34           ` [PATCH v4] " Peiyong Lin
  2020-10-26 22:25             ` Peiyong Lin
@ 2020-11-16 20:55             ` Peiyong Lin
  2020-11-16 21:05               ` Steven Rostedt
  2020-11-17 21:31             ` Peiyong Lin
  2 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-11-16 20:55 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Amit Kucheria, android-kernel, Greg Kroah-Hartman, linux-kernel,
	Ingo Molnar, Paul Walmsley, Pavel Machek, Prahlad Kilambi,
	Rafael J. Wysocki, Ulf Hansson, Masahiro Yamada, zzyiwei,
	Sidath Senanayake

On Thu, Oct 22, 2020 at 10:34 AM Peiyong Lin <lpy@google.com> wrote:
>
> Historically there is no common trace event for GPU frequency, in
> downstream Android each different hardware vendor implements their own
> way to expose GPU frequency, for example as a debugfs node.  This patch
> standardize it as a common trace event in upstream linux kernel to help
> the ecosystem have a common implementation across hardware vendors.
> Toolings in the Linux ecosystem will benefit from this especially in the
> downstream Android, where this information is critical to graphics
> developers.
>
> Signed-off-by: Peiyong Lin <lpy@google.com>
> ---
>
> Changelog since v3:
>  - Correct copyright title.
>
> Changelog since v2:
>  - Add more comments to indicate when the event should be emitted.
>  - Change state to frequency.
>
> Changelog since v1:
>  - Use %u in TP_printk
>
>  drivers/gpu/Makefile                    |  1 +
>  drivers/gpu/trace/Kconfig               |  3 +++
>  drivers/gpu/trace/Makefile              |  1 +
>  drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
>  include/trace/events/power.h            | 33 +++++++++++++++++++++++++
>  5 files changed, 51 insertions(+)
>  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
>
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 835c88318cec..f289a47eb031 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
>  obj-y                  += drm/ vga/
>  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
>  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> index c24e9edd022e..ac4aec8d5845 100644
> --- a/drivers/gpu/trace/Kconfig
> +++ b/drivers/gpu/trace/Kconfig
> @@ -2,3 +2,6 @@
>
>  config TRACE_GPU_MEM
>         bool
> +
> +config TRACE_GPU_FREQUENCY
> +       bool
> diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> index b70fbdc5847f..2b7ae69327d6 100644
> --- a/drivers/gpu/trace/Makefile
> +++ b/drivers/gpu/trace/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> new file mode 100644
> index 000000000000..668fabd6b77a
> --- /dev/null
> +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * GPU frequency trace points
> + *
> + * Copyright (C) 2020 Google LLC
> + */
> +
> +#include <linux/module.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/power.h>
> +
> +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index af5018aa9517..343825a76953 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
>
>         TP_ARGS(name, type, new_value)
>  );
> +
> +/**
> + * gpu_frequency - Reports the GPU frequency in GPU clock domains.
> + *
> + * This event should be emitted whenever there's a GPU frequency change happens,
> + * or a GPU goes from idle state to active state, or vice versa.
> + *
> + * When the GPU goes from idle state to active state, this event should report
> + * the GPU frequency of the active state. When the GPU goes from active state to
> + * idle state, this event should report a zero frequency value.
> + *
> + * @frequency:  New frequency (in KHz)
> + * @gpu_id: Id for each GPU clock domain
> + */
> +TRACE_EVENT(gpu_frequency,
> +
> +       TP_PROTO(unsigned int frequency, unsigned int gpu_id),
> +
> +       TP_ARGS(frequency, gpu_id),
> +
> +       TP_STRUCT__entry(
> +               __field(unsigned int, frequency)
> +               __field(unsigned int, gpu_id)
> +       ),
> +
> +       TP_fast_assign(
> +               __entry->frequency = frequency;
> +               __entry->gpu_id = gpu_id;
> +       ),
> +
> +       TP_printk("frequency=%u gpu_id=%u",
> +               __entry->frequency, __entry->gpu_id)
> +);
>  #endif /* _TRACE_POWER_H */
>
>  /* This part must be outside protection */
> --
> 2.29.0.rc1.297.gfa9743e501-goog
>

Hi there,

May I ask whether the merge window has passed? If so is it possible to
ask for a review?

Thanks,
Peiyong

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-11-16 20:55             ` Peiyong Lin
@ 2020-11-16 21:05               ` Steven Rostedt
  2020-11-17 18:00                 ` Rafael J. Wysocki
  0 siblings, 1 reply; 21+ messages in thread
From: Steven Rostedt @ 2020-11-16 21:05 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: Amit Kucheria, android-kernel, Greg Kroah-Hartman, linux-kernel,
	Ingo Molnar, Paul Walmsley, Pavel Machek, Prahlad Kilambi,
	Rafael J. Wysocki, Ulf Hansson, Masahiro Yamada, zzyiwei,
	Sidath Senanayake

On Mon, 16 Nov 2020 12:55:29 -0800
Peiyong Lin <lpy@google.com> wrote:

> Hi there,
> 
> May I ask whether the merge window has passed? If so is it possible to
> ask for a review?

This is up to the maintainers of power management to accept this.

Rafael?

-- Steve

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-11-16 21:05               ` Steven Rostedt
@ 2020-11-17 18:00                 ` Rafael J. Wysocki
  2020-11-17 18:10                   ` Alex Deucher
  0 siblings, 1 reply; 21+ messages in thread
From: Rafael J. Wysocki @ 2020-11-17 18:00 UTC (permalink / raw)
  To: Steven Rostedt, Peiyong Lin
  Cc: Amit Kucheria, android-kernel, Greg Kroah-Hartman, linux-kernel,
	Ingo Molnar, Paul Walmsley, Pavel Machek, Prahlad Kilambi,
	Ulf Hansson, Masahiro Yamada, zzyiwei, Sidath Senanayake,
	dri-devel

On 11/16/2020 10:05 PM, Steven Rostedt wrote:
> On Mon, 16 Nov 2020 12:55:29 -0800
> Peiyong Lin <lpy@google.com> wrote:
>
>> Hi there,
>>
>> May I ask whether the merge window has passed? If so is it possible to
>> ask for a review?
> This is up to the maintainers of power management to accept this.
>
> Rafael?

I'd say up to the GPU people rather (dri-devel CCed) since that's where 
it is going to be used.

Also it would be good to see at least one in-the-tree user of this (or a 
usage example at least).

Cheers!



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-11-17 18:00                 ` Rafael J. Wysocki
@ 2020-11-17 18:10                   ` Alex Deucher
  0 siblings, 0 replies; 21+ messages in thread
From: Alex Deucher @ 2020-11-17 18:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Steven Rostedt, Peiyong Lin, Ulf Hansson, Greg Kroah-Hartman,
	Sidath Senanayake, LKML, Amit Kucheria, Masahiro Yamada,
	Prahlad Kilambi, Ingo Molnar, dri-devel, Pavel Machek,
	Paul Walmsley, zzyiwei, android-kernel

On Tue, Nov 17, 2020 at 1:00 PM Rafael J. Wysocki
<rafael.j.wysocki@intel.com> wrote:
>
> On 11/16/2020 10:05 PM, Steven Rostedt wrote:
> > On Mon, 16 Nov 2020 12:55:29 -0800
> > Peiyong Lin <lpy@google.com> wrote:
> >
> >> Hi there,
> >>
> >> May I ask whether the merge window has passed? If so is it possible to
> >> ask for a review?
> > This is up to the maintainers of power management to accept this.
> >
> > Rafael?
>
> I'd say up to the GPU people rather (dri-devel CCed) since that's where
> it is going to be used.
>
> Also it would be good to see at least one in-the-tree user of this (or a
> usage example at least).

Can you resend the patches and cc dri-devel or point out the previous
patch discussion?

Alex

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-10-22 17:34           ` [PATCH v4] " Peiyong Lin
  2020-10-26 22:25             ` Peiyong Lin
  2020-11-16 20:55             ` Peiyong Lin
@ 2020-11-17 21:31             ` Peiyong Lin
  2020-11-30 22:33               ` Peiyong Lin
  2 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-11-17 21:31 UTC (permalink / raw)
  To: Steven Rostedt, alexdeucher
  Cc: android-kernel, Greg Kroah-Hartman, linux-kernel, Ingo Molnar,
	Paul Walmsley, Pavel Machek, Prahlad Kilambi, Rafael J. Wysocki,
	Ulf Hansson, Masahiro Yamada, zzyiwei, Sidath Senanayake,
	dri-devel

On Thu, Oct 22, 2020 at 10:34 AM Peiyong Lin <lpy@google.com> wrote:
>
> Historically there is no common trace event for GPU frequency, in
> downstream Android each different hardware vendor implements their own
> way to expose GPU frequency, for example as a debugfs node.  This patch
> standardize it as a common trace event in upstream linux kernel to help
> the ecosystem have a common implementation across hardware vendors.
> Toolings in the Linux ecosystem will benefit from this especially in the
> downstream Android, where this information is critical to graphics
> developers.
>
> Signed-off-by: Peiyong Lin <lpy@google.com>
> ---
>
> Changelog since v3:
>  - Correct copyright title.
>
> Changelog since v2:
>  - Add more comments to indicate when the event should be emitted.
>  - Change state to frequency.
>
> Changelog since v1:
>  - Use %u in TP_printk
>
>  drivers/gpu/Makefile                    |  1 +
>  drivers/gpu/trace/Kconfig               |  3 +++
>  drivers/gpu/trace/Makefile              |  1 +
>  drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
>  include/trace/events/power.h            | 33 +++++++++++++++++++++++++
>  5 files changed, 51 insertions(+)
>  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
>
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 835c88318cec..f289a47eb031 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
>  obj-y                  += drm/ vga/
>  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
>  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> index c24e9edd022e..ac4aec8d5845 100644
> --- a/drivers/gpu/trace/Kconfig
> +++ b/drivers/gpu/trace/Kconfig
> @@ -2,3 +2,6 @@
>
>  config TRACE_GPU_MEM
>         bool
> +
> +config TRACE_GPU_FREQUENCY
> +       bool
> diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> index b70fbdc5847f..2b7ae69327d6 100644
> --- a/drivers/gpu/trace/Makefile
> +++ b/drivers/gpu/trace/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> new file mode 100644
> index 000000000000..668fabd6b77a
> --- /dev/null
> +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * GPU frequency trace points
> + *
> + * Copyright (C) 2020 Google LLC
> + */
> +
> +#include <linux/module.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/power.h>
> +
> +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index af5018aa9517..343825a76953 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
>
>         TP_ARGS(name, type, new_value)
>  );
> +
> +/**
> + * gpu_frequency - Reports the GPU frequency in GPU clock domains.
> + *
> + * This event should be emitted whenever there's a GPU frequency change happens,
> + * or a GPU goes from idle state to active state, or vice versa.
> + *
> + * When the GPU goes from idle state to active state, this event should report
> + * the GPU frequency of the active state. When the GPU goes from active state to
> + * idle state, this event should report a zero frequency value.
> + *
> + * @frequency:  New frequency (in KHz)
> + * @gpu_id: Id for each GPU clock domain
> + */
> +TRACE_EVENT(gpu_frequency,
> +
> +       TP_PROTO(unsigned int frequency, unsigned int gpu_id),
> +
> +       TP_ARGS(frequency, gpu_id),
> +
> +       TP_STRUCT__entry(
> +               __field(unsigned int, frequency)
> +               __field(unsigned int, gpu_id)
> +       ),
> +
> +       TP_fast_assign(
> +               __entry->frequency = frequency;
> +               __entry->gpu_id = gpu_id;
> +       ),
> +
> +       TP_printk("frequency=%u gpu_id=%u",
> +               __entry->frequency, __entry->gpu_id)
> +);
>  #endif /* _TRACE_POWER_H */
>
>  /* This part must be outside protection */
> --
> 2.29.0.rc1.297.gfa9743e501-goog
>


Hi there,

Per request, re-send this patch with dri-devel@ list CCed.

Thanks,
Peiyong

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-11-17 21:31             ` Peiyong Lin
@ 2020-11-30 22:33               ` Peiyong Lin
  2020-12-02 12:34                 ` Brian Starkey
  0 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-11-30 22:33 UTC (permalink / raw)
  To: Steven Rostedt, alexdeucher
  Cc: android-kernel, Greg Kroah-Hartman, linux-kernel, Ingo Molnar,
	Paul Walmsley, Pavel Machek, Prahlad Kilambi, Rafael J. Wysocki,
	Ulf Hansson, Masahiro Yamada, zzyiwei, Sidath Senanayake,
	dri-devel

On Tue, Nov 17, 2020 at 1:31 PM Peiyong Lin <lpy@google.com> wrote:
>
> On Thu, Oct 22, 2020 at 10:34 AM Peiyong Lin <lpy@google.com> wrote:
> >
> > Historically there is no common trace event for GPU frequency, in
> > downstream Android each different hardware vendor implements their own
> > way to expose GPU frequency, for example as a debugfs node.  This patch
> > standardize it as a common trace event in upstream linux kernel to help
> > the ecosystem have a common implementation across hardware vendors.
> > Toolings in the Linux ecosystem will benefit from this especially in the
> > downstream Android, where this information is critical to graphics
> > developers.
> >
> > Signed-off-by: Peiyong Lin <lpy@google.com>
> > ---
> >
> > Changelog since v3:
> >  - Correct copyright title.
> >
> > Changelog since v2:
> >  - Add more comments to indicate when the event should be emitted.
> >  - Change state to frequency.
> >
> > Changelog since v1:
> >  - Use %u in TP_printk
> >
> >  drivers/gpu/Makefile                    |  1 +
> >  drivers/gpu/trace/Kconfig               |  3 +++
> >  drivers/gpu/trace/Makefile              |  1 +
> >  drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
> >  include/trace/events/power.h            | 33 +++++++++++++++++++++++++
> >  5 files changed, 51 insertions(+)
> >  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
> >
> > diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> > index 835c88318cec..f289a47eb031 100644
> > --- a/drivers/gpu/Makefile
> > +++ b/drivers/gpu/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
> >  obj-y                  += drm/ vga/
> >  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
> >  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> > +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> > diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> > index c24e9edd022e..ac4aec8d5845 100644
> > --- a/drivers/gpu/trace/Kconfig
> > +++ b/drivers/gpu/trace/Kconfig
> > @@ -2,3 +2,6 @@
> >
> >  config TRACE_GPU_MEM
> >         bool
> > +
> > +config TRACE_GPU_FREQUENCY
> > +       bool
> > diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> > index b70fbdc5847f..2b7ae69327d6 100644
> > --- a/drivers/gpu/trace/Makefile
> > +++ b/drivers/gpu/trace/Makefile
> > @@ -1,3 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >
> >  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> > +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> > diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> > new file mode 100644
> > index 000000000000..668fabd6b77a
> > --- /dev/null
> > +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * GPU frequency trace points
> > + *
> > + * Copyright (C) 2020 Google LLC
> > + */
> > +
> > +#include <linux/module.h>
> > +
> > +#define CREATE_TRACE_POINTS
> > +#include <trace/events/power.h>
> > +
> > +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> > diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> > index af5018aa9517..343825a76953 100644
> > --- a/include/trace/events/power.h
> > +++ b/include/trace/events/power.h
> > @@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
> >
> >         TP_ARGS(name, type, new_value)
> >  );
> > +
> > +/**
> > + * gpu_frequency - Reports the GPU frequency in GPU clock domains.
> > + *
> > + * This event should be emitted whenever there's a GPU frequency change happens,
> > + * or a GPU goes from idle state to active state, or vice versa.
> > + *
> > + * When the GPU goes from idle state to active state, this event should report
> > + * the GPU frequency of the active state. When the GPU goes from active state to
> > + * idle state, this event should report a zero frequency value.
> > + *
> > + * @frequency:  New frequency (in KHz)
> > + * @gpu_id: Id for each GPU clock domain
> > + */
> > +TRACE_EVENT(gpu_frequency,
> > +
> > +       TP_PROTO(unsigned int frequency, unsigned int gpu_id),
> > +
> > +       TP_ARGS(frequency, gpu_id),
> > +
> > +       TP_STRUCT__entry(
> > +               __field(unsigned int, frequency)
> > +               __field(unsigned int, gpu_id)
> > +       ),
> > +
> > +       TP_fast_assign(
> > +               __entry->frequency = frequency;
> > +               __entry->gpu_id = gpu_id;
> > +       ),
> > +
> > +       TP_printk("frequency=%u gpu_id=%u",
> > +               __entry->frequency, __entry->gpu_id)
> > +);
> >  #endif /* _TRACE_POWER_H */
> >
> >  /* This part must be outside protection */
> > --
> > 2.29.0.rc1.297.gfa9743e501-goog
> >
>
>
> Hi there,
>
> Per request, re-send this patch with dri-devel@ list CCed.
>
> Thanks,
> Peiyong

Hi there,

For GPU drivers folks in dri-devel@ list, any input?

Thanks,
Peiyong

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] Add power/gpu_frequency tracepoint.
  2020-11-30 22:33               ` Peiyong Lin
@ 2020-12-02 12:34                 ` Brian Starkey
  2020-12-21 20:10                   ` [PATCH v5] " Peiyong Lin
  0 siblings, 1 reply; 21+ messages in thread
From: Brian Starkey @ 2020-12-02 12:34 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: Steven Rostedt, alexdeucher, Ulf Hansson, Greg Kroah-Hartman,
	Sidath Senanayake, Rafael J. Wysocki, linux-kernel, dri-devel,
	Masahiro Yamada, Prahlad Kilambi, Ingo Molnar, Pavel Machek,
	Paul Walmsley, zzyiwei, android-kernel, nd

Hi Peiyong,

On Mon, Nov 30, 2020 at 02:33:59PM -0800, Peiyong Lin wrote:
> On Tue, Nov 17, 2020 at 1:31 PM Peiyong Lin <lpy@google.com> wrote:
> >
> > On Thu, Oct 22, 2020 at 10:34 AM Peiyong Lin <lpy@google.com> wrote:
> > >
> > > Historically there is no common trace event for GPU frequency, in
> > > downstream Android each different hardware vendor implements their own
> > > way to expose GPU frequency, for example as a debugfs node.  This patch
> > > standardize it as a common trace event in upstream linux kernel to help
> > > the ecosystem have a common implementation across hardware vendors.
> > > Toolings in the Linux ecosystem will benefit from this especially in the
> > > downstream Android, where this information is critical to graphics
> > > developers.
> > >
> > > Signed-off-by: Peiyong Lin <lpy@google.com>
> > > ---
> > >
> > > Changelog since v3:
> > >  - Correct copyright title.
> > >
> > > Changelog since v2:
> > >  - Add more comments to indicate when the event should be emitted.
> > >  - Change state to frequency.
> > >
> > > Changelog since v1:
> > >  - Use %u in TP_printk
> > >
> > >  drivers/gpu/Makefile                    |  1 +
> > >  drivers/gpu/trace/Kconfig               |  3 +++
> > >  drivers/gpu/trace/Makefile              |  1 +
> > >  drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++++
> > >  include/trace/events/power.h            | 33 +++++++++++++++++++++++++
> > >  5 files changed, 51 insertions(+)
> > >  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
> > >
> > > diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> > > index 835c88318cec..f289a47eb031 100644
> > > --- a/drivers/gpu/Makefile
> > > +++ b/drivers/gpu/Makefile
> > > @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
> > >  obj-y                  += drm/ vga/
> > >  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
> > >  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> > > +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> > > diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> > > index c24e9edd022e..ac4aec8d5845 100644
> > > --- a/drivers/gpu/trace/Kconfig
> > > +++ b/drivers/gpu/trace/Kconfig
> > > @@ -2,3 +2,6 @@
> > >
> > >  config TRACE_GPU_MEM
> > >         bool
> > > +
> > > +config TRACE_GPU_FREQUENCY
> > > +       bool
> > > diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> > > index b70fbdc5847f..2b7ae69327d6 100644
> > > --- a/drivers/gpu/trace/Makefile
> > > +++ b/drivers/gpu/trace/Makefile
> > > @@ -1,3 +1,4 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >
> > >  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> > > +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> > > diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> > > new file mode 100644
> > > index 000000000000..668fabd6b77a
> > > --- /dev/null
> > > +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> > > @@ -0,0 +1,13 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * GPU frequency trace points
> > > + *
> > > + * Copyright (C) 2020 Google LLC
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +
> > > +#define CREATE_TRACE_POINTS
> > > +#include <trace/events/power.h>
> > > +
> > > +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> > > diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> > > index af5018aa9517..343825a76953 100644
> > > --- a/include/trace/events/power.h
> > > +++ b/include/trace/events/power.h
> > > @@ -500,6 +500,39 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
> > >
> > >         TP_ARGS(name, type, new_value)
> > >  );
> > > +
> > > +/**
> > > + * gpu_frequency - Reports the GPU frequency in GPU clock domains.
> > > + *
> > > + * This event should be emitted whenever there's a GPU frequency change happens,
> > > + * or a GPU goes from idle state to active state, or vice versa.
> > > + *
> > > + * When the GPU goes from idle state to active state, this event should report
> > > + * the GPU frequency of the active state. When the GPU goes from active state to
> > > + * idle state, this event should report a zero frequency value.
> > > + *
> > > + * @frequency:  New frequency (in KHz)
> > > + * @gpu_id: Id for each GPU clock domain

Thinking about options for how to assign this, there are a few
existing tracepoints (I see block device and jb2) which use dev_t, so
perhaps that would be an option. We'd still want to be able to expose
multiple clocks per device though, either with a separate field or
with a defined packing into this one.

dev_t is a little bit tricky because the packing can (is?) different
between the kernel and C library. major + minor + clock domain as
3 separate fields would be the most explicit.

As the intended use-case is tools, I imagine userspace will want to
link this to data exposed via other APIs, so gpu_id needs to somehow
allow that. That probably means some opaque integer assigned within
the kernel isn't OK.

Hopefully some other vendors have thoughts on what would work in their
systems.

Thanks,
-Brian

> > > + */
> > > +TRACE_EVENT(gpu_frequency,
> > > +
> > > +       TP_PROTO(unsigned int frequency, unsigned int gpu_id),
> > > +
> > > +       TP_ARGS(frequency, gpu_id),
> > > +
> > > +       TP_STRUCT__entry(
> > > +               __field(unsigned int, frequency)
> > > +               __field(unsigned int, gpu_id)
> > > +       ),
> > > +
> > > +       TP_fast_assign(
> > > +               __entry->frequency = frequency;
> > > +               __entry->gpu_id = gpu_id;
> > > +       ),
> > > +
> > > +       TP_printk("frequency=%u gpu_id=%u",
> > > +               __entry->frequency, __entry->gpu_id)
> > > +);
> > >  #endif /* _TRACE_POWER_H */
> > >
> > >  /* This part must be outside protection */
> > > --
> > > 2.29.0.rc1.297.gfa9743e501-goog
> > >
> >
> >
> > Hi there,
> >
> > Per request, re-send this patch with dri-devel@ list CCed.
> >
> > Thanks,
> > Peiyong
> 
> Hi there,
> 
> For GPU drivers folks in dri-devel@ list, any input?
> 
> Thanks,
> Peiyong
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5] Add power/gpu_frequency tracepoint.
  2020-12-02 12:34                 ` Brian Starkey
@ 2020-12-21 20:10                   ` Peiyong Lin
  2021-03-10 21:56                     ` Peiyong Lin
  0 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2020-12-21 20:10 UTC (permalink / raw)
  To: brian.starkey
  Cc: alexdeucher, android-kernel, dri-devel, gregkh, linux-kernel,
	lpy, mingo, nd, paul.walmsley, pavel, prahladk, rafael.j.wysocki,
	rostedt, sidaths, ulf.hansson, yamada.masahiro, zzyiwei

Historically there is no common trace event for GPU frequency, in
downstream Android each different hardware vendor implements their own
way to expose GPU frequency, for example as a debugfs node.  This patch
standardize it as a common trace event in upstream linux kernel to help
the ecosystem have a common implementation across hardware vendors.
Toolings in the Linux ecosystem will benefit from this especially in the
downstream Android, where this information is critical to graphics
developers.

Signed-off-by: Peiyong Lin <lpy@google.com>
---

Changelog since v4:
 - Explicitly use class id and instance id to identify a GPU instance.
 - Change gpu_id to clock_id to call out its the clock domain in
   the GPU instance.

Changelog since v3:
 - Correct copyright title.

Changelog since v2:
 - Add more comments to indicate when the event should be emitted.
 - Change state to frequency.

Changelog since v1:
 - Use %u in TP_printk

 drivers/gpu/Makefile                    |  1 +
 drivers/gpu/trace/Kconfig               |  3 ++
 drivers/gpu/trace/Makefile              |  1 +
 drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++
 include/trace/events/power.h            | 41 +++++++++++++++++++++++++
 5 files changed, 59 insertions(+)
 create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..f289a47eb031 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)	+= host1x/
 obj-y			+= drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_TRACE_GPU_FREQUENCY)		+= trace/
diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
index c24e9edd022e..ac4aec8d5845 100644
--- a/drivers/gpu/trace/Kconfig
+++ b/drivers/gpu/trace/Kconfig
@@ -2,3 +2,6 @@
 
 config TRACE_GPU_MEM
 	bool
+
+config TRACE_GPU_FREQUENCY
+	bool
diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
index b70fbdc5847f..2b7ae69327d6 100644
--- a/drivers/gpu/trace/Makefile
+++ b/drivers/gpu/trace/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
+obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
new file mode 100644
index 000000000000..668fabd6b77a
--- /dev/null
+++ b/drivers/gpu/trace/trace_gpu_frequency.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPU frequency trace points
+ *
+ * Copyright (C) 2020 Google LLC
+ */
+
+#include <linux/module.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/power.h>
+
+EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index af5018aa9517..590e16169dd1 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -500,6 +500,47 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
 
 	TP_ARGS(name, type, new_value)
 );
+
+/**
+ * gpu_frequency - Reports the GPU frequency in GPU clock domains.
+ *
+ * This event should be emitted whenever there's a GPU frequency change happens,
+ * or a GPU goes from idle state to active state, or vice versa.
+ *
+ * When the GPU goes from idle state to active state, this event should report
+ * the GPU frequency of the active state. When the GPU goes from active state to
+ * idle state, this event should report a zero frequency value.
+ *
+ * @frequency:  New frequency (in KHz)
+ * @gpu_class_id: Id representing the class of the GPU
+ * @gpu_instance_id: Id representing the instance of class &gpu_class_id
+ * @clock_id: Id for the clock domain in &gpu_instance_id running at &frequency
+ */
+TRACE_EVENT(gpu_frequency,
+
+	TP_PROTO(unsigned int frequency, unsigned int gpu_class_id,
+		 unsigned int gpu_instance_id, unsigned int clock_id),
+
+	TP_ARGS(frequency, gpu_class_id, gpu_instance_id, clock_id),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, frequency)
+		__field(unsigned int, gpu_class_id)
+		__field(unsigned int, gpu_instance_id)
+		__field(unsigned int, clock_id)
+	),
+
+	TP_fast_assign(
+		__entry->frequency = frequency;
+		__entry->gpu_class_id = gpu_class_id;
+		__entry->gpu_instance_id = gpu_instance_id;
+		__entry->clock_id = clock_id;
+	),
+
+	TP_printk("frequency=%u gpu_class_id=%u gpu_instance_id=%u clock_id=%u",
+		__entry->frequency, __entry->gpu_class_id,
+		__entry->gpu_instance_id, __entry->clock_id)
+);
 #endif /* _TRACE_POWER_H */
 
 /* This part must be outside protection */
-- 
2.29.2.684.gfbc64c5ab5-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v5] Add power/gpu_frequency tracepoint.
  2020-12-21 20:10                   ` [PATCH v5] " Peiyong Lin
@ 2021-03-10 21:56                     ` Peiyong Lin
  2021-03-12 18:08                       ` Pavel Machek
  0 siblings, 1 reply; 21+ messages in thread
From: Peiyong Lin @ 2021-03-10 21:56 UTC (permalink / raw)
  To: Brian Starkey
  Cc: alexdeucher, android-kernel, dri-devel, Greg Kroah-Hartman,
	linux-kernel, Ingo Molnar, nd, Paul Walmsley, Pavel Machek,
	Prahlad Kilambi, Rafael J. Wysocki, Steven Rostedt,
	Sidath Senanayake, Ulf Hansson, Masahiro Yamada, zzyiwei

On Mon, Dec 21, 2020 at 12:10 PM Peiyong Lin <lpy@google.com> wrote:
>
> Historically there is no common trace event for GPU frequency, in
> downstream Android each different hardware vendor implements their own
> way to expose GPU frequency, for example as a debugfs node.  This patch
> standardize it as a common trace event in upstream linux kernel to help
> the ecosystem have a common implementation across hardware vendors.
> Toolings in the Linux ecosystem will benefit from this especially in the
> downstream Android, where this information is critical to graphics
> developers.
>
> Signed-off-by: Peiyong Lin <lpy@google.com>
> ---
>
> Changelog since v4:
>  - Explicitly use class id and instance id to identify a GPU instance.
>  - Change gpu_id to clock_id to call out its the clock domain in
>    the GPU instance.
>
> Changelog since v3:
>  - Correct copyright title.
>
> Changelog since v2:
>  - Add more comments to indicate when the event should be emitted.
>  - Change state to frequency.
>
> Changelog since v1:
>  - Use %u in TP_printk
>
>  drivers/gpu/Makefile                    |  1 +
>  drivers/gpu/trace/Kconfig               |  3 ++
>  drivers/gpu/trace/Makefile              |  1 +
>  drivers/gpu/trace/trace_gpu_frequency.c | 13 ++++++++
>  include/trace/events/power.h            | 41 +++++++++++++++++++++++++
>  5 files changed, 59 insertions(+)
>  create mode 100644 drivers/gpu/trace/trace_gpu_frequency.c
>
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 835c88318cec..f289a47eb031 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_TEGRA_HOST1X)      += host1x/
>  obj-y                  += drm/ vga/
>  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
>  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY)              += trace/
> diff --git a/drivers/gpu/trace/Kconfig b/drivers/gpu/trace/Kconfig
> index c24e9edd022e..ac4aec8d5845 100644
> --- a/drivers/gpu/trace/Kconfig
> +++ b/drivers/gpu/trace/Kconfig
> @@ -2,3 +2,6 @@
>
>  config TRACE_GPU_MEM
>         bool
> +
> +config TRACE_GPU_FREQUENCY
> +       bool
> diff --git a/drivers/gpu/trace/Makefile b/drivers/gpu/trace/Makefile
> index b70fbdc5847f..2b7ae69327d6 100644
> --- a/drivers/gpu/trace/Makefile
> +++ b/drivers/gpu/trace/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  obj-$(CONFIG_TRACE_GPU_MEM) += trace_gpu_mem.o
> +obj-$(CONFIG_TRACE_GPU_FREQUENCY) += trace_gpu_frequency.o
> diff --git a/drivers/gpu/trace/trace_gpu_frequency.c b/drivers/gpu/trace/trace_gpu_frequency.c
> new file mode 100644
> index 000000000000..668fabd6b77a
> --- /dev/null
> +++ b/drivers/gpu/trace/trace_gpu_frequency.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * GPU frequency trace points
> + *
> + * Copyright (C) 2020 Google LLC
> + */
> +
> +#include <linux/module.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/power.h>
> +
> +EXPORT_TRACEPOINT_SYMBOL(gpu_frequency);
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index af5018aa9517..590e16169dd1 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -500,6 +500,47 @@ DEFINE_EVENT(dev_pm_qos_request, dev_pm_qos_remove_request,
>
>         TP_ARGS(name, type, new_value)
>  );
> +
> +/**
> + * gpu_frequency - Reports the GPU frequency in GPU clock domains.
> + *
> + * This event should be emitted whenever there's a GPU frequency change happens,
> + * or a GPU goes from idle state to active state, or vice versa.
> + *
> + * When the GPU goes from idle state to active state, this event should report
> + * the GPU frequency of the active state. When the GPU goes from active state to
> + * idle state, this event should report a zero frequency value.
> + *
> + * @frequency:  New frequency (in KHz)
> + * @gpu_class_id: Id representing the class of the GPU
> + * @gpu_instance_id: Id representing the instance of class &gpu_class_id
> + * @clock_id: Id for the clock domain in &gpu_instance_id running at &frequency
> + */
> +TRACE_EVENT(gpu_frequency,
> +
> +       TP_PROTO(unsigned int frequency, unsigned int gpu_class_id,
> +                unsigned int gpu_instance_id, unsigned int clock_id),
> +
> +       TP_ARGS(frequency, gpu_class_id, gpu_instance_id, clock_id),
> +
> +       TP_STRUCT__entry(
> +               __field(unsigned int, frequency)
> +               __field(unsigned int, gpu_class_id)
> +               __field(unsigned int, gpu_instance_id)
> +               __field(unsigned int, clock_id)
> +       ),
> +
> +       TP_fast_assign(
> +               __entry->frequency = frequency;
> +               __entry->gpu_class_id = gpu_class_id;
> +               __entry->gpu_instance_id = gpu_instance_id;
> +               __entry->clock_id = clock_id;
> +       ),
> +
> +       TP_printk("frequency=%u gpu_class_id=%u gpu_instance_id=%u clock_id=%u",
> +               __entry->frequency, __entry->gpu_class_id,
> +               __entry->gpu_instance_id, __entry->clock_id)
> +);
>  #endif /* _TRACE_POWER_H */
>
>  /* This part must be outside protection */
> --
> 2.29.2.684.gfbc64c5ab5-goog
>

Hi there,

Could you please take a look at this patch?

Thanks,
Peiyong

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5] Add power/gpu_frequency tracepoint.
  2021-03-10 21:56                     ` Peiyong Lin
@ 2021-03-12 18:08                       ` Pavel Machek
  0 siblings, 0 replies; 21+ messages in thread
From: Pavel Machek @ 2021-03-12 18:08 UTC (permalink / raw)
  To: Peiyong Lin
  Cc: Brian Starkey, alexdeucher, android-kernel, dri-devel,
	Greg Kroah-Hartman, linux-kernel, Ingo Molnar, nd, Paul Walmsley,
	Prahlad Kilambi, Rafael J. Wysocki, Steven Rostedt,
	Sidath Senanayake, Ulf Hansson, Masahiro Yamada, zzyiwei

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

On Wed 2021-03-10 13:56:29, Peiyong Lin wrote:
> On Mon, Dec 21, 2020 at 12:10 PM Peiyong Lin <lpy@google.com> wrote:
> >
> > Historically there is no common trace event for GPU frequency, in
> > downstream Android each different hardware vendor implements their own
> > way to expose GPU frequency, for example as a debugfs node.  This patch
> > standardize it as a common trace event in upstream linux kernel to help
> > the ecosystem have a common implementation across hardware vendors.
> > Toolings in the Linux ecosystem will benefit from this especially in the
> > downstream Android, where this information is critical to graphics
> > developers.
...
> >  /* This part must be outside protection */
> > --
> > 2.29.2.684.gfbc64c5ab5-goog
> >
> 
> Hi there,
> 
> Could you please take a look at this patch?

Please trim email quotes when replying.

Also, you might want to state _name_ of person who should apply this,
so there's no ambiguity. You cc-ed many people...
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-03-12 18:09 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13 21:03 [PATCH] Add power/gpu_frequency tracepoint Peiyong Lin
2020-08-13 21:22 ` Steven Rostedt
2020-08-13 21:37   ` [PATCH v2] " Peiyong Lin
2020-08-13 21:50     ` Steven Rostedt
2020-08-20 19:41       ` [PATCH v3] " Peiyong Lin
2020-08-20 20:27         ` Steven Rostedt
2020-10-22 16:59           ` [PATCH] " Peiyong Lin
2020-10-22 17:34           ` [PATCH v4] " Peiyong Lin
2020-10-26 22:25             ` Peiyong Lin
2020-10-27  6:16               ` Greg Kroah-Hartman
2020-11-16 20:55             ` Peiyong Lin
2020-11-16 21:05               ` Steven Rostedt
2020-11-17 18:00                 ` Rafael J. Wysocki
2020-11-17 18:10                   ` Alex Deucher
2020-11-17 21:31             ` Peiyong Lin
2020-11-30 22:33               ` Peiyong Lin
2020-12-02 12:34                 ` Brian Starkey
2020-12-21 20:10                   ` [PATCH v5] " Peiyong Lin
2021-03-10 21:56                     ` Peiyong Lin
2021-03-12 18:08                       ` Pavel Machek
     [not found] ` <159741850487.10342.14268227307882225260@build.alporthouse.com>
2020-08-14 19:25   ` [PATCH] " Peiyong Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).