On Fri, Jan 11, 2019 at 3:32 PM Russell King - ARM Linux <li...@armlinux.org.uk> wrote: > On Fri, Jan 11, 2019 at 03:26:44PM +0100, Rafael J. Wysocki wrote: > > On Fri, Jan 11, 2019 at 3:20 PM Daniel Vetter <dan...@ffwll.ch> wrote: > > > > > > On Wed, Jan 9, 2019 at 10:31 AM Russell King - ARM Linux > > > <li...@armlinux.org.uk> wrote: > > > > On Wed, Jan 09, 2019 at 10:24:01AM +0100, Rafael J. Wysocki wrote: > > > > > On Wed, Jan 9, 2019 at 10:13 AM Andrzej Hajda <a.ha...@samsung.com> > > > > > wrote: > > > > > > > > > > > > +CC: Rafael J. Wysocki <raf...@kernel.org> > > > > > > > > > > > > On 08.01.2019 16:07, Russell King - ARM Linux wrote: > > > > > > > On Tue, Jan 08, 2019 at 03:33:54PM +0100, Andrzej Hajda wrote: > > > > > > >> On 08.01.2019 14:21, Russell King - ARM Linux 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. > > > > > > >> > > > > > > >> Supplier usually is registered in it's probe time, so there is > > > > > > >> short > > > > > > >> period of time when supplier is available, but the probe is not > > > > > > >> yet > > > > > > >> finished - quite unsafe, but not impossible, especially if there > > > > > > >> exists > > > > > > >> some kind of notifications about resource appearance (MIPI-DSI > > > > > > >> case). > > > > > > > At some point during the supplier probe, the resource becomes > > > > > > > available > > > > > > > to consumers. At that point, device links can be setup before the > > > > > > > supplier has finished probing. So any driver that provides > > > > > > > resources > > > > > > > to another driver will, at some point during the provider's probe, > > > > > > > make resources available, and therefore be a candidate for device > > > > > > > links > > > > > > > to be created _before_ the probe function has returned. > > > > > > > > > > > > > > What is a problem is if the provider publishes resources, and > > > > > > > then fails > > > > > > > its probe function, causing the resource to disappear. > > > > > > > > > > > > > > > > > > But creating link to not-probed provider is still incorrect usage > > > > > > from > > > > > > device_links framework PoV, and my tests showed indeed that device > > > > > > link > > > > > > created before end of provider's probe does not work at all - and > > > > > > since > > > > > > it is stated in the documentation I guess it is by design. > > > > > > > > > > Yes, it is. > > > > > > > > So is the regulator support and the use of it being proposed for the CCF > > > > all going against the design of device links? In both those cases, > > > > device links _can_ be created while the supplier is still in the probe > > > > function by a consumer finding the resource. > > > > > > > > This seems fragile by design. > > > > > > Rafael, can you confirm? > > > > Let me quote from > > https://www.kernel.org/doc/html/latest/driver-api/device_link.html : > > > > "When driver presence on the supplier is irrelevant and only correct > > suspend/resume and shutdown ordering is needed, the device link may > > simply be set up with the DL_FLAG_STATELESS flag. In other words, > > enforcing driver presence on the supplier is optional." > > > > Which is exactly the case at hand here AFAICS. > > That is not what we're discussing. We are discussing using device > links to solve the problem with DRM bridges which may be removed > today from DRM without the rest of the DRM system being aware.
Yeah, we want device links not just for the suspend/resume ordering, but also as full dependencies. Atm that's solved with component (for the full generic case), but that needs more hand-rolled code. device_links would be much easier to integrate into all these subsystems (and you really want to unload the drm driver if clocks/transcoders/whatever disappears). For most drivers there's really no difference between the runtime dependencies for suspend/resume and the load/unload dependencies. Having two use 2 completely different solutions for the common case where they match exactly seems suboptimal. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
