Re: [PATCH v3] drm: of: Lookup if child node has panel or bridge

[Maxime Ripard <maxime@cerno.tech>]

[-- Attachment #1: Type: text/plain, Size: 3406 bytes --]

On Wed, Jan 12, 2022 at 12:45:23PM +0100, Andrzej Hajda wrote:
> 
> On 12.01.2022 11:14, Jagan Teki wrote:
> > On Wed, Jan 12, 2022 at 3:33 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Wed, Jan 12, 2022 at 12:01:52AM +0530, Jagan Teki wrote:
> > > > Some OF graphs don't require 'port' or 'ports' to represent the
> > > > downstream panel or bridge; instead it simply adds a child node
> > > > on a given parent node.
> > > All bindings using OF graph nodes require either port or ports.
> > > 
> > > DSI Host however don't have to use the OF graph, and that's what you're
> > > talking about.
> > Yes, right now I can see DSI but this change is generic to any OF graph.
> > 
> > > > drm_of_find_panel_or_bridge can lookup panel or bridge for a given
> > > > node based on the OF graph port and endpoint and it fails to use
> > > > if the given node has a child panel or bridge.
> > > > 
> > > > This patch add support to lookup that given node has child panel
> > > > or bridge however that child node is neither a 'port' nor a 'ports'
> > > > node.
> > > > 
> > > > Example OF graph representation of DSI host, which has 'port'
> > > > but not has 'ports' and has child panel node.
> > > > 
> > > > dsi {
> > > >        compatible = "allwinner,sun6i-a31-mipi-dsi";
> > > >        #address-cells = <1>;
> > > >        #size-cells = <0>;
> > > > 
> > > >        port {
> > > >                dsi_in_tcon0: endpoint {
> > > >                        remote-endpoint = <tcon0_out_dsi>;
> > > >        };
> > > > 
> > > >        panel@0 {
> > > >                reg = <0>;
> > > >        };
> > > > };
> > > > 
> > > > Example OF graph representation of DSI host, which has 'ports'
> > > > but not has 'port' and has child panel node.
> > > > 
> > > > dsi {
> > > >          compatible = "samsung,exynos5433-mipi-dsi";
> > > >          #address-cells = <1>;
> > > >          #size-cells = <0>;
> > > > 
> > > >        ports {
> > > >                #address-cells = <1>;
> > > >                #size-cells = <0>;
> > > > 
> > > >                port@0 {
> > > >                        reg = <0>;
> > > > 
> > > >                        dsi_to_mic: endpoint {
> > > >                                remote-endpoint = <&mic_to_dsi>;
> > > >                        };
> > > >                  };
> > > >          };
> > > > 
> > > >          panel@0 {
> > > >                  reg = <0>;
> > > >          };
> > > > };
> > > I can't see how that one makes sense. The endpoint seems to have a
> > > single output, yet you also have a panel under it which is also an
> > > output? You should have at least the virtual channel of that endpoint
> > > somewhere to differentiate data between the panel and whatever is
> > > connected on the other side of that endpoint.
> > Same that I understood so far (based on v2 change), However we have
> > exynos5433 has this pipeline and Andrzej mentioned it is valid
> > pipeline on other thread.
> > 
> > May be Andrzej, can give more conclusive evidence for it.
> 
> 
> Hmm, this is DSI bridge (or encoder), which has one input (connected to mic,
> described by port 0) and one output (connected to DSI panel described by
> child relationship).
> 
> It looks for me quite natural.

Yeah, the dsi_to_mic feels weird if DSI is the receiver here, but it
makes sense, thanks

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]