On Tue, Aug 27, 2013 at 9:46 AM, Lothar Waßmann <l...@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.vet...@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to