| Message ID | 20221209152343.180139-8-jagan@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm: bridge: Add Samsung MIPI DSIM bridge | expand |
On 12/9/22 16:23, Jagan Teki wrote: > The child devices in MIPI DSI can be binding with OF-graph > and also via child nodes. > > The OF-graph interface represents the child devices via > remote and associated endpoint numbers like > > dsi { > compatible = "fsl,imx8mm-mipi-dsim"; > > ports { > port@0 { > reg = <0>; > > dsi_in_lcdif: endpoint@0 { > reg = <0>; > remote-endpoint = <&lcdif_out_dsi>; > }; > }; > > port@1 { > reg = <1>; > > dsi_out_bridge: endpoint { > remote-endpoint = <&bridge_in_dsi>; > }; > }; > }; > > The child node interface represents the child devices via > conventional child nodes on given DSI parent like > > dsi { > compatible = "samsung,exynos5433-mipi-dsi"; > > ports { > port@0 { > reg = <0>; > > dsi_to_mic: endpoint { > remote-endpoint = <&mic_to_dsi>; > }; > }; > }; > > panel@0 { > reg = <0>; > }; > }; > > As Samsung DSIM bridge is common DSI IP across all Exynos DSI > and NXP i.MX8M host controllers, this patch adds support to > lookup the child devices whether its bindings on the associated > host represent OF-graph or child node interfaces. > > v9, v8, v7, v6, v5, v4, v3: > * none > > v2: > * new patch This looks like a good candidate for common/helper code which can be reused by other similar drivers.
On Sun, Dec 11, 2022 at 7:58 AM Marek Vasut <marex@denx.de> wrote: > > On 12/9/22 16:23, Jagan Teki wrote: > > The child devices in MIPI DSI can be binding with OF-graph > > and also via child nodes. > > > > The OF-graph interface represents the child devices via > > remote and associated endpoint numbers like > > > > dsi { > > compatible = "fsl,imx8mm-mipi-dsim"; > > > > ports { > > port@0 { > > reg = <0>; > > > > dsi_in_lcdif: endpoint@0 { > > reg = <0>; > > remote-endpoint = <&lcdif_out_dsi>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > > > dsi_out_bridge: endpoint { > > remote-endpoint = <&bridge_in_dsi>; > > }; > > }; > > }; > > > > The child node interface represents the child devices via > > conventional child nodes on given DSI parent like > > > > dsi { > > compatible = "samsung,exynos5433-mipi-dsi"; > > > > ports { > > port@0 { > > reg = <0>; > > > > dsi_to_mic: endpoint { > > remote-endpoint = <&mic_to_dsi>; > > }; > > }; > > }; > > > > panel@0 { > > reg = <0>; > > }; > > }; > > > > As Samsung DSIM bridge is common DSI IP across all Exynos DSI > > and NXP i.MX8M host controllers, this patch adds support to > > lookup the child devices whether its bindings on the associated > > host represent OF-graph or child node interfaces. > > > > v9, v8, v7, v6, v5, v4, v3: > > * none > > > > v2: > > * new patch > > This looks like a good candidate for common/helper code which can be > reused by other similar drivers. Yes, I have responded to the same comment of yours in v7 [1]. It is hard to make this code work in a generic way. [1] https://lore.kernel.org/all/CAMty3ZBtRZ-vDPQYX+m8uWmsD+vmbFOnCGVba8swQ8GWtWaKJQ@mail.gmail.com/
On 12/11/22 06:42, Jagan Teki wrote: > On Sun, Dec 11, 2022 at 7:58 AM Marek Vasut <marex@denx.de> wrote: >> >> On 12/9/22 16:23, Jagan Teki wrote: >>> The child devices in MIPI DSI can be binding with OF-graph >>> and also via child nodes. >>> >>> The OF-graph interface represents the child devices via >>> remote and associated endpoint numbers like >>> >>> dsi { >>> compatible = "fsl,imx8mm-mipi-dsim"; >>> >>> ports { >>> port@0 { >>> reg = <0>; >>> >>> dsi_in_lcdif: endpoint@0 { >>> reg = <0>; >>> remote-endpoint = <&lcdif_out_dsi>; >>> }; >>> }; >>> >>> port@1 { >>> reg = <1>; >>> >>> dsi_out_bridge: endpoint { >>> remote-endpoint = <&bridge_in_dsi>; >>> }; >>> }; >>> }; >>> >>> The child node interface represents the child devices via >>> conventional child nodes on given DSI parent like >>> >>> dsi { >>> compatible = "samsung,exynos5433-mipi-dsi"; >>> >>> ports { >>> port@0 { >>> reg = <0>; >>> >>> dsi_to_mic: endpoint { >>> remote-endpoint = <&mic_to_dsi>; >>> }; >>> }; >>> }; >>> >>> panel@0 { >>> reg = <0>; >>> }; >>> }; >>> >>> As Samsung DSIM bridge is common DSI IP across all Exynos DSI >>> and NXP i.MX8M host controllers, this patch adds support to >>> lookup the child devices whether its bindings on the associated >>> host represent OF-graph or child node interfaces. >>> >>> v9, v8, v7, v6, v5, v4, v3: >>> * none >>> >>> v2: >>> * new patch >> >> This looks like a good candidate for common/helper code which can be >> reused by other similar drivers. > > Yes, I have responded to the same comment of yours in v7 [1]. It is > hard to make this code work in a generic way. It seems the patch adds a for_each...() loop and a function call. Should be easy enough to turn that into a helper. What am I missing ?
On Sun, Dec 11, 2022 at 11:36 PM Marek Vasut <marex@denx.de> wrote: > > On 12/11/22 06:42, Jagan Teki wrote: > > On Sun, Dec 11, 2022 at 7:58 AM Marek Vasut <marex@denx.de> wrote: > >> > >> On 12/9/22 16:23, Jagan Teki wrote: > >>> The child devices in MIPI DSI can be binding with OF-graph > >>> and also via child nodes. > >>> > >>> The OF-graph interface represents the child devices via > >>> remote and associated endpoint numbers like > >>> > >>> dsi { > >>> compatible = "fsl,imx8mm-mipi-dsim"; > >>> > >>> ports { > >>> port@0 { > >>> reg = <0>; > >>> > >>> dsi_in_lcdif: endpoint@0 { > >>> reg = <0>; > >>> remote-endpoint = <&lcdif_out_dsi>; > >>> }; > >>> }; > >>> > >>> port@1 { > >>> reg = <1>; > >>> > >>> dsi_out_bridge: endpoint { > >>> remote-endpoint = <&bridge_in_dsi>; > >>> }; > >>> }; > >>> }; > >>> > >>> The child node interface represents the child devices via > >>> conventional child nodes on given DSI parent like > >>> > >>> dsi { > >>> compatible = "samsung,exynos5433-mipi-dsi"; > >>> > >>> ports { > >>> port@0 { > >>> reg = <0>; > >>> > >>> dsi_to_mic: endpoint { > >>> remote-endpoint = <&mic_to_dsi>; > >>> }; > >>> }; > >>> }; > >>> > >>> panel@0 { > >>> reg = <0>; > >>> }; > >>> }; > >>> > >>> As Samsung DSIM bridge is common DSI IP across all Exynos DSI > >>> and NXP i.MX8M host controllers, this patch adds support to > >>> lookup the child devices whether its bindings on the associated > >>> host represent OF-graph or child node interfaces. > >>> > >>> v9, v8, v7, v6, v5, v4, v3: > >>> * none > >>> > >>> v2: > >>> * new patch > >> > >> This looks like a good candidate for common/helper code which can be > >> reused by other similar drivers. > > > > Yes, I have responded to the same comment of yours in v7 [1]. It is > > hard to make this code work in a generic way. > > It seems the patch adds a for_each...() loop and a function call. Should > be easy enough to turn that into a helper. What am I missing ? What I'm saying here is, initially, I added for_each in the existing drm_of_find_panel_or_bridge helper but it fails to handle all drm_of use cases generically. You can find more information on this commit 80253168dbfd. I keep this in mind and will see whether it can support the new dsi helper once this supported is merged. Hope I'm clear on it. Jagan.
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c index baad09b2daeb..a4379c2ccb77 100644 --- a/drivers/gpu/drm/bridge/samsung-dsim.c +++ b/drivers/gpu/drm/bridge/samsung-dsim.c @@ -1356,18 +1356,52 @@ static int samsung_dsim_host_attach(struct mipi_dsi_host *host, struct samsung_dsim *dsi = host_to_dsi(host); const struct samsung_dsim_plat_data *pdata = dsi->plat_data; struct device *dev = dsi->dev; + struct device_node *np = dev->of_node; + struct device_node *remote; struct drm_panel *panel; int ret; - panel = of_drm_find_panel(device->dev.of_node); + /** + * 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; + + 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, 1, 0); + +of_find_panel_or_bridge: + if (!remote) + return -ENODEV; + + panel = of_drm_find_panel(remote); if (!IS_ERR(panel)) { dsi->out_bridge = devm_drm_panel_bridge_add(dev, panel); } else { - dsi->out_bridge = of_drm_find_bridge(device->dev.of_node); + dsi->out_bridge = of_drm_find_bridge(remote); if (!dsi->out_bridge) dsi->out_bridge = ERR_PTR(-EINVAL); } + of_node_put(remote); + if (IS_ERR(dsi->out_bridge)) { ret = PTR_ERR(dsi->out_bridge); DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret);
The child devices in MIPI DSI can be binding with OF-graph and also via child nodes. The OF-graph interface represents the child devices via remote and associated endpoint numbers like dsi { compatible = "fsl,imx8mm-mipi-dsim"; ports { port@0 { reg = <0>; dsi_in_lcdif: endpoint@0 { reg = <0>; remote-endpoint = <&lcdif_out_dsi>; }; }; port@1 { reg = <1>; dsi_out_bridge: endpoint { remote-endpoint = <&bridge_in_dsi>; }; }; }; The child node interface represents the child devices via conventional child nodes on given DSI parent like dsi { compatible = "samsung,exynos5433-mipi-dsi"; ports { port@0 { reg = <0>; dsi_to_mic: endpoint { remote-endpoint = <&mic_to_dsi>; }; }; }; panel@0 { reg = <0>; }; }; As Samsung DSIM bridge is common DSI IP across all Exynos DSI and NXP i.MX8M host controllers, this patch adds support to lookup the child devices whether its bindings on the associated host represent OF-graph or child node interfaces. v9, v8, v7, v6, v5, v4, v3: * none v2: * new patch Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- drivers/gpu/drm/bridge/samsung-dsim.c | 38 +++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)