[PATCH v4] drm: of: Lookup if child node has panel or bridge

[Paul Kocialkowski]

Hi Jagan,

On Wed 02 Feb 22, 21:34, Jagan Teki wrote:
> Devices can also be child nodes when we also control that device
> through the upstream device (ie, MIPI-DCS for a MIPI-DSI device).
> 
> drm_of_find_panel_or_bridge can lookup panel or bridge for a given
> device has port and endpoint and it fails to lookup if the device
> has a child nodes.

This patch breaks the logicvc drm driver that I'm currently developping.
The symptom is that drm_of_find_panel_or_bridge now always returns
-EPROBE_DEFER even after the panel has probed and is running well.
It seems that the function can no longer find the panel.

I haven't figured out the details, but reverting your patch makes
it work again. I suspect other drivers might be affected as well, so
it would probably be a good idea to revert the patch until the root
cause is clearly understood and the patch can be adapted accordingly.

Here is what the device-tree looks like:

/ {
	panel: panel-lvds {
		compatible = "panel-lvds";

		[...]

		port {
			#address-cells = <1>;
			#size-cells = <0>;

			panel_input: endpoint at 0 {
				reg = <0>;
				remote-endpoint = <&logicvc_output>;
			};
		};
	};
};

&amba {
	logicvc: logicvc at 43c00000 {
		compatible = "xylon,logicvc-3.02.a", "syscon", "simple-mfd";
		reg = <0x43c00000 0x6000>;

		#address-cells = <1>;
		#size-cells = <1>;

		[...]

		logicvc_display: display-engine at 0 {
			compatible = "xylon,logicvc-4.01.a-display";

			[...]

			port {
				#address-cells = <1>;
				#size-ce/lls = <0>;

				logicvc_output: endpoint at 0 {
					reg = <0>;
					remote-endpoint = <&panel_input>;
				};
			};
		};
	};
};

Cheers,

Paul

> This patch add support to lookup for a child node of the given parent
> that isn't either port or ports.
> 
> Example OF graph representation of DSI host, which has port but
> not has ports and has child panel node.
> 
> dsi {
> 	compatible = "allwinner,sun6i-a31-mipi-dsi";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	port {
> 		dsi_in_tcon0: endpoint {
> 			remote-endpoint = <tcon0_out_dsi>;
> 	};
> 
> 	panel at 0 {
> 		reg = <0>;
> 	};
> };
> 
> Example OF graph representation of DSI host, which has ports but
> not has port and has child panel node.
> 
> dsi {
>         compatible = "samsung,exynos5433-mipi-dsi";
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
> 	ports {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		port at 0 {
> 			reg = <0>;
> 
>                 	dsi_to_mic: endpoint {
>                         	remote-endpoint = <&mic_to_dsi>;
>                 	};
>                 };
>         };
> 
>         panel at 0 {
>                 reg = <0>;
>         };
> };
> 
> Example OF graph representation of DSI host, which has neither a port
> nor a ports but has child panel node.
> 
> dsi0 {
> 	compatible = "ste,mcde-dsi";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	panel at 0 {
> 		reg = <0>;
> 	};
> };
> 
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> Reviewed-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> Changes for v4:
> - update comments and commit message
> Changes for v3:
> - updated based on other usecase where 'ports' used along with child
> Changes for v2:
> - drop of helper
> https://patchwork.kernel.org/project/dri-devel/cover/20211207054747.461029-1-jagan@amarulasolutions.com/
> - support 'port' alone OF graph
> - updated comments
> - added simple code
> 
>  drivers/gpu/drm/drm_of.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 59d368ea006b..9d90cd75c457 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -249,6 +249,21 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
>  	if (panel)
>  		*panel = NULL;
>  
> +	/**
> +	 * 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,
> @@ -259,6 +274,8 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
>  		return -ENODEV;
>  
>  	remote = of_graph_get_remote_node(np, port, endpoint);
> +
> +of_find_panel_or_bridge:
>  	if (!remote)
>  		return -ENODEV;
>  

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220303/560d338e/attachment.sig>