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

[John Harrison <john.c.harrison@intel.com>]

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?

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'.

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
>>