On Tue, Jul 27, 2021 at 02:10:37PM +0200, Daniel Vetter wrote: > The module init code is somewhat misplaced in i915_pci.c, since it > needs to pull in init/exit functions from every part of the driver and > pollutes the include list a lot. > > Extract an i915_module.c file which pulls all the bits together, and > allows us to massively trim the include list of i915_pci.c. > > The downside is that have to drop the error path check Jason added to > catch when we set up the pci driver too early. I think that risk is > acceptable for this pretty nice include. > > Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > Reviewed-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx> With gcc and CONFIG_GCC_PLUGIN_RANDSTRUCT=y, this patch results in: drivers/gpu/drm/i915/i915_module.c:50:11: error: positional initialization of field in 'struct' declared with 'designated_init' attribute This is seen for each of the initializers. Guenter > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/i915_module.c | 113 ++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_pci.c | 117 +---------------------------- > drivers/gpu/drm/i915/i915_pci.h | 8 ++ > 4 files changed, 125 insertions(+), 114 deletions(-) > create mode 100644 drivers/gpu/drm/i915/i915_module.c > create mode 100644 drivers/gpu/drm/i915/i915_pci.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 9022dc638ed6..4ebd9f417ddb 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -38,6 +38,7 @@ i915-y += i915_drv.o \ > i915_irq.o \ > i915_getparam.o \ > i915_mitigations.o \ > + i915_module.o \ > i915_params.o \ > i915_pci.o \ > i915_scatterlist.o \ > diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c > new file mode 100644 > index 000000000000..c578ea8f56a0 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_module.c > @@ -0,0 +1,113 @@ > +/* > + * SPDX-License-Identifier: MIT > + * > + * Copyright © 2021 Intel Corporation > + */ > + > +#include <linux/console.h> > + > +#include "gem/i915_gem_context.h" > +#include "gem/i915_gem_object.h" > +#include "i915_active.h" > +#include "i915_buddy.h" > +#include "i915_params.h" > +#include "i915_pci.h" > +#include "i915_perf.h" > +#include "i915_request.h" > +#include "i915_scheduler.h" > +#include "i915_selftest.h" > +#include "i915_vma.h" > + > +static int i915_check_nomodeset(void) > +{ > + bool use_kms = true; > + > + /* > + * Enable KMS by default, unless explicitly overriden by > + * either the i915.modeset prarameter or by the > + * vga_text_mode_force boot option. > + */ > + > + if (i915_modparams.modeset == 0) > + use_kms = false; > + > + if (vgacon_text_force() && i915_modparams.modeset == -1) > + use_kms = false; > + > + if (!use_kms) { > + /* Silently fail loading to not upset userspace. */ > + DRM_DEBUG_DRIVER("KMS disabled.\n"); > + return 1; > + } > + > + return 0; > +} > + > +static const struct { > + int (*init)(void); > + void (*exit)(void); > +} init_funcs[] = { > + { i915_check_nomodeset, NULL }, > + { i915_active_module_init, i915_active_module_exit }, > + { i915_buddy_module_init, i915_buddy_module_exit }, > + { i915_context_module_init, i915_context_module_exit }, > + { i915_gem_context_module_init, i915_gem_context_module_exit }, > + { i915_objects_module_init, i915_objects_module_exit }, > + { i915_request_module_init, i915_request_module_exit }, > + { i915_scheduler_module_init, i915_scheduler_module_exit }, > + { i915_vma_module_init, i915_vma_module_exit }, > + { i915_mock_selftests, NULL }, > + { i915_pmu_init, i915_pmu_exit }, > + { i915_register_pci_driver, i915_unregister_pci_driver }, > + { i915_perf_sysctl_register, i915_perf_sysctl_unregister }, > +}; > +static int init_progress; > + > +static int __init i915_init(void) > +{ > + int err, i; > + > + for (i = 0; i < ARRAY_SIZE(init_funcs); i++) { > + err = init_funcs[i].init(); > + if (err < 0) { > + while (i--) { > + if (init_funcs[i].exit) > + init_funcs[i].exit(); > + } > + return err; > + } else if (err > 0) { > + /* > + * Early-exit success is reserved for things which > + * don't have an exit() function because we have no > + * idea how far they got or how to partially tear > + * them down. > + */ > + WARN_ON(init_funcs[i].exit); > + break; > + } > + } > + > + init_progress = i; > + > + return 0; > +} > + > +static void __exit i915_exit(void) > +{ > + int i; > + > + for (i = init_progress - 1; i >= 0; i--) { > + GEM_BUG_ON(i >= ARRAY_SIZE(init_funcs)); > + if (init_funcs[i].exit) > + init_funcs[i].exit(); > + } > +} > + > +module_init(i915_init); > +module_exit(i915_exit); > + > +MODULE_AUTHOR("Tungsten Graphics, Inc."); > +MODULE_AUTHOR("Intel Corporation"); > + > +MODULE_DESCRIPTION(DRIVER_DESC); > +MODULE_LICENSE("GPL and additional rights"); > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index b4f5e88aaae6..08651ca03478 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -22,24 +22,13 @@ > * > */ > > -#include <linux/console.h> > #include <linux/vga_switcheroo.h> > > #include <drm/drm_drv.h> > #include <drm/i915_pciids.h> > > -#include "display/intel_fbdev.h" > - > -#include "i915_active.h" > -#include "i915_buddy.h" > #include "i915_drv.h" > -#include "gem/i915_gem_context.h" > -#include "gem/i915_gem_object.h" > -#include "i915_request.h" > -#include "i915_perf.h" > -#include "i915_selftest.h" > -#include "i915_scheduler.h" > -#include "i915_vma.h" > +#include "i915_pci.h" > > #define PLATFORM(x) .platform = (x) > #define GEN(x) \ > @@ -1251,31 +1240,6 @@ static void i915_pci_shutdown(struct pci_dev *pdev) > i915_driver_shutdown(i915); > } > > -static int i915_check_nomodeset(void) > -{ > - bool use_kms = true; > - > - /* > - * Enable KMS by default, unless explicitly overriden by > - * either the i915.modeset prarameter or by the > - * vga_text_mode_force boot option. > - */ > - > - if (i915_modparams.modeset == 0) > - use_kms = false; > - > - if (vgacon_text_force() && i915_modparams.modeset == -1) > - use_kms = false; > - > - if (!use_kms) { > - /* Silently fail loading to not upset userspace. */ > - DRM_DEBUG_DRIVER("KMS disabled.\n"); > - return 1; > - } > - > - return 0; > -} > - > static struct pci_driver i915_pci_driver = { > .name = DRIVER_NAME, > .id_table = pciidlist, > @@ -1285,87 +1249,12 @@ static struct pci_driver i915_pci_driver = { > .driver.pm = &i915_pm_ops, > }; > > -static int i915_register_pci_driver(void) > +int i915_register_pci_driver(void) > { > return pci_register_driver(&i915_pci_driver); > } > > -static void i915_unregister_pci_driver(void) > +void i915_unregister_pci_driver(void) > { > pci_unregister_driver(&i915_pci_driver); > } > - > -static const struct { > - int (*init)(void); > - void (*exit)(void); > -} init_funcs[] = { > - { i915_check_nomodeset, NULL }, > - { i915_active_module_init, i915_active_module_exit }, > - { i915_buddy_module_init, i915_buddy_module_exit }, > - { i915_context_module_init, i915_context_module_exit }, > - { i915_gem_context_module_init, i915_gem_context_module_exit }, > - { i915_objects_module_init, i915_objects_module_exit }, > - { i915_request_module_init, i915_request_module_exit }, > - { i915_scheduler_module_init, i915_scheduler_module_exit }, > - { i915_vma_module_init, i915_vma_module_exit }, > - { i915_mock_selftests, NULL }, > - { i915_pmu_init, i915_pmu_exit }, > - { i915_register_pci_driver, i915_unregister_pci_driver }, > - { i915_perf_sysctl_register, i915_perf_sysctl_unregister }, > -}; > -static int init_progress; > - > -static int __init i915_init(void) > -{ > - int err, i; > - > - for (i = 0; i < ARRAY_SIZE(init_funcs); i++) { > - err = init_funcs[i].init(); > - if (err < 0) { > - while (i--) { > - if (init_funcs[i].exit) > - init_funcs[i].exit(); > - } > - return err; > - } else if (err > 0) { > - /* > - * Early-exit success is reserved for things which > - * don't have an exit() function because we have no > - * idea how far they got or how to partially tear > - * them down. > - */ > - WARN_ON(init_funcs[i].exit); > - > - /* > - * We don't want to advertise devices with an only > - * partially initialized driver. > - */ > - WARN_ON(i915_pci_driver.driver.owner); > - break; > - } > - } > - > - init_progress = i; > - > - return 0; > -} > - > -static void __exit i915_exit(void) > -{ > - int i; > - > - for (i = init_progress - 1; i >= 0; i--) { > - GEM_BUG_ON(i >= ARRAY_SIZE(init_funcs)); > - if (init_funcs[i].exit) > - init_funcs[i].exit(); > - } > -} > - > -module_init(i915_init); > -module_exit(i915_exit); > - > -MODULE_AUTHOR("Tungsten Graphics, Inc."); > -MODULE_AUTHOR("Intel Corporation"); > - > -MODULE_DESCRIPTION(DRIVER_DESC); > -MODULE_LICENSE("GPL and additional rights"); > diff --git a/drivers/gpu/drm/i915/i915_pci.h b/drivers/gpu/drm/i915/i915_pci.h > new file mode 100644 > index 000000000000..b386f319f52e > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_pci.h > @@ -0,0 +1,8 @@ > +/* > + * SPDX-License-Identifier: MIT > + * > + * Copyright © 2021 Intel Corporation > + */ > + > +int i915_register_pci_driver(void); > +void i915_unregister_pci_driver(void);