On Mon, Oct 31, 2016 at 08:58:43AM +0000, Russell King - ARM Linux wrote: > On Mon, Oct 24, 2016 at 10:24:42PM +0200, Daniel Vetter wrote: > > On Mon, Oct 24, 2016 at 4:52 PM, Brian Starkey <brian.starkey@xxxxxxx> wrote: > > >> > > >>> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > > >>> b/drivers/gpu/drm/i2c/tda998x_drv.c > > >>> index f4315bc..6e6fca2 100644 > > >>> --- a/drivers/gpu/drm/i2c/tda998x_drv.c > > >>> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > > >>> @@ -1369,7 +1369,6 @@ const struct drm_connector_helper_funcs > > >>> tda998x_connector_helper_funcs = { > > >>> > > >>> static void tda998x_connector_destroy(struct drm_connector *connector) > > >>> { > > >>> - drm_connector_unregister(connector); > > >>> drm_connector_cleanup(connector); > > >>> } > > >>> > > >>> @@ -1441,16 +1440,10 @@ static int tda998x_bind(struct device *dev, > > >>> struct device *master, void *data) > > >>> if (ret) > > >>> goto err_connector; > > >>> > > >>> - ret = drm_connector_register(&priv->connector); > > >>> - if (ret) > > >>> - goto err_sysfs; > > >>> - > > >> > > >> > > >> Instead of smashing all these patches into one, what about checking here > > >> for midlayer driver set with: > > >> > > >> /* register here for drivers still using midlayer load/unload */ > > >> if (dev->driver->load) > > >> drm_connector_register(connector), > > >> > > >> Similar in other places. That way we wouldn't need to switch the world in > > >> one patch. > > > > > > > > > I don't think that helps. If we do that in isolation (first), then > > > mali-dp and hdlcd won't get their connectors registered because their > > > bind order is: > > > > > > drm_dev_register(); > > > component_bind_all(); > > > > > > If we change the mali-dp/hdlcd bind order first, then tda998x will > > > explode on drm_connector_register() until it's patched to remove that. > > > > > > As I mentioned in my mail to Russell, the only way I can see to avoid > > > patching all three drivers in one go is: > > > 1) Add (probably open-coded) drm_connector_register_all() to the end > > > of bind in hdlcd and mali-dp > > > 2) Patch tda998x to remove drm_connector_register() > > > 3) Reorder hdlcd/mali-dp bind and remove the connector registration > > > added in 1) > > > > > > We can do that, but it's extra churn for the same result, and none of > > > the 5 patches will really make sense in isolation anyway. > > > > I thought there's also armada to take care of, which this patch would > > break? > > NO NO NO NO NO. I've said this several times. Let's try it again, > and see if it sticks. > > Because Armada has not been converted from a mid-layered driver, it > is _IMMUNE_ from any patch removing the drm_connector_register() call > in TDA998x. It does _NOT_ break in any way. > > Only those drivers which are de-mid-layered, and worked around the > drm_connector_register() call inside TDA998x (eg, mali) break, because > of the order in which they are _forced_ to call stuff. > > In a de-mid-layered driver, with the drm_connector_register() call in > place in TDA998x, drm_dev_register() _MUST_ be called prior to > component_bind_all(), otherwise you get a WARN_ON() dump from the > kobject code. With the drm_connector_register() call removed, > drm_dev_register() _MUST_ be called after component_bind_all() so that > the connector is registered. > > It's the de-mid-layered drivers which are the problem here, not the > mid-layered ones like Armada. > > > Maybe even another driver, so the hack would still be useful > > for those other drivers. And it would have been useful if malidp/hdlcd > > wouldn't have started out with the wrong init ordering ;-) > > It's forced into the "wrong init ordering" due to the kobject WARN_ON. Hm, I entirely missed that part of the troubles. Anyway, if you all agree on a patch I certainly won't block it, feel free to merge through suitable trees (or I can smash it into drm-misc if that's wanted). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel