[RFC PATCH 1/3] drm/exynos: make kms drivers to be independent modules
Inki Dae
inki.dae at samsung.com
Wed Nov 19 05:17:32 PST 2014
2014-11-19 17:49 GMT+09:00 Andrzej Hajda <a.hajda at samsung.com>:
> 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 at 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 at 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
Right, hdmi/mixer cannot be probed, which means module doesn't work correctly.
> and when it should create drmdev.
>
> DRM system is rather monolithic splitting its components to modules
> seems strange.
Yes, good point. I missed a big point that DRM system should be
monolithic, integrated driver.
However I strongly think it's better to separate them into independent
drivers so that entry point of each sub driver can be called itself.
With this, the probe order of each sub driver can be handled by
component and Exynos drm core properly. it's really not good for all
register codes of sub drivers to be placed in exynos_drm_drv,
and this was my plan for cleanning up Exynos drm long ago: but still
remaining one more thing, applying super device node.
So I will just modify for them to be built in kernel image by
reverting the change of Kconfig, which means that all sub drivers
cannot be configured as a separeated module like now.
Thanks,
Inki Dae
>
> 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 at samsung.com>");
>> MODULE_AUTHOR("Andrzej Hajda <a.hajda at 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);
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list