From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965569AbcJYKTZ (ORCPT ); Tue, 25 Oct 2016 06:19:25 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:33514 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933028AbcJYKTX (ORCPT ); Tue, 25 Oct 2016 06:19:23 -0400 Date: Tue, 25 Oct 2016 12:19:19 +0200 From: Daniel Vetter To: Brian Starkey Cc: Daniel Vetter , Russell King , dri-devel , Linux Kernel Mailing List Subject: Re: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration Message-ID: <20161025101919.35c53zoyf435urp3@phenom.ffwll.local> Mail-Followup-To: Brian Starkey , Russell King , dri-devel , Linux Kernel Mailing List 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161025095233.GD1988@e106950-lin.cambridge.arm.com> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration Date: Tue, 25 Oct 2016 12:19:19 +0200 Message-ID: <20161025101919.35c53zoyf435urp3@phenom.ffwll.local> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id B64FC6E69D for ; Tue, 25 Oct 2016 10:19:23 +0000 (UTC) Received: by mail-wm0-x243.google.com with SMTP id y138so610471wme.1 for ; Tue, 25 Oct 2016 03:19:23 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20161025095233.GD1988@e106950-lin.cambridge.arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Starkey Cc: Russell King , dri-devel , Linux Kernel Mailing List List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBPY3QgMjUsIDIwMTYgYXQgMTA6NTI6MzNBTSArMDEwMCwgQnJpYW4gU3RhcmtleSB3 cm90ZToKPiBIaSBEYW5pZWwsCj4gCj4gT24gTW9uLCBPY3QgMjQsIDIwMTYgYXQgMTA6MjQ6NDJQ TSArMDIwMCwgRGFuaWVsIFZldHRlciB3cm90ZToKPiA+IE9uIE1vbiwgT2N0IDI0LCAyMDE2IGF0 IDQ6NTIgUE0sIEJyaWFuIFN0YXJrZXkgPGJyaWFuLnN0YXJrZXlAYXJtLmNvbT4gd3JvdGU6Cj4g PiA+ID4gCj4gPiA+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2kyYy90ZGE5OTh4 X2Rydi5jCj4gPiA+ID4gPiBiL2RyaXZlcnMvZ3B1L2RybS9pMmMvdGRhOTk4eF9kcnYuYwo+ID4g PiA+ID4gaW5kZXggZjQzMTViYy4uNmU2ZmNhMiAxMDA2NDQKPiA+ID4gPiA+IC0tLSBhL2RyaXZl cnMvZ3B1L2RybS9pMmMvdGRhOTk4eF9kcnYuYwo+ID4gPiA+ID4gKysrIGIvZHJpdmVycy9ncHUv ZHJtL2kyYy90ZGE5OTh4X2Rydi5jCj4gPiA+ID4gPiBAQCAtMTM2OSw3ICsxMzY5LDYgQEAgY29u c3Qgc3RydWN0IGRybV9jb25uZWN0b3JfaGVscGVyX2Z1bmNzCj4gPiA+ID4gPiB0ZGE5OTh4X2Nv bm5lY3Rvcl9oZWxwZXJfZnVuY3MgPSB7Cj4gPiA+ID4gPiAKPiA+ID4gPiA+ICBzdGF0aWMgdm9p ZCB0ZGE5OTh4X2Nvbm5lY3Rvcl9kZXN0cm95KHN0cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0 b3IpCj4gPiA+ID4gPiAgewo+ID4gPiA+ID4gLSAgICAgICBkcm1fY29ubmVjdG9yX3VucmVnaXN0 ZXIoY29ubmVjdG9yKTsKPiA+ID4gPiA+ICAgICAgICAgZHJtX2Nvbm5lY3Rvcl9jbGVhbnVwKGNv bm5lY3Rvcik7Cj4gPiA+ID4gPiAgfQo+ID4gPiA+ID4gCj4gPiA+ID4gPiBAQCAtMTQ0MSwxNiAr MTQ0MCwxMCBAQCBzdGF0aWMgaW50IHRkYTk5OHhfYmluZChzdHJ1Y3QgZGV2aWNlICpkZXYsCj4g PiA+ID4gPiBzdHJ1Y3QgZGV2aWNlICptYXN0ZXIsIHZvaWQgKmRhdGEpCj4gPiA+ID4gPiAgICAg ICAgIGlmIChyZXQpCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgZ290byBlcnJfY29ubmVjdG9y Owo+ID4gPiA+ID4gCj4gPiA+ID4gPiAtICAgICAgIHJldCA9IGRybV9jb25uZWN0b3JfcmVnaXN0 ZXIoJnByaXYtPmNvbm5lY3Rvcik7Cj4gPiA+ID4gPiAtICAgICAgIGlmIChyZXQpCj4gPiA+ID4g PiAtICAgICAgICAgICAgICAgZ290byBlcnJfc3lzZnM7Cj4gPiA+ID4gPiAtCj4gPiA+ID4gCj4g PiA+ID4gCj4gPiA+ID4gSW5zdGVhZCBvZiBzbWFzaGluZyBhbGwgdGhlc2UgcGF0Y2hlcyBpbnRv IG9uZSwgd2hhdCBhYm91dCBjaGVja2luZyBoZXJlCj4gPiA+ID4gZm9yIG1pZGxheWVyIGRyaXZl ciBzZXQgd2l0aDoKPiA+ID4gPiAKPiA+ID4gPiAgICAgICAgIC8qIHJlZ2lzdGVyIGhlcmUgZm9y IGRyaXZlcnMgc3RpbGwgdXNpbmcgbWlkbGF5ZXIgbG9hZC91bmxvYWQgKi8KPiA+ID4gPiAgICAg ICAgIGlmIChkZXYtPmRyaXZlci0+bG9hZCkKPiA+ID4gPiAgICAgICAgICAgICAgICAgZHJtX2Nv bm5lY3Rvcl9yZWdpc3Rlcihjb25uZWN0b3IpLAo+ID4gPiA+IAo+ID4gPiA+IFNpbWlsYXIgaW4g b3RoZXIgcGxhY2VzLiBUaGF0IHdheSB3ZSB3b3VsZG4ndCBuZWVkIHRvIHN3aXRjaCB0aGUgd29y bGQgaW4KPiA+ID4gPiBvbmUgcGF0Y2guCj4gPiA+IAo+ID4gPiAKPiA+ID4gSSBkb24ndCB0aGlu ayB0aGF0IGhlbHBzLiBJZiB3ZSBkbyB0aGF0IGluIGlzb2xhdGlvbiAoZmlyc3QpLCB0aGVuCj4g PiA+IG1hbGktZHAgYW5kIGhkbGNkIHdvbid0IGdldCB0aGVpciBjb25uZWN0b3JzIHJlZ2lzdGVy ZWQgYmVjYXVzZSB0aGVpcgo+ID4gPiBiaW5kIG9yZGVyIGlzOgo+ID4gPiAKPiA+ID4gICAgICAg ICBkcm1fZGV2X3JlZ2lzdGVyKCk7Cj4gPiA+ICAgICAgICAgY29tcG9uZW50X2JpbmRfYWxsKCk7 Cj4gPiA+IAo+ID4gPiBJZiB3ZSBjaGFuZ2UgdGhlIG1hbGktZHAvaGRsY2QgYmluZCBvcmRlciBm aXJzdCwgdGhlbiB0ZGE5OTh4IHdpbGwKPiA+ID4gZXhwbG9kZSBvbiBkcm1fY29ubmVjdG9yX3Jl Z2lzdGVyKCkgdW50aWwgaXQncyBwYXRjaGVkIHRvIHJlbW92ZSB0aGF0Lgo+ID4gPiAKPiA+ID4g QXMgSSBtZW50aW9uZWQgaW4gbXkgbWFpbCB0byBSdXNzZWxsLCB0aGUgb25seSB3YXkgSSBjYW4g c2VlIHRvIGF2b2lkCj4gPiA+IHBhdGNoaW5nIGFsbCB0aHJlZSBkcml2ZXJzIGluIG9uZSBnbyBp czoKPiA+ID4gIDEpIEFkZCAocHJvYmFibHkgb3Blbi1jb2RlZCkgZHJtX2Nvbm5lY3Rvcl9yZWdp c3Rlcl9hbGwoKSB0byB0aGUgZW5kCj4gPiA+ICAgICBvZiBiaW5kIGluIGhkbGNkIGFuZCBtYWxp LWRwCj4gPiA+ICAyKSBQYXRjaCB0ZGE5OTh4IHRvIHJlbW92ZSBkcm1fY29ubmVjdG9yX3JlZ2lz dGVyKCkKPiA+ID4gIDMpIFJlb3JkZXIgaGRsY2QvbWFsaS1kcCBiaW5kIGFuZCByZW1vdmUgdGhl IGNvbm5lY3RvciByZWdpc3RyYXRpb24KPiA+ID4gICAgIGFkZGVkIGluIDEpCj4gPiA+IAo+ID4g PiBXZSBjYW4gZG8gdGhhdCwgYnV0IGl0J3MgZXh0cmEgY2h1cm4gZm9yIHRoZSBzYW1lIHJlc3Vs dCwgYW5kIG5vbmUgb2YKPiA+ID4gdGhlIDUgcGF0Y2hlcyB3aWxsIHJlYWxseSBtYWtlIHNlbnNl IGluIGlzb2xhdGlvbiBhbnl3YXkuCj4gPiAKPiA+IEkgdGhvdWdodCB0aGVyZSdzIGFsc28gYXJt YWRhIHRvIHRha2UgY2FyZSBvZiwgd2hpY2ggdGhpcyBwYXRjaCB3b3VsZAo+ID4gYnJlYWs/IE1h eWJlIGV2ZW4gYW5vdGhlciBkcml2ZXIsIHNvIHRoZSBoYWNrIHdvdWxkIHN0aWxsIGJlIHVzZWZ1 bAo+ID4gZm9yIHRob3NlIG90aGVyIGRyaXZlcnMuCj4gCj4gT0sgaXQgc2VlbXMgbGlrZSB0aGlz IHNpdHVhdGlvbiBoYXMgZ290IHZlcnkgY29uZnVzZWQuIEluIHNob3J0LCB0aGlzCj4gcGF0Y2gg ZG9lcyBub3QgYnJlYWsgYXJtYWRhLiBSdXNzZWxsIHByZXZpb3VzbHkgdGVzdGVkIHRoZSB0ZGE5 OTh4Cj4gcGF0Y2ggYWdhaW5zdCBhcm1hZGEgYW5kIHJlcG9ydGVkIG5vIGlzc3Vlcy4KPiBEcml2 ZXJzIHdpdGggYSAtPmxvYWQoKSBjYWxsYmFjayBkb24ndCBuZWVkIHRvIHJlZ2lzdGVyIHRoZSBj b25uZWN0b3IKPiBhbnltb3JlLCBiZWNhdXNlIGRybV9kZXZfcmVnaXN0ZXIoKSBkb2VzIGl0IGZv ciB0aGVtLgo+IAo+IEFzIGZhciBhcyBJIGtub3csIHRoaXMgcGF0Y2ggdG91Y2hpbmcgdGhlc2Ug dGhyZWUgZHJpdmVycyBpcwo+IHN1ZmZpY2llbnQgdG8gYWxsb3cgYWxsIGN1cnJlbnQgdXNlcnMg b2YgdGRhOTk4eCB0byBjb250aW51ZSB1c2luZyBpdCwKPiB3aGlsc3QgYWxzbyBhbGxvd2luZyBh cm1hZGEgYW5kIHRpbGNkYyB0byBiZSBkZS1taWRsYXllcmVkLgoKQWgsIG1ha2VzIHNlbnNlLiBT aG91bGQgSSBhcHBseSB0aGlzIHRvIGRybS1taXNjIHNvIGl0J3MgaW4gYSBzaGFyZWQgdHJlZT8K LURhbmllbAotLSAKRGFuaWVsIFZldHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9y YXRpb24KaHR0cDovL2Jsb2cuZmZ3bGwuY2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vZHJpLWRldmVsCg==