Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752877AbdFVJbu (ORCPT ); Thu, 22 Jun 2017 05:31:50 -0400 Received: from mx08-00178001.pphosted.com ([91.207.212.93]:51281 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751116AbdFVJbs (ORCPT ); Thu, 22 Jun 2017 05:31:48 -0400 From: Philippe CORNU To: Archit Taneja , Benjamin Gaignard , Eric Anholt CC: "dri-devel@lists.freedesktop.org" , Andrzej Hajda , Laurent Pinchart , Thierry Reding , Rob Herring , Mark Rutland , "devicetree@vger.kernel.org" , Linux Kernel Mailing List , Boris Brezillon Subject: Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge. Thread-Topic: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge. Thread-Index: AQHS6zAM03cpATVi30+MCv6R5dS2CaIwfLIA Date: Thu, 22 Jun 2017 09:31:18 +0000 Message-ID: <479892b4-909b-e2c0-025d-844f75d72899@st.com> 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> In-Reply-To: Accept-Language: fr-FR, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.75.127.47] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-06-22_05:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v5M9W2XH028384 Content-Length: 4146 Lines: 98 On 06/22/2017 10:17 AM, 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. > >>> >>> 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? > > Thanks, > Archit > >> + Philippe in copy because we have the same probing issue when adding >> panel-bridge >> with the dsi bridge >> When adding panel-bridge support to the Synopsys DesignWare DSI bridge, I came to the conclusion that the only solution to make it works properly (without patching drm) was to "add the DSI bridge" at the end of the dw_mipi_dsi_host_attach() (see https://lists.freedesktop.org/archives/dri-devel/2017-June/144717.html) ie. defers crtc & encoder probing until the panel-bridge connector is created. Before that, I spent a lot of time trying different solutions like patching panel-bridge as Eric did (drm_connector_register...), adding bind/unbind() everywhere, debugging around __drm_mode_object_add... Surely DSI Host & device mechanism + panels add complexity to the related connector creation... After reading this thread, I have no good solution to suggest... and "deferring probing until connector registration" works fine now on my side and seems to be the way others drivers work... Philippe >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> >