From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge. Date: Thu, 22 Jun 2017 11:23:32 +0200 Message-ID: <20170622112332.7ec7b65c@bbrezillon> References: <20170615204130.19255-1-eric@anholt.net> <20170615204130.19255-2-eric@anholt.net> <777ee1b1-e5ce-ea29-8a48-f792354a22d1@codeaurora.org> <871sqkouvr.fsf@eliezer.anholt.net> <8e148170-b626-b426-3c94-b93d2746f4ce@codeaurora.org> <871sqe7ei0.fsf@eliezer.anholt.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Archit Taneja , Laurent Pinchart Cc: Mark Rutland , devicetree@vger.kernel.org, Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" , Philippe Cornu , Rob Herring , Thierry Reding List-Id: devicetree@vger.kernel.org T24gVGh1LCAyMiBKdW4gMjAxNyAxMzo0Nzo0MyArMDUzMApBcmNoaXQgVGFuZWphIDxhcmNoaXR0 QGNvZGVhdXJvcmEub3JnPiB3cm90ZToKCj4gT24gMDYvMjIvMjAxNyAwMToyMCBQTSwgQmVuamFt aW4gR2FpZ25hcmQgd3JvdGU6Cj4gPiAyMDE3LTA2LTIwIDE5OjMxIEdNVCswMjowMCBFcmljIEFu aG9sdCA8ZXJpY0BhbmhvbHQubmV0PjogIAo+ID4+IEFyY2hpdCBUYW5lamEgPGFyY2hpdHRAY29k ZWF1cm9yYS5vcmc+IHdyaXRlczoKPiA+PiAgCj4gPj4+IE9uIDA2LzE2LzIwMTcgMDg6MTMgUE0s IEVyaWMgQW5ob2x0IHdyb3RlOiAgCj4gPj4+PiBBcmNoaXQgVGFuZWphIDxhcmNoaXR0QGNvZGVh dXJvcmEub3JnPiB3cml0ZXM6Cj4gPj4+PiAgCj4gPj4+Pj4gT24gMDYvMTYvMjAxNyAwMjoxMSBB TSwgRXJpYyBBbmhvbHQgd3JvdGU6ICAKPiA+Pj4+Pj4gSWYgdGhlIHBhbmVsLWJyaWRnZSBpcyBi ZWluZyBzZXQgdXAgYWZ0ZXIgdGhlIGRybV9tb2RlX2NvbmZpZ19yZXNldCgpLAo+ID4+Pj4+PiB0 aGVuIHRoZSBjb25uZWN0b3IncyBzdGF0ZSB3b3VsZCBuZXZlciBnZXQgaW5pdGlhbGl6ZWQsIGFu ZCB3ZSdkCj4gPj4+Pj4+IGRlcmVmZXJlbmNlIHRoZSBOVUxMIGluIHRoZSBob3RwbHVnIHBhdGgu ICBXZSBhbHNvIG5lZWQgdG8gcmVnaXN0ZXIKPiA+Pj4+Pj4gdGhlIGNvbm5lY3Rvciwgc28gdGhh dCB1c2Vyc3BhY2UgY2FuIGdldCBhdCBpdC4KPiA+Pj4+Pj4gIAo+ID4+Pj4+Cj4gPj4+Pj4gU2hv dWxkbid0IHRoZSBLTVMgZHJpdmVyIG1ha2Ugc3VyZSB0aGUgcGFuZWwtYnJpZGdlIGlzIHNldCB1 cCBiZWZvcmUKPiA+Pj4+PiBkcm1fbW9kZV9jb25maWdfcmVzZXQ/IElzIGl0IHRoZSBjYXNlIHdo ZW4gd2UncmUgaW5zZXJ0aW5nIHRoZQo+ID4+Pj4+IHBhbmVsLWJyaWRnZSBkcml2ZXIgYXMgYSBt b2R1bGU/Cj4gPj4+Pj4KPiA+Pj4+Pgo+ID4+Pj4+IEFsbCB0aGUgY29ubmVjdG9ycyB0aGF0IGhh dmUgYmVlbiBhZGRlZCBhcmUgcmVnaXN0ZXJlZCBhdXRvbWF0aWNhbGx5Cj4gPj4+Pj4gd2hlbiBk cm1fZGV2X3JlZ2lzdGVyKCkgaXMgY2FsbGVkIGJ5IHRoZSBLTVMgZHJpdmVyLiBSZWdpc3Rlcmlu ZyBhCj4gPj4+Pj4gY29ubmVjdG9yIGluIHRoZSBtaWRkbGUgb2Ygc2V0dGluZyB1cCBvdXIgZHJp dmVyIGlzIHByb25lIHRvIHJhY2UKPiA+Pj4+PiBjb25kaXRpb25zIGlmIHRoZSB1c2Vyc3BhY2Ug ZGVjaWRlcyB0byB1c2UgdGhlbSBpbW1lZGlhdGVseS4gIAo+ID4+Pj4KPiA+Pj4+IFllYWgsIHRo aXMgaXMgZml4aW5nIGluaXRpYWxpemluZyBwYW5lbF9icmlkZ2UgYXQgRFNJIGhvc3RfYXR0YWNo IHRpbWUsCj4gPj4+PiB3aGljaCBpbiB0aGUgY2FzZSBvZiBhIHBhbmVsIG1vZHVsZSB0aGF0IGNy ZWF0ZXMgdGhlIERTSSBkZXZpY2UKPiA+Pj4+IChhZHY3NTMzLXN0eWxlLCBsaWtlIHlvdSBzYWlk IEkgc2hvdWxkIHVzZSBhcyBhIHJlZmVyZW5jZSkgd2lsbCBiZSBhZnRlcgo+ID4+Pj4gZHJtX21v ZGVfY29uZmlnX3Jlc2V0KCkgYW5kIGRybV9kZXZfcmVnaXN0ZXIoKS4gIAo+ID4+Pgo+ID4+PiBP a2F5LiBJbiB0aGUgY2FzZSBvZiB0aGUgbXNtIGttcyBkcml2ZXIsIHdlIGRlZmVyIHByb2JlIHVu dGlsIHRoZQo+ID4+PiBhZHY3NTMzIG1vZHVsZSBpcyBpbnNlcnRlZCwgb25seSB0aGVuIHdlIHBy b2NlZWQgdG8gZHJtX21vZGVfY29uZmlnX3Jlc2V0KCkKPiA+Pj4gYW5kIGRybV9kZXZfcmVnaXN0 ZXIoKS4gSSBhc3N1bWVkIHRoaXMgd2FzIHRoZSBnZW5lcmFsIHByYWN0aWNlIGZvbGxvd2VkIGJ5 Cj4gPj4+IG1vc3Qga21zIGRyaXZlcnMuIEkuLGUgdGhlIGttcyBkcml2ZXIgZGVmZXJzIHByb2Jl IHVudGlsIGFsbCBjb25uZWN0b3IKPiA+Pj4gcmVsYXRlZCBtb2R1bGVzIGFyZSBpbnNlcnRlZCwg YW5kIG9ubHkgdGhlbiBwcm9jZWVkIHRvIGNyZWF0ZSBhIGRybSBkZXZpY2UuICAKPiA+Pgo+ID4+ IFRoZSBwcm9ibGVtLCB0aG91Z2gsIGlzIHRoZSBwYW5lbCBkcml2ZXIgbmVlZHMgdGhlIE1JUEkg RFNJIGhvc3QgdG8KPiA+PiBleGlzdCB0byBjYWxsIG1pcGlfZHNpX2RldmljZV9yZWdpc3Rlcl9m dWxsKCkgZHVyaW5nIHRoZSBwcm9iZSBwcm9jZXNzLgo+ID4+IFRoZSBhZHY3NTMzIGRyaXZlciBn ZXRzIGFyb3VuZCB0aGlzIGJ5IHJlZ2lzdGVyaW5nIHRoZSBEU0kgZGV2aWNlIGluIHRoZQo+ID4+ IGJyaWRnZSBhdHRhY2ggc3RlcCwgYnV0IGRybV9wYW5lbCBkb2Vzbid0IGhhdmUgYW4gYXR0YWNo IHN0ZXAuICAKPiAKPiBJJ20gbm90IHN1cmUgaG93IHdlIGNhbiBnZXQgYXJvdW5kIHRoaXMuIFdl IGhhZCBkaXNjdXNzaW9uIGFib3V0IHRoaXMgb24gaXJjCj4gcmVjZW50bHksIGJ1dCBjb3VsZG4n dCBjb21lIHVwIHdpdGggYSBnb29kIGNvbmNsdXNpb24uIFdlIGNvdWxkIGNvbWUgdXAgd2l0aCBh Cj4gcGFuZWxfYXR0YWNoKCkgY2FsbGJhY2sgdG8gbWFrZSBpdCBzaW1pbGFyIHRvIGJyaWRnZXMs IGJ1dCB0aGF0J3MganVzdCB1cyBhdm9pZGluZwo+IHRoZSByZWFsIGlzc3VlLgoKSG93IGFib3V0 IG1ha2luZyBEU0kgZGV2IHJlZ2lzdHJhdGlvbiBmdWxseSBhc3luY2hyb25vdXMsIHRoYXQgaXMs IERTSQpkZXZzIGRlY2xhcmVkIGluIHRoZSBEVCB1bmRlciB0aGUgRFNJIGhvc3Qgbm9kZSB3aWxs IGJlCnJlZ2lzdGVyZWQvYXR0YWNoZWQgYXQgcHJvYmUgdGltZSwgYW5kIGRldnMgdXNpbmcgYW5v dGhlciBjb250cm9sIGJ1cwoobGlrZSB0aGUgYWR2NzUzMyBjb250cm9sbGVyIG92ZXIgaTJjKSB3 aWxsIGJlIHJlZ2lzdGVyZWQgYWZ0ZXJ3YXJkcy4KClRoYXQgaW1wbGllcyBtb3ZpbmcgdGhlIGRy bV9icmlnZSByZWdpc3RyYXRpb24gbG9naWMgb3V0c2lkZSBvZiB0aGUgRFNJCmhvc3QgLT5wcm9i ZSgpIHBhdGguIFRoZSBpZGVhIHdvdWxkIGJlIHRvIGNoZWNrIGlmIGFsbCBkZXZzIGNvbm5lY3Rl ZAp0byB0aGUgRFNJIGJ1cyBhcmUgcmVhZHkgYXQgZHNpX2hvc3QtPmF0dGFjaCgpIHRpbWUuIElm IHRoZXkgYXJlLCB3ZQpjYW4gZmluYWxseSByZWdpc3RlciB0aGUgWFhYIC0+IERTSSBicmlkZ2Uu IElmIHRoZXkncmUgbm90IChiZWNhdXNlCnNvbWUgZGV2cyBjb25uZWN0ZWQgdG8gdGhlIERTSSBi dXMgaGF2ZSBub3QgYmVlbiBwcm9iZWQgeWV0KSwgdGhlbiB3ZQpkbyBub3QgcmVnaXN0ZXIgdGhl IGRybV9icmlkZ2UgYW5kIHdhaXQgZm9yIHRoZSBuZXh0IGRzaV9ob3N0LT5hdHRhY2goKQpldmVu dC4KCldpdGggdGhpcyBzb2x1dGlvbiwgSSB0aGluayB3ZSBjYW4gYXZvaWQgdGhlIGNoaWNrZW4m ZWdnIHByb2JsZW0gd2hlcmUKdGhlIGFkdjc1MzMgZGV2IGlzIHdhaXRpbmcgZm9yIHRoZSBEU0kg aG9zdCB0byBiZSBwcm9iZWQgdG8gYmUgYWJsZSB0bwpyZWdpc3RlciBhIERTSSBkZXYgd2l0aCBt aXBpX2RzaV9kZXZpY2VfcmVnaXN0ZXJfZnVsbCgpIGFuZCB0aGUgRFNJCmhvc3QgbmVlZHMgYWxs IGRldnMgdG8gYmUgcmVnaXN0ZXJlZCB0byBub3QgcmV0dXJuIC1FUFJPQkVfREVGRVIuCgo+IAo+ ID4+Cj4gPj4gQW5vdGhlciBhbHRlcm5hdGl2ZSBpcyBteSBvcmlnaW5hbCB2ZXJzaW9uIG9mIHRo ZSBwYW5lbCBkcml2ZXIgdGhhdCB3YXMKPiA+PiBhIG1pcGlfZHNpX2RldmljZSBkcml2ZXIgdGhh dCByZWdpc3RlcmVkIHRoZSBwYW5lbCBkdXJpbmcgdGhlIERTSSBkZXZpY2UKPiA+PiBwcm9iZS4g IFRoYXQncyB3aHkgdmM0J3MgcGFuZWwgbG9va3VwIGlzIGR1cmluZyB0aGUgTUlQSSBEU0kgYXR0 YWNoCj4gPj4gcGhhc2UsIGN1cnJlbnRseS4gIAo+ID4gICAKPiAKPiBUaGlzIHdvdWxkIHJlcXVp cmUgeW91IHRvIGhhdmUgYSBEU0kgZGV2aWNlIG5vZGUgaW4gRFQsIHJhdGhlciB0aGFuIGFuIGky Ywo+IG5vZGUsIHJpZ2h0PyBJIGRvbid0IGtub3cgaWYgd2Ugc2hvdWxkIGRvIHRoYXQgYmVjYXVz ZSBvZiBhIGxpbWl0YXRpb24gaW4KPiBvdXIgZHJtX21pcGlfZHNpIGFuZCBkcm1fcGFuZWwgZnJh bWV3b3Jrcy4KPiAKPiBEb2VzIGFueW9uZSBoYXZlIGJldHRlciBpZGVhcz8KCldlbGwsIHRoZXNl IG1peGVkLWJ1cyBjYXNlcyBhcmUgcmVhbGx5IHVucHJhY3RpY2FsIHRvIHJlcHJlc2VudCBpbiB0 aGUKRFQgKGFuZCBhbHNvIG5vdCBlYXN5IHRvIGRlYWwgd2l0aCB0aGUgZGV2aWNlLW1vZGVsKS4g SSB1bmRlcnN0YW5kIHRoYXQKdGhlIGluaXRpYWwgc29sdXRpb24gd2FzIHRvIHB1dCB0aGUgZGV2 aWNlIHVuZGVyIHRoZSBjb250cm9sLWJ1cywgYW5kCm5vdCB0aGUgZGF0YS1idXMsIGJ1dCB0aGVu IGl0IGxlYWRzIHRvIHRoZSBjdXJyZW50IHNpdHVhdGlvbiB3aGVyZSB3ZQpkb24ndCBrbm93IGV4 YWN0bHkgd2hlbiB0aGluZ3MgYXJlIHJlYWR5IHRvIGJlIHVzZWQuCgpUaGVvcmV0aWNhbGx5LCB3 ZSBzaG91bGQgd2FpdCBmb3IgYm90aCB0aGUgaTJjIGFuZCBEU0kgZW5kcyB0byBiZQphdHRhY2hl ZCB0byB0aGVpciByZXNwZWN0aXZlIGNvbnRyb2xsZXIgYmVmb3JlIHN0YXJ0aW5nIHVzaW5nIHRo ZQpkZXZpY2UuIEJ1dCB0aGF0IHJlcXVpcmVzIHJlcHJlc2VudGluZyB0aGluZ3Mgd2l0aCBhIDNy ZCBEVCBub2RlCmFnZ3JlZ2F0aW5nIGJvdGggY29tcG9uZW50czoKCglpMmMtYnVzIHsKCQkuLi4K CgkJYWR2NzUzM19pMmNAeHggewoJCQkuLi4KCQkJYWR2LGFkdjc1MzMtbWFzdGVyID0gPCZhZHY3 NTMzPjsKCQl9OwoJfTsKCgkuLi4KCWRzaS1idXMgewoJCS4uLgoJCWFkdjc1MzNfZHNpOiBhZHY3 NTMzX2RzaUB4eCB7CgkJCS4uLgoJCQlhZHYsYWR2NzUzMy1tYXN0ZXIgPSA8JmFkdjc1MzM+OwoJ CX07Cgl9OwoKCWFkdjc1MzM6IGFkdjc1MzMgewoJCS4uLgoJCWFkdixhZHY3NTMzLWRzaSA9IDwm YWR2NzUzM19kc2k+OwoJCWFkdixhZHY3NTMzLWkyYyA9IDwmYWR2NzUzM19pMmM+OwoJfTsKCgpJ IGd1ZXNzIHRoaXMga2luZCBvZiByZXByZXNlbnRhdGlvbiBoYXMgYWxyZWFkeSBiZWVuIGRpc2N1 c3NlZCBhbmQKcmVqZWN0ZWQgZm9yIGdvb2QgcmVhc29ucyAobm90ZSB0aGF0IHdlIGNvdWxkIGFs c28gdXNlIE9GIGdyYXBoCnJlcHJlc2VudGF0aW9uIHRvIGxpbmsgYWR2NzUzMyBtYXN0ZXIgYW5k IGl0cyBjb21wb25lbnRzKS4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJp LWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752938AbdFVJXr (ORCPT ); Thu, 22 Jun 2017 05:23:47 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:33625 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751163AbdFVJXp (ORCPT ); Thu, 22 Jun 2017 05:23:45 -0400 Date: Thu, 22 Jun 2017 11:23:32 +0200 From: Boris Brezillon To: Archit Taneja , Laurent Pinchart Cc: Benjamin Gaignard , Eric Anholt , Mark Rutland , devicetree@vger.kernel.org, Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" , Philippe Cornu , Rob Herring , Thierry Reding Subject: Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge. Message-ID: <20170622112332.7ec7b65c@bbrezillon> In-Reply-To: References: <20170615204130.19255-1-eric@anholt.net> <20170615204130.19255-2-eric@anholt.net> <777ee1b1-e5ce-ea29-8a48-f792354a22d1@codeaurora.org> <871sqkouvr.fsf@eliezer.anholt.net> <8e148170-b626-b426-3c94-b93d2746f4ce@codeaurora.org> <871sqe7ei0.fsf@eliezer.anholt.net> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 22 Jun 2017 13:47:43 +0530 Archit Taneja wrote: > On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: > > 2017-06-20 19:31 GMT+02:00 Eric Anholt : > >> Archit Taneja writes: > >> > >>> On 06/16/2017 08:13 PM, Eric Anholt wrote: > >>>> Archit Taneja writes: > >>>> > >>>>> On 06/16/2017 02:11 AM, Eric Anholt wrote: > >>>>>> If the panel-bridge is being set up after the drm_mode_config_reset(), > >>>>>> then the connector's state would never get initialized, and we'd > >>>>>> dereference the NULL in the hotplug path. We also need to register > >>>>>> the connector, so that userspace can get at it. > >>>>>> > >>>>> > >>>>> Shouldn't the KMS driver make sure the panel-bridge is set up before > >>>>> drm_mode_config_reset? Is it the case when we're inserting the > >>>>> panel-bridge driver as a module? > >>>>> > >>>>> > >>>>> All the connectors that have been added are registered automatically > >>>>> when drm_dev_register() is called by the KMS driver. Registering a > >>>>> connector in the middle of setting up our driver is prone to race > >>>>> conditions if the userspace decides to use them immediately. > >>>> > >>>> Yeah, this is fixing initializing panel_bridge at DSI host_attach time, > >>>> which in the case of a panel module that creates the DSI device > >>>> (adv7533-style, like you said I should use as a reference) will be after > >>>> drm_mode_config_reset() and drm_dev_register(). > >>> > >>> Okay. In the case of the msm kms driver, we defer probe until the > >>> adv7533 module is inserted, only then we proceed to drm_mode_config_reset() > >>> and drm_dev_register(). I assumed this was the general practice followed by > >>> most kms drivers. I.,e the kms driver defers probe until all connector > >>> related modules are inserted, and only then proceed to create a drm device. > >> > >> The problem, though, is the panel driver needs the MIPI DSI host to > >> exist to call mipi_dsi_device_register_full() during the probe process. > >> The adv7533 driver gets around this by registering the DSI device in the > >> bridge attach step, but drm_panel doesn't have an attach step. > > I'm not sure how we can get around this. We had discussion about this on irc > recently, but couldn't come up with a good conclusion. We could come up with a > panel_attach() callback to make it similar to bridges, but that's just us avoiding > the real issue. How about making DSI dev registration fully asynchronous, that is, DSI devs declared in the DT under the DSI host node will be registered/attached at probe time, and devs using another control bus (like the adv7533 controller over i2c) will be registered afterwards. That implies moving the drm_brige registration logic outside of the DSI host ->probe() path. The idea would be to check if all devs connected to the DSI bus are ready at dsi_host->attach() time. If they are, we can finally register the XXX -> DSI bridge. If they're not (because some devs connected to the DSI bus have not been probed yet), then we do not register the drm_bridge and wait for the next dsi_host->attach() event. With this solution, I think we can avoid the chicken&egg problem where the adv7533 dev is waiting for the DSI host to be probed to be able to register a DSI dev with mipi_dsi_device_register_full() and the DSI host needs all devs to be registered to not return -EPROBE_DEFER. > > >> > >> Another alternative is my original version of the panel driver that was > >> a mipi_dsi_device driver that registered the panel during the DSI device > >> probe. That's why vc4's panel lookup is during the MIPI DSI attach > >> phase, currently. > > > > This would require you to have a DSI device node in DT, rather than an i2c > node, right? I don't know if we should do that because of a limitation in > our drm_mipi_dsi and drm_panel frameworks. > > Does anyone have better ideas? Well, these mixed-bus cases are really unpractical to represent in the DT (and also not easy to deal with the device-model). I understand that the initial solution was to put the device under the control-bus, and not the data-bus, but then it leads to the current situation where we don't know exactly when things are ready to be used. Theoretically, we should wait for both the i2c and DSI ends to be attached to their respective controller before starting using the device. But that requires representing things with a 3rd DT node aggregating both components: i2c-bus { ... adv7533_i2c@xx { ... adv,adv7533-master = <&adv7533>; }; }; ... dsi-bus { ... adv7533_dsi: adv7533_dsi@xx { ... adv,adv7533-master = <&adv7533>; }; }; adv7533: adv7533 { ... adv,adv7533-dsi = <&adv7533_dsi>; adv,adv7533-i2c = <&adv7533_i2c>; }; I guess this kind of representation has already been discussed and rejected for good reasons (note that we could also use OF graph representation to link adv7533 master and its components).