On Tue, Jan 8, 2019 at 2:22 PM Russell King - ARM Linux linux@armlinux.org.uk wrote:
On Tue, Jan 08, 2019 at 01:27:56PM +0100, Andrzej Hajda wrote:
On 08.01.2019 12:38, Russell King - ARM Linux wrote:
On Tue, Jan 08, 2019 at 12:25:34PM +0100, Andrzej Hajda wrote:
Issues with device links have nothing to do with hotplugging, they are generic - lifetime of the objects (drm_bridge, drm_panel) is just slightly different of lifetime of device links, and this is racy even if you do not want hotplugging. Moreover since drm_dev is not device (has no associated struct device) assuming we can reuse its parent to create device link results in circular dependencies.
How about having the device links created depending on whether the main drm driver wants them or not - that would mean that Exynos could continue avoiding them, but others that want them can have the links?
That should be OK for Exynos. But regardless of Exynos device_links at the current state will not work correctly with bridges/panels as I described earlier.
However, I'm not sure you're correct with your interpretation of the documentation. Firstly, the documentation says:
Another example for an inconsistent state would be a device link that represents a driver presence dependency, yet is added from the consumer's ->probe callback while the supplier hasn't probed yet: Had the driver core known about the device link earlier, it wouldn't have probed the consumer in the first place. The onus is thus on the consumer to check presence of the supplier after adding the link, and defer probing on non-presence.
This is fine - if we add the device link from of_drm_find_bridge(), we will be in the consumer's ->probe function, and the supplier must have been probed for us to find the struct drm_bridge.
Secondly, device links are created by the regulator support whenever a regulator is "got" - between the supplier and the consumer. I'm not sure that regulator uses this in a safe way since it looks to me like there could be a race between the point where the regulator has been found and the point that the device link is created, and the regulator supplier being unbound. Regulator uses stateless device links to order PM, but not to remove consumers when the supplier goes away.
Finally, I believe that CCF is looking at using device links as well, which will mean a link is established by a clk_get() type operation and released in a clk_put() operation. There hasn't been any code merged for this, but I have seen it discussed.
These all have in common one thing - a device link is created at the point that the resource is obtained and removed when the resource is released.
I don't see how DRM bridges are any different from any other resource in the system, and why you think that device links wouldn't work for DRM bridges.
Yeah, for PM ordering device links should work for bridges and panels. I think all the questions/confusion is whether they also work for load/unload ordering, which they atm don't anyway because if you unload a provider and then reload it, the consumer isn't added to the reprobe list (unlikely what component achieves, which does call master bind again in this case).
I also think the entire discussion around whether you can hotadd bridges/panels after drm_dev_register is orthogonal, or at least should be. If you want to support hot-adding, they neither can you use device links nor component, because they both create a more static connection between producer and consumer. Aside from hotadding anything aside from drm_connector isn't really supported, and you need to roll your own locking and everything (which exynos really doesn't seem to do - the fact it works if you're careful doesn't really change that). -Daniel