[Intel-xe] [PATCH 2/6] drm/xe: place all modprobe parameters at the same place

[Jani Nikula]

On Fri, 03 Feb 2023, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> From: Mauro Carvalho Chehab <mchehab at kernel.org>
>
> It makes easier to identify the module parameters if they're
> placed it at the same place.
>
> Place them together with the module author/description/license.
>
> While here, fix a checkpatch.pl warning on force_probe description:
> 	WARNING: quoted string split across lines
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab at kernel.org>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

I don't really see why this needs to reinvent the wheel when we could
use the same model as i915. Separate file, all params in a struct that
provides a namespace for the params. It's hideous to have them all in
the driver local namespace.

> ---
>  drivers/gpu/drm/xe/xe_device.c  | 10 +---------
>  drivers/gpu/drm/xe/xe_guc_log.c |  5 +----
>  drivers/gpu/drm/xe/xe_mmio.c    |  5 +----
>  drivers/gpu/drm/xe/xe_module.c  | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_module.h  | 13 +++++++++++++
>  drivers/gpu/drm/xe/xe_pci.c     |  7 +------
>  6 files changed, 39 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_module.h
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 2e1f4beba9b0..60938c2deee2 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -20,6 +20,7 @@
>  #include "xe_exec.h"
>  #include "xe_gt.h"
>  #include "xe_irq.h"
> +#include "xe_module.h"
>  #include "xe_mmio.h"
>  #include "xe_pcode.h"
>  #include "xe_pm.h"
> @@ -42,15 +43,6 @@
>  #include "display/ext/intel_pm.h"
>  #endif
>  
> -/* FIXME: Move to common param infrastructure */
> -static bool enable_guc = true;
> -module_param_named_unsafe(enable_guc, enable_guc, bool, 0444);
> -MODULE_PARM_DESC(enable_guc, "Enable GuC submission");
> -
> -static bool enable_display = true;
> -module_param_named(enable_display, enable_display, bool, 0444);
> -MODULE_PARM_DESC(enable_display, "Enable display");
> -
>  static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>  {
>  	struct xe_file *xef;
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 3f19fbf243d1..7ec1b2bb1f8e 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -9,10 +9,7 @@
>  #include "xe_gt.h"
>  #include "xe_guc_log.h"
>  #include "xe_map.h"
> -
> -static int xe_guc_log_level = 5;
> -module_param_named(guc_log_level, xe_guc_log_level, int, 0600);
> -MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (0=disable, 1..5=enable with verbosity min..max)");
> +#include "xe_module.h"
>  
>  static struct xe_gt *
>  log_to_gt(struct xe_guc_log *log)
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index f20734cf15ba..8a953df2b468 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -12,6 +12,7 @@
>  #include "xe_gt.h"
>  #include "xe_gt_mcr.h"
>  #include "xe_macros.h"
> +#include "xe_module.h"
>  
>  #include "i915_reg.h"
>  #include "gt/intel_engine_regs.h"
> @@ -21,10 +22,6 @@
>  #define TILE_COUNT		REG_GENMASK(15, 8)
>  #define GEN12_LMEM_BAR		2
>  
> -static u32 xe_force_lmem_bar_size;
> -module_param_named(lmem_bar_size, xe_force_lmem_bar_size, uint, 0600);
> -MODULE_PARM_DESC(lmem_bar_size, "Set the lmem bar size(in MiB)");
> -
>  static int xe_set_dma_info(struct xe_device *xe)
>  {
>  	unsigned int mask_size = xe->info.dma_mask_size;
> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
> index d6b50f1c2a05..9cd1663f83f6 100644
> --- a/drivers/gpu/drm/xe/xe_module.c
> +++ b/drivers/gpu/drm/xe/xe_module.c
> @@ -8,9 +8,31 @@
>  
>  #include "xe_drv.h"
>  #include "xe_hw_fence.h"
> +#include "xe_module.h"
>  #include "xe_pci.h"
>  #include "xe_sched_job.h"
>  
> +bool enable_guc = true;
> +module_param_named_unsafe(enable_guc, enable_guc, bool, 0444);
> +MODULE_PARM_DESC(enable_guc, "Enable GuC submission");
> +
> +bool enable_display = true;
> +module_param_named(enable_display, enable_display, bool, 0444);
> +MODULE_PARM_DESC(enable_display, "Enable display");
> +
> +u32 xe_force_lmem_bar_size;
> +module_param_named(lmem_bar_size, xe_force_lmem_bar_size, uint, 0600);
> +MODULE_PARM_DESC(lmem_bar_size, "Set the lmem bar size(in MiB)");
> +
> +int xe_guc_log_level = 5;
> +module_param_named(guc_log_level, xe_guc_log_level, int, 0600);
> +MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (0=disable, 1..5=enable with verbosity min..max)");
> +
> +char *xe_param_force_probe = CONFIG_DRM_XE_FORCE_PROBE;
> +module_param_named_unsafe(force_probe, xe_param_force_probe, charp, 0400);
> +MODULE_PARM_DESC(force_probe,
> +		 "Force probe options for specified devices. See CONFIG_DRM_XE_FORCE_PROBE for details.");
> +
>  struct init_funcs {
>  	int (*init)(void);
>  	void (*exit)(void);
> diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h
> new file mode 100644
> index 000000000000..2c6ee46f5595
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_module.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include <linux/init.h>
> +
> +/* Module modprobe variables */
> +extern bool enable_guc;
> +extern bool enable_display;
> +extern u32 xe_force_lmem_bar_size;
> +extern int xe_guc_log_level;
> +extern char *xe_param_force_probe;
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 1d5b6afed2c3..20aa2b5ca9ac 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -17,17 +17,12 @@
>  #include "xe_drv.h"
>  #include "xe_device.h"
>  #include "xe_macros.h"
> +#include "xe_module.h"
>  #include "xe_pm.h"
>  #include "xe_step.h"
>  
>  #include "i915_reg.h"
>  
> -static char *xe_param_force_probe = CONFIG_DRM_XE_FORCE_PROBE;
> -module_param_named_unsafe(force_probe, xe_param_force_probe, charp, 0400);
> -MODULE_PARM_DESC(force_probe,
> -		 "Force probe options for specified devices. "
> -		 "See CONFIG_DRM_XE_FORCE_PROBE for details.");
> -
>  #define DEV_INFO_FOR_EACH_FLAG(func) \
>  	func(require_force_probe); \
>  	func(is_dgfx); \

-- 
Jani Nikula, Intel Open Source Graphics Center