[PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing

[Marek Vasut]

On 3/8/22 14:49, Maxime Ripard wrote:
> On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
>> On 3/8/22 13:51, Maxime Ripard wrote:
>>> On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
>>>> On 3/8/22 11:07, Jagan Teki wrote:
>>>>> On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut <marex at denx.de> wrote:
>>>>>>
>>>>>> On 3/8/22 09:03, Jagan Teki wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>> @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs chipone_bridge_funcs = {
>>>>>>>>      static int chipone_parse_dt(struct chipone *icn)
>>>>>>>>      {
>>>>>>>>             struct device *dev = icn->dev;
>>>>>>>> +       struct device_node *endpoint;
>>>>>>>>             struct drm_panel *panel;
>>>>>>>> +       int dsi_lanes;
>>>>>>>>             int ret;
>>>>>>>>
>>>>>>>>             icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
>>>>>>>> @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
>>>>>>>>                     return PTR_ERR(icn->enable_gpio);
>>>>>>>>             }
>>>>>>>>
>>>>>>>> +       endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
>>>>>>>> +       dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
>>>>>>>> +       icn->host_node = of_graph_get_remote_port_parent(endpoint);
>>>>>>>> +       of_node_put(endpoint);
>>>>>>>> +
>>>>>>>> +       if (!icn->host_node)
>>>>>>>> +               return -ENODEV;
>>>>>>>
>>>>>>> The non-ports-based OF graph returns a -19 example on the Allwinner
>>>>>>> Display pipeline in R16 [1].
>>>>>>>
>>>>>>> We need to have a helper to return host_node for non-ports as I have
>>>>>>> done it for drm_of_find_bridge.
>>>>>>>
>>>>>>> [1] https://patchwork.amarulasolutions.com/patch/1805/
>>>>>>
>>>>>> The link points to a patch marked "DO NOT MERGE", maybe that patch is
>>>>>> missing the DSI host port at 0 OF graph link ? Both port at 0 and port at 1 are
>>>>>> required, see:
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
>>>>>>
>>>>>> What is "non-ports-based OF graph" ?
>>>>>>
>>>>>> I don't see drm_of_find_bridge() in linux-next , what is that ?
>>>>>
>>>>> port at 0 is optional as some of the DSI host OF-graph represent the
>>>>> bridge or panel as child nodes instead of ports. (i think dt-binding
>>>>> has to fix it to make port at 0 optional)
>>>>
>>>> The current upstream DT binding document says:
>>>>
>>>>       required:
>>>>         - port at 0
>>>>         - port at 1
>>>>
>>>> So port at 0 is mandatory.
>>>
>>> In the binding, sure, but fundamentally the DT excerpt Jagan provided is
>>> correct. If the bridge supports DCS, there's no reason to use the OF
>>> graph in the first place: the bridge node will be a child node of the
>>> MIPI-DSI controller (and there's no obligation to use the OF-graph for a
>>> MIPI-DSI controller).
>>>
>>> I believe port at 0 should be made optional (or downright removed if
>>> MIPI-DCS in the only control bus).
>>
>> That's out of scope of this series anyway, so Jagan can implement patches
>> for that mode if needed.
> 
> Not really? You can't count on the port at 0 being there generally
> speaking, so you can't count on data-lanes being there either, which
> exactly what you're doing in this patch.

I can because the upstream DT bindings currently say port at 0 must be 
present, see above. If that requirement should be relaxed, sure, but 
that's a separate series.