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

On 20.06.2017 08:56, Archit Taneja wrote:

On 06/19/2017 03:42 PM, Boris Brezillon wrote:

quoted

On Tue, 13 Jun 2017 11:02:47 +0200
Andrzej Hajda [off-list ref] wrote:

quoted

Hi,

Just spotted this thread.

On 06.06.2017 14:58, Tomi Valkeinen wrote:

quoted

On 06/06/17 15:48, Boris Brezillon wrote:
  

quoted

Okay. Thanks for the clarification. Can you confirm that this version
is correct?

  	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_out: port@1 {
  				reg = <1>;
				#address-cells = <1>;
  				#size-cells = <0>;
  
  				dsi_out_vc0: endpoint@0 {
					reg = <0>;
  					remote-endpoint = <&dsi_panel0_in>;
				};

  				dsi_out_vc1: endpoint@1 {
					reg = <1>;
  					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 {
					reg = <0>;
  					remote-endpoint = <&dsi_out_vc0>;
  				};
  			};
  		};
  
  		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 {
					reg = <0>;
  					remote-endpoint = <&dsi_out_vc1>;
  				};
  			};
  		};
  	};
  

Looks correct to me. I think it can be a bit shorter though:

- You don't need #address-cells and #size-cells for all. I think those
are inherited from the parent.
- If there's just one port and one endpoint, you can leave the 'reg'
out, as it's considered to be 0 by default.

So for the panel, you can have just:

port {
	dsi_panel1_in: endpoint {
		remote-endpoint = <&dsi_out_vc1>;
	};
};

In case DSI bus is used to both control and sending video signal you can
skip video links from dsi-host to dsi-child, so nodes can look like:


  	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>;
  				};
  			};
  
  		};
  
  		panel@0 {
  			compatible = "...";
  			reg = <0>;
  		};
  
  		panel@1 {
  			compatible = "...";
  			reg = <1>;
  		};
  	};

Does that mean I should make port1 (AKA DSI ouput port) optional?
IMHO, it's clearer when these links are explicitly described in the DT,
but maybe there are good reasons to keep it implicit for the "control
through DSI" case.

Tomi, Archit, any opinion on this?

I guess there isn't any harm in having the links explicitly described. It's
just that those ports won't be used by the driver in the "control through DSI"
case.

For the MSM DSI host bindings, we actually keep the DSI 'data-lanes' param in the
DSI output port, so it's mandatory even if the panel/bridge is controlled via
the host DSI bus.

Andrzej,

Are there any reasons why keeping the host-to-child links in the "control through
DSI" case could be harmful?

They are redundant - DSI bus already describes the 'link', so there are
classical potential issues connected with redundancy:
- which info should be parsed by the driver,
- what to do if links provide different information than DSI bus.
And as I understand current device-tree policy is to avoid them in such
case, see [1].

[1]: http://marc.info/?l=dri-devel&m=148354108702517&w=2

Regards
Andrzej

Archit

quoted

Regards,

Boris

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html