Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"

[Jagan Teki]

Hi Paul,

On Wed, Apr 27, 2022 at 5:49 PM Paul Kocialkowski
<paul.kocialkowski@xxxxxxxxxxx> wrote:
>
> Hi Jagan,
>
> On Wed 27 Apr 22, 17:22, Jagan Teki wrote:
> > On Wed, Apr 27, 2022 at 12:29 PM Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Apr 21, 2022 at 1:54 PM Maxime Ripard <maxime@xxxxxxxxxx> wrote:
> > > >
> > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > + Linus
> > > > > + Marek
> > > > > + Laurent
> > > > > + Robert
> > > > >
> > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > > <bjorn.andersson@xxxxxxxxxx> wrote:
> > > > > >
> > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > > bridge")' attempted to simplify the case of expressing a simple panel
> > > > > > under a DSI controller, by assuming that the first non-graph child node
> > > > > > was a panel or bridge.
> > > > > >
> > > > > > Unfortunately for non-trivial cases the first child node might not be a
> > > > > > panel or bridge.  Examples of this can be a aux-bus in the case of
> > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > >
> > > > > > In these cases the reverted commit prevents the caller from ever finding
> > > > > > a reference to the panel.
> > > > > >
> > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > > panel or bridge")', in favor of using an explicit graph reference to the
> > > > > > panel in the trivial case as well.
> > > > >
> > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > switched drivers.  Do you have any suggestions on how to proceed to
> > > > > succeed in those use cases as well?
> > > >
> > > > I guess we could create a new helper for those, like
> > > > devm_drm_of_get_bridge_with_panel, or something.
> > >
> > > I think using the same existing helper and updating child support is
> > > make sense, as there is a possibility to use the same host for child
> > > and OF-graph bindings.
> > >
> > > I can see two possible solutions (as of now)
> > >
> > > 1. adding "dcs-child-type" bindings for child-based panel or bridge
> > > 2. iterate child and skip those nodes other than panel or bridge. or
> > > iterate sub-child to find it has a panel or bridge-like aux-bus (which
> > > is indeed hard as this configuration seems not 'standard' i think )
> > >
> > > Any inputs?
> >
> > Checking aux-bus with the sub-node panel can be a possible check to
> > look at it, any comments?
>
> That looks very fragile and oddly specific. Also why base changes on the
> original patch that you made?

It is just showcased a snippet to check the child's conditions.

>
> With the follow-up fixes, we are checking the of graph first and only
> considering child nodes if the of graph and remote are missing, so there isn't
> really a need to be more specific in the child noise discrimination.

So, does it handle child panel or bridge finding? just keep in mind
the same call from the host need to handle with and without OF-graph
representation.

>
> Actually I should also make a new version of "drm: of: Improve error handling in
> bridge/panel detection" to also return -ENODEV if of_graph_get_remote_node
> fails, so that it doesn't try to use the child node when the graph is defined
> but not remote is defined.

Jagan.