commit b5dc0d108cd3c0b50ddcb6f6c54be1bea4c39e01 breaks imx-drm support

[Rob Clark]

On Tue, Aug 27, 2013 at 7:19 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Tue, Aug 27, 2013 at 4:56 AM, Sascha Hauer <s.hauer at pengutronix.de> wrote:
>> On Tue, Aug 27, 2013 at 10:41:20AM +0200, Daniel Vetter wrote:
>>> On Tue, Aug 27, 2013 at 10:26 AM, Lucas Stach <l.stach at pengutronix.de> wrote:
>>> >> If I understand stuff correctly you have a main driver plus a bunch of
>>> >> encoder/crtc modules and you expect that everything gets loaded and then
>>> >> only when the kms driver is first opened. The current drm core just
>>> >> doesn't support hotplugging of encoder/crtc objects after driver init has
>>> >> completed. If you try to do that it'll go down in flames due to all the
>>> >> races involved.
>>> >>
>>> > You know the logic you broke here was just a moderately sane approach to
>>> > get around the monolithic DRM driver nonsense, while not having to use
>>> > real hotplug in DRM.
>>> >
>>> > The thing is we don't know if we will ever have all submodules loaded,
>>> > as this driver is mostly used on embedded devices where people randomly
>>> > decide to exclude things from their kernel config, because they don't
>>> > use the feature on their board. The current logic is under the premise
>>> > that there are no early DRM clients in something like Plymouth, which is
>>> > a bit flaky, but worked very well for the targeted use-cases.
>>>
>>> Imo the imxdrm->references logic is broken already and simply removing
>>> those checks would be the right course of action - pretending to have
>>> fixed races but not actually having fixed much is imo much worse than
>>> openly admitting that the code is racy and needs work.
>>>
>>> Wrt embedded guys shaving off a few kb by not loading a bunch of
>>> modules I think you should simply make that compile-time options
>>> instead of modules. More hassle but at least it should work.
>>
>> It's not about the few kb. The problem is that our devices are not
>> monolithic, but instead we have multiple devices in and around the SoC
>> which form a DRM device.
>
> right, but the cores on the SoC, and even any external encoder chips,
> are not actually hot-pluggable.  I have a similar scenario w/ msm drm,

oh, and come to think of it, same approach it tilcdc.  And I'm sure
there are other drivers with the same scenario.

BR,
-R

> where there is the core display controller (for ex, 'mdp4'), plus
> hdmi/dsi/etc blocks around that (with their own irq, io/register
> region, etc).  It would be nice if the kernel provided a better
> mechanism for composite drivers, but what I do is just register the
> platform drivers for those other blocks first (in the init code,
> before calling 'platform_driver_register(&msm_platform_driver)'.  This
> way, if the device is actually present, I know before drm dev_load.  I
> do not attempt to build hdmi/dsi/etc as separate modules.  I could if
> I wanted to include/exclude them at compile time, but separate modules
> seems like a bad idea.
>
> BR,
> -R
>
>> Sascha
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel