| Message ID | 20220420231230.58499-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] Revert "drm: of: Properly try all possible cases for bridge/panel detection" | expand |
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. 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@linaro.org> > --- > 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 >
Hi Bjorn, On Wed 20 Apr 22, 16:12, Bjorn Andersson wrote: > 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@linaro.org> Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Cheers, Paul > --- > 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 >
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@linaro.org> > > --- > > 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 > >
On Wed, 20 Apr 2022 16:12:29 -0700, Bjorn Andersson wrote: > 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. > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
On Thu, Apr 21, 2022 at 09:13:02AM +0200, Paul Kocialkowski wrote: > 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). I was the one that asked for it because I thought this would be more convenient for everyone. Turns out I was very wrong :) Maxime
Hi On 4/21/22 09:13, Paul Kocialkowski wrote: > 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). This patch was merged within the last mainline kernel and was breaking our STM32MP15 platforms. Switching back to drm-based kernel (drm-next-fixes branch) made me realize this patch was faulty. I'm glad it was reverted. Thanks, Raphaël > > 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@linaro.org> >>> --- >>> 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 >>>
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);
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@linaro.org> --- drivers/gpu/drm/drm_of.c | 99 ++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 50 deletions(-)