Re: [RFC PATCH 1/3] drm/exynos: make kms drivers to be independent modules

On 11/18/2014 04:26 PM, Inki Dae wrote:

This patch makes kms drivers to be independent modules. For this, it removes all register codes to kms drivers from exynos_drm_drv module and adds module_init/exit for each kms driver so that each kms driver can be called independently.

It is hard to test if modules are working because I am not able to build modules :), see comments below.

Signed-off-by: Inki Dae inki.dae@samsung.com

drivers/gpu/drm/exynos/Kconfig | 8 +++--- drivers/gpu/drm/exynos/exynos_dp_core.c | 13 +++++++++ drivers/gpu/drm/exynos/exynos_drm_drv.c | 43 +++--------------------------- drivers/gpu/drm/exynos/exynos_drm_drv.h | 5 ---- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 +++++++++ drivers/gpu/drm/exynos/exynos_drm_fimd.c | 13 +++++++++ drivers/gpu/drm/exynos/exynos_hdmi.c | 13 +++++++++ drivers/gpu/drm/exynos/exynos_mixer.c | 13 +++++++++ 8 files changed, 72 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 7f9f6f9..3d5fa69 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -25,7 +25,7 @@ config DRM_EXYNOS_DMABUF Choose this option if you want to use DMABUF feature for DRM.

config DRM_EXYNOS_FIMD

• bool "Exynos DRM FIMD"

• tristate "Exynos DRM FIMD" depends on DRM_EXYNOS && !FB_S3C select FB_MODE_HELPERS select MFD_SYSCON

@@ -41,7 +41,7 @@ config DRM_EXYNOS_DPI This enables support for Exynos parallel output.

config DRM_EXYNOS_DSI

• bool "EXYNOS DRM MIPI-DSI driver support"

• tristate "EXYNOS DRM MIPI-DSI driver support" depends on DRM_EXYNOS_FIMD select DRM_MIPI_DSI select DRM_PANEL

@@ -50,7 +50,7 @@ config DRM_EXYNOS_DSI This enables support for Exynos MIPI-DSI device.

config DRM_EXYNOS_DP

• bool "EXYNOS DRM DP driver support"

• tristate "EXYNOS DRM DP driver support" depends on DRM_EXYNOS_FIMD && ARCH_EXYNOS && (DRM_PTN3460=n || DRM_PTN3460=y || DRM_PTN3460=DRM_EXYNOS) default DRM_EXYNOS select DRM_PANEL

@@ -58,7 +58,7 @@ config DRM_EXYNOS_DP This enables support for DP device.

config DRM_EXYNOS_HDMI

• bool "Exynos DRM HDMI"

• tristate "Exynos DRM HDMI" depends on DRM_EXYNOS && !VIDEO_SAMSUNG_S5P_TV help Choose this option if you want to use Exynos HDMI for DRM.

Without corresponding changes in Makefiles you cannot build it as modules.

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index ed818b9..b08d97b 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1408,6 +1408,19 @@ struct platform_driver dp_driver = { }, };

+static int exynos_dp_init(void) +{

• return platform_driver_register(&dp_driver);

+}

+static void exynos_dp_exit(void) +{

• platform_driver_unregister(&dp_driver);

+}

+module_init(exynos_dp_init); +module_exit(exynos_dp_exit);

Here and in other modules you can use module_platform_driver macro.

MODULE_AUTHOR("Jingoo Han jg1.han@samsung.com"); MODULE_DESCRIPTION("Samsung SoC DP Driver"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index eab12f0..02d4772 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -484,12 +484,6 @@ static struct component_match *exynos_drm_match_add(struct device *dev)

mutex_lock(&drm_component_lock);

• /* Do not retry to probe if there is no any kms driver regitered. */
• if (list_empty(&drm_component_list)) {

• mutex_unlock(&drm_component_lock);
  

• return ERR_PTR(-ENODEV);
  

• }

It is hard to guess how and why it should work with modules. For example what should happen if fimd/dsi modules are loaded but hdmi/mixer not. When exynos_drv should wait with initialization for hdmi and when it should create drmdev.

DRM system is rather monolithic splitting its components to modules seems strange.

Regards Andrzej

list_for_each_entry(cdev, &drm_component_list, list) { /* * Add components to master only in case that crtc and @@ -545,22 +539,6 @@ static const struct component_master_ops exynos_drm_ops = { .unbind = exynos_drm_unbind, };

-static struct platform_driver *const exynos_drm_kms_drivers[] = { -#ifdef CONFIG_DRM_EXYNOS_FIMD

• &fimd_driver,

-#endif -#ifdef CONFIG_DRM_EXYNOS_DP

• &dp_driver,

-#endif -#ifdef CONFIG_DRM_EXYNOS_DSI

• &dsi_driver,

-#endif -#ifdef CONFIG_DRM_EXYNOS_HDMI

• &mixer_driver,
• &hdmi_driver,

-#endif -};

static struct platform_driver *const exynos_drm_non_kms_drivers[] = { #ifdef CONFIG_DRM_EXYNOS_G2D &g2d_driver, @@ -587,22 +565,14 @@ static int exynos_drm_platform_probe(struct platform_device *pdev) pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); exynos_drm_driver.num_ioctls = ARRAY_SIZE(exynos_ioctls);

• for (i = 0; i < ARRAY_SIZE(exynos_drm_kms_drivers); ++i) {

• ret = platform_driver_register(exynos_drm_kms_drivers[i]);
  

• if (ret < 0)
  

• 	goto err_unregister_kms_drivers;
  

• }
• match = exynos_drm_match_add(&pdev->dev);
• if (IS_ERR(match)) {

• ret = PTR_ERR(match);
  

• goto err_unregister_kms_drivers;
  

• }

• if (IS_ERR(match))

• return PTR_ERR(match);
  
  

  ret = component_master_add_with_match(&pdev->dev, &exynos_drm_ops, match); if (ret < 0)

• goto err_unregister_kms_drivers;
  

• return ret;
  
  

  for (j = 0; j < ARRAY_SIZE(exynos_drm_non_kms_drivers); ++j) { ret = platform_driver_register(exynos_drm_non_kms_drivers[j]);

@@ -632,10 +602,6 @@ err_unregister_non_kms_drivers: err_del_component_master: component_master_del(&pdev->dev, &exynos_drm_ops);

-err_unregister_kms_drivers:

• while (--i >= 0)

• platform_driver_unregister(exynos_drm_kms_drivers[i]);
  

• return ret;

}

@@ -654,9 +620,6 @@ static int exynos_drm_platform_remove(struct platform_device *pdev)

component_master_del(&pdev->dev, &exynos_drm_ops);

• for (i = ARRAY_SIZE(exynos_drm_kms_drivers) - 1; i >= 0; --i)

• platform_driver_unregister(exynos_drm_kms_drivers[i]);
  

• return 0;

}

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 262a459..352a9f9 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -331,11 +331,6 @@ int exynos_drm_component_add(struct device *dev, void exynos_drm_component_del(struct device *dev, enum exynos_drm_device_type dev_type);

-extern struct platform_driver fimd_driver; -extern struct platform_driver dp_driver; -extern struct platform_driver dsi_driver; -extern struct platform_driver mixer_driver; -extern struct platform_driver hdmi_driver; extern struct platform_driver exynos_drm_common_hdmi_driver; extern struct platform_driver vidi_driver; extern struct platform_driver g2d_driver; diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 66d427e..ebc3383 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1799,6 +1799,19 @@ struct platform_driver dsi_driver = { }, };

+static int exynos_dsi_driver_init(void) +{

• return platform_driver_register(&dsi_driver);

+}

+static void exynos_dsi_driver_exit(void) +{

• platform_driver_unregister(&dsi_driver);

+}

+module_init(exynos_dsi_driver_init); +module_exit(exynos_dsi_driver_exit);

MODULE_AUTHOR("Tomasz Figa t.figa@samsung.com"); MODULE_AUTHOR("Andrzej Hajda a.hajda@samsung.com"); MODULE_DESCRIPTION("Samsung SoC MIPI DSI Master"); diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 0673a39..eed51c6 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -1240,3 +1240,16 @@ struct platform_driver fimd_driver = { .of_match_table = fimd_driver_dt_match, }, };

+static int exynos_drm_fimd_init(void) +{

• return platform_driver_register(&fimd_driver);

+}

+static void exynos_drm_fimd_exit(void) +{

• platform_driver_unregister(&fimd_driver);

+}

+module_init(exynos_drm_fimd_init); +module_exit(exynos_drm_fimd_exit); diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 563a19e..83198f7 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2537,3 +2537,16 @@ struct platform_driver hdmi_driver = { .of_match_table = hdmi_match_types, }, };

+static int hdmi_init(void) +{

• return platform_driver_register(&hdmi_driver);

+}

+static void hdmi_exit(void) +{

• platform_driver_unregister(&hdmi_driver);

+}

+module_init(hdmi_init); +module_exit(hdmi_exit); diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index a41c84e..4d812f1 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1349,3 +1349,16 @@ struct platform_driver mixer_driver = { .remove = mixer_remove, .id_table = mixer_driver_types, };

+static int mixer_init(void) +{

• return platform_driver_register(&mixer_driver);

+}

+static void mixer_exit(void) +{

• platform_driver_unregister(&mixer_driver);

+}

+module_init(mixer_init); +module_exit(mixer_exit);