[PATCH v2 3/3] drm/xe: Normalize default param values
John Harrison
john.c.harrison at intel.com
Mon Jun 16 19:14:51 UTC 2025
On 6/16/2025 11:52 AM, Lucas De Marchi wrote:
> On Mon, Jun 16, 2025 at 11:46:57AM -0700, John Harrison wrote:
>> On 6/13/2025 1:00 PM, Lucas De Marchi wrote:
>>> Document xe module params with the default values following a similar
>>> strategy for all of them:
>>>
>>> 1) Define a DEFAULT_* macro with the default value. When the
>>> value can't be directly stringified, also define a *_STR
>>> variant
>>> 2) Use __stringify() or the _STR variant to make sure the
>>> default value shows up in the param description
>>>
>>> This allows us to show the correct default according to the
>>> configuration. max_vfs for example was wrongly documented for
>>> CONFIG_DRM_XE_DEBUG and svm_notifier_size didn't have its default
>>> documented.
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>> ---
>>> drivers/gpu/drm/xe/xe_module.c | 44
>>> +++++++++++++++++++++++++++---------------
>>> 1 file changed, 28 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_module.c
>>> b/drivers/gpu/drm/xe/xe_module.c
>>> index 192908fa074cb..2068c579d5c3b 100644
>>> --- a/drivers/gpu/drm/xe/xe_module.c
>>> +++ b/drivers/gpu/drm/xe/xe_module.c
>>> @@ -19,34 +19,43 @@
>>> #include "xe_sched_job.h"
>>> #if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
>>> -#define DEFAULT_GUC_LOG_LEVEL 3
>>> +#define DEFAULT_GUC_LOG_LEVEL 3
>>> +#define DEFAULT_MAX_VFS ~0
>>> +#define DEFAULT_MAX_VFS_STR "unlimited"
>>> #else
>>> -#define DEFAULT_GUC_LOG_LEVEL 1
>>> +#define DEFAULT_GUC_LOG_LEVEL 1
>>> +#define DEFAULT_MAX_VFS 0
>>> +#define DEFAULT_MAX_VFS_STR "0"
>> Could be 'stringify(MAX_VFS)'.
>
> I think it's better to pair with the one above.
> We don't want to print the the default is ~0 in the other case so we
> create a manual _STR.
I meant:
#define DEFAULT_MAX_VFS ~0
#define DEFAULT_MAX_VFS_STR "unlimited"
#else
#define DEFAULT_MAX_VFS 0
#define DEFAULT_MAX_VFS_STR __stringify(DEFAULT_MAX_VFS)
John.
>
>
>>
>>> #endif
>>> +#define DEFAULT_PROBE_DISPLAY true
>>> +#define DEFAULT_FORCE_PROBE CONFIG_DRM_XE_FORCE_PROBE
>>> +#define DEFAULT_WEDGED_MODE 1
>>> +#define DEFAULT_SVM_NOTIFIER_SIZE 512
>>> +
>>> struct xe_modparam xe_modparam = {
>>> - .probe_display = true,
>>> - .guc_log_level = DEFAULT_GUC_LOG_LEVEL,
>>> - .force_probe = CONFIG_DRM_XE_FORCE_PROBE,
>>> -#ifdef CONFIG_PCI_IOV
>> This ifdef is lost from the new version?
>
> yeah, dropped it by mistake... will need to add it back.
>>
>>> - .max_vfs = IS_ENABLED(CONFIG_DRM_XE_DEBUG) ? ~0 : 0,
>>> -#endif
>>> - .wedged_mode = 1,
>>> - .svm_notifier_size = 512,
>>> + .probe_display = DEFAULT_PROBE_DISPLAY,
>>> + .guc_log_level = DEFAULT_GUC_LOG_LEVEL,
>>> + .force_probe = DEFAULT_FORCE_PROBE,
>>> + .max_vfs = DEFAULT_MAX_VFS,
>>> + .wedged_mode = DEFAULT_WEDGED_MODE,
>>> + .svm_notifier_size = DEFAULT_SVM_NOTIFIER_SIZE,
>>> /* the rest are 0 by default */
>>> };
>>> module_param_named(svm_notifier_size,
>>> xe_modparam.svm_notifier_size, uint, 0600);
>>> -MODULE_PARM_DESC(svm_notifier_size, "Set the svm notifier size(in
>>> MiB), must be power of 2");
>>> +MODULE_PARM_DESC(svm_notifier_size, "Set the svm notifier size in
>>> MiB, must be power of 2 "
>>> + "[default=" __stringify(DEFAULT_SVM_NOTIFIER_SIZE) "]");
>>> module_param_named_unsafe(force_execlist,
>>> xe_modparam.force_execlist, bool, 0444);
>>> MODULE_PARM_DESC(force_execlist, "Force Execlist submission");
>>> module_param_named(probe_display, xe_modparam.probe_display, bool,
>>> 0444);
>>> -MODULE_PARM_DESC(probe_display, "Probe display HW, otherwise it's
>>> left untouched (default: true)");
>>> +MODULE_PARM_DESC(probe_display, "Probe display HW, otherwise it's
>>> left untouched "
>>> + "[default=" __stringify(DEFAULT_PROBE_DISPLAY) "])");
>>> module_param_named(vram_bar_size, xe_modparam.force_vram_bar_size,
>>> int, 0600);
>>> -MODULE_PARM_DESC(vram_bar_size, "Set the vram bar size (in MiB) -
>>> <0=disable-resize, 0=max-needed-size[default], >0=force-size");
>>> +MODULE_PARM_DESC(vram_bar_size, "Set the vram bar size in MiB
>>> (<0=disable-resize, 0=max-needed-size, >0=force-size [default=0])");
>> Why leave this one as hard coded value rather than a define?
>
> there wasn't an assignment xe_modparam, so I overlooked it. I will fix
> those up in next rev
>
> thanks
> Lucas De Marchi
>
>
>>
>> John.
>>
>>> module_param_named(guc_log_level, xe_modparam.guc_log_level, int,
>>> 0600);
>>> MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level
>>> (0=disable, 1=normal, 2..5=verbose-levels "
>>> @@ -66,18 +75,21 @@ MODULE_PARM_DESC(gsc_firmware_path,
>>> module_param_named_unsafe(force_probe, xe_modparam.force_probe,
>>> charp, 0400);
>>> MODULE_PARM_DESC(force_probe,
>>> - "Force probe options for specified devices. See
>>> CONFIG_DRM_XE_FORCE_PROBE for details.");
>>> + "Force probe options for specified devices. See
>>> CONFIG_DRM_XE_FORCE_PROBE for details "
>>> + "[default=" DEFAULT_FORCE_PROBE "])");
>>> #ifdef CONFIG_PCI_IOV
>>> module_param_named(max_vfs, xe_modparam.max_vfs, uint, 0400);
>>> MODULE_PARM_DESC(max_vfs,
>>> "Limit number of Virtual Functions (VFs) that could be
>>> managed. "
>>> - "(0 = no VFs [default]; N = allow up to N VFs)");
>>> + "(0=no VFs; N=allow up to N VFs "
>>> + "[default=" DEFAULT_MAX_VFS_STR "])");
>>> #endif
>>> module_param_named_unsafe(wedged_mode, xe_modparam.wedged_mode,
>>> int, 0600);
>>> MODULE_PARM_DESC(wedged_mode,
>>> - "Module's default policy for the wedged mode - 0=never,
>>> 1=upon-critical-errors[default], 2=upon-any-hang");
>>> + "Module's default policy for the wedged mode (0=never,
>>> 1=upon-critical-errors, 2=upon-any-hang "
>>> + "[default=" __stringify(DEFAULT_WEDGED_MODE) "])");
>>> static int xe_check_nomodeset(void)
>>> {
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20250616/0b6d4c37/attachment-0001.htm>
More information about the Intel-xe
mailing list