Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941248AbcJXUYs (ORCPT ); Mon, 24 Oct 2016 16:24:48 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:33400 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932580AbcJXUYo (ORCPT ); Mon, 24 Oct 2016 16:24:44 -0400 MIME-Version: 1.0 X-Originating-IP: [2a02:168:56b5:0:ac27:b86c:7764:9429] In-Reply-To: <20161024145202.GB1988@e106950-lin.cambridge.arm.com> 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> From: Daniel Vetter Date: Mon, 24 Oct 2016 22:24:42 +0200 X-Google-Sender-Auth: YE2eNsW8CoiuYWVWg9s7Q0O8LOA Message-ID: Subject: Re: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration To: Brian Starkey Cc: Russell King , dri-devel , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2515 Lines: 68 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. And it would have been useful if malidp/hdlcd wouldn't have started out with the wrong init ordering ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch