commit b5dc0d108cd3c0b50ddcb6f6c54be1bea4c39e01 breaks imx-drm support

[Daniel Vetter]

On Tue, Aug 27, 2013 at 9:46 AM, Lothar Waßmann <LW at karo-electronics.de> wrote:
> the function imx_drm_driver_load() must have been called before
> calling imx_drm_add_crtc(), but the following hunk in the commit:
> @@ -446,6 +434,9 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
>          */
>         imxdrm->drm->vblank_disable_allowed = 1;
>
> +       if (!imx_drm_device_get())
> +               ret = -EINVAL;
> +
>         ret = 0;
>
>  err_init:
>
> makes imx_drm_add_crtc() bail out at:
>
>         if (imxdrm->references) {
>                 ret = -EBUSY;
>                 goto err_busy;
>         }
>
> Thus it is not possible to register any CRTCs.

Ok I've now read around a bit in the imx core and I think my call to rip
this out was right ;-)

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.

So as a stopgap measuret I sugges you rip out the imxdrm->references
trickery since it won't actually protect you from anything truly nasty
happening. And I wouldn't worry about module unloading and refcounting for
now since core drm is terminally broken in that area already anyway.

Then we can move ahead and how to really fix this init ordering. So I
think we have another TODO here:

Cheers, Daniel

---
diff --git a/drivers/staging/imx-drm/TODO b/drivers/staging/imx-drm/TODO
index f806415..f8ef245 100644
--- a/drivers/staging/imx-drm/TODO
+++ b/drivers/staging/imx-drm/TODO
@@ -6,6 +6,11 @@ TODO:
 - Factor out more code to common helper functions
 - decide where to put the base driver. It is not specific to a subsystem
   and would be used by DRM/KMS and media/V4L2
+- Fix up the driver load sequence to make sure all submodules for encoders/crtcs
+  are fully loaded before the drm driver is registered. Might require a core drm
+  rework to break away the drm core init sequence from its midlayer drug and
+  assorted control inversion issues. Or we restructure imx to just built in
+  everything, dunno. Requested by Daniel Vetter <daniel.vetter at ffwll.ch>
 
 Missing features (not necessarily for moving out of staging):
 
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch