Currently we just define the display tracepoints with
TRACE_SYSTEM i915. However the code gets included separately
in i915 and xe, and now both modules are competing for the
same tracepoints. Apparently whichever module is loaded first
gets the tracepoints and the other guy is left with nothing.
Give each module its own set of display tracepoints so that
things work even when both modules are loaded.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250127213055.640-1-ville.syrjala@linux.intel.com
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Using the plane->state pointer in the tracepoints is incorrect
as technically a different state could already have been swapped
in (though in reality that is currently prevented by the stall
hacks in the commit machinery). But let's not leave such footguns
lying around when we can just pass in the correct state by hand.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241218173650.19782-4-ville.syrjala@linux.intel.com
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Out plane names already include the "plane" part (or
"primary","sprite","cursor" in some cases). Don't duplicate
that in the tracepoints as that leadst to weird stuff like
"plane plane 1A".
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241218173650.19782-3-ville.syrjala@linux.intel.com
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Tracepoints that display frame and scanline counters for all pipes were
added with commit 1489bba824 ("drm/i915: Add cxsr toggle tracepoint")
and commit 0b2599a43c ("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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241016135300.21428-6-gustavo.sousa@intel.com
The first part[1] of the LWN series on using TRACE_EVENT() mentions
about TP_printk():
"Do not create new tracepoint-specific helpers, because that will
confuse user-space tools that know about the TRACE_EVENT() helper
macros but will not know how to handle ones created for individual
tracepoints."
It seems this is what we ended up doing when using pipe_name() in
TP_printk().
For example, the format for the intel_pipe_update_start event is as
follows:
# cat /sys/kernel/debug/tracing/events/i915/intel_pipe_update_start/format
name: intel_pipe_update_start
ID: 1136
format:
field:unsigned short common_type; offset:0; size:2; signed:0;
field:unsigned char common_flags; offset:2; size:1; signed:0;
field:unsigned char common_preempt_count; offset:3; size:1; signed:0;
field:int common_pid; offset:4; size:4; signed:1;
field:__data_loc char[] dev; offset:8; size:4; signed:0;
field:enum pipe pipe; offset:12; size:4; signed:1;
field:u32 frame; offset:16; size:4; signed:0;
field:u32 scanline; offset:20; size:4; signed:0;
field:u32 min; offset:24; size:4; signed:0;
field:u32 max; offset:28; size:4; signed:0;
print fmt: "dev %s, pipe %c, frame=%u, scanline=%u, min=%u, max=%u", __get_str(dev), ((REC->pipe) + 'A'), REC->frame, REC->scanline, REC->min, REC->max
The call to pipe_name(__entry->pipe) is expanted to ((REC->pipe) + 'A')
and that's how the format is saved.
Even though the output from /sys/kernel/debug/tracing/trace will look
correct (because it is generated in the kernel), we will see corrupted
lines when using a tool like trace-cmd to view the data.
While the output looks correct when looking at
/sys/kernel/debug/tracing/trace, we see corrupted lines when viewing the
trace data with "trace-cmd report":
$ trace-cmd report \
> | sed -n 's/.*dev 0000:00:02\.0, \(pipe .\).*/\1/p' \
> | cat -v | uniq -c
34 pipe ^A
, where ^A is a non-printable character.
As a fix, let's store the pipe name directly in the event. The fix was
done by applying the following sed script:
s/__field\s*(\s*enum\s\+pipe\s*,\s*pipe\s*)/__field(char, pipe_name)/
s/__entry\s*->\s*pipe\s*=\s*\([^;]\+\);/__entry->pipe_name = pipe_name(\1);/
s/pipe_name\s*(\s*__entry\s*->\s*pipe\s*)/__entry->pipe_name/
After these changes, using the same example, we have the following:
$ trace-cmd report \
> | sed -n 's/.*dev 0000:00:02\.0, \(pipe .\).*/\1/p' \
> | cat -v | sort | uniq -c
396 pipe A
34 pipe B
[1] https://lwn.net/Articles/379903/
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241016135300.21428-4-gustavo.sousa@intel.com
In an upcoming change, we will also add support for logging
frame/scanline counts for pipe D in relevant tracepoints.
In [1], Matt mentioned the possibility of having garbage in those counts
for pipe D on a platform containing only 3 pipes. Indeed, it has been
verified that the counts for the extra pipe would not be
zero-initialized by the tracing system.
Since it is also possible that the same would happen for a fused-off
pipe, let's go ahead and add the logic to zero-initialize the arrays
now.
[1] https://lore.kernel.org/all/20240918224927.GU5091@mdroper-desk1.amr.corp.intel.com/
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241016135300.21428-3-gustavo.sousa@intel.com
Going forward, struct intel_display shall replace struct
drm_i915_private as the main display device data pointer type. Convert
intel_display_trace.h to struct intel_display.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240904130633.3831492-2-jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
With the rework of how the __string() handles dynamic strings where it
saves off the source string in field in the helper structure[1], the
assignment of that value to the trace event field is stored in the helper
value and does not need to be passed in again.
This means that with:
__string(field, mystring)
Which use to be assigned with __assign_str(field, mystring), no longer
needs the second parameter and it is unused. With this, __assign_str()
will now only get a single parameter.
There's over 700 users of __assign_str() and because coccinelle does not
handle the TRACE_EVENT() macro I ended up using the following sed script:
git grep -l __assign_str | while read a ; do
sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
mv /tmp/test-file $a;
done
I then searched for __assign_str() that did not end with ';' as those
were multi line assignments that the sed script above would fail to catch.
Note, the same updates will need to be done for:
__assign_str_len()
__assign_rel_str()
__assign_rel_str_len()
I tested this with both an allmodconfig and an allyesconfig (build only for both).
[1] https://lore.kernel.org/linux-trace-kernel/20240222211442.634192653@goodmis.org/
Link: https://lore.kernel.org/linux-trace-kernel/20240516133454.681ba6a0@rorschach.local.home
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Julia Lawall <Julia.Lawall@inria.fr>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Christian König <christian.koenig@amd.com> for the amdgpu parts.
Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> #for
Acked-by: Rafael J. Wysocki <rafael@kernel.org> # for thermal
Acked-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Darrick J. Wong <djwong@kernel.org> # xfs
Tested-by: Guenter Roeck <linux@roeck-us.net>
I'm working on improving the __assign_str() and __string() macros to be
more efficient, and removed some unneeded semicolons. This triggered a bug
in the build as some of the __assign_str() macros in intel_display_trace
was missing a terminating semicolon.
Link: https://lore.kernel.org/linux-trace-kernel/20240222133057.2af72a19@gandalf.local.home
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: stable@vger.kernel.org
Fixes: 2ceea5d880 ("drm/i915: Print plane name in fbc tracepoints")
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
A lot of places include i915_reg.h implicitly via i915_irq.h, which gets
included implicitly via intel_display_trace.h. Remove the includes from
the headers, and include i915_reg.h and i915_irq.h explicitly where
needed.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230419094243.366821-1-jani.nikula@intel.com
Include dev_name() in the tracpoints so one can filter based on
the device.
Example:
echo 'dev=="0000:00:02.0"' > events/i915/intel_cpu_fifo_underrun/filter
v2: Reduce the magic macros, rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221111123120.7759-5-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Print the name of the plane in the fbc tracepoints. As the
pipe<->plane assignment can vary on old hw it's probably
more helpful to see both the plane and the pipe names together.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221111123120.7759-3-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Pass intel_plane rather than drm_plane to the plane tracepoints.
Matches what we do eg. with the fbc tracepoints. Using the same
type for everything will help with digging out the device name
from the plane in the future.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221111123120.7759-2-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Remove the local onoff() implementation and adopt the
str_on_off() from linux/string_helpers.h.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220225234631.3725943-4-lucas.demarchi@intel.com
Remove the local yesno() implementation and adopt the str_yes_no() from
linux/string_helpers.h.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220225234631.3725943-1-lucas.demarchi@intel.com
Add display/intel_display_trace.[ch] for defining display
tracepoints. The main goal is to reduce cross-includes between gem and
display. It would be possible split up tracing even further, but that
would lead to more boilerplate.
We end up having to include intel_crtc.h in a few places because it was
pulled in implicitly via intel_de.h -> i915_trace.h -> intel_crtc.h, and
that's no longer the case.
There should be no changes to tracepoints.
v3:
- Rebase
v2:
- Define TRACE_INCLUDE_PATH relative to define_trace.h (Chris)
- Remove useless comments (Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/7862ad764fbd0748d903c76bc632d3d277874e5b.1638961423.git.jani.nikula@intel.com
