From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Bommithi Sakeena <bommithi.sakeena@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>,
intel-xe@lists.freedesktop.org
Subject: Re: [Intel-xe] [PATCH] drm/xe: Encapsulate all the module parameters
Date: Tue, 28 Nov 2023 10:58:36 -0500 [thread overview]
Message-ID: <ZWYOLHSEn4rI/Or9@intel.com> (raw)
In-Reply-To: <20231117160618.4134-1-bommithi.sakeena@intel.com>
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@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Bommithi Sakeena <bommithi.sakeena@intel.com>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@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
>
next prev parent reply other threads:[~2023-11-28 15:58 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-17 16:06 [Intel-xe] [PATCH] drm/xe: Encapsulate all the module parameters Bommithi Sakeena
2023-11-20 12:46 ` [Intel-xe] ✓ CI.Patch_applied: success for drm/xe: Encapsulate all the module parameters (rev5) Patchwork
2023-11-20 12:46 ` [Intel-xe] ✓ CI.checkpatch: " Patchwork
2023-11-20 12:48 ` [Intel-xe] ✓ CI.KUnit: " Patchwork
2023-11-20 12:55 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-11-20 12:55 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-11-20 12:56 ` [Intel-xe] ✓ CI.checksparse: " Patchwork
2023-11-20 13:32 ` [Intel-xe] ✓ CI.BAT: " Patchwork
2023-11-28 15:58 ` Rodrigo Vivi [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-11-09 17:47 [Intel-xe] [PATCH] drm/xe: Encapsulate all the module parameters Bommithi Sakeena
2023-11-13 14:23 ` Lucas De Marchi
2023-11-13 19:52 ` Rodrigo Vivi
2023-11-02 19:51 Bommithi Sakeena
2023-11-03 3:48 ` Lucas De Marchi
2023-11-03 15:55 ` Rodrigo Vivi
2023-10-11 17:42 Bommithi Sakeena
2023-10-12 9:16 ` Jani Nikula
2023-10-12 14:08 ` Lucas De Marchi
2023-10-12 14:15 ` Jani Nikula
2023-10-13 20:03 ` Rodrigo Vivi
2023-10-13 22:36 ` Lucas De Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZWYOLHSEn4rI/Or9@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=bommithi.sakeena@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.