Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932785AbcJaJAY (ORCPT ); Mon, 31 Oct 2016 05:00:24 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:55490 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763303AbcJaJAW (ORCPT ); Mon, 31 Oct 2016 05:00:22 -0400 Date: Mon, 31 Oct 2016 09:00:08 +0000 From: Russell King - ARM Linux To: Daniel Vetter Cc: Brian Starkey , dri-devel , Linux Kernel Mailing List Subject: Re: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration Message-ID: <20161031090008.GV1041@n2100.armlinux.org.uk> References: <20161024142312.GA1988@e106950-lin.cambridge.arm.com> <1477319279-21726-1-git-send-email-brian.starkey@arm.com> <20161024143627.GT20761@phenom.ffwll.local> <20161024145202.GB1988@e106950-lin.cambridge.arm.com> <20161025095233.GD1988@e106950-lin.cambridge.arm.com> <20161025101919.35c53zoyf435urp3@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161025101919.35c53zoyf435urp3@phenom.ffwll.local> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3903 Lines: 88 On Tue, Oct 25, 2016 at 12:19:19PM +0200, Daniel Vetter wrote: > On Tue, Oct 25, 2016 at 10:52:33AM +0100, Brian Starkey wrote: > > Hi Daniel, > > > > On Mon, Oct 24, 2016 at 10:24:42PM +0200, Daniel Vetter wrote: > > > On Mon, Oct 24, 2016 at 4:52 PM, Brian Starkey 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? Maybe even another driver, so the hack would still be useful > > > for those other drivers. > > > > OK it seems like this situation has got very confused. In short, this > > patch does not break armada. Russell previously tested the tda998x > > patch against armada and reported no issues. > > Drivers with a ->load() callback don't need to register the connector > > anymore, because drm_dev_register() does it for them. > > > > As far as I know, this patch touching these three drivers is > > sufficient to allow all current users of tda998x to continue using it, > > whilst also allowing armada and tilcdc to be de-midlayered. > > Ah, makes sense. Should I apply this to drm-misc so it's in a shared tree? I need the patch against v4.8. I'm happy to pick it up and add it to my drm-tda998x-devel branch, which you can then merge into drm-misc if you wish. -- 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.