[PATCH v5 0/7] drm/sun4i: Add support for dual-link LVDS on the A20

[PATCH v5 0/7] drm/sun4i: Add support for dual-link LVDS on the A20

[Maxime Ripard]

Hi,

This is a fourth attempt at supporting the LVDS dual-link output on the
Allwinner A20.

Let me know what you think,
Maxime

Changes from v4:
  - Rebased on 5.15

Changes from v3:
  - Reintroduce the endpoint sanity check (and make of_graph_get_port_by_id
    const in the process)
  - Make a few doc fixes
  - Fix device_node leakage
  - Changed allwinner,lvds-companion to more generic link-companion
  - Rebased on 5.11

Changes from v2:
  - Added the DT binding description
  - Split the patch to enable the A20
  - Reworked a bit the error messages

Changes from v1:
  - Reworked the DT bindings
  - Refactored a bit the panel registration in the tcon code.

Maxime Ripard (7):
  of: Make of_graph_get_port_by_id take a const device_node
  drm/of: Change the prototype of drm_of_lvds_get_dual_link_pixel_order
  dt-bindings: display: sun4i: Add LVDS Dual-Link property
  drm/sun4i: tcon: Refactor the LVDS and panel probing
  drm/sun4i: tcon: Support the LVDS Dual-Link
  drm/sun4i: tcon: Enable the A20 dual-link output
  [DO NOT MERGE] ARM: dts: sun7i: Enable LVDS Dual-Link on the
    Cubieboard

 .../display/allwinner,sun4i-a10-tcon.yaml     |   6 +
 arch/arm/boot/dts/sun7i-a20-cubieboard2.dts   |  69 ++++++++
 drivers/gpu/drm/drm_of.c                      | 138 +++++++++++----
 drivers/gpu/drm/rcar-du/rcar_lvds.c           |   8 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c            | 164 ++++++++++--------
 drivers/gpu/drm/sun4i/sun4i_tcon.h            |   4 +
 drivers/of/property.c                         |   2 +-
 include/drm/drm_of.h                          |  16 +-
 include/linux/of_graph.h                      |   4 +-
 9 files changed, 298 insertions(+), 113 deletions(-)

-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 1/7] of: Make of_graph_get_port_by_id take a const device_node

[Maxime Ripard]

of_graph_get_port_by_id doesn't modify the device_node pointer it takes
as argument, so we can make it const.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/of/property.c    | 2 +-
 include/linux/of_graph.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 3fd74bb34819..739d5d1c8c3a 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -603,7 +603,7 @@ EXPORT_SYMBOL(of_graph_parse_endpoint);
  * Return: A 'port' node pointer with refcount incremented. The caller
  * has to use of_node_put() on it when done.
  */
-struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
+struct device_node *of_graph_get_port_by_id(const struct device_node *parent, u32 id)
 {
 	struct device_node *node, *port;
 
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 4d7756087b6b..87f3f4d0d0df 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -42,7 +42,7 @@ bool of_graph_is_present(const struct device_node *node);
 int of_graph_parse_endpoint(const struct device_node *node,
 				struct of_endpoint *endpoint);
 int of_graph_get_endpoint_count(const struct device_node *np);
-struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id);
+struct device_node *of_graph_get_port_by_id(const struct device_node *node, u32 id);
 struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 					struct device_node *previous);
 struct device_node *of_graph_get_endpoint_by_regs(
@@ -74,7 +74,7 @@ static inline int of_graph_get_endpoint_count(const struct device_node *np)
 }
 
 static inline struct device_node *of_graph_get_port_by_id(
-					struct device_node *node, u32 id)
+					const struct device_node *node, u32 id)
 {
 	return NULL;
 }
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/7] of: Make of_graph_get_port_by_id take a const device_node

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Wed, Sep 29, 2021 at 10:42:28AM +0200, Maxime Ripard wrote:
> of_graph_get_port_by_id doesn't modify the device_node pointer it takes
> as argument, so we can make it const.

From a C point of view that's right, but conceptually speaking, is it
right to return a non-const child port node of a const device_node ?

> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/of/property.c    | 2 +-
>  include/linux/of_graph.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 3fd74bb34819..739d5d1c8c3a 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -603,7 +603,7 @@ EXPORT_SYMBOL(of_graph_parse_endpoint);
>   * Return: A 'port' node pointer with refcount incremented. The caller
>   * has to use of_node_put() on it when done.
>   */
> -struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
> +struct device_node *of_graph_get_port_by_id(const struct device_node *parent, u32 id)
>  {
>  	struct device_node *node, *port;
>  
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index 4d7756087b6b..87f3f4d0d0df 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -42,7 +42,7 @@ bool of_graph_is_present(const struct device_node *node);
>  int of_graph_parse_endpoint(const struct device_node *node,
>  				struct of_endpoint *endpoint);
>  int of_graph_get_endpoint_count(const struct device_node *np);
> -struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id);
> +struct device_node *of_graph_get_port_by_id(const struct device_node *node, u32 id);
>  struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
>  					struct device_node *previous);
>  struct device_node *of_graph_get_endpoint_by_regs(
> @@ -74,7 +74,7 @@ static inline int of_graph_get_endpoint_count(const struct device_node *np)
>  }
>  
>  static inline struct device_node *of_graph_get_port_by_id(
> -					struct device_node *node, u32 id)
> +					const struct device_node *node, u32 id)
>  {
>  	return NULL;
>  }

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/7] of: Make of_graph_get_port_by_id take a const device_node

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 832 bytes --]

On Tue, Oct 19, 2021 at 03:02:13PM +0300, Laurent Pinchart wrote:
> Hi Maxime,
> 
> Thank you for the patch.
> 
> On Wed, Sep 29, 2021 at 10:42:28AM +0200, Maxime Ripard wrote:
> > of_graph_get_port_by_id doesn't modify the device_node pointer it takes
> > as argument, so we can make it const.
> 
> From a C point of view that's right, but conceptually speaking, is it
> right to return a non-const child port node of a const device_node ?

I mean, I guess not, but you're the one that asked for it:
https://lore.kernel.org/dri-devel/YBAiztjg0Jji9voK@pendragon.ideasonboard.com/

I can change it if you want, but certainly not if the only comment I get
on this series for the next year is going to be over whether or not
arguments of functions unrelated to the main intent should be constified
or not.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 1/7] of: Make of_graph_get_port_by_id take a const device_node

[Laurent Pinchart]

Hi Maxime,

On Thu, Oct 21, 2021 at 09:48:43AM +0200, Maxime Ripard wrote:
> On Tue, Oct 19, 2021 at 03:02:13PM +0300, Laurent Pinchart wrote:
> > On Wed, Sep 29, 2021 at 10:42:28AM +0200, Maxime Ripard wrote:
> > > of_graph_get_port_by_id doesn't modify the device_node pointer it takes
> > > as argument, so we can make it const.
> > 
> > From a C point of view that's right, but conceptually speaking, is it
> > right to return a non-const child port node of a const device_node ?
> 
> I mean, I guess not, but you're the one that asked for it:
> https://lore.kernel.org/dri-devel/YBAiztjg0Jji9voK@pendragon.ideasonboard.com/

Oops. Looks like I must be wrong with at least one of the two comments.
Please pick the option you like best.

> I can change it if you want, but certainly not if the only comment I get
> on this series for the next year is going to be over whether or not
> arguments of functions unrelated to the main intent should be constified
> or not.

DRM/KMS seems to suffer from a deficit of reviewers in all non-desktop
areas :-(

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 2/7] drm/of: Change the prototype of drm_of_lvds_get_dual_link_pixel_order

[Maxime Ripard]

The drm_of_lvds_get_dual_link_pixel_order() function took so far the
device_node of the two ports used together to make up a dual-link LVDS
output.

This assumes that a binding would use an entire port for the LVDS output.
However, some bindings have used endpoints instead and thus we need to
operate at the endpoint level. Change slightly the arguments to allow that.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/drm_of.c            | 138 +++++++++++++++++++++-------
 drivers/gpu/drm/rcar-du/rcar_lvds.c |   8 +-
 include/drm/drm_of.h                |  16 +++-
 3 files changed, 121 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 997b8827fed2..681664a8875f 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -300,13 +300,35 @@ static int drm_of_lvds_get_port_pixels_type(struct device_node *port_node)
 	       (odd_pixels ? DRM_OF_LVDS_ODD : 0);
 }
 
-static int drm_of_lvds_get_remote_pixels_type(
-			const struct device_node *port_node)
+static int drm_of_lvds_get_remote_pixels_type(const struct device_node *endpoint)
 {
-	struct device_node *endpoint = NULL;
-	int pixels_type = -EPIPE;
+	struct device_node *remote_port;
+	int pixels_type;
 
-	for_each_child_of_node(port_node, endpoint) {
+	remote_port = of_graph_get_remote_port(endpoint);
+	if (!remote_port)
+		return -EPIPE;
+
+	pixels_type = drm_of_lvds_get_port_pixels_type(remote_port);
+	of_node_put(remote_port);
+
+	if (pixels_type < 0)
+		return -EPIPE;
+
+	return pixels_type;
+}
+
+static int drm_of_lvds_check_remote_port(const struct device_node *dev, int id)
+{
+	struct device_node *endpoint;
+	struct device_node *port;
+	int previous_pt = -EPIPE;
+
+	port = of_graph_get_port_by_id(dev, id);
+	if (!port)
+		return -EINVAL;
+
+	for_each_child_of_node(port, endpoint) {
 		struct device_node *remote_port;
 		int current_pt;
 
@@ -315,14 +337,19 @@ static int drm_of_lvds_get_remote_pixels_type(
 
 		remote_port = of_graph_get_remote_port(endpoint);
 		if (!remote_port) {
-			of_node_put(endpoint);
+			of_node_put(port);
 			return -EPIPE;
 		}
 
 		current_pt = drm_of_lvds_get_port_pixels_type(remote_port);
 		of_node_put(remote_port);
-		if (pixels_type < 0)
-			pixels_type = current_pt;
+		if (!current_pt) {
+			of_node_put(port);
+			return -EINVAL;
+		}
+
+		if (previous_pt < 0)
+			previous_pt = current_pt;
 
 		/*
 		 * Sanity check, ensure that all remote endpoints have the same
@@ -331,19 +358,26 @@ static int drm_of_lvds_get_remote_pixels_type(
 		 * configurations by passing the endpoints explicitly to
 		 * drm_of_lvds_get_dual_link_pixel_order().
 		 */
-		if (!current_pt || pixels_type != current_pt) {
-			of_node_put(endpoint);
+		if (previous_pt != current_pt) {
+			of_node_put(port);
 			return -EINVAL;
 		}
+
+		previous_pt = current_pt;
 	}
 
-	return pixels_type;
+	of_node_put(port);
+	return previous_pt < 0 ? previous_pt : 0;
 }
 
 /**
  * drm_of_lvds_get_dual_link_pixel_order - Get LVDS dual-link pixel order
- * @port1: First DT port node of the Dual-link LVDS source
- * @port2: Second DT port node of the Dual-link LVDS source
+ * @dev1: First DT device node of the Dual-Link LVDS source
+ * @port1_id: ID of the first DT port node of the Dual-Link LVDS source
+ * @endpoint1_id: ID of the first DT port node of the Dual-Link LVDS source
+ * @dev2: Second DT device node of the Dual-Link LVDS source
+ * @port2_id: ID of the second DT port node of the Dual-Link LVDS source
+ * @endpoint2_id: ID of the second DT port node of the Dual-Link LVDS source
  *
  * An LVDS dual-link connection is made of two links, with even pixels
  * transitting on one link, and odd pixels on the other link. This function
@@ -357,43 +391,85 @@ static int drm_of_lvds_get_remote_pixels_type(
  *
  * If either port is not connected, this function returns -EPIPE.
  *
- * @port1 and @port2 are typically DT sibling nodes, but may have different
- * parents when, for instance, two separate LVDS encoders carry the even and odd
- * pixels.
+ * @port1_id and @port2_id are typically DT sibling nodes, but may have
+ * different parents when, for instance, two separate LVDS encoders carry the
+ * even and odd pixels.
+ *
+ * If @port1_id, @port2_id, @endpoint1_id or @endpoint2_id are set to -1, their
+ * value is going to be ignored and the first port or endpoint will be used.
  *
  * Return:
- * * DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS - @port1 carries even pixels and @port2
- *   carries odd pixels
- * * DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS - @port1 carries odd pixels and @port2
- *   carries even pixels
- * * -EINVAL - @port1 and @port2 are not connected to a dual-link LVDS sink, or
- *   the sink configuration is invalid
- * * -EPIPE - when @port1 or @port2 are not connected
+ * * DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS - @endpoint1_id carries even pixels and
+ *   @endpoint2_id carries odd pixels
+ * * DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS - @endpoint1_id carries odd pixels and
+ *   @endpoint2_id carries even pixels
+ * * -EINVAL - @endpoint1_id and @endpoint2_id are not connected to a dual-link
+ *   LVDS sink, or the sink configuration is invalid
+ * * -EPIPE - when @endpoint1_id or @endpoint2_id are not connected
  */
-int drm_of_lvds_get_dual_link_pixel_order(const struct device_node *port1,
-					  const struct device_node *port2)
+int drm_of_lvds_get_dual_link_pixel_order(const struct device_node *dev1,
+					  int port1_id,
+					  int endpoint1_id,
+					  const struct device_node *dev2,
+					  int port2_id,
+					  int endpoint2_id)
 {
+	struct device_node *endpoint1, *endpoint2;
 	int remote_p1_pt, remote_p2_pt;
+	int ret;
 
-	if (!port1 || !port2)
+	if (!dev1 || !dev2)
 		return -EINVAL;
 
-	remote_p1_pt = drm_of_lvds_get_remote_pixels_type(port1);
-	if (remote_p1_pt < 0)
+	if (endpoint1_id == -1) {
+		ret = drm_of_lvds_check_remote_port(dev1, port1_id);
+		if (ret)
+			return ret;
+	}
+
+	if (endpoint2_id == -1) {
+		ret = drm_of_lvds_check_remote_port(dev2, port2_id);
+		if (ret)
+			return ret;
+	}
+
+	endpoint1 = of_graph_get_endpoint_by_regs(dev1, port1_id, endpoint1_id);
+	if (!endpoint1)
+		return -EINVAL;
+
+	endpoint2 = of_graph_get_endpoint_by_regs(dev2, port2_id, endpoint2_id);
+	if (!endpoint2) {
+		of_node_put(endpoint1);
+		return -EINVAL;
+	}
+
+	remote_p1_pt = drm_of_lvds_get_remote_pixels_type(endpoint1);
+	if (remote_p1_pt < 0) {
+		of_node_put(endpoint2);
+		of_node_put(endpoint1);
 		return remote_p1_pt;
+	}
 
-	remote_p2_pt = drm_of_lvds_get_remote_pixels_type(port2);
-	if (remote_p2_pt < 0)
+	remote_p2_pt = drm_of_lvds_get_remote_pixels_type(endpoint2);
+	if (remote_p2_pt < 0) {
+		of_node_put(endpoint2);
+		of_node_put(endpoint1);
 		return remote_p2_pt;
+	}
 
 	/*
 	 * A valid dual-lVDS bus is found when one remote port is marked with
 	 * "dual-lvds-even-pixels", and the other remote port is marked with
 	 * "dual-lvds-odd-pixels", bail out if the markers are not right.
 	 */
-	if (remote_p1_pt + remote_p2_pt != DRM_OF_LVDS_EVEN + DRM_OF_LVDS_ODD)
+	if (remote_p1_pt + remote_p2_pt != DRM_OF_LVDS_EVEN + DRM_OF_LVDS_ODD) {
+		of_node_put(endpoint2);
+		of_node_put(endpoint1);
 		return -EINVAL;
+	}
 
+	of_node_put(endpoint2);
+	of_node_put(endpoint1);
 	return remote_p1_pt == DRM_OF_LVDS_EVEN ?
 		DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS :
 		DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index d061b8de748f..40389e76e04b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -606,7 +606,6 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
 {
 	const struct of_device_id *match;
 	struct device_node *companion;
-	struct device_node *port0, *port1;
 	struct rcar_lvds *companion_lvds;
 	struct device *dev = lvds->dev;
 	int dual_link;
@@ -634,11 +633,8 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
 	 * connected to, if they are marked as expecting even pixels and
 	 * odd pixels than we need to enable vertical stripe output.
 	 */
-	port0 = of_graph_get_port_by_id(dev->of_node, 1);
-	port1 = of_graph_get_port_by_id(companion, 1);
-	dual_link = drm_of_lvds_get_dual_link_pixel_order(port0, port1);
-	of_node_put(port0);
-	of_node_put(port1);
+	dual_link = drm_of_lvds_get_dual_link_pixel_order(dev->of_node, 1, -1,
+							  companion, 1, -1);
 
 	switch (dual_link) {
 	case DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS:
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index b9b093add92e..7bb1f6603beb 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -47,8 +47,12 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
 				int port, int endpoint,
 				struct drm_panel **panel,
 				struct drm_bridge **bridge);
-int drm_of_lvds_get_dual_link_pixel_order(const struct device_node *port1,
-					  const struct device_node *port2);
+int drm_of_lvds_get_dual_link_pixel_order(const struct device_node *dev1,
+					  int port1_id,
+					  int endpoint1_id,
+					  const struct device_node *dev2,
+					  int port2_id,
+					  int endpoint2_id);
 #else
 static inline uint32_t drm_of_crtc_port_mask(struct drm_device *dev,
 					  struct device_node *port)
@@ -93,8 +97,12 @@ static inline int drm_of_find_panel_or_bridge(const struct device_node *np,
 }
 
 static inline int
-drm_of_lvds_get_dual_link_pixel_order(const struct device_node *port1,
-				      const struct device_node *port2)
+drm_of_lvds_get_dual_link_pixel_order(const struct device_node *dev1,
+				      int port1_id,
+				      int endpoint1_id,
+				      const struct device_node *dev2,
+				      int port2_id,
+				      int endpoint2_id)
 {
 	return -EINVAL;
 }
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 3/7] dt-bindings: display: sun4i: Add LVDS Dual-Link property

[Maxime Ripard]

The Allwinner SoCs with two TCONs and LVDS output can use both to drive an
LVDS dual-link. Add a new property to express that link between these two
TCONs.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 .../bindings/display/allwinner,sun4i-a10-tcon.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
index 3a7d5d731712..d01b0689785c 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
@@ -113,6 +113,12 @@ properties:
           - const: edp
           - const: lvds
 
+  link-companion:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: >
+      Phandle to the other TCON in the system used to drive a dual-link LVDS
+      output.
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 3/7] dt-bindings: display: sun4i: Add LVDS Dual-Link property

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

I may have written "Add LVDS link companion" property in the subject
line.

On Wed, Sep 29, 2021 at 10:42:30AM +0200, Maxime Ripard wrote:
> The Allwinner SoCs with two TCONs and LVDS output can use both to drive an
> LVDS dual-link. Add a new property to express that link between these two
> TCONs.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../bindings/display/allwinner,sun4i-a10-tcon.yaml          | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> index 3a7d5d731712..d01b0689785c 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> @@ -113,6 +113,12 @@ properties:
>            - const: edp
>            - const: lvds
>  
> +  link-companion:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: >
> +      Phandle to the other TCON in the system used to drive a dual-link LVDS
> +      output.

Should this be expanded to indicate that the property shall be set in
both TCON instances ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>  

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 4/7] drm/sun4i: tcon: Refactor the LVDS and panel probing

[Maxime Ripard]

The current code to parse the DT, deal with the older device trees, and
register either the RGB or LVDS output has so far grown organically into
the bind function and has become quite hard to extend properly.

Let's move it into a single function that grabs all the resources it needs
and registers the proper panel output.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 127 +++++++++++++----------------
 1 file changed, 58 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 9f06dec0fc61..e38e60faafd2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -868,6 +868,63 @@ static int sun4i_tcon_init_regmap(struct device *dev,
 	return 0;
 }
 
+static int sun4i_tcon_register_panel(struct drm_device *drm,
+				     struct sun4i_tcon *tcon)
+{
+	struct device_node *companion;
+	struct device_node *remote;
+	struct device *dev = tcon->dev;
+	int ret;
+
+	/*
+	 * If we have an LVDS panel connected to the TCON, we should
+	 * just probe the LVDS connector. Otherwise, let's just register
+	 * an RGB panel.
+	 */
+	remote = of_graph_get_remote_node(dev->of_node, 1, 0);
+	if (!tcon->quirks->supports_lvds ||
+	    !of_device_is_compatible(remote, "panel-lvds"))
+		return sun4i_rgb_init(drm, tcon);
+
+	/*
+	 * This can only be made optional since we've had DT
+	 * nodes without the LVDS reset properties.
+	 *
+	 * If the property is missing, just disable LVDS, and
+	 * print a warning.
+	 */
+	tcon->lvds_rst = devm_reset_control_get_optional(dev, "lvds");
+	if (IS_ERR(tcon->lvds_rst)) {
+		dev_err(dev, "Couldn't get our reset line\n");
+		return PTR_ERR(tcon->lvds_rst);
+	} else if (!tcon->lvds_rst) {
+		dev_warn(dev, "Missing LVDS reset property, please upgrade your DT\n");
+		return -ENODEV;
+	}
+
+	reset_control_reset(tcon->lvds_rst);
+
+	/*
+	 * This can only be made optional since we've had DT
+	 * nodes without the LVDS clocks properties.
+	 *
+	 * If the property is missing, just disable LVDS, and
+	 * print a warning.
+	 */
+	if (tcon->quirks->has_lvds_alt) {
+		tcon->lvds_pll = devm_clk_get_optional(dev, "lvds-alt");
+		if (IS_ERR(tcon->lvds_pll)) {
+			dev_err(dev, "Couldn't get the LVDS PLL\n");
+			return PTR_ERR(tcon->lvds_pll);
+		} else if (!tcon->lvds_pll) {
+			dev_warn(dev, "Missing LVDS PLL clock, please upgrade your DT\n");
+			return -ENODEV;
+		}
+	}
+
+	return sun4i_lvds_init(drm, tcon);
+}
+
 /*
  * On SoCs with the old display pipeline design (Display Engine 1.0),
  * the TCON is always tied to just one backend. Hence we can traverse
@@ -1115,10 +1172,8 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 	struct drm_device *drm = data;
 	struct sun4i_drv *drv = drm->dev_private;
 	struct sunxi_engine *engine;
-	struct device_node *remote;
 	struct sun4i_tcon *tcon;
 	struct reset_control *edp_rstc;
-	bool has_lvds_rst, has_lvds_alt, can_lvds;
 	int ret;
 
 	engine = sun4i_tcon_find_engine(drv, dev->of_node);
@@ -1163,58 +1218,6 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	if (tcon->quirks->supports_lvds) {
-		/*
-		 * This can only be made optional since we've had DT
-		 * nodes without the LVDS reset properties.
-		 *
-		 * If the property is missing, just disable LVDS, and
-		 * print a warning.
-		 */
-		tcon->lvds_rst = devm_reset_control_get_optional(dev, "lvds");
-		if (IS_ERR(tcon->lvds_rst)) {
-			dev_err(dev, "Couldn't get our reset line\n");
-			return PTR_ERR(tcon->lvds_rst);
-		} else if (tcon->lvds_rst) {
-			has_lvds_rst = true;
-			reset_control_reset(tcon->lvds_rst);
-		} else {
-			has_lvds_rst = false;
-		}
-
-		/*
-		 * This can only be made optional since we've had DT
-		 * nodes without the LVDS reset properties.
-		 *
-		 * If the property is missing, just disable LVDS, and
-		 * print a warning.
-		 */
-		if (tcon->quirks->has_lvds_alt) {
-			tcon->lvds_pll = devm_clk_get(dev, "lvds-alt");
-			if (IS_ERR(tcon->lvds_pll)) {
-				if (PTR_ERR(tcon->lvds_pll) == -ENOENT) {
-					has_lvds_alt = false;
-				} else {
-					dev_err(dev, "Couldn't get the LVDS PLL\n");
-					return PTR_ERR(tcon->lvds_pll);
-				}
-			} else {
-				has_lvds_alt = true;
-			}
-		}
-
-		if (!has_lvds_rst ||
-		    (tcon->quirks->has_lvds_alt && !has_lvds_alt)) {
-			dev_warn(dev, "Missing LVDS properties, Please upgrade your DT\n");
-			dev_warn(dev, "LVDS output disabled\n");
-			can_lvds = false;
-		} else {
-			can_lvds = true;
-		}
-	} else {
-		can_lvds = false;
-	}
-
 	ret = sun4i_tcon_init_clocks(dev, tcon);
 	if (ret) {
 		dev_err(dev, "Couldn't init our TCON clocks\n");
@@ -1249,21 +1252,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 	}
 
 	if (tcon->quirks->has_channel_0) {
-		/*
-		 * If we have an LVDS panel connected to the TCON, we should
-		 * just probe the LVDS connector. Otherwise, just probe RGB as
-		 * we used to.
-		 */
-		remote = of_graph_get_remote_node(dev->of_node, 1, 0);
-		if (of_device_is_compatible(remote, "panel-lvds"))
-			if (can_lvds)
-				ret = sun4i_lvds_init(drm, tcon);
-			else
-				ret = -EINVAL;
-		else
-			ret = sun4i_rgb_init(drm, tcon);
-		of_node_put(remote);
-
+		ret = sun4i_tcon_register_panel(drm, tcon);
 		if (ret < 0)
 			goto err_free_dotclock;
 	}
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 5/7] drm/sun4i: tcon: Support the LVDS Dual-Link

[Maxime Ripard]

The A20 and other SoC with two TCONs (A31, R40, etc.) can use its second
TCON as the secondary LVDS link in a dual-link setup, with the TCON0 being
the main link. Extend a bit the parsing code to leverage the DRM dual-link
code, register only the LVDS output on the primary TCON, and add the needed
bits to setup the TCON properly.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 36 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  4 ++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index e38e60faafd2..c7c94b7c6439 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -475,6 +475,10 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
 		     SUN4I_TCON0_BASIC2_V_BACKPORCH(bp));
 
 	reg = SUN4I_TCON0_LVDS_IF_CLK_SEL_TCON0;
+
+	if (tcon->lvds_dual_link)
+		reg |= SUN4I_TCON0_LVDS_IF_DUAL_LINK;
+
 	if (sun4i_tcon_get_pixel_depth(encoder) == 24)
 		reg |= SUN4I_TCON0_LVDS_IF_BITWIDTH_24BITS;
 	else
@@ -886,6 +890,16 @@ static int sun4i_tcon_register_panel(struct drm_device *drm,
 	    !of_device_is_compatible(remote, "panel-lvds"))
 		return sun4i_rgb_init(drm, tcon);
 
+	/*
+	 * Only the TCON0 will be relevant for the LVDS output, so if
+	 * our ID is something else, let's prevent our TCON from
+	 * registering its own LVDS output
+	 */
+	if (tcon->id) {
+		dev_dbg(dev, "TCON used as an LVDS secondary link.");
+		return 0;
+	}
+
 	/*
 	 * This can only be made optional since we've had DT
 	 * nodes without the LVDS reset properties.
@@ -922,6 +936,28 @@ static int sun4i_tcon_register_panel(struct drm_device *drm,
 		}
 	}
 
+	/*
+	 * If we don't have a second TCON, we will never be able to do
+	 * dual-link LVDS, so we don't have much more to do.
+	 */
+	companion = of_parse_phandle(dev->of_node, "link-companion", 0);
+	if (!companion)
+		return sun4i_lvds_init(drm, tcon);
+
+	/*
+	 * Let's do a sanity check on the dual-link setup to make sure
+	 * everything is properly described.
+	 */
+	ret = drm_of_lvds_get_dual_link_pixel_order(dev->of_node, 1, 0,
+						    companion, 1, 0);
+	if (ret < 0) {
+		dev_err(dev, "Invalid Dual-Link Configuration.\n");
+		return ret;
+	}
+
+	dev_info(dev, "Primary TCON, enabling LVDS Dual-Link");
+	tcon->lvds_dual_link = true;
+
 	return sun4i_lvds_init(drm, tcon);
 }
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index e624f6977eb8..7609a880a8a7 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -98,6 +98,7 @@
 
 #define SUN4I_TCON0_LVDS_IF_REG			0x84
 #define SUN4I_TCON0_LVDS_IF_EN				BIT(31)
+#define SUN4I_TCON0_LVDS_IF_DUAL_LINK			BIT(30)
 #define SUN4I_TCON0_LVDS_IF_BITWIDTH_MASK		BIT(26)
 #define SUN4I_TCON0_LVDS_IF_BITWIDTH_18BITS		(1 << 26)
 #define SUN4I_TCON0_LVDS_IF_BITWIDTH_24BITS		(0 << 26)
@@ -281,6 +282,9 @@ struct sun4i_tcon {
 	/* Associated crtc */
 	struct sun4i_crtc		*crtc;
 
+	/* Is the LVDS link a dual-channel link? */
+	bool				lvds_dual_link;
+
 	int				id;
 
 	/* TCON list management */
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 6/7] drm/sun4i: tcon: Enable the A20 dual-link output

[Maxime Ripard]

The A20 second TCON (TCON1) can be used as a secondary output to drive a
dual-link LVDS output. Let's add it to our capabilities.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c7c94b7c6439..2855ff70418d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -1521,6 +1521,7 @@ static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = {
 };
 
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+	.supports_lvds		= true,
 	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.dclk_min_div		= 4,
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 7/7] [DO NOT MERGE] ARM: dts: sun7i: Enable LVDS Dual-Link on the Cubieboard

[Maxime Ripard]

For the sake of the example, let's enable an LVDS Dual-Link display on a
Cubieboard.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 69 +++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
index e35e6990c4b2..b10d387385d9 100644
--- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
@@ -85,6 +85,49 @@ led-1 {
 			gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>;
 		};
 	};
+
+	panel: panel {
+		compatible = "panel-lvds";
+		width-mm = <153>;
+		height-mm = <90>;
+		data-mapping = "vesa-24";
+
+		panel-timing {
+			clock-frequency = <148500000>;
+			hfront-porch = <88>;
+			hactive = <1920>;
+			hback-porch = <148>;
+			hsync-len = <44>;
+
+			vfront-porch = <4>;
+			vactive = <1080>;
+			vback-porch = <36>;
+			vsync-len = <5>;
+		};
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				dual-lvds-even-pixels;
+
+				panel_input_0: endpoint {
+					remote-endpoint = <&tcon0_out_panel>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				dual-lvds-odd-pixels;
+
+				panel_input_1: endpoint {
+					remote-endpoint = <&tcon1_out_panel>;
+				};
+			};
+		};
+	};
 };
 
 &ahci {
@@ -218,6 +261,32 @@ &reg_usb2_vbus {
 	status = "okay";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd_lvds0_pins>;
+	link-companion = <&tcon1>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_panel: endpoint@0 {
+		remote-endpoint = <&panel_input_0>;
+	};
+};
+
+&tcon1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd_lvds1_pins>;
+	link-companion = <&tcon0>;
+	status = "okay";
+};
+
+&tcon1_out {
+	tcon1_out_panel: endpoint@0 {
+		remote-endpoint = <&panel_input_1>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pb_pins>;
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel