[Intel-xe] [PATCH 2/6] drm/xe: place all modprobe parameters at the same place
Rodrigo Vivi
rodrigo.vivi at intel.com
Fri Feb 17 16:28:06 UTC 2023
On Thu, Feb 16, 2023 at 03:12:13PM +0200, Jani Nikula wrote:
> 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.
I was willing to avoid any special infra in order to discourage the
addition of many parameters.
But if we start growing we can definitely have the split and all.
Also, I'd like to avoid i915isms. if we need those macros and all like
we have in i915 we should probably get that into a common kernel
infra for debugfs because they would likely benefit other drivers as
well.
>
> > ---
> > 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
More information about the Intel-xe
mailing list