Re: [PATCH v2 1/7] drm/exynos: add super device support

[Tomasz Figa <t.figa@samsung.com>]

Hi Inki,

On 01.04.2014 14:37, Inki Dae wrote:
> This patch adds super device support to bind sub drivers
> using device tree.
>
> For this, you should add a super device node to each machine dt files
> like belows,
>
> In case of using MIPI-DSI,
> 	display-subsystem {
> 		compatible = "samsung,exynos-display-subsystem";
> 		ports = <&fimd>, <&dsi>;
> 	};
>
> In case of using DisplayPort,
> 	display-subsystem {
> 		compatible = "samsung,exynos-display-subsystem";
> 		ports = <&fimd>, <&dp>;
> 	};
>
> In case of using Parallel panel,
> 	display-subsystem {
> 		compatible = "samsung,exynos-display-subsystem";
> 		ports = <&fimd>;
> 	};
>
> And if you don't add connector device node to ports property,
> default parallel panel driver, exynos_drm_dpi module, will be used.
>
> ports property can have the following device nodes,
> 	fimd, mixer, Image Enhancer, MIPI-DSI, eDP, LVDS Bridge, or HDMI
>
> With this patch, we can resolve the probing order issue without
> some global lists. So this patch also removes the unnecessary lists and
> stuff related to these lists.

I can see several problems with this approach:

1) It breaks compatibility with existing DT. After this patch it is no 
longer possible to use old device trees and get a working DRM. However, 
in my opinion, this requirement can be relaxed if we make sure that any 
users are properly converted.

2) What happens if in Kconfig you disable a driver for a component that 
is listed in supernode? If I'm reading the code correctly, Exynos DRM 
will not register, which is completely wrong. Users should be able to 
select which drivers should be compiled into their kernels.

3) Such approach leads to complete integration of all Exynos DRM 
drivers, without possibility of loading some sub-drivers as modules. I 
know that current driver design doesn't support it either, but if this 
series is claimed to improve things, it should really do so.

4) Exactly the same can be achieved without changing the DT bindings at 
all. In fact even without adding any new single property or node to DT. 
We discussed this with Andrzej and Marek today and came to a solution in 
which just by adding a little bit of code to Exynos DRM subdrivers, you 
could guarantee correct registration of Exynos DRM platform and also get 
rid of #ifdeffery in exynos_drm_drv.c. Andrzej will send an RFC after 
the weekend.

5) This series seems to break DPI display support with runtime PM 
enabled. Universal C210 just hangs on second FIMD probe, after first one 
fails with probe deferral. This needs more investigation, though.

Best regards,
Tomasz