Re: [PATCH v2 2/2] dt-bindings: drm/bridge: Document Cadence DSI bridge bindings

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Sat, 3 Jun 2017 23:43:17 +0530
Archit Taneja <architt@codeaurora.org> 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@free-electrons.com>
> > ---
> >  .../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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel