[PATCH 0/2] drm: of: Lookup child panel_or_bridge

[PATCH 0/2] drm: of: Lookup child panel_or_bridge

[Jagan Teki]

This patch set add support to lookup if child node is panel or bridge
via drm_of_find_panel_or_bridge.

Any inputs?
Jagan.

Jagan Teki (2):
  of: Add helper to lookup non port child node
  drm: of: Lookup if child node is panel or bridge

 drivers/gpu/drm/drm_of.c | 36 ++++++++++++++++++++++++++----------
 drivers/of/base.c        | 29 +++++++++++++++++++++++++++++
 include/linux/of.h       |  6 ++++++
 3 files changed, 61 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH 1/2] of: Add helper to lookup non port child node

[Jagan Teki]

Add of_get_non_port_child() helper that can be used to lookup
non port child nodes.

Some OF graphs don't require 'ports' to represent the next output
instead, it simply adds a child node on a given parent node. This
helper lookup that child node, however that child node is not a
'port' on given parent as 'port' based nodes are looked up via
of_graph_get_remote_node().

Example OF graph representation of DSI host, which doesn't
have 'ports'.

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

	port {
		dsi_in_tcon0: endpoint {
			remote-endpoint = <tcon0_out_dsi>;
	};

	panel@0 {
		reg = <0>;
	};
};

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
 include/linux/of.h |  6 ++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 61de453b885c..31bbf885b0f8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -854,6 +854,35 @@ struct device_node *of_get_compatible_child(const struct device_node *parent,
 }
 EXPORT_SYMBOL(of_get_compatible_child);
 
+/**
+ * of_get_non_port_child - Find the non port child node for a given parent
+ * @node:	parent node
+ *
+ * This function looks for child node which is not port child for given parent.
+ *
+ * Return: A node pointer if found, with refcount incremented, use
+ * of_node_put() on it when done.
+ * Returns NULL if node is not found.
+ */
+struct device_node *of_get_non_port_child(const struct device_node *parent)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(parent, child) {
+		if (of_node_name_eq(child, "port"))
+			continue;
+
+		if (!of_device_is_available(child)) {
+			of_node_put(child);
+			continue;
+		}
+		break;
+	}
+
+	return child;
+}
+EXPORT_SYMBOL(of_get_non_port_child);
+
 /**
  * of_get_child_by_name - Find the child node by name for a given parent
  * @node:	parent node
diff --git a/include/linux/of.h b/include/linux/of.h
index ff143a027abc..3e699becef82 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -290,6 +290,7 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
 extern struct device_node *of_get_next_available_child(
 	const struct device_node *node, struct device_node *prev);
 
+extern struct device_node *of_get_non_port_child(const struct device_node *parent);
 extern struct device_node *of_get_compatible_child(const struct device_node *parent,
 					const char *compatible);
 extern struct device_node *of_get_child_by_name(const struct device_node *node,
@@ -678,6 +679,11 @@ static inline bool of_have_populated_dt(void)
 	return false;
 }
 
+static inline struct device_node *of_get_non_port_child(const struct device_node *parent)
+{
+	return NULL;
+}
+
 static inline struct device_node *of_get_compatible_child(const struct device_node *parent,
 					const char *compatible)
 {
-- 
2.25.1

[PATCH 2/2] drm: of: Lookup if child node is panel or bridge

[Jagan Teki]

drm_of_find_panel_or_bridge can lookup panel or bridge for
a given node based on the OF graph port and endpoint and it
fails to use if the given node has a child panel or bridge.

This patch add support to lookup that given node has child
panel or bridge however that child node cannot be a 'port'.

Examples OF graph representation of DSI host, which doesn't
have 'ports'

dsi {
	compatible = "allwinner,sun6i-a31-mipi-dsi";
	#address-cells = <1>;
	#size-cells = <0>;

	port {
		dsi_in_tcon0: endpoint {
			remote-endpoint = <tcon0_out_dsi>;
	};

	panel@0 {
		reg = <0>;
	};
};

dsi {
	compatible = "allwinner,sun6i-a31-mipi-dsi";
	#address-cells = <1>;
	#size-cells = <0>;

	port {
		dsi_in_tcon0: endpoint {
			remote-endpoint = <tcon0_out_dsi>;
	};

	bridge@0 {
		reg = <0>;

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

			bridge_out: port@1 {
				reg = <1>;

				bridge_out_panel: endpoint {
					remote-endpoint = <&panel_out_bridge>;
				};
			};
		};
	};
};

dsi0 {
	compatible = "ste,mcde-dsi";
	#address-cells = <1>;
	#size-cells = <0>;

	panel@0 {
		reg = <0>;
	};
};

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/drm_of.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 59d368ea006b..1c4cb809d7bc 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -249,18 +249,34 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
 	if (panel)
 		*panel = NULL;
 
-	/*
-	 * 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.
+	/**
+	 * Some OF graphs don't require 'ports' to represent the next output
+	 * instead, it simply adds a child node on a given parent node.
+	 * Lookup that child node for a given parent however that child
+	 * cannot be a 'port'.
+	 *
+	 * Add precedence to lookup non port child as of_graph_get_remote_node()
+	 * returns valid even if OF graph has 'port' but that OF graph remote
+	 * node is not register panel or bridge.
 	 */
-	if (!of_graph_is_present(np))
-		return -ENODEV;
+	if (!of_get_child_by_name(np, "ports")) {
+		remote = of_get_non_port_child(np);
+		if (!remote)
+			return -ENODEV;
+	} else {
+		/*
+		 * 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);
-	if (!remote)
-		return -ENODEV;
+		remote = of_graph_get_remote_node(np, port, endpoint);
+		if (!remote)
+			return -ENODEV;
+	}
 
 	if (panel) {
 		*panel = of_drm_find_panel(remote);
-- 
2.25.1

Re: [PATCH 1/2] of: Add helper to lookup non port child node

[Rob Herring]

On Mon, Dec 6, 2021 at 11:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Add of_get_non_port_child() helper that can be used to lookup
> non port child nodes.
>
> Some OF graphs don't require 'ports' to represent the next output
> instead, it simply adds a child node on a given parent node. This
> helper lookup that child node, however that child node is not a
> 'port' on given parent as 'port' based nodes are looked up via
> of_graph_get_remote_node().
>
> Example OF graph representation of DSI host, which doesn't
> have 'ports'.

This seems pretty specific to DSI and also can't handle there being
more than 1 non-port node. That's allowed for DSI too, but I don't
think I've ever seen a case. Anyways, I'd just move this to DRM rather
than common DT code. One comment on the implementation that will
shrink it.

>
> dsi {
>         #address-cells = <1>;
>         #size-cells = <0>;
>
>         port {
>                 dsi_in_tcon0: endpoint {
>                         remote-endpoint = <tcon0_out_dsi>;
>         };
>
>         panel@0 {
>                 reg = <0>;
>         };
> };
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
>  include/linux/of.h |  6 ++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 61de453b885c..31bbf885b0f8 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -854,6 +854,35 @@ struct device_node *of_get_compatible_child(const struct device_node *parent,
>  }
>  EXPORT_SYMBOL(of_get_compatible_child);
>
> +/**
> + * of_get_non_port_child - Find the non port child node for a given parent
> + * @node:      parent node
> + *
> + * This function looks for child node which is not port child for given parent.
> + *
> + * Return: A node pointer if found, with refcount incremented, use
> + * of_node_put() on it when done.
> + * Returns NULL if node is not found.
> + */
> +struct device_node *of_get_non_port_child(const struct device_node *parent)
> +{
> +       struct device_node *child;
> +
> +       for_each_child_of_node(parent, child) {

for_each_available_child_of_node

> +               if (of_node_name_eq(child, "port"))
> +                       continue;
> +
> +               if (!of_device_is_available(child)) {
> +                       of_node_put(child);
> +                       continue;
> +               }
> +               break;
> +       }
> +
> +       return child;
> +}
> +EXPORT_SYMBOL(of_get_non_port_child);
> +
>  /**
>   * of_get_child_by_name - Find the child node by name for a given parent
>   * @node:      parent node
> diff --git a/include/linux/of.h b/include/linux/of.h
> index ff143a027abc..3e699becef82 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -290,6 +290,7 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
>  extern struct device_node *of_get_next_available_child(
>         const struct device_node *node, struct device_node *prev);
>
> +extern struct device_node *of_get_non_port_child(const struct device_node *parent);
>  extern struct device_node *of_get_compatible_child(const struct device_node *parent,
>                                         const char *compatible);
>  extern struct device_node *of_get_child_by_name(const struct device_node *node,
> @@ -678,6 +679,11 @@ static inline bool of_have_populated_dt(void)
>         return false;
>  }
>
> +static inline struct device_node *of_get_non_port_child(const struct device_node *parent)
> +{
> +       return NULL;
> +}
> +
>  static inline struct device_node *of_get_compatible_child(const struct device_node *parent,
>                                         const char *compatible)
>  {
> --
> 2.25.1
>

Re: [PATCH 1/2] of: Add helper to lookup non port child node

[Jagan Teki]

On Wed, Dec 8, 2021 at 2:20 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Mon, Dec 6, 2021 at 11:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Add of_get_non_port_child() helper that can be used to lookup
> > non port child nodes.
> >
> > Some OF graphs don't require 'ports' to represent the next output
> > instead, it simply adds a child node on a given parent node. This
> > helper lookup that child node, however that child node is not a
> > 'port' on given parent as 'port' based nodes are looked up via
> > of_graph_get_remote_node().
> >
> > Example OF graph representation of DSI host, which doesn't
> > have 'ports'.
>
> This seems pretty specific to DSI and also can't handle there being
> more than 1 non-port node. That's allowed for DSI too, but I don't
> think I've ever seen a case. Anyways, I'd just move this to DRM rather
> than common DT code. One comment on the implementation that will
> shrink it.

I think it can be possible to OF graph even for non-DSI, however if
the end-node outputs to a panel or bridge. At the moment, I can one
use case on the non-DSI side is rcar du encoder.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rcar-du/rcar_du_encoder.c#n68

I'm not sure this can be done existing of_graph_get_remote_node, but an example.

>
> >
> > dsi {
> >         #address-cells = <1>;
> >         #size-cells = <0>;
> >
> >         port {
> >                 dsi_in_tcon0: endpoint {
> >                         remote-endpoint = <tcon0_out_dsi>;
> >         };
> >
> >         panel@0 {
> >                 reg = <0>;
> >         };
> > };
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/of/base.c  | 29 +++++++++++++++++++++++++++++
> >  include/linux/of.h |  6 ++++++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 61de453b885c..31bbf885b0f8 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -854,6 +854,35 @@ struct device_node *of_get_compatible_child(const struct device_node *parent,
> >  }
> >  EXPORT_SYMBOL(of_get_compatible_child);
> >
> > +/**
> > + * of_get_non_port_child - Find the non port child node for a given parent
> > + * @node:      parent node
> > + *
> > + * This function looks for child node which is not port child for given parent.
> > + *
> > + * Return: A node pointer if found, with refcount incremented, use
> > + * of_node_put() on it when done.
> > + * Returns NULL if node is not found.
> > + */
> > +struct device_node *of_get_non_port_child(const struct device_node *parent)
> > +{
> > +       struct device_node *child;
> > +
> > +       for_each_child_of_node(parent, child) {
>
> for_each_available_child_of_node

Look like this simplifies not required to use of_device_is_available,
I will use it in v2.

Thanks,
Jagan.

Re: [PATCH 1/2] of: Add helper to lookup non port child node

[Jagan Teki]

Hi Rob and Laurent,

On Wed, Dec 8, 2021 at 11:56 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Wed, Dec 8, 2021 at 2:20 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Mon, Dec 6, 2021 at 11:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Add of_get_non_port_child() helper that can be used to lookup
> > > non port child nodes.
> > >
> > > Some OF graphs don't require 'ports' to represent the next output
> > > instead, it simply adds a child node on a given parent node. This
> > > helper lookup that child node, however that child node is not a
> > > 'port' on given parent as 'port' based nodes are looked up via
> > > of_graph_get_remote_node().
> > >
> > > Example OF graph representation of DSI host, which doesn't
> > > have 'ports'.
> >
> > This seems pretty specific to DSI and also can't handle there being
> > more than 1 non-port node. That's allowed for DSI too, but I don't
> > think I've ever seen a case. Anyways, I'd just move this to DRM rather
> > than common DT code. One comment on the implementation that will
> > shrink it.
>
> I think it can be possible to OF graph even for non-DSI, however if
> the end-node outputs to a panel or bridge. At the moment, I can one
> use case on the non-DSI side is rcar du encoder.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rcar-du/rcar_du_encoder.c#n68

Do you see any point to make this helper in of/base based on above
rcar_du_encoder usage? if not i can directly use this functionality in
panel_or_bridge finding code itself.

Please let me know.

Thanks,
Jagan.