| Message ID | 20250127213055.640-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915: Give i915 and xe each their own display tracepoints | expand |
-----Original Message----- From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala Sent: Monday, January 27, 2025 1:31 PM To: intel-gfx@lists.freedesktop.org Cc: intel-xe@lists.freedesktop.org Subject: [PATCH] drm/i915: Give i915 and xe each their own display tracepoints > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently we just define the display tracpoints with > TRACE_SYSTEM i915. However the code gets included separately > in i915 and xe, and now both modules are competing for the > same tracpoints. 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 tracpoints so that > things work even when both modules are loaded. > > This one had me stumped for a bit when after a reboot I lost > all i915 display tracpoints (on account of the new kernel > also including xe, and something also ended up loading it > before I manually loaded i915). s/tracpoints/tracepoints Also, the last sentence in this commit message probably isn't necessary, but it doesn't detract from anything, so I won't block on its removal. Just fix the tracepoints spelling and this is: Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Though it would probably be good to file an issue report detailing the issue this fixes, then mark this patch as having fixed that reported issue. -Jonathan Cavitt > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display_trace.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h > index 54a6e2a46b82..0e10c2856058 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_trace.h > +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h > @@ -4,7 +4,11 @@ > */ > > #undef TRACE_SYSTEM > +#ifdef I915 > #define TRACE_SYSTEM i915 > +#else > +#define TRACE_SYSTEM xe > +#endif > > #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ) > #define __INTEL_DISPLAY_TRACE_H__ > -- > 2.45.3 > >
On Mon, Jan 27, 2025 at 11:30:55PM +0200, Ville Syrjälä wrote: >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >Currently we just define the display tracpoints with >TRACE_SYSTEM i915. However the code gets included separately >in i915 and xe, and now both modules are competing for the >same tracpoints. 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 tracpoints so that >things work even when both modules are loaded. > >This one had me stumped for a bit when after a reboot I lost >all i915 display tracpoints (on account of the new kernel >also including xe, and something also ended up loading it >before I manually loaded i915). > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >--- > drivers/gpu/drm/i915/display/intel_display_trace.h | 4 ++++ > 1 file changed, 4 insertions(+) > >diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h >index 54a6e2a46b82..0e10c2856058 100644 >--- a/drivers/gpu/drm/i915/display/intel_display_trace.h >+++ b/drivers/gpu/drm/i915/display/intel_display_trace.h >@@ -4,7 +4,11 @@ > */ > > #undef TRACE_SYSTEM >+#ifdef I915 > #define TRACE_SYSTEM i915 >+#else >+#define TRACE_SYSTEM xe looking forward to the day this will be intel_display or i915_display, but until then Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> is tracpoints above intentional? I'd say it's a typo, but it's repeated 4 times. Lucas De Marchi >+#endif > > #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ) > #define __INTEL_DISPLAY_TRACE_H__ >-- >2.45.3 >
On Mon, 27 Jan 2025, "Cavitt, Jonathan" <jonathan.cavitt@intel.com> wrote: > Though it would probably be good to file an issue report detailing > the issue this fixes, then mark this patch as having fixed that > reported issue. While I occasionally do request that when we get patches out of left field, and I want to ensure we have a full understanding of the problem, please let's not add extra hurdles for ourselves for fixing stuff. BR, Jani.
On Mon, Jan 27, 2025 at 05:00:38PM -0600, Lucas De Marchi wrote: > On Mon, Jan 27, 2025 at 11:30:55PM +0200, Ville Syrjälä wrote: > >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > >Currently we just define the display tracpoints with > >TRACE_SYSTEM i915. However the code gets included separately > >in i915 and xe, and now both modules are competing for the > >same tracpoints. 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 tracpoints so that > >things work even when both modules are loaded. > > > >This one had me stumped for a bit when after a reboot I lost > >all i915 display tracpoints (on account of the new kernel > >also including xe, and something also ended up loading it > >before I manually loaded i915). > > > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >--- > > drivers/gpu/drm/i915/display/intel_display_trace.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > >diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h > >index 54a6e2a46b82..0e10c2856058 100644 > >--- a/drivers/gpu/drm/i915/display/intel_display_trace.h > >+++ b/drivers/gpu/drm/i915/display/intel_display_trace.h > >@@ -4,7 +4,11 @@ > > */ > > > > #undef TRACE_SYSTEM > >+#ifdef I915 > > #define TRACE_SYSTEM i915 > >+#else > >+#define TRACE_SYSTEM xe > > looking forward to the day this will be intel_display or i915_display, intel_display might be the right choice at that point, but yeah can't go there yet. > but until then > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Ta. > > is tracpoints above intentional? I'd say it's a typo, but it's repeated > 4 times. Apparently just bad muscle memory on my part. > > Lucas De Marchi > > >+#endif > > > > #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ) > > #define __INTEL_DISPLAY_TRACE_H__ > >-- > >2.45.3 > >
On Mon, Jan 27, 2025 at 09:40:12PM +0000, Cavitt, Jonathan wrote: > -----Original Message----- > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala > Sent: Monday, January 27, 2025 1:31 PM > To: intel-gfx@lists.freedesktop.org > Cc: intel-xe@lists.freedesktop.org > Subject: [PATCH] drm/i915: Give i915 and xe each their own display tracepoints > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Currently we just define the display tracpoints with > > TRACE_SYSTEM i915. However the code gets included separately > > in i915 and xe, and now both modules are competing for the > > same tracpoints. 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 tracpoints so that > > things work even when both modules are loaded. > > > > This one had me stumped for a bit when after a reboot I lost > > all i915 display tracpoints (on account of the new kernel > > also including xe, and something also ended up loading it > > before I manually loaded i915). > > s/tracpoints/tracepoints > > Also, the last sentence in this commit message probably isn't > necessary, but it doesn't detract from anything, so I won't block > on its removal. Yeah, probably doesn't add anything of value. Dropped. > Just fix the tracepoints spelling and this is: > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Thanks.
diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h index 54a6e2a46b82..0e10c2856058 100644 --- a/drivers/gpu/drm/i915/display/intel_display_trace.h +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h @@ -4,7 +4,11 @@ */ #undef TRACE_SYSTEM +#ifdef I915 #define TRACE_SYSTEM i915 +#else +#define TRACE_SYSTEM xe +#endif #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ) #define __INTEL_DISPLAY_TRACE_H__