Re: [PATCH v2 11/11] drm/i915: Extract i915_module.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 27/07/2021 13:10, 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.

i915_module.c is an improvement and the rest for me is not extremely objectionable by the end of this incarnation, but I also do not see it as an improvement really.

There was a bug to fix relating to mock tests, but that is where the exercise should have stopped for now. After that it IMHO spiraled out of control, not least the unjustifiably expedited removal of cache shrinking. On balance for me it is too churny and boils down to two extremely capable people spending time on kind of really unimportant side fiddles. And I do not intend to prescribe you what to do, just expressing my bewilderment. FWIW... I can only say my opinion as it, not that it matters a lot.

Regards,

Tvrtko

Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
---
  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);




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux