Hi,
On Wed 20 Apr 22, 16:19, Bjorn Andersson wrote:
> On Wed 20 Apr 16:12 PDT 2022, Bjorn Andersson wrote:
>
> Sorry, I missed Jagan and Linus, author and reviewer of the reverted
> patch 2, among the recipients.
I'd be curious to have Jagan's feedback on why the change was needed in the
first place and whether an accepted dt binding relies on it.
We might be able to just keep the whole thing reverted (forever).
Paul
> Regards,
> Bjorn
>
> > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > bridge")' introduced the ability to describe a panel under a display
> > controller without having to use a graph to connect the controller to
> > its single child panel (or bridge).
> >
> > The implementation of this would find the first non-graph node and
> > attempt to acquire the related panel or bridge. This prevents cases
> > where any other child node, such as a aux bus for a DisplayPort
> > controller, or an opp-table to find the referenced panel.
> >
> > Commit '67bae5f28c89 ("drm: of: Properly try all possible cases for
> > bridge/panel detection")' attempted to solve this problem by not
> > bypassing the graph reference lookup before attempting to find the panel
> > or bridge.
> >
> > While this does solve the case where a proper graph reference is
> > present, it does not allow the caller to distinguish between a
> > yet-to-be-probed panel or bridge and the absence of a reference to a
> > panel.
> >
> > One such case is a DisplayPort controller that on some boards have an
> > explicitly described reference to a panel, but on others have a
> > discoverable DisplayPort display attached (which doesn't need to be
> > expressed in DeviceTree).
> >
> > This reverts commit '67bae5f28c89 ("drm: of: Properly try all possible
> > cases for bridge/panel detection")', as a step towards reverting commit
> > '80253168dbfd ("drm: of: Lookup if child node has panel or bridge")'.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/drm_of.c | 99 ++++++++++++++++++++--------------------
> > 1 file changed, 49 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > index f4df344509a8..026e4e29a0f3 100644
> > --- a/drivers/gpu/drm/drm_of.c
> > +++ b/drivers/gpu/drm/drm_of.c
> > @@ -214,29 +214,6 @@ int drm_of_encoder_active_endpoint(struct device_node *node,
> > }
> > EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
> >
> > -static int find_panel_or_bridge(struct device_node *node,
> > - struct drm_panel **panel,
> > - struct drm_bridge **bridge)
> > -{
> > - if (panel) {
> > - *panel = of_drm_find_panel(node);
> > - if (!IS_ERR(*panel))
> > - return 0;
> > -
> > - /* Clear the panel pointer in case of error. */
> > - *panel = NULL;
> > - }
> > -
> > - /* No panel found yet, check for a bridge next. */
> > - if (bridge) {
> > - *bridge = of_drm_find_bridge(node);
> > - if (*bridge)
> > - return 0;
> > - }
> > -
> > - return -EPROBE_DEFER;
> > -}
> > -
> > /**
> > * drm_of_find_panel_or_bridge - return connected panel or bridge device
> > * @np: device tree node containing encoder output ports
> > @@ -259,44 +236,66 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
> > struct drm_panel **panel,
> > struct drm_bridge **bridge)
> > {
> > - struct device_node *node;
> > - int ret;
> > + int ret = -EPROBE_DEFER;
> > + struct device_node *remote;
> >
> > if (!panel && !bridge)
> > return -EINVAL;
> > -
> > if (panel)
> > *panel = NULL;
> > - if (bridge)
> > - *bridge = NULL;
> > -
> > - /* Check for a graph on the device node first. */
> > - if (of_graph_is_present(np)) {
> > - node = of_graph_get_remote_node(np, port, endpoint);
> > - if (node) {
> > - ret = find_panel_or_bridge(node, panel, bridge);
> > - of_node_put(node);
> > -
> > - if (!ret)
> > - return 0;
> > - }
> > - }
> >
> > - /* Otherwise check for any child node other than port/ports. */
> > - for_each_available_child_of_node(np, node) {
> > - if (of_node_name_eq(node, "port") ||
> > - of_node_name_eq(node, "ports"))
> > + /**
> > + * Devices can also be child nodes when we also control that device
> > + * through the upstream device (ie, MIPI-DCS for a MIPI-DSI device).
> > + *
> > + * Lookup for a child node of the given parent that isn't either port
> > + * or ports.
> > + */
> > + for_each_available_child_of_node(np, remote) {
> > + if (of_node_name_eq(remote, "port") ||
> > + of_node_name_eq(remote, "ports"))
> > continue;
> >
> > - ret = find_panel_or_bridge(node, panel, bridge);
> > - of_node_put(node);
> > + goto of_find_panel_or_bridge;
> > + }
> > +
> > + /*
> > + * of_graph_get_remote_node() produces a noisy error message if port
> > + * node isn't found and the absence of the port is a legit case here,
> > + * so at first we silently check whether graph presents in the
> > + * device-tree node.
> > + */
> > + if (!of_graph_is_present(np))
> > + return -ENODEV;
> > +
> > + remote = of_graph_get_remote_node(np, port, endpoint);
> > +
> > +of_find_panel_or_bridge:
> > + if (!remote)
> > + return -ENODEV;
> > +
> > + if (panel) {
> > + *panel = of_drm_find_panel(remote);
> > + if (!IS_ERR(*panel))
> > + ret = 0;
> > + else
> > + *panel = NULL;
> > + }
> > +
> > + /* No panel found yet, check for a bridge next. */
> > + if (bridge) {
> > + if (ret) {
> > + *bridge = of_drm_find_bridge(remote);
> > + if (*bridge)
> > + ret = 0;
> > + } else {
> > + *bridge = NULL;
> > + }
> >
> > - /* Stop at the first found occurrence. */
> > - if (!ret)
> > - return 0;
> > }
> >
> > - return -EPROBE_DEFER;
> > + of_node_put(remote);
> > + return ret;
> > }
> > EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
> >
> > --
> > 2.35.1
> >
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
