On Fri, Dec 10, 2021 at 6:49 PM Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> devm_drm_of_get_bridge is capable of looking up the downstream
> bridge and panel and trying to add a panel bridge if the panel
> is found.
>
> Replace explicit finding calls with devm_drm_of_get_bridge.
>
> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> Cc: Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
Nice overall!
> - /* Look for a panel as a child to this node */
> - for_each_available_child_of_node(dev->of_node, child) {
> - panel = of_drm_find_panel(child);
> - if (IS_ERR(panel)) {
> - dev_err(dev, "failed to find panel try bridge (%ld)\n",
> - PTR_ERR(panel));
> - panel = NULL;
> -
> - bridge = of_drm_find_bridge(child);
> - if (!bridge) {
> - dev_err(dev, "failed to find bridge\n");
> - return -EINVAL;
> - }
> - }
> - }
> - if (panel) {
> - bridge = drm_panel_bridge_add_typed(panel,
> - DRM_MODE_CONNECTOR_DSI);
And we are guaranteed that the right type of connector will be
used here? (Just checking.)
> - if (IS_ERR(bridge)) {
> - dev_err(dev, "error adding panel bridge\n");
> - return PTR_ERR(bridge);
> - }
> - dev_info(dev, "connected to panel\n");
> - d->panel = panel;
How does this assignment happen after your patch?
I'm using that...
devm_drm_of_get_bridge() needs some more argument right?
> - } else if (bridge) {
> - /* TODO: AV8100 HDMI encoder goes here for example */
> - dev_info(dev, "connected to non-panel bridge (unsupported)\n");
> - return -ENODEV;
> - } else {
> - dev_err(dev, "no panel or bridge\n");
> - return -ENODEV;
> + bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
> + if (IS_ERR(bridge)) {
> + dev_err(dev, "error to get bridge\n");
> + return PTR_ERR(bridge);
I'm gonna want to test this somehow on the hardware. But the TODO comment
there wasn't supposed to be deleted if I will still need to take some special
action whether this is a panel bridge or some other bridge.
Yours,
Linus Walleij
