[PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties

[Thierry Reding]

On Tue, Sep 23, 2014 at 02:15:54PM +0300, Tomi Valkeinen wrote:
> On 23/09/14 12:28, Thierry Reding wrote:
> 
> >> But, for example, let's say that the board is designed in a way that for
> >> panel0 the bridge needs to output a bit higher level voltages than for
> >> panel1. That's not a property of the panel, so it can't be queried from
> >> the panel.
> >>
> >> That feature (varying the voltage level) is specific to the bridge, and
> >> thus the properties should be in the bridge's DT data. So we need two
> >> different configurations in the bridge, one for panel0 and one for
> >> panel1. This is what endpoints give us.
> > 
> > You could get the same by moving the mux in front of the bridge instead.
> 
> I didn't understand. Moving the mux between the SoC and the bridge? How
> would that solve the problem. Or what do you mean with "in the front of
> the bridge"?

Because then you'd have multiple instances of the bridge that could
describe which parameters it wants. But writing it out loud this is
equally bad. More below.

> > Trying to do this within the bridge's node directly has two flaws:
> > 
> > 	1) It violates the DT principle of describing hardware. The
> > 	   device itself does not know anything about multiple streams
> > 	   and deals only with a single input and output. You'd be
> > 	   describing a board specific aspect in the device binding.
> 
> We _are_ describing board specific aspects in the board.dts file. That's
> what it is about.
> 
> If the board's hardware is such that the bridge's voltage level needs to
> be higher when using panel0, that's very much describing hardware.

You misunderstood. It's describing a use-case rather than the device
itself. You describe, *within the device node*, that it can be muxed
whereas the device itself doesn't know anything about muxing. That's
where the violation is. You require a board-integration issue to be
described in the binding of a specific device.

> > 	2) Such a solution would have to be implemented for all bridge
> > 	   devices that can potentially be muxed (really all of them).
> > 	   If you describe it accurately in a separate mux node you get
> > 	   genericity for free and it will work for all bridges.
> 
> I do agree that a generic mux is better if there's nothing special to be
> configured. But how do you use a generic mux node for bridge device
> specific features?

Good question. I don't have an immediate answer to that and would have
to think about it for some more.

> >> I disagree. If we don't have a common way to describe the connections
> >> between video devices, we cannot:
> >>
> >> - have a common helper library to parse the connections
> > 
> > We have a common helper already. It's called of_parse_phandle().
> 
> Yes, but what is the name of the property, and where does it go? Does
> the panel have a phandle to the bridge? Does the bridge have a phandle
> to the panel?

My point was that we don't need a common helper library if we have a way
of getting at the phandle and resolve the phandle to a bridge or a
panel. While I think standard names can be advantageous, using the
regular of_parse_phandle() and a lookup function we don't even need
them.

> >> - study the connections before the drivers are loaded
> > 
> > Why would you need that?
> 
> I think this was discussed earlier, maybe Laurent remembers the use cases.
> 
> I think one possibility here is to have an understanding of the
> pipelines even if the modules have not been loaded or a single device
> has failed to probe.

I don't see how that would be useful.

> >> - handle the connections anywhere else than the specific driver for the
> >> component
> > 
> > Why should we do that?
> 
> We could, for example, have a single component (common or SoC specific)
> that manages the video pipelines. The drivers for the
> encoders/bridges/panels would just probe the devices, without doing
> anything else. The master component would then parse the links and
> connect the devices in whatever way it sees best.

You don't need any of that. The SoC driver simply obtains a phandle to
the first bridge connected to it. That bridge will already have done the
same for any chained bridges. There's no need to connect them in any way
anymore because they are already properly set up.

> > Having a standard representation only matters if you want to be able to
> > parse the pipeline without knowing about the device specifics. But I'm
> > not sure I understand why you would want to do that. This sounds like
> > you'd want some generic code to be able to construct a pipeline. But at
> > the same time you can't do anything meaningful with that pipeline
> > because the generic code doesn't know how to program the pipeline. The
> > only thing you'll get is a list of devices in the pipeline, but you can
> > do that by looking at the bindings and the DT content, no matter what
> > the properties are named
> 
> You can, but then you need to know all the possible variations and ways
> the phandles are used.

Which is described in the DT bindings. Really, the DT should be
describing the device in whatever is the most natural way for that
device. It is the job of the OS to abstract things away so that
components can be seemlessly integrated. The DT doesn't need to (and
shouldn't) have that kind of abstraction.

> >> Even if the points I gave about why a common way to describe connections
> >> is important would not be relevant today, it sounds much safer to have a
> >> standard representation in the DT for the connections. I'd only go for
> >> device specific custom descriptions if there's a very important reason
> >> for that. And I can't come up with any reason.
> > 
> > One of the good practices in DT is to keep property names similar to the
> > signal names of the chip to make it easy to correlate. So if you have a
> > bridge that converts RGB to LVDS and DSI for example, you'd have to
> > describe two outputs. That's kind of difficult to do with standard
> > property names. Well, it depends, you could do this:
> > 
> > 	bridge {
> > 		panels = <&lvds &dsi>;
> > 	};
> > 
> > Alternatively:
> > 
> > 	bridge {
> > 		lvds-panel = <&lvds>;
> > 		dsi-panel = <&dsi>;
> > 	};
> 
> Nothing says the bridge is connected to a panel, so "output" or such
> would probably be better there.

Hmm? How does the above not say that the bridge is connected to a panel?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140923/b32c1a4e/attachment.sig>