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

[Jagan Teki]

Hi Marek,

On Tue, Mar 8, 2022 at 4:00 PM Marek Vasut <marex at denx.de> 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.
>
> So I don't think any change is needed to this patch ?
>
> > Example OF-graph is on the commit message.
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_of.c?id=80253168dbfd256bca97cf7f13312863c5a7f2e5
>
> It seems the current upstream DT bindings only support DSI hosts which
> do provide an endpoint, because port at 0 is required per DT binding
> document. If you want to support the other options as listed in the
> aforementioned commit, then you would have to extend this driver and its
> bindings, but that is out of scope of this series.

Your comments seem to be valid, thanks.

Jagan.