On Sat, 3 Jun 2017 23:43:17 +0530 Archit Taneja <architt@xxxxxxxxxxxxxx> wrote: > Hi, > > On 06/02/2017 05:34 PM, Boris Brezillon wrote: > > Document the bindings used for the Cadence DSI bridge. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> > > --- > > .../bindings/display/bridge/cdns,dsi.txt | 55 ++++++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt > > new file mode 100644 > > index 000000000000..770c5c5b1e93 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt > > @@ -0,0 +1,55 @@ > > +Cadence DSI bridge > > +================== > > + > > +The Cadence DSI bridge is a DPI to DSI bridge supporting up to 4 DSI lanes. > > Is this a separate chip, or an IP integrated into SoCs? It's supposed to be integrated into SoCs. > If it's the > latter, I don't think DPI on the its input side is the right term to > use. Maybe RGB would be more appropriate here. Well, the datasheet explicitly mentions DPI, and you can also send pixels in YUV422 and YUV420 format on this bus, so I don't think RGB is appropriate, but if you really want me to use RGB I can change that. BTW, can you detail why DPI is not appropriate for internal parallel busses. I don't understand why it makes a difference when the bus is exposed through external pins. > > > + > > +Required properties: > > +- compatible: should be set to "cdns,dsi". > > Would it be better to take a dw-hdmi like approach here? I.e, the > binding should be specific to the SoC that integrates this DSI > bridge? Hm, I was considering something slightly different: adding new compat strings to the DSI bridge driver and keep the interface to the display controller drivers as simple as possible to avoid duplicating the glue used to bind the component in all display controller drivers embedding this bridge. Note that right now we don't have any SoC embedding this IP. It has been tested on an FPGA with a very basic display controller (designed for testing purpose only). By exposing this IP as a simple bridge, we can easily attach it to any kind of display controller, and if we ever need to use the component framework, then it should be pretty easy to add support for this feature as a follow-up patch. > > > +- reg: physical base address and length of the controller's registers. > > +- interrupts: interrupt line connected to the DSI bridge. > > +- clocks: DSI bridge clocks. > > +- clock-names: must contain "pclk" and "sysclk". > > +- phys: phandle link to the MIPI D-PHY controller. > > +- phy-names: must contain "dphy". > > +- #address-cells: must be set to 1. > > +- #size-cells: must be set to 0. > > + > > +Required subnodes: > > +- ports: Ports as described in Documentation/devicetree/bindings/graph.txt. > > + Currently contains a single input port at address 0 representing the DPI > > + input. Other ports will be added later to support the SDI inputs. > > + Port 0 should be connected to a DPI encoder output. > > The output of the DSI bridge may be another bridge, which could be i2c > controlled. In that case, it won't be a child of the DSI bridge. For > such scenarios, we might want to define an output port for the bridge. Hm, okay. IIRC, this is something you mentioned when I asked how to describe input/output ports for a DSI bridge a while ago. I'm still not sure how the links between input and output endpoint are supposed to be described. For example, if you take the case where you have the DSI device directly described under the DSI host controller, should I create another node for this output port? Something like the following? dsi@xxx { #address-cells = <1>; #size-cells = <0>; ports { #address-cells = <1>; #size-cells = <0>; dpi_in: port@0 { reg = <0>; #address-cells = <1>; #size-cells = <0>; endpoint@0 { remote-endpoint = <&dpi_out>; }; }; dsi_out0: port@1 { reg = <1>; #address-cells = <1>; #size-cells = <0>; dsi_out0: endpoint@0 { remote-endpoint = <&dsi_panel0_in>; }; }; dsi_out0: port@2 { reg = <2>; #address-cells = <1>; #size-cells = <0>; dsi_out1: endpoint@0 { remote-endpoint = <&dsi_panel1_in>; }; }; }; panel@0 { compatible = "..."; reg = <0>; #address-cells = <1>; #size-cells = <0>; port@0 { #address-cells = <1>; #size-cells = <0>; reg = <0>; dsi_panel0_in: endpoint@0 { remote-endpoint = <&dsi_out0>; }; }; }; panel@1 { compatible = "..."; reg = <1>; #address-cells = <1>; #size-cells = <0>; port@0 { #address-cells = <1>; #size-cells = <0>; reg = <0>; dsi_panel1_in: endpoint@0 { remote-endpoint = <&dsi_out1>; }; }; }; }; Thanks for the review, Boris -- 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