diff mbox

[3/6] drm/imx: Build monolithic driver

Message ID 1443114161-7965-3-git-send-email-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding Sept. 24, 2015, 5:02 p.m. UTC 
From: Thierry Reding <treding@nvidia.com>

There's no use building the individual drivers as separate modules
because they are all only useful if combined into a single DRM/KMS
device.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/imx/Kconfig            | 15 +++++++--------
 drivers/gpu/drm/imx/Makefile           | 17 +++++++----------
 drivers/gpu/drm/imx/dw_hdmi-imx.c      |  4 +---
 drivers/gpu/drm/imx/imx-drm-core.c     | 22 +++++++++++++++++++++-
 drivers/gpu/drm/imx/imx-drm.h          |  6 ++++++
 drivers/gpu/drm/imx/imx-ldb.c          |  4 +---
 drivers/gpu/drm/imx/imx-tve.c          |  4 +---
 drivers/gpu/drm/imx/ipuv3-crtc.c       |  3 +--
 drivers/gpu/drm/imx/parallel-display.c |  4 +---
 9 files changed, 46 insertions(+), 33 deletions(-)

Comments

Philipp Zabel Sept. 25, 2015, 7:16 a.m. UTC | #1 
Hi Thierry,

Am Donnerstag, den 24.09.2015, 19:02 +0200 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
> 
> There's no use building the individual drivers as separate modules
> because they are all only useful if combined into a single DRM/KMS
> device.

Why not? On an i.MX5 device with VGA(tve) output only, it is nice not to
have to load the i.MX6 HDMI driver.

regards
Philipp
Thierry Reding Sept. 25, 2015, 12:17 p.m. UTC | #2 
On Fri, Sep 25, 2015 at 09:16:06AM +0200, Philipp Zabel wrote:
> Hi Thierry,
> 
> Am Donnerstag, den 24.09.2015, 19:02 +0200 schrieb Thierry Reding:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > There's no use building the individual drivers as separate modules
> > because they are all only useful if combined into a single DRM/KMS
> > device.
> 
> Why not? On an i.MX5 device with VGA(tve) output only, it is nice not to
> have to load the i.MX6 HDMI driver.

I think you gain much less by splitting up than you realize. Compare
this from before the series:

	$ du -ch drivers/gpu/drm/imx/*.ko
	8.0K    drivers/gpu/drm/imx/dw_hdmi-imx.ko
	16K     drivers/gpu/drm/imx/imxdrm.ko
	16K     drivers/gpu/drm/imx/imx-ipuv3-crtc.ko
	12K     drivers/gpu/drm/imx/imx-ldb.ko
	12K     drivers/gpu/drm/imx/imx-tve.ko
	8.0K    drivers/gpu/drm/imx/parallel-display.ko
	72K     total

	$ du -ch drivers/gpu/drm/sti/*.ko
	36K     drivers/gpu/drm/sti/sticompositor.ko
	12K     drivers/gpu/drm/sti/sti_drv.ko
	16K     drivers/gpu/drm/sti/stidvo.ko
	16K     drivers/gpu/drm/sti/sti_hda.ko
	24K     drivers/gpu/drm/sti/stihdmi.ko
	8.0K    drivers/gpu/drm/sti/sti_hqvdp.ko
	12K     drivers/gpu/drm/sti/sti_tvout.ko
	8.0K    drivers/gpu/drm/sti/sti_vtac.ko
	8.0K    drivers/gpu/drm/sti/sti_vtg.ko
	140K    total

with this after the series:

	$ du -ch drivers/gpu/drm/imx/*.ko
	44K     drivers/gpu/drm/imx/imx-drm.ko
	44K     total

	$ du -ch drivers/gpu/drm/sti/*.ko
	92K     drivers/gpu/drm/sti/sti-drm.ko
	92K     total

There are other things to consider as well, such as the additional
memory overhead caused by merely having multiple modules, or all of the
exported functions that unnecessarily clutter up the symbol table, and
which end up slowing down every symbol lookup.

Thierry
Philipp Zabel Sept. 25, 2015, 1:09 p.m. UTC | #3 
Am Freitag, den 25.09.2015, 14:17 +0200 schrieb Thierry Reding:
> I think you gain much less by splitting up than you realize. Compare
> this from before the series:
> 
> 	$ du -ch drivers/gpu/drm/imx/*.ko
> 	8.0K    drivers/gpu/drm/imx/dw_hdmi-imx.ko

Oh right, I didn't realize that this is just the shim. The bulk of the
HDMI driver is in bridge/dw_hdmi.ko.

> 	16K     drivers/gpu/drm/imx/imxdrm.ko
> 	16K     drivers/gpu/drm/imx/imx-ipuv3-crtc.ko
> 	12K     drivers/gpu/drm/imx/imx-ldb.ko
> 	12K     drivers/gpu/drm/imx/imx-tve.ko
> 	8.0K    drivers/gpu/drm/imx/parallel-display.ko
> 	72K     total
[...]
> with this after the series:
> 
> 	$ du -ch drivers/gpu/drm/imx/*.ko
> 	44K     drivers/gpu/drm/imx/imx-drm.ko
> 	44K     total
[...]
> There are other things to consider as well, such as the additional
> memory overhead caused by merely having multiple modules, or all of the
> exported functions that unnecessarily clutter up the symbol table, and
> which end up slowing down every symbol lookup.

Thanks, can't argue with those numbers.
I'll queue patches 3 and 4 for imx-drm.

regards
Philipp
Philipp Zabel Sept. 25, 2015, 1:13 p.m. UTC | #4 
Am Freitag, den 25.09.2015, 15:09 +0200 schrieb Philipp Zabel:
> Am Freitag, den 25.09.2015, 14:17 +0200 schrieb Thierry Reding:
> > I think you gain much less by splitting up than you realize. Compare
> > this from before the series:
> > 
> > 	$ du -ch drivers/gpu/drm/imx/*.ko
> > 	8.0K    drivers/gpu/drm/imx/dw_hdmi-imx.ko
> 
> Oh right, I didn't realize that this is just the shim. The bulk of the
> HDMI driver is in bridge/dw_hdmi.ko.
> 
> > 	16K     drivers/gpu/drm/imx/imxdrm.ko
> > 	16K     drivers/gpu/drm/imx/imx-ipuv3-crtc.ko
> > 	12K     drivers/gpu/drm/imx/imx-ldb.ko
> > 	12K     drivers/gpu/drm/imx/imx-tve.ko
> > 	8.0K    drivers/gpu/drm/imx/parallel-display.ko
> > 	72K     total
> [...]
> > with this after the series:
> > 
> > 	$ du -ch drivers/gpu/drm/imx/*.ko
> > 	44K     drivers/gpu/drm/imx/imx-drm.ko
> > 	44K     total
> [...]
> > There are other things to consider as well, such as the additional
> > memory overhead caused by merely having multiple modules, or all of the
> > exported functions that unnecessarily clutter up the symbol table, and
> > which end up slowing down every symbol lookup.
> 
> Thanks, can't argue with those numbers.
> I'll queue patches 3 and 4 for imx-drm.

Wait, I won't because they depend on the helper function in patch 1.

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

then. Can I get a stable tag so I can solve potential merge conflicts in
imx-drm-core.c?

regards
Philipp
Thierry Reding Sept. 25, 2015, 2:21 p.m. UTC | #5 
On Fri, Sep 25, 2015 at 03:13:54PM +0200, Philipp Zabel wrote:
> Am Freitag, den 25.09.2015, 15:09 +0200 schrieb Philipp Zabel:
> > Am Freitag, den 25.09.2015, 14:17 +0200 schrieb Thierry Reding:
> > > I think you gain much less by splitting up than you realize. Compare
> > > this from before the series:
> > > 
> > > 	$ du -ch drivers/gpu/drm/imx/*.ko
> > > 	8.0K    drivers/gpu/drm/imx/dw_hdmi-imx.ko
> > 
> > Oh right, I didn't realize that this is just the shim. The bulk of the
> > HDMI driver is in bridge/dw_hdmi.ko.
> > 
> > > 	16K     drivers/gpu/drm/imx/imxdrm.ko
> > > 	16K     drivers/gpu/drm/imx/imx-ipuv3-crtc.ko
> > > 	12K     drivers/gpu/drm/imx/imx-ldb.ko
> > > 	12K     drivers/gpu/drm/imx/imx-tve.ko
> > > 	8.0K    drivers/gpu/drm/imx/parallel-display.ko
> > > 	72K     total
> > [...]
> > > with this after the series:
> > > 
> > > 	$ du -ch drivers/gpu/drm/imx/*.ko
> > > 	44K     drivers/gpu/drm/imx/imx-drm.ko
> > > 	44K     total
> > [...]
> > > There are other things to consider as well, such as the additional
> > > memory overhead caused by merely having multiple modules, or all of the
> > > exported functions that unnecessarily clutter up the symbol table, and
> > > which end up slowing down every symbol lookup.
> > 
> > Thanks, can't argue with those numbers.
> > I'll queue patches 3 and 4 for imx-drm.
> 
> Wait, I won't because they depend on the helper function in patch 1.
> 
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> then. Can I get a stable tag so I can solve potential merge conflicts in
> imx-drm-core.c?

Let's see what Greg thinks about patch 1. There are a bunch of other
drivers that can use these helpers, so it might be best to get the
helper merged first and hold off on the depending patches until the
next release.

I'll let you know when we've figured out how to merge this.

Thierry
diff mbox

Patch

diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
index 2b81a417cf29..999c21ba94b7 100644
--- a/drivers/gpu/drm/imx/Kconfig
+++ b/drivers/gpu/drm/imx/Kconfig
@@ -11,7 +11,7 @@  config DRM_IMX
 	  enable i.MX graphics support
 
 config DRM_IMX_FB_HELPER
-	tristate "provide legacy framebuffer /dev/fb0"
+	bool "provide legacy framebuffer /dev/fb0"
 	select DRM_KMS_CMA_HELPER
 	depends on DRM_IMX
 	help
@@ -20,13 +20,13 @@  config DRM_IMX_FB_HELPER
 	  and also for applications using the legacy framebuffer API
 
 config DRM_IMX_PARALLEL_DISPLAY
-	tristate "Support for parallel displays"
+	bool "Support for parallel displays"
 	select DRM_PANEL
 	depends on DRM_IMX
 	select VIDEOMODE_HELPERS
 
 config DRM_IMX_TVE
-	tristate "Support for TV and VGA displays"
+	bool "Support for TV and VGA displays"
 	depends on DRM_IMX
 	select REGMAP_MMIO
 	help
@@ -34,7 +34,7 @@  config DRM_IMX_TVE
 	  found on i.MX53 processors.
 
 config DRM_IMX_LDB
-	tristate "Support for LVDS displays"
+	bool "Support for LVDS displays"
 	depends on DRM_IMX && MFD_SYSCON
 	select DRM_PANEL
 	help
@@ -42,14 +42,13 @@  config DRM_IMX_LDB
 	  found on i.MX53 and i.MX6 processors.
 
 config DRM_IMX_IPUV3
-	tristate
+	bool
 	depends on DRM_IMX
 	depends on IMX_IPUV3_CORE
-	default y if DRM_IMX=y
-	default m if DRM_IMX=m
+	default y
 
 config DRM_IMX_HDMI
-	tristate "Freescale i.MX DRM HDMI"
+	bool "Freescale i.MX DRM HDMI"
 	select DRM_DW_HDMI
 	depends on DRM_IMX
 	help
diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile
index f3ecd8903d97..48b3844928d9 100644
--- a/drivers/gpu/drm/imx/Makefile
+++ b/drivers/gpu/drm/imx/Makefile
@@ -1,12 +1,9 @@ 
+imx-drm-y := imx-drm-core.o
 
-imxdrm-objs := imx-drm-core.o
+imx-drm-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o
+imx-drm-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
+imx-drm-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
+imx-drm-$(CONFIG_DRM_IMX_IPUV3) += ipuv3-crtc.o ipuv3-plane.o
+imx-drm-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
 
-obj-$(CONFIG_DRM_IMX) += imxdrm.o
-
-obj-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o
-obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
-obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
-
-imx-ipuv3-crtc-objs  := ipuv3-crtc.o ipuv3-plane.o
-obj-$(CONFIG_DRM_IMX_IPUV3)	+= imx-ipuv3-crtc.o
-obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
+obj-$(CONFIG_DRM_IMX) += imx-drm.o
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index 644edf65dbe0..eacf32d36c7b 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -271,7 +271,7 @@  static int dw_hdmi_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver dw_hdmi_imx_platform_driver = {
+struct platform_driver dw_hdmi_imx_platform_driver = {
 	.probe  = dw_hdmi_imx_probe,
 	.remove = dw_hdmi_imx_remove,
 	.driver = {
@@ -280,8 +280,6 @@  static struct platform_driver dw_hdmi_imx_platform_driver = {
 	},
 };
 
-module_platform_driver(dw_hdmi_imx_platform_driver);
-
 MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
 MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
 MODULE_DESCRIPTION("IMX6 Specific DW-HDMI Driver Extension");
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 40950e15b759..4b49a7cb0a14 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -636,7 +636,27 @@  static struct platform_driver imx_drm_pdrv = {
 		.of_match_table = imx_drm_dt_ids,
 	},
 };
-module_platform_driver(imx_drm_pdrv);
+
+static struct platform_driver * const drivers[] = {
+	&ipu_drm_driver,
+	&imx_ldb_driver,
+	&imx_pd_driver,
+	&dw_hdmi_imx_platform_driver,
+	&imx_tve_driver,
+	&imx_drm_pdrv,
+};
+
+static int imx_drm_init(void)
+{
+	return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
+}
+module_init(imx_drm_init);
+
+static void imx_drm_exit(void)
+{
+	platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
+}
+module_exit(imx_drm_exit);
 
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
 MODULE_DESCRIPTION("i.MX drm driver core");
diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h
index eebf0e2fefd0..87bdbf5cabb1 100644
--- a/drivers/gpu/drm/imx/imx-drm.h
+++ b/drivers/gpu/drm/imx/imx-drm.h
@@ -53,4 +53,10 @@  int imx_drm_encoder_parse_of(struct drm_device *drm,
 void imx_drm_connector_destroy(struct drm_connector *connector);
 void imx_drm_encoder_destroy(struct drm_encoder *encoder);
 
+extern struct platform_driver ipu_drm_driver;
+extern struct platform_driver imx_ldb_driver;
+extern struct platform_driver imx_pd_driver;
+extern struct platform_driver dw_hdmi_imx_platform_driver;
+extern struct platform_driver imx_tve_driver;
+
 #endif /* _IMX_DRM_H_ */
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index abacc8f67469..3fcb221accd7 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -673,7 +673,7 @@  static int imx_ldb_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver imx_ldb_driver = {
+struct platform_driver imx_ldb_driver = {
 	.probe		= imx_ldb_probe,
 	.remove		= imx_ldb_remove,
 	.driver		= {
@@ -682,8 +682,6 @@  static struct platform_driver imx_ldb_driver = {
 	},
 };
 
-module_platform_driver(imx_ldb_driver);
-
 MODULE_DESCRIPTION("i.MX LVDS driver");
 MODULE_AUTHOR("Sascha Hauer, Pengutronix");
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index e671ad369416..cf6d0b171074 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -722,7 +722,7 @@  static const struct of_device_id imx_tve_dt_ids[] = {
 	{ /* sentinel */ }
 };
 
-static struct platform_driver imx_tve_driver = {
+struct platform_driver imx_tve_driver = {
 	.probe		= imx_tve_probe,
 	.remove		= imx_tve_remove,
 	.driver		= {
@@ -731,8 +731,6 @@  static struct platform_driver imx_tve_driver = {
 	},
 };
 
-module_platform_driver(imx_tve_driver);
-
 MODULE_DESCRIPTION("i.MX Television Encoder driver");
 MODULE_AUTHOR("Philipp Zabel, Pengutronix");
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index a10da8e011c2..af48ffee752f 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -501,14 +501,13 @@  static int ipu_drm_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver ipu_drm_driver = {
+struct platform_driver ipu_drm_driver = {
 	.driver = {
 		.name = "imx-ipuv3-crtc",
 	},
 	.probe = ipu_drm_probe,
 	.remove = ipu_drm_remove,
 };
-module_platform_driver(ipu_drm_driver);
 
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
 MODULE_DESCRIPTION(DRIVER_DESC);
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index b4deb9cf9d71..ec61b5753bc6 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -295,7 +295,7 @@  static const struct of_device_id imx_pd_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, imx_pd_dt_ids);
 
-static struct platform_driver imx_pd_driver = {
+struct platform_driver imx_pd_driver = {
 	.probe		= imx_pd_probe,
 	.remove		= imx_pd_remove,
 	.driver		= {
@@ -304,8 +304,6 @@  static struct platform_driver imx_pd_driver = {
 	},
 };
 
-module_platform_driver(imx_pd_driver);
-
 MODULE_DESCRIPTION("i.MX parallel display driver");
 MODULE_AUTHOR("Sascha Hauer, Pengutronix");
 MODULE_LICENSE("GPL");