Archit Taneja <architt@xxxxxxxxxxxxxx> writes: > 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. All versions of this patch have had one of those, because either that's where you attach the driver (the first, no-core-changes version of the driver) or you need it for the of-graph connections anyway. These later versions just haven't had a compatible string on the DSI device node.
Attachment:
signature.asc
Description: PGP signature
