On Sat, Mar 08, 2014 at 01:05:50AM +0800, Grant Likely wrote: > On Wed, 26 Feb 2014 15:48:49 +0100, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote: > > Or two separate display controllers with two parallel outputs each, a > > 4-port multiplexer, and an encoder (e.g. lvds): > > > > ,------------------. > > | display [port@0]--. ,-------------. > > | controller [port@1]-. `-[port@0] | > > `------------------´ `--[port@1] mux | > > ,------------------. ,--[port@2] | ,-------------. > > | display [port@0]-´ ,-[port@3] [port@4]--[port] encoder | > > | controller [port@1]--´ `-------------´ `-------------´ > > `------------------´ > > Similar question here, how is the above complex assembled into a logical > device? Is there an overarching controller device for this component > complex? This is exactly the problem we discussed at Kernel Summit during the DRM et.al. session. It's the multiple component problem. We have this solved in a subsystem and firmware independent manner. The way this works in imx-drm is that the graph of connections is scanned to discover which component devices are required, and once all the components are present, we initialise the logical device. Further than that, the driver structure is an implemention specific detail. The starting point is the CPU facing part, because that's what provides the logical interfaces, especially as there may be more than one logical interface (which each of which may be entirely separate kernel subsystems - display vs capture.) > > ,--------. ,---------. > > | dc [port]--[port@0] | > > `--------´ | encoder | ,-----------. > > | [port@1]--[port] panel | > > `---------´ `-----------´ > > In all of the above examples I see a unidirectional data flow. There are > producer ports and consumer ports. Is there any (reasonable) possibility > of peer ports that are bidirectional? Generally, they will be unidirectional, but when you realise that you want to be able to walk the graph from the CPU point of view, the data flow direction is meaningless. What you need to know is how the components are connected together. If you model the links as per the data flow direction, then while you may be able to walk the display side - eg, dc -> encoder -> panel due to your DT links, you wouldn't be able to walk from the CPU interface down to your camera sensor - you would need to do a full scan of the DT for every interation to reverse the direction of the linkages. > > According to video-interfaces.txt, it is expected that endpoints contain > > phandles pointing to the remote endpoint on both sides. I'd like to > > leave this up to the more specialized bindings, but I can see that this > > makes enumerating the connections starting from each device tree node > > easier, for example at probe time. > > This has come up in the past. That approach comes down to premature > optimization at the expense of making the data structure more prone to > inconsistency. I consider it to be a bad pattern. > > Backlinks are good for things like dynamically linked lists that need to > be fast and the software fully manages the links. For a data structure like > the FDT it is better to have the data in one place, and one place only. > Besides, computers are *good* at parsing data structures. :-) > > I appreciate that existing drivers may be using the backlinks, but I > strongly recommend not doing it for new users. "Backlinks" doesn't convey to me exactly what you mean here. Are you talking about the ones in the opposite direction to data flow, or the ones pointing upstream towards the CPU? (I'm going to use upstream to refer to the ones pointing towards the CPU.) That matters: as I've said above, when creating logical devices, you tend to start at the CPU interfaces and work your way down the graph. Working the opposite way is much harder, especially if you hit some kind of muxing arranagement which results in two sensors which combine into one logical grabbing device. > > If the back links are not provided in the device tree, a device at the > > receiving end of a remote-endpoint phandle can only know about connected > > remote ports after the kernel parsed the whole graph and created the > > back links itself. > > A whole tree parse isn't expensive. We do it all the time. A while tree parse may not be expensive, but to do it multiple times increases the cost manyfold. Taking the first example above, it may be needed to do that five times, one time per link, and that may be just one subsystem. It could very easily become 10 times, at which point the expense is an order of magnitude more than it was. We do have real hardware which is arranged as per that first diagram. It's the iMX6Q - two IPUs each with two display controllers, giving a total of four output video streams. Those are then fed into a set of muxes which then feed into a set of encoders. Effectively, all those muxes make a switching matrix, allowing you to connect any encoder to any of the four output video streams - even connecting two or more encoders to the same video stream. There's HDMI, TV, LDB, and parallel. So that's 20 links in total (in reality, it's 16 because we don't represent the mux-to-encoder link because it's not relevant.) So, is 16x "a whole tree parse which isn't expensive" still not expensive? I'm not saying that we /need/ this, I'm merely pointing out that your comment may be correct for one-offs, but that may not be what happens in reality. I think for the case I mention above, it's entirely possible to do without the upstream links - whether that suits everyone, I'm not sure. > > > Graphs should be one direction only. > > > > But this is not what the current binding in video-interfaces.txt > > describes. I don't think it is a good idea to explicitly forbid > > backlinks in this binding. > > Nah, I disagree. The binding document /should/ forbid it to make it > really clear what the common pattern is. The code doesn't have to > enforce it (you don't want to break existing platforms), but the > document should be blunt. > > Another thought. In terms of the pattern, I would add a recommendation > that there should be a way to identify ports of a particular type. ie. > If I were using the pattern to implement an patch bay of DSP filters, > where each input and output, then each target node should have a unique > identifier property analogous to "interrupt-controller" or > "gpio-controller". In this fictitious example I would probably choose > "audiostream-input-port" and "audiostream-output-port" as empty > properties in the port nodes. (I'm not suggesting a change to the > existing binding, but making a recommendation to new users). Given the amount of discussion of these bindings which are already in the kernel, I have to ask a really obvious question: were they reviewed before they were merged into the kernel? It sounds like the above discussion should already have happened... -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
