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.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
