Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Sousa <gustavo.sousa@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Matt Roper <matthew.d.roper@intel.com>
Subject: [PATCH v2 5/5] drm/i915/display: Cover all possible pipes in TP_printk()
Date: Mon, 23 Sep 2024 16:02:54 -0300	[thread overview]
Message-ID: <20240923190324.83013-6-gustavo.sousa@intel.com> (raw)
In-Reply-To: <20240923190324.83013-1-gustavo.sousa@intel.com>

Tracepoints that display frame and scanline counters for all pipes were
added with commit 1489bba82433 ("drm/i915: Add cxsr toggle tracepoint")
and commit 0b2599a43ca9 ("drm/i915: Add pipe enable/disable
tracepoints"). At that time, we only had pipes A, B and C. Now that we
can also have pipe D, the TP_printk() calls are missing it.

As a quick and dirty fix for that, let's define two common macros to be
used for the format and values respectively, and also ensure we raise a
build bug if more pipes are added to enum pipe.

In the future, we should probably have a way of printing information for
available pipes only.

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 .../drm/i915/display/intel_display_trace.h    | 43 +++++++++++++------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h
index eec9aeddad96..9bd8f1e505b0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_trace.h
+++ b/drivers/gpu/drm/i915/display/intel_display_trace.h
@@ -31,6 +31,29 @@
 #define _TRACE_PIPE_A	0
 #define _TRACE_PIPE_B	1
 #define _TRACE_PIPE_C	2
+#define _TRACE_PIPE_D	3
+
+/*
+ * FIXME: Several TP_printk() calls below display frame and scanline numbers for
+ * all possible pipes (regardless of whether they are available) and that is
+ * done with a constant format string. A better approach would be to generate
+ * that info dynamically based on available pipes, but, while we do not have
+ * that implemented yet, let's assert that the constant format string indeed
+ * covers all possible pipes.
+ */
+static_assert(I915_MAX_PIPES - 1 == _TRACE_PIPE_D);
+
+#define _PIPES_FRAME_AND_SCANLINE_FMT		\
+	"pipe A: frame=%u, scanline=%u"		\
+	", pipe B: frame=%u, scanline=%u"	\
+	", pipe C: frame=%u, scanline=%u"	\
+	", pipe D: frame=%u, scanline=%u"
+
+#define _PIPES_FRAME_AND_SCANLINE_VALUES					\
+	__entry->frame[_TRACE_PIPE_A], __entry->scanline[_TRACE_PIPE_A]		\
+	, __entry->frame[_TRACE_PIPE_B], __entry->scanline[_TRACE_PIPE_B]	\
+	, __entry->frame[_TRACE_PIPE_C], __entry->scanline[_TRACE_PIPE_C]	\
+	, __entry->frame[_TRACE_PIPE_D], __entry->scanline[_TRACE_PIPE_D]
 
 /*
  * Paranoid sanity check that at least the enumeration starts at the
@@ -63,11 +86,8 @@ TRACE_EVENT(intel_pipe_enable,
 			   __entry->pipe_name = pipe_name(crtc->pipe);
 			   ),
 
-	    TP_printk("dev %s, pipe %c enable, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u",
-		      __get_str(dev), __entry->pipe_name,
-		      __entry->frame[_TRACE_PIPE_A], __entry->scanline[_TRACE_PIPE_A],
-		      __entry->frame[_TRACE_PIPE_B], __entry->scanline[_TRACE_PIPE_B],
-		      __entry->frame[_TRACE_PIPE_C], __entry->scanline[_TRACE_PIPE_C])
+	    TP_printk("dev %s, pipe %c enable, " _PIPES_FRAME_AND_SCANLINE_FMT,
+		      __get_str(dev), __entry->pipe_name, _PIPES_FRAME_AND_SCANLINE_VALUES)
 );
 
 TRACE_EVENT(intel_pipe_disable,
@@ -96,11 +116,8 @@ TRACE_EVENT(intel_pipe_disable,
 			   __entry->pipe_name = pipe_name(crtc->pipe);
 			   ),
 
-	    TP_printk("dev %s, pipe %c disable, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u",
-		      __get_str(dev), __entry->pipe_name,
-		      __entry->frame[_TRACE_PIPE_A], __entry->scanline[_TRACE_PIPE_A],
-		      __entry->frame[_TRACE_PIPE_B], __entry->scanline[_TRACE_PIPE_B],
-		      __entry->frame[_TRACE_PIPE_C], __entry->scanline[_TRACE_PIPE_C])
+	    TP_printk("dev %s, pipe %c disable, " _PIPES_FRAME_AND_SCANLINE_FMT,
+		      __get_str(dev), __entry->pipe_name, _PIPES_FRAME_AND_SCANLINE_VALUES)
 );
 
 TRACE_EVENT(intel_crtc_flip_done,
@@ -230,11 +247,9 @@ TRACE_EVENT(intel_memory_cxsr,
 			   __entry->new = new;
 			   ),
 
-	    TP_printk("dev %s, cxsr %s->%s, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u",
+	    TP_printk("dev %s, cxsr %s->%s, " _PIPES_FRAME_AND_SCANLINE_FMT,
 		      __get_str(dev), str_on_off(__entry->old), str_on_off(__entry->new),
-		      __entry->frame[_TRACE_PIPE_A], __entry->scanline[_TRACE_PIPE_A],
-		      __entry->frame[_TRACE_PIPE_B], __entry->scanline[_TRACE_PIPE_B],
-		      __entry->frame[_TRACE_PIPE_C], __entry->scanline[_TRACE_PIPE_C])
+		      _PIPES_FRAME_AND_SCANLINE_VALUES)
 );
 
 TRACE_EVENT(g4x_wm,
-- 
2.46.1


  parent reply	other threads:[~2024-09-23 19:03 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-23 19:02 [PATCH v2 0/5] Miscelaneous fixes for display tracepoints Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 1/5] drm/i915/display: Fix out-of-bounds access in pipe-related tracepoints Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 2/5] drm/i915/display: Zero-initialize frame/scanline counts in tracepoints Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 3/5] drm/i915/display: Store pipe name in trace events Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 4/5] drm/i915/display: Do not use ids from enum pipe in TP_printk() Gustavo Sousa
2024-09-23 19:02 ` Gustavo Sousa [this message]
2024-09-23 19:23   ` [PATCH v2 5/5] drm/i915/display: Cover all possible pipes " Ville Syrjälä
2024-09-23 20:06     ` Gustavo Sousa
2024-09-23 20:18       ` Ville Syrjälä
2024-09-23 20:47         ` Gustavo Sousa
2024-09-23 20:48           ` Gustavo Sousa
2024-09-23 21:08             ` Ville Syrjälä
2024-09-26 19:26 ` ✗ Fi.CI.CHECKPATCH: warning for Miscelaneous fixes for display tracepoints (rev2) Patchwork
2024-09-26 19:28 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-10-04 11:12   ` Gustavo Sousa
2024-10-04 11:33     ` Jani Nikula
2024-10-04 11:43       ` Gustavo Sousa
2024-10-04 12:03         ` Saarinen, Jani
2024-10-04 12:09           ` Saarinen, Jani
2024-10-08 19:46 ` ✗ Fi.CI.CHECKPATCH: warning for Miscelaneous fixes for display tracepoints (rev3) Patchwork
2024-10-08 19:55 ` ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240923190324.83013-6-gustavo.sousa@intel.com \
    --to=gustavo.sousa@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.d.roper@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox