<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 6/16/2025 11:52 AM, Lucas De Marchi wrote: </div> <blockquote type="cite" cite="mid:jfkgpylqiujyudvqr4ivn3653arznibeofzqrdyhiy4fv3tgsk@tjf4hde4izg3">On Mon, Jun 16, 2025 at 11:46:57AM -0700, John Harrison wrote: <blockquote type="cite">On 6/13/2025 1:00 PM, Lucas De Marchi wrote: <blockquote type="cite">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 <a class="moz-txt-link-rfc2396E" href="mailto:lucas.demarchi@intel.com"><lucas.demarchi@intel.com></a> --- 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" </blockquote> Could be 'stringify(MAX_VFS)'. </blockquote> 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. </blockquote> I meant: <blockquote>#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) </blockquote> John. <blockquote type="cite" cite="mid:jfkgpylqiujyudvqr4ivn3653arznibeofzqrdyhiy4fv3tgsk@tjf4hde4izg3"> <blockquote type="cite"> <blockquote type="cite"> #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 </blockquote> This ifdef is lost from the new version? </blockquote> yeah, dropped it by mistake... will need to add it back. <blockquote type="cite"> <blockquote type="cite">- .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])"); </blockquote> Why leave this one as hard coded value rather than a define? </blockquote> there wasn't an assignment xe_modparam, so I overlooked it. I will fix those up in next rev thanks Lucas De Marchi <blockquote type="cite"> John. <blockquote type="cite"> 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) { </blockquote> </blockquote> </blockquote> </body> </html>