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

[Inki Dae]

2014-04-04 22:55 GMT+09:00 Tomasz Figa <t.figa at 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

I'm not sure what you meant but there wouldn't be no way that users
*cannot disable* the driver for a component. The driver would be
exynos_drm_drv, not separated module, and would always be built as
long as users want to use *exynos drm driver*.

> listed in supernode? If I'm reading the code correctly, Exynos DRM will not

And the only case a component isn't added to the list is when users
disabled sub driver.

> register, which is completely wrong. Users should be able to select which
> drivers should be compiled into their kernels.

So users are be able to select drivers they want to use, and will be
compiled correctly. So no, the only thing users can disable is each
sub driver, not core module.

>
> 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

No, current drm driver *must also be built* as one integrated single
drm driver without super device approach. So the super device approach
*has no any effect on existing design*,  and what the super device
approch tries to do is to resolve the probe order issue to sub drivers
*without some codes specific to Exynos drm*.

> 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.

I'm not sure but I had implemented below prototype codes for that, see
the below link,
https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/commit/?h=exynos-bridge-test&id=2860ad02d736e300034ddeabce3da92614922b4e

I guess what you said would be similler approach.

And I still think the use of the component framework would be the best
solution and *Linux generic way* for resolving the probe order issue
without any specific codes. So I'm not advocating the compoent
framework but I tend not to want to use specific codes.

>
> 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.

For -next, I never expect that pm operations would be operated
perfactly. They are really not for product. Just adding new feature
and drivers to -next, and then fixing them while in RC. And RC process
would also be for it. Actually, I see pm interfaces of exynos_drm_dsi
driver leads to break a single driver model because pm operations
should be done at top level of exynos drm, and some codes Sean didn't
fix. So such things are in my fix-todo-list. I tend to accept new
features and drivers for -next as long as they have no big problem,
And that is my style I maintain.

Thanks,
Inki Dae

>
> Best regards,
> Tomasz
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel