Re: [PATCH 1/2] drm/xe/guc: Default log level to non-verbose

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Jun 12, 2025 at 11:26:08AM -0700, John Harrison wrote:
> On 6/12/2025 11:21 AM, Matthew Brost wrote:
> > On Thu, Jun 12, 2025 at 11:05:32AM -0700, Lucas De Marchi wrote:
> > > Currently xe sets the guc log level to a verbose level since it's useful
> > > to debug hangs and general development. However the verbose level may
> > > already be too much and affect performance.
> > > 
> > > Michal Mrozek did some tests with the L0 compute stack for submission
> > > latency with ULLS disabled. Below are the normalized numbers with log
> > > level 3 (the current default) as baseline for each test:
> > > 
> > >                            Test \ Log Level                        3      0      1      2
> > >   ----------------------------------------------------------- ------ ------ ------ ------
> > >    BestWalkerNthCommandListSubmission(CmdListCount=2)           1.00   0.63   0.63   0.96
> > >    BestWalkerNthSubmission(KernelCount=2)                       1.00   0.62   0.63   0.96
> > >    BestWalkerNthSubmissionImmediate(KernelCount=2)              1.00   0.58   0.58   0.85
> > >    BestWalkerSubmission                                         1.00   0.62   0.62   0.96
> > >    BestWalkerSubmissionImmediate                                1.00   0.63   0.62   0.96
> > >    BestWalkerSubmissionImmediateMultiCmdlists(cmdlistCount=2)   1.00   0.58   0.58   0.86
> > >    BestWalkerSubmissionImmediateMultiCmdlists(cmdlistCount=4)   1.00   0.70   0.70   0.83
> > >    BestWalkerSubmissionImmediateMultiCmdlists(cmdlistCount=8)   1.00   0.53   0.52   0.78
> > > 
> > > Log level 2 is the first "verbose level" for GuC, where the biggest
> > > difference happens. Keep log level 3 for CONFIG_DRM_XE_DEBUG, but switch
> > > to 1, i.e.  GUC_LOG_LEVEL_NON_VERBOSE, for "normal" builds.
> > > 
> > For series in general, I think this is a good idea - no need to slow
> > down performance for customers.
> > 
> > Downside is devcoredump from customers are going to have less
> > information. I'm wondering if the GuC log level is low, if we should
> > print something in devcoredump indicating this, and instructions on how
> We already include the log level in the GuC info section of the core dump
> (and the debugfs dump), I thought?
> 

Yes, GuC log level is in there.

> And note that the minimum log level is meant to include the really important
> logs. It certainly isn't as complete as higher levels but it is hopefully
> enough to give us a first thought of what is going on. And if we need more
> then it is easy to add a comment to the bug report saying 'please re-run
> with this extra setting'.
> 

Sure. Typically for really difficult problems you need ftrace (which is
not available either) in addition to the verbose GuC logs to really
figure out what went sideways. I guess I'd hope customers are not
hitting those types of bugs too.

Matt

> John.
> 
> > to turn up the GuC log level for filing verbose bug reports? e.g., A
> > similar message to what we print in xe_device_declare_wedged?
> > 
> > Matt
> > 
> > > Cc: Michal Mrozek <michal.mrozek@intel.com>
> > > Cc: John Harrison <John.C.Harrison@Intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_module.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
> > > index 1c4dfafbcd0bc..4809afa7ce3f9 100644
> > > --- a/drivers/gpu/drm/xe/xe_module.c
> > > +++ b/drivers/gpu/drm/xe/xe_module.c
> > > @@ -20,7 +20,7 @@
> > >   struct xe_modparam xe_modparam = {
> > >   	.probe_display = true,
> > > -	.guc_log_level = 3,
> > > +	.guc_log_level = IS_ENABLED(CONFIG_DRM_XE_DEBUG) ? 3 : 1,
> > >   	.force_probe = CONFIG_DRM_XE_FORCE_PROBE,
> > >   #ifdef CONFIG_PCI_IOV
> > >   	.max_vfs = IS_ENABLED(CONFIG_DRM_XE_DEBUG) ? ~0 : 0,
> > > 
> > > -- 
> > > 2.49.0
> > > 
>