[Intel-xe] [PATCH] drm/xe: Encapsulate all the module parameters

Rodrigo Vivi rodrigo.vivi at intel.com
Tue Nov 28 15:58:36 UTC 2023


On Fri, Nov 17, 2023 at 04:06:18PM +0000, Bommithi Sakeena wrote:
> Encapsulate all the module parameters in one single global struct
> variable. This also removes the extra xe_module.h from includes.
> 
> v2: naming consistency as suggested by Jani and Lucas
> v3: fix checkpatch errors/warnings
> v4: adding blank line after struct declaration
> 
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: Bommithi Sakeena <bommithi.sakeena at intel.com>
> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

pushed to drm-xe-next, thanks for the patch and reviews

> ---
>  drivers/gpu/drm/xe/xe_device.c  |  2 +-
>  drivers/gpu/drm/xe/xe_display.c |  4 ++--
>  drivers/gpu/drm/xe/xe_guc_log.c |  2 +-
>  drivers/gpu/drm/xe/xe_mmio.c    |  2 +-
>  drivers/gpu/drm/xe/xe_module.c  | 29 ++++++++++++++---------------
>  drivers/gpu/drm/xe/xe_module.h  | 24 +++++++++++++++++-------
>  drivers/gpu/drm/xe/xe_pci.c     |  6 +++---
>  drivers/gpu/drm/xe/xe_uc_fw.c   |  6 +++---
>  8 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 1202f8007f79..f956846c1330 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -225,7 +225,7 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
>  
>  	xe->info.devid = pdev->device;
>  	xe->info.revid = pdev->revision;
> -	xe->info.force_execlist = force_execlist;
> +	xe->info.force_execlist = xe_modparam.force_execlist;
>  
>  	spin_lock_init(&xe->irq.lock);
>  
> diff --git a/drivers/gpu/drm/xe/xe_display.c b/drivers/gpu/drm/xe/xe_display.c
> index da10f16e1c12..74391d9b11ae 100644
> --- a/drivers/gpu/drm/xe/xe_display.c
> +++ b/drivers/gpu/drm/xe/xe_display.c
> @@ -45,7 +45,7 @@ static bool has_display(struct xe_device *xe)
>   */
>  bool xe_display_driver_probe_defer(struct pci_dev *pdev)
>  {
> -	if (!enable_display)
> +	if (!xe_modparam.enable_display)
>  		return 0;
>  
>  	return intel_display_driver_probe_defer(pdev);
> @@ -69,7 +69,7 @@ static void xe_display_last_close(struct drm_device *dev)
>   */
>  void xe_display_driver_set_hooks(struct drm_driver *driver)
>  {
> -	if (!enable_display)
> +	if (!xe_modparam.enable_display)
>  		return;
>  
>  	driver->driver_features |= DRIVER_MODESET | DRIVER_ATOMIC;
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 45c60a9c631c..27c3827bfd05 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -100,7 +100,7 @@ int xe_guc_log_init(struct xe_guc_log *log)
>  
>  	xe_map_memset(xe, &bo->vmap, 0, 0, guc_log_size());
>  	log->bo = bo;
> -	log->level = xe_guc_log_level;
> +	log->level = xe_modparam.guc_log_level;
>  
>  	err = drmm_add_action_or_reset(&xe->drm, guc_log_fini, log);
>  	if (err)
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index d8f9fabf715e..37a6861e83c7 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -73,7 +73,7 @@ _resize_bar(struct xe_device *xe, int resno, resource_size_t size)
>   */
>  static void xe_resize_vram_bar(struct xe_device *xe)
>  {
> -	u64 force_vram_bar_size = xe_force_vram_bar_size;
> +	u64 force_vram_bar_size = xe_modparam.force_vram_bar_size;
>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>  	struct pci_bus *root = pdev->bus;
>  	resource_size_t current_size;
> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
> index 7194595e7f31..1ea883f48c63 100644
> --- a/drivers/gpu/drm/xe/xe_module.c
> +++ b/drivers/gpu/drm/xe/xe_module.c
> @@ -10,39 +10,38 @@
>  
>  #include "xe_drv.h"
>  #include "xe_hw_fence.h"
> -#include "xe_module.h"
>  #include "xe_pci.h"
>  #include "xe_pmu.h"
>  #include "xe_sched_job.h"
>  
> -bool force_execlist = false;
> -module_param_named_unsafe(force_execlist, force_execlist, bool, 0444);
> +struct xe_modparam xe_modparam = {
> +	.enable_display = true,
> +	.guc_log_level = 5,
> +	.force_probe = CONFIG_DRM_XE_FORCE_PROBE,
> +	/* the rest are 0 by default */
> +};
> +
> +module_param_named_unsafe(force_execlist, xe_modparam.force_execlist, bool, 0444);
>  MODULE_PARM_DESC(force_execlist, "Force Execlist submission");
>  
> -bool enable_display = true;
> -module_param_named(enable_display, enable_display, bool, 0444);
> +module_param_named(enable_display, xe_modparam.enable_display, bool, 0444);
>  MODULE_PARM_DESC(enable_display, "Enable display");
>  
> -u32 xe_force_vram_bar_size;
> -module_param_named(vram_bar_size, xe_force_vram_bar_size, uint, 0600);
> +module_param_named(vram_bar_size, xe_modparam.force_vram_bar_size, uint, 0600);
>  MODULE_PARM_DESC(vram_bar_size, "Set the vram bar size(in MiB)");
>  
> -int xe_guc_log_level = 5;
> -module_param_named(guc_log_level, xe_guc_log_level, int, 0600);
> +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..5=enable with verbosity min..max)");
>  
> -char *xe_guc_firmware_path;
> -module_param_named_unsafe(guc_firmware_path, xe_guc_firmware_path, charp, 0400);
> +module_param_named_unsafe(guc_firmware_path, xe_modparam.guc_firmware_path, charp, 0400);
>  MODULE_PARM_DESC(guc_firmware_path,
>  		 "GuC firmware path to use instead of the default one");
>  
> -char *xe_huc_firmware_path;
> -module_param_named_unsafe(huc_firmware_path, xe_huc_firmware_path, charp, 0400);
> +module_param_named_unsafe(huc_firmware_path, xe_modparam.huc_firmware_path, charp, 0400);
>  MODULE_PARM_DESC(huc_firmware_path,
>  		 "HuC firmware path to use instead of the default one - empty string disables");
>  
> -char *xe_param_force_probe = CONFIG_DRM_XE_FORCE_PROBE;
> -module_param_named_unsafe(force_probe, xe_param_force_probe, charp, 0400);
> +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.");
>  
> diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h
> index e1da1e9ca5cb..51d75ff12376 100644
> --- a/drivers/gpu/drm/xe/xe_module.h
> +++ b/drivers/gpu/drm/xe/xe_module.h
> @@ -3,13 +3,23 @@
>   * Copyright © 2023 Intel Corporation
>   */
>  
> +#ifndef _XE_MODULE_H_
> +#define _XE_MODULE_H_
> +
>  #include <linux/types.h>
>  
>  /* Module modprobe variables */
> -extern bool force_execlist;
> -extern bool enable_display;
> -extern u32 xe_force_vram_bar_size;
> -extern int xe_guc_log_level;
> -extern char *xe_guc_firmware_path;
> -extern char *xe_huc_firmware_path;
> -extern char *xe_param_force_probe;
> +struct xe_modparam {
> +	bool force_execlist;
> +	bool enable_display;
> +	u32 force_vram_bar_size;
> +	int guc_log_level;
> +	char *guc_firmware_path;
> +	char *huc_firmware_path;
> +	char *force_probe;
> +};
> +
> +extern struct xe_modparam xe_modparam;
> +
> +#endif
> +
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 682ba188e456..d56a89da7ff3 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -418,12 +418,12 @@ static bool device_id_in_list(u16 device_id, const char *devices, bool negative)
>  
>  static bool id_forced(u16 device_id)
>  {
> -	return device_id_in_list(device_id, xe_param_force_probe, false);
> +	return device_id_in_list(device_id, xe_modparam.force_probe, false);
>  }
>  
>  static bool id_blocked(u16 device_id)
>  {
> -	return device_id_in_list(device_id, xe_param_force_probe, true);
> +	return device_id_in_list(device_id, xe_modparam.force_probe, true);
>  }
>  
>  static const struct xe_subplatform_desc *
> @@ -592,7 +592,7 @@ static int xe_info_init(struct xe_device *xe,
>  	xe->info.has_range_tlb_invalidation = graphics_desc->has_range_tlb_invalidation;
>  
>  	xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
> -				  enable_display &&
> +				  xe_modparam.enable_display &&
>  				  desc->has_display;
>  	/*
>  	 * All platforms have at least one primary GT.  Any platform with media
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index 3032c4f148d4..25778887d846 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -220,11 +220,11 @@ uc_fw_override(struct xe_uc_fw *uc_fw)
>  	/* empty string disables, but it's not allowed for GuC */
>  	switch (uc_fw->type) {
>  	case XE_UC_FW_TYPE_GUC:
> -		if (xe_guc_firmware_path && *xe_guc_firmware_path)
> -			path_override = xe_guc_firmware_path;
> +		if (xe_modparam.guc_firmware_path && *xe_modparam.guc_firmware_path)
> +			path_override = xe_modparam.guc_firmware_path;
>  		break;
>  	case XE_UC_FW_TYPE_HUC:
> -		path_override = xe_huc_firmware_path;
> +		path_override = xe_modparam.huc_firmware_path;
>  		break;
>  	default:
>  		break;
> -- 
> 2.34.1
> 


More information about the Intel-xe mailing list