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.ha...@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@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
>
>> Regards,
>>
>> Boris
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to