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 <eric@xxxxxxxxxx>: >>> Archit Taneja <architt@xxxxxxxxxxxxxx> writes: >>> >>>> On 06/16/2017 08:13 PM, Eric Anholt wrote: >>>>> Archit Taneja <architt@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> > ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f