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

2014-11-21 Thread Andrzej Hajda 
On 11/20/2014 03:37 PM, Inki Dae wrote:
 On 2014년 11월 20일 23:23, Andrzej Hajda wrote:
 On 11/20/2014 02:56 PM, Inki Dae wrote:
 On 2014년 11월 20일 22:19, Andrzej Hajda wrote:
 On 11/20/2014 11:24 AM, Inki Dae wrote:
 This patch makes kms drivers to be independent drivers.
 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.

 Changelog v3:
 - Use module_platform_driver macro instead module_init/exit.
 - Configure all kms drivers to be built in kernel image.

 Changelog v2:
 - none

 Signed-off-by: Inki Dae inki@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_dp_core.c  |2 ++
  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  |2 ++
  drivers/gpu/drm/exynos/exynos_drm_fimd.c |2 ++
  drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
  drivers/gpu/drm/exynos/exynos_mixer.c|2 ++
  7 files changed, 13 insertions(+), 45 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index ed818b9..30ebf5d 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -1408,6 +1408,8 @@ struct platform_driver dp_driver = {
   },
  };
  
 +module_platform_driver(dp_driver);
 If you try to build exynosdrm as module you will receive errors due to
 multiple definitions of init_module, ie module_init/module_*_driver
 macros can be used once per module.
 Ah, right. we had ever tried same way before but failed in same problem.
 I didn't realize that. Anyway, I will try to find out a better way. I'd
 really like to remove all register codes of sub drivers from
 exynos_drm_drv somehow.
 I have proposed once solution with sparse arrays, using linker scripts
 [1]. Something similar is used with clock controllers as I remember.
 I do not see other possibilities to fully separate components of
 exynos_drm_drv.

 [1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/103816

 I know this patch. I wasn't sure that the use of the private linker
 section is reasonable and overuse it. Actually, Mr. Park opposed this
 way. It might be a good idea if no problem for device drivers use the
 private linker section. Is there any device driver using the private
 linker section?

No, there are no dev drivers using it, but it is hardly used anyway.

Quick look at include/asm-generic/vmlinux.lds.h shows following sparse
arrays:
#define CLKSRC_OF_TABLES()  OF_TABLE(CONFIG_CLKSRC_OF, clksrc)
#define IRQCHIP_OF_MATCH_TABLE() OF_TABLE(CONFIG_IRQCHIP, irqchip)
#define CLK_OF_TABLES() OF_TABLE(CONFIG_COMMON_CLK, clk)
#define RESERVEDMEM_OF_TABLES() OF_TABLE(CONFIG_OF_RESERVED_MEM,
reservedmem)
#define CPU_METHOD_OF_TABLES()  OF_TABLE(CONFIG_SMP, cpu_method)
#define EARLYCON_OF_TABLES()OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)

The number of arrays slowly grows over time, so some day they can start
appear in drivers as well :)

Such array in driver doesn't look like much different to me, it is just
a workaround for language limitations,
but it would be good to have an opinion of people more involved in
linker scripts.

On the other side having list of component drivers in exynos_drm_drv and
registering them
in module init maybe is not pretty, but it does the same thing and is
quite clear and standard.

Regards
Andrzej


 Thanks,
 Inki Dae

 Regards
 Andrzej

 Anyway I am afraid exynosdrm seems to be still fragile to order of
 successful probes of their components.
 It can be easily observed on the system with two display pipelines with
 one of them probe deferring. For example universal with fimd/dpi + vidi:
 1. fimd defers probe because panel is not yet probed.
 2. vidi probes ok.
 3. drmdev is initialized.
 4. fimd probes ok, but it is too late!!!

 So you can reproduce the scenario on any target when one of the
 components defers and vidi is present (vidi never defers I suppose).
 Thanks for checking,
 Inki Dae

 Regards
 Andrzej

 +
  MODULE_AUTHOR(Jingoo Han jg1@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);
 - }
 -
   list_for_each_entry(cdev, drm_component_list, list) {
   /*
* Add components to master only in case that crtc and
 @@ -545,22

[RFC PATCH v3 1/4] drm/exynos: make kms drivers to be independent drivers

2014-11-20 Thread Inki Dae 
This patch makes kms drivers to be independent drivers.
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.

Changelog v3:
- Use module_platform_driver macro instead module_init/exit.
- Configure all kms drivers to be built in kernel image.

Changelog v2:
- none

Signed-off-by: Inki Dae inki@samsung.com
---
 drivers/gpu/drm/exynos/exynos_dp_core.c  |2 ++
 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  |2 ++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |2 ++
 drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
 drivers/gpu/drm/exynos/exynos_mixer.c|2 ++
 7 files changed, 13 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index ed818b9..30ebf5d 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1408,6 +1408,8 @@ struct platform_driver dp_driver = {
},
 };
 
+module_platform_driver(dp_driver);
+
 MODULE_AUTHOR(Jingoo Han jg1@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);
-   }
-
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

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

2014-11-20 Thread Andrzej Hajda 
On 11/20/2014 11:24 AM, Inki Dae wrote:
 This patch makes kms drivers to be independent drivers.
 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.
 
 Changelog v3:
 - Use module_platform_driver macro instead module_init/exit.
 - Configure all kms drivers to be built in kernel image.
 
 Changelog v2:
 - none
 
 Signed-off-by: Inki Dae inki@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_dp_core.c  |2 ++
  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  |2 ++
  drivers/gpu/drm/exynos/exynos_drm_fimd.c |2 ++
  drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
  drivers/gpu/drm/exynos/exynos_mixer.c|2 ++
  7 files changed, 13 insertions(+), 45 deletions(-)
 
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index ed818b9..30ebf5d 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -1408,6 +1408,8 @@ struct platform_driver dp_driver = {
   },
  };
  
 +module_platform_driver(dp_driver);

If you try to build exynosdrm as module you will receive errors due to
multiple definitions of init_module, ie module_init/module_*_driver
macros can be used once per module.

Anyway I am afraid exynosdrm seems to be still fragile to order of
successful probes of their components.
It can be easily observed on the system with two display pipelines with
one of them probe deferring. For example universal with fimd/dpi + vidi:
1. fimd defers probe because panel is not yet probed.
2. vidi probes ok.
3. drmdev is initialized.
4. fimd probes ok, but it is too late!!!

So you can reproduce the scenario on any target when one of the
components defers and vidi is present (vidi never defers I suppose).

Regards
Andrzej

 +
  MODULE_AUTHOR(Jingoo Han jg1@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);
 - }
 -
   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 =

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

2014-11-20 Thread Inki Dae 
On 2014년 11월 20일 22:19, Andrzej Hajda wrote:
 On 11/20/2014 11:24 AM, Inki Dae wrote:
 This patch makes kms drivers to be independent drivers.
 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.

 Changelog v3:
 - Use module_platform_driver macro instead module_init/exit.
 - Configure all kms drivers to be built in kernel image.

 Changelog v2:
 - none

 Signed-off-by: Inki Dae inki@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_dp_core.c  |2 ++
  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  |2 ++
  drivers/gpu/drm/exynos/exynos_drm_fimd.c |2 ++
  drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
  drivers/gpu/drm/exynos/exynos_mixer.c|2 ++
  7 files changed, 13 insertions(+), 45 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index ed818b9..30ebf5d 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -1408,6 +1408,8 @@ struct platform_driver dp_driver = {
  },
  };
  
 +module_platform_driver(dp_driver);
 
 If you try to build exynosdrm as module you will receive errors due to
 multiple definitions of init_module, ie module_init/module_*_driver
 macros can be used once per module.

Ah, right. we had ever tried same way before but failed in same problem.
I didn't realize that. Anyway, I will try to find out a better way. I'd
really like to remove all register codes of sub drivers from
exynos_drm_drv somehow.

 
 Anyway I am afraid exynosdrm seems to be still fragile to order of
 successful probes of their components.
 It can be easily observed on the system with two display pipelines with
 one of them probe deferring. For example universal with fimd/dpi + vidi:
 1. fimd defers probe because panel is not yet probed.
 2. vidi probes ok.
 3. drmdev is initialized.
 4. fimd probes ok, but it is too late!!!
 
 So you can reproduce the scenario on any target when one of the
 components defers and vidi is present (vidi never defers I suppose).

Thanks for checking,
Inki Dae

 
 Regards
 Andrzej
 
 +
  MODULE_AUTHOR(Jingoo Han jg1@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);
 -}
 -
  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)

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

2014-11-20 Thread Javier Martinez Canillas 
Hello Inki,

On Thu, Nov 20, 2014 at 2:56 PM, Inki Dae inki@samsung.com wrote:
 If you try to build exynosdrm as module you will receive errors due to
 multiple definitions of init_module, ie module_init/module_*_driver
 macros can be used once per module.

 Ah, right. we had ever tried same way before but failed in same problem.
 I didn't realize that. Anyway, I will try to find out a better way. I'd
 really like to remove all register codes of sub drivers from
 exynos_drm_drv somehow.


Maybe as an immediate fix we can move the platform driver registration
out of probe as suggested by Andrzej?

I posted an RFC patch a couple of days ago [0] that fixes both the
deadlock and the infinite loop bug so it would be great if you can
review/test so I can post a proper patch.

And then we can look how to better refactor the exynos drm driver.

Best regards,
Javier

[0]: http://www.spinics.net/lists/linux-samsung-soc/msg39106.html
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-20 Thread Inki Dae 
Hi Javier,

On 2014년 11월 20일 23:06, Javier Martinez Canillas wrote:
 Hello Inki,
 
 On Thu, Nov 20, 2014 at 2:56 PM, Inki Dae inki@samsung.com wrote:
 If you try to build exynosdrm as module you will receive errors due to
 multiple definitions of init_module, ie module_init/module_*_driver
 macros can be used once per module.

 Ah, right. we had ever tried same way before but failed in same problem.
 I didn't realize that. Anyway, I will try to find out a better way. I'd
 really like to remove all register codes of sub drivers from
 exynos_drm_drv somehow.

 
 Maybe as an immediate fix we can move the platform driver registration
 out of probe as suggested by Andrzej?
 
 I posted an RFC patch a couple of days ago [0] that fixes both the
 deadlock and the infinite loop bug so it would be great if you can
 review/test so I can post a proper patch.

Fantastical timing.:) I gave you a same opinion almost simultaneous.
Check your email thread.

Thanks,
Inki Dae

 
 And then we can look how to better refactor the exynos drm driver.
 
 Best regards,
 Javier
 
 [0]: http://www.spinics.net/lists/linux-samsung-soc/msg39106.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-11-20 Thread Andrzej Hajda 
On 11/20/2014 02:56 PM, Inki Dae wrote:
 On 2014년 11월 20일 22:19, Andrzej Hajda wrote:
 On 11/20/2014 11:24 AM, Inki Dae wrote:
 This patch makes kms drivers to be independent drivers.
 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.

 Changelog v3:
 - Use module_platform_driver macro instead module_init/exit.
 - Configure all kms drivers to be built in kernel image.

 Changelog v2:
 - none

 Signed-off-by: Inki Dae inki@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_dp_core.c  |2 ++
  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  |2 ++
  drivers/gpu/drm/exynos/exynos_drm_fimd.c |2 ++
  drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
  drivers/gpu/drm/exynos/exynos_mixer.c|2 ++
  7 files changed, 13 insertions(+), 45 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index ed818b9..30ebf5d 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -1408,6 +1408,8 @@ struct platform_driver dp_driver = {
 },
  };
  
 +module_platform_driver(dp_driver);

 If you try to build exynosdrm as module you will receive errors due to
 multiple definitions of init_module, ie module_init/module_*_driver
 macros can be used once per module.
 
 Ah, right. we had ever tried same way before but failed in same problem.
 I didn't realize that. Anyway, I will try to find out a better way. I'd
 really like to remove all register codes of sub drivers from
 exynos_drm_drv somehow.

I have proposed once solution with sparse arrays, using linker scripts
[1]. Something similar is used with clock controllers as I remember.
I do not see other possibilities to fully separate components of
exynos_drm_drv.

[1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/103816

Regards
Andrzej

 

 Anyway I am afraid exynosdrm seems to be still fragile to order of
 successful probes of their components.
 It can be easily observed on the system with two display pipelines with
 one of them probe deferring. For example universal with fimd/dpi + vidi:
 1. fimd defers probe because panel is not yet probed.
 2. vidi probes ok.
 3. drmdev is initialized.
 4. fimd probes ok, but it is too late!!!

 So you can reproduce the scenario on any target when one of the
 components defers and vidi is present (vidi never defers I suppose).
 
 Thanks for checking,
 Inki Dae
 

 Regards
 Andrzej

 +
  MODULE_AUTHOR(Jingoo Han jg1@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);
 -   }
 -
 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

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

2014-11-20 Thread Inki Dae 
On 2014년 11월 20일 23:23, Andrzej Hajda wrote:
 On 11/20/2014 02:56 PM, Inki Dae wrote:
 On 2014년 11월 20일 22:19, Andrzej Hajda wrote:
 On 11/20/2014 11:24 AM, Inki Dae wrote:
 This patch makes kms drivers to be independent drivers.
 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.

 Changelog v3:
 - Use module_platform_driver macro instead module_init/exit.
 - Configure all kms drivers to be built in kernel image.

 Changelog v2:
 - none

 Signed-off-by: Inki Dae inki@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_dp_core.c  |2 ++
  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  |2 ++
  drivers/gpu/drm/exynos/exynos_drm_fimd.c |2 ++
  drivers/gpu/drm/exynos/exynos_hdmi.c |2 ++
  drivers/gpu/drm/exynos/exynos_mixer.c|2 ++
  7 files changed, 13 insertions(+), 45 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index ed818b9..30ebf5d 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -1408,6 +1408,8 @@ struct platform_driver dp_driver = {
},
  };
  
 +module_platform_driver(dp_driver);

 If you try to build exynosdrm as module you will receive errors due to
 multiple definitions of init_module, ie module_init/module_*_driver
 macros can be used once per module.

 Ah, right. we had ever tried same way before but failed in same problem.
 I didn't realize that. Anyway, I will try to find out a better way. I'd
 really like to remove all register codes of sub drivers from
 exynos_drm_drv somehow.
 
 I have proposed once solution with sparse arrays, using linker scripts
 [1]. Something similar is used with clock controllers as I remember.
 I do not see other possibilities to fully separate components of
 exynos_drm_drv.
 
 [1]: http://permalink.gmane.org/gmane.comp.video.dri.devel/103816
 

I know this patch. I wasn't sure that the use of the private linker
section is reasonable and overuse it. Actually, Mr. Park opposed this
way. It might be a good idea if no problem for device drivers use the
private linker section. Is there any device driver using the private
linker section?

Thanks,
Inki Dae

 Regards
 Andrzej
 


 Anyway I am afraid exynosdrm seems to be still fragile to order of
 successful probes of their components.
 It can be easily observed on the system with two display pipelines with
 one of them probe deferring. For example universal with fimd/dpi + vidi:
 1. fimd defers probe because panel is not yet probed.
 2. vidi probes ok.
 3. drmdev is initialized.
 4. fimd probes ok, but it is too late!!!

 So you can reproduce the scenario on any target when one of the
 components defers and vidi is present (vidi never defers I suppose).

 Thanks for checking,
 Inki Dae


 Regards
 Andrzej

 +
  MODULE_AUTHOR(Jingoo Han jg1@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);
 -  }
 -
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);
 -