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

[Andrzej Hajda]

On 20.06.2017 08:56, Archit Taneja wrote:
>
> On 06/19/2017 03:42 PM, Boris Brezillon wrote:
>> On Tue, 13 Jun 2017 11:02:47 +0200
>> Andrzej Hajda <a.hajda at samsung.com> wrote:
>>
>>> Hi,
>>>
>>> Just spotted this thread.
>>>
>>> On 06.06.2017 14:58, Tomi Valkeinen wrote:
>>>> On 06/06/17 15:48, Boris Brezillon wrote:
>>>>   
>>>>> Okay. Thanks for the clarification. Can you confirm that this version
>>>>> is correct?
>>>>>
>>>>>   	dsi at xxx {
>>>>>   		#address-cells = <1>;
>>>>>   		#size-cells = <0>;
>>>>>   
>>>>>   		ports {
>>>>>   			#address-cells = <1>;
>>>>>   			#size-cells = <0>;
>>>>> 			dpi_in: port at 0 {
>>>>>   				reg = <0>;
>>>>>   				#address-cells = <1>;
>>>>>   				#size-cells = <0>;
>>>>>   
>>>>>   				endpoint at 0 {
>>>>>   					remote-endpoint = <&dpi_out>;
>>>>>   				};
>>>>>   			};
>>>>>   
>>>>>   			dsi_out: port at 1 {
>>>>>   				reg = <1>;
>>>>> 				#address-cells = <1>;
>>>>>   				#size-cells = <0>;
>>>>>   
>>>>>   				dsi_out_vc0: endpoint at 0 {
>>>>> 					reg = <0>;
>>>>>   					remote-endpoint = <&dsi_panel0_in>;
>>>>> 				};
>>>>>
>>>>>   				dsi_out_vc1: endpoint at 1 {
>>>>> 					reg = <1>;
>>>>>   					remote-endpoint = <&dsi_panel1_in>;
>>>>>   				};
>>>>>   			};
>>>>>   		};
>>>>>   
>>>>>   		panel at 0 {
>>>>>   			compatible = "...";
>>>>>   			reg = <0>;
>>>>>   			#address-cells = <1>;
>>>>>   			#size-cells = <0>;
>>>>>   
>>>>>   			port at 0 {
>>>>>   				#address-cells = <1>;
>>>>>   		                #size-cells = <0>;
>>>>>   				reg = <0>;
>>>>>   
>>>>>   				dsi_panel0_in: endpoint at 0 {
>>>>> 					reg = <0>;
>>>>>   					remote-endpoint = <&dsi_out_vc0>;
>>>>>   				};
>>>>>   			};
>>>>>   		};
>>>>>   
>>>>>   		panel at 1 {
>>>>>   			compatible = "...";
>>>>>   			reg = <1>;
>>>>>   			#address-cells = <1>;
>>>>>   			#size-cells = <0>;
>>>>>   
>>>>>   			port at 0 {
>>>>>   				#address-cells = <1>;
>>>>>   		                #size-cells = <0>;
>>>>>   				reg = <0>;
>>>>>   
>>>>>   				dsi_panel1_in: endpoint at 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 at xxx {
>>>   		#address-cells = <1>;
>>>   		#size-cells = <0>;
>>>   
>>>   		ports {
>>>   			#address-cells = <1>;
>>>   			#size-cells = <0>;
>>> 			dpi_in: port at 0 {
>>>   				reg = <0>;
>>>   				#address-cells = <1>;
>>>   				#size-cells = <0>;
>>>   
>>>   				endpoint at 0 {
>>>   					remote-endpoint = <&dpi_out>;
>>>   				};
>>>   			};
>>>   
>>>   		};
>>>   
>>>   		panel at 0 {
>>>   			compatible = "...";
>>>   			reg = <0>;
>>>   		};
>>>   
>>>   		panel at 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
>
>> Regards,
>>
>> Boris
>>