[PATCH 0/8] drm/sun4i: Make both display pipeline work together

[PATCH 0/8] drm/sun4i: Make both display pipeline work together

[Chen-Yu Tsai]

Hi everyone,

Previously I added support for two display pipelines for the sun4i-drm
driver. At the time we did not have support for downstream encoders to
actually test the second pipeline, nor having both pipelines active at
the same time.

With HDMI encoder support now merged and support for it on sun6i in
progress, we are now able to test both pipelines being active at the
same time, with LCD output from one and HDMI from the other. Testing
has revealed some more issues, such as component probing order. Also,
we found out that muxing between the display backend and TCON was
possible, and required for the second pipeline to work as intended.
Last, the number of CRTCs provided to drm_vblank_init needs to be
increased for vblanks to work properly.

This patch series does a few things:

  - Fix endpoint IDs so they conform to what the DT binding stipulates.

  - Change the order the individual components are queued up.

  - Fix how the TCON gets its ID when the backend-TCON mux is present.

  - Support the input mux in the TCON (which selects which backend to
    use on the A31/A31s).

  - Fix the drm_vblank_init call and pass the correct number of crtcs.

  - Add the cross pipeline connections between the DRCs and TCONs on
    the A31, conforming to the DT binding and what the hardware is
    capable of.

More details are available in each individual commit. Please have a look.

Regards
ChenYu


Chen-Yu Tsai (8):
  ARM: dts: sun6i: Fix endpoint IDs in second display pipeline
  drm/sun4i: add components in breadth first traversal order
  drm/sun4i: tcon: Check for multiple paths between TCONs and backends
  drm/sun4i: tcon: get TCON ID and matching engine with remote endpoint
    ID
  drm/sun4i: tcon: Simplify sun4i_tcon_find_engine_traverse for one
    input
  drm/sun4i: tcon: Support backend input mux
  drm/sun4i: call drm_vblank_init with correct number of crtcs
  ARM: dts: sun6i: Add cross pipeline connections between DRCs and TCONs

 arch/arm/boot/dts/sun6i-a31.dtsi   |  32 +++++--
 drivers/gpu/drm/sun4i/sun4i_drv.c  |  91 +++++++++++++++---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 191 +++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/sun4i/sun4i_tcon.h |   3 +
 4 files changed, 276 insertions(+), 41 deletions(-)

-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/8] ARM: dts: sun6i: Fix endpoint IDs in second display pipeline

[Chen-Yu Tsai]

When the second display pipeline device nodes for the A31/A31s were
added, it was not known that the TCONs could (through either DRCs)
select either backend as their input. Thus in the endpoints connecting
these components together, the endpoint IDs were set to 0, while in
fact they should have been set to 1.

Fixes: 9a26882a7378 ("ARM: dts: sun6i: Add second display pipeline device
		      nodes")
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index f3d74dc5b292..00a4c7614e0a 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -311,8 +311,8 @@
 					#size-cells = <0>;
 					reg = <0>;
 
-					tcon1_in_drc1: endpoint@0 {
-						reg = <0>;
+					tcon1_in_drc1: endpoint@1 {
+						reg = <1>;
 						remote-endpoint = <&drc1_out_tcon1>;
 					};
 				};
@@ -1038,8 +1038,8 @@
 					#size-cells = <0>;
 					reg = <1>;
 
-					be1_out_drc1: endpoint@0 {
-						reg = <0>;
+					be1_out_drc1: endpoint@1 {
+						reg = <1>;
 						remote-endpoint = <&drc1_in_be1>;
 					};
 				};
@@ -1068,8 +1068,8 @@
 					#size-cells = <0>;
 					reg = <0>;
 
-					drc1_in_be1: endpoint@0 {
-						reg = <0>;
+					drc1_in_be1: endpoint@1 {
+						reg = <1>;
 						remote-endpoint = <&be1_out_drc1>;
 					};
 				};
@@ -1079,8 +1079,8 @@
 					#size-cells = <0>;
 					reg = <1>;
 
-					drc1_out_tcon1: endpoint@0 {
-						reg = <0>;
+					drc1_out_tcon1: endpoint@1 {
+						reg = <1>;
 						remote-endpoint = <&tcon1_in_drc1>;
 					};
 				};
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] drm/sun4i: add components in breadth first traversal order

[Chen-Yu Tsai]

The encoder drivers use drm_of_find_possible_crtcs to get upstream
crtcs from the device tree using of_graph. For the results to be
correct, encoders must be probed/bound after _all_ crtcs have been
created. The existing code uses a depth first recursive traversal
of the of_graph, which means the encoders downstream of the TCON
get add right after the first TCON. The second TCON or CRTC will
never be properly associated with encoders connected to it.

Other platforms, such as Rockchip, deal with this by probing all
CRTCs first, then all subsequent components. This is easy to do
since the CRTCs correspond to just one device node, and are the
first nodes in the pipeline.

However with Allwinner SoCs, the function of the CRTC is split
between the display backend (DE 1.0) or mixer (DE 2.0), which does
scan-out and compositing, and the TCON, which generates the display
timing signals. Further complicating the process, there may be a
Dynamic Range Controller between the backend and the TCON. Also, the
backend is preceded by the frontend, with a Display Enhancement Unit
possibly in between.

In a dual display pipeline setup, both frontends can feed either
backend, and both backends can feed either TCON. We want all
components of the same type to be added before the next type in the
pipeline. Fortunately, the pipelines are perfectly symmetric, i.e.
components of the same type are at the same depth when counted from
the frontend. The only exception is the third pipeline in the A80
SoC, which we do not support anyway.

Hence we can use a breadth first search traversal order to add
components. We do not need to check for duplicates. The component
matching system handles this for us.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 81 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 72 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index ace59651892f..2ff4233fe2da 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -200,11 +200,39 @@ static int compare_of(struct device *dev, void *data)
 	return dev->of_node == data;
 }
 
+/*
+ * The encoder drivers use drm_of_find_possible_crtcs to get upstream
+ * crtcs from the device tree using of_graph. For the results to be
+ * correct, encoders must be probed/bound after _all_ crtcs have been
+ * created. The existing code uses a depth first recursive traversal
+ * of the of_graph, which means the encoders downstream of the TCON
+ * get add right after the first TCON. The second TCON or CRTC will
+ * never be properly associated with encoders connected to it.
+ *
+ * Also, in a dual display pipeline setup, both frontends can feed
+ * either backend, and both backends can feed either TCON, we want
+ * all components of the same type to be added before the next type
+ * in the pipeline. Fortunately, the pipelines are perfectly symmetric,
+ * i.e. components of the same type are at the same depth when counted
+ * from the frontend. The only exception is the third pipeline in
+ * the A80 SoC, which we do not support anyway.
+ *
+ * Hence we can use a breadth first search traversal order to add
+ * components. We do not need to check for duplicates. The component
+ * matching system handles this for us.
+ */
+struct endpoint_list {
+	struct device_node *node;
+	struct list_head list;
+};
+
 static int sun4i_drv_add_endpoints(struct device *dev,
+				   struct list_head *endpoints,
 				   struct component_match **match,
 				   struct device_node *node)
 {
 	struct device_node *port, *ep, *remote;
+	struct endpoint_list *endpoint;
 	int count = 0;
 
 	/*
@@ -264,10 +292,15 @@ static int sun4i_drv_add_endpoints(struct device *dev,
 			}
 		}
 
-		/* Walk down our tree */
-		count += sun4i_drv_add_endpoints(dev, match, remote);
+		/* Add downstream nodes to the queue */
+		endpoint = kzalloc(sizeof(*endpoint), GFP_KERNEL);
+		if (!endpoint) {
+			of_node_put(remote);
+			return -ENOMEM;
+		}
 
-		of_node_put(remote);
+		endpoint->node = remote;
+		list_add_tail(&endpoint->list, endpoints);
 	}
 
 	return count;
@@ -277,7 +310,9 @@ static int sun4i_drv_probe(struct platform_device *pdev)
 {
 	struct component_match *match = NULL;
 	struct device_node *np = pdev->dev.of_node;
-	int i, count = 0;
+	struct endpoint_list *endpoint, *endpoint_temp;
+	int i, ret, count = 0;
+	LIST_HEAD(endpoints);
 
 	for (i = 0;; i++) {
 		struct device_node *pipeline = of_parse_phandle(np,
@@ -286,12 +321,31 @@ static int sun4i_drv_probe(struct platform_device *pdev)
 		if (!pipeline)
 			break;
 
-		count += sun4i_drv_add_endpoints(&pdev->dev, &match,
-						pipeline);
-		of_node_put(pipeline);
+		endpoint = kzalloc(sizeof(*endpoint), GFP_KERNEL);
+		if (!endpoint) {
+			ret = -ENOMEM;
+			goto err_free_endpoints;
+		}
+
+		endpoint->node = pipeline;
+		list_add_tail(&endpoint->list, &endpoints);
+	}
+
+	list_for_each_entry_safe(endpoint, endpoint_temp, &endpoints, list) {
+		/* process this endpoint */
+		ret = sun4i_drv_add_endpoints(&pdev->dev, &endpoints, &match,
+					      endpoint->node);
+
+		/* sun4i_drv_add_endpoints can fail to allocate memory */
+		if (ret < 0)
+			goto err_free_endpoints;
+
+		count += ret;
 
-		DRM_DEBUG_DRIVER("Queued %d outputs on pipeline %d\n",
-				 count, i);
+		/* delete and cleanup the current entry */
+		list_del(&endpoint->list);
+		of_node_put(endpoint->node);
+		kfree(endpoint);
 	}
 
 	if (count)
@@ -300,6 +354,15 @@ static int sun4i_drv_probe(struct platform_device *pdev)
 						       match);
 	else
 		return 0;
+
+err_free_endpoints:
+	list_for_each_entry_safe(endpoint, endpoint_temp, &endpoints, list) {
+		list_del(&endpoint->list);
+		of_node_put(endpoint->node);
+		kfree(endpoint);
+	}
+
+	return ret;
 }
 
 static int sun4i_drv_remove(struct platform_device *pdev)
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] drm/sun4i: tcon: Check for multiple paths between TCONs and backends

[Chen-Yu Tsai]

The patch b317fa3ba11a ("drm/sun4i: tcon: Find matching display backend
by device node matching") assumed a one-to-one mapping between TCONs
and backends. This turned out wrong, as we found muxing controls in the
TCON of the A31, and undocumented usage of the backend output selector
of the A20.

Make sun4i_tcon_find_engine() bail out if the current node has multiple
input connections.

Fixes: b317fa3ba11a ("drm/sun4i: tcon: Find matching display backend
		      by device node matching")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index eb32676d5b01..065654dbfb2c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -473,6 +473,20 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
 	if (!port)
 		return ERR_PTR(-EINVAL);
 
+	/*
+	 * This only works if there is only one path from the TCON
+	 * to any display engine. Otherwise the probe order of the
+	 * TCONs and display engines is not guaranteed. They may
+	 * either bind to the wrong one, or worse, bind to the same
+	 * one if additional checks are not done.
+	 *
+	 * Bail out if there are multiple input connections.
+	 */
+	if (of_get_available_child_count(port) != 1) {
+		of_node_put(port);
+		return ERR_PTR(-EINVAL);
+	}
+
 	for_each_available_child_of_node(port, ep) {
 		remote = of_graph_get_remote_port_parent(ep);
 		if (!remote)
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/8] drm/sun4i: tcon: get TCON ID and matching engine with remote endpoint ID

[Chen-Yu Tsai]

The device tree binding for sun4i-drm says:

    For all connections between components up to the TCONs in the display
    pipeline, when there are multiple components of the same type at the
    same depth, the local endpoint ID must be the same as the remote
    component's index. For example, if the remote endpoint is Frontend 1,
    then the local endpoint ID must be 1.

We should be able to get the TCON's ID directly from any of the remote
endpoints from its input port. With the ID, we can then go through the
list of registered engines and find a matching one by ID.

However the A31 device tree is incorrect. We assumed that there were no
cross pipeline connections between the backends and TCONs. As a result,
in all the endpoints between the backends and TCONs of the second
display pipeline, the endpoint IDs were incorrectly set to 0, when in
fact they should've been set to 1.

To maintain compatibility with this incorrect device tree, we first
check if the TCON's input port has 1 or many endpoints. If there are
more than 1, then it is likely a fixed version, and we can proceed
with the new method. If there is only 1 endpoint, then it is possibly
an incorrect version, or it could be the SoC only has one pipeline.
In either case we fall back to using the old method of traversing
the input connections to find a matching engine, and then get its
ID.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 121 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 065654dbfb2c..33c20d2f9fb1 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -463,8 +463,9 @@ static int sun4i_tcon_init_regmap(struct device *dev,
  * function in fact searches the corresponding engine, and the ID is
  * requested via the get_id function of the engine.
  */
-static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
-						   struct device_node *node)
+static struct sunxi_engine *
+sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv,
+				struct device_node *node)
 {
 	struct device_node *port, *ep, *remote;
 	struct sunxi_engine *engine;
@@ -502,7 +503,7 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
 		}
 
 		/* keep looking through upstream ports */
-		engine = sun4i_tcon_find_engine(drv, remote);
+		engine = sun4i_tcon_find_engine_traverse(drv, remote);
 		if (!IS_ERR(engine)) {
 			of_node_put(remote);
 			of_node_put(port);
@@ -513,6 +514,120 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
 	return ERR_PTR(-EINVAL);
 }
 
+/*
+ * The device tree binding says that the remote endpoint ID of any
+ * connection between components, up to and including the TCON, of
+ * the display pipeline should be equal to the actual ID of the local
+ * component. Thus we can look at any one of the input connections of
+ * the TCONs, and use that connection's remote endpoint ID as our own.
+ *
+ * Since the user of this function already finds the input port,
+ * the port is passed in directly without further checks.
+ */
+static int sun4i_tcon_of_get_id_from_port(struct device_node *port)
+{
+	struct device_node *ep;
+	int ret = -EINVAL;
+
+	/* try finding an upstream endpoint */
+	for_each_available_child_of_node(port, ep) {
+		struct device_node *remote;
+		u32 reg;
+
+		remote = of_graph_get_remote_endpoint(ep);
+		if (!remote)
+			continue;
+
+		ret = of_property_read_u32(remote, "reg", &reg);
+		if (ret)
+			continue;
+
+		ret = reg;
+	}
+
+	return ret;
+}
+
+/*
+ * Once we know the TCON's id, we can look through the list of
+ * engines to find a matching one. We assume all engines have
+ * been probed and added to the list.
+ */
+static struct sunxi_engine *sun4i_tcon_get_engine_by_id(struct sun4i_drv *drv,
+							int id)
+{
+	struct sunxi_engine *engine;
+
+	list_for_each_entry(engine, &drv->engine_list, list)
+		if (engine->id == id)
+			return engine;
+
+	return ERR_PTR(-EINVAL);
+}
+
+/*
+ * On SoCs with the old display pipeline design (Display Engine 1.0),
+ * we assumed the TCON was always tied to just one backend. However
+ * this proved not to be the case. On the A31, the TCON can select
+ * either backend as its source. On the A20 (and likely on the A10),
+ * the backend can choose which TCON to output to.
+ *
+ * The device tree binding says that the remote endpoint ID of any
+ * connection between components, up to and including the TCON, of
+ * the display pipeline should be equal to the actual ID of the local
+ * component. Thus we should be able to look at any one of the input
+ * connections of the TCONs, and use that connection's remote endpoint
+ * ID as our own.
+ *
+ * However  the connections between the backend and TCON were assumed
+ * to be always singular, and their endpoit IDs were all incorrectly
+ * set to 0. This means for these old device trees, we cannot just look
+ * up the remote endpoint ID of a TCON input endpoint. TCON1 would be
+ * incorrectly identified as TCON0.
+ *
+ * This function first checks if the TCON node has 2 input endpoints.
+ * If so, then the device tree is a corrected version, and it will use
+ * sun4i_tcon_of_get_id() and sun4i_tcon_get_engine_by_id() from above
+ * to fetch the ID and engine directly. If not, then it is likely an
+ * old device trees, where the endpoint IDs were incorrect, but did not
+ * have endpoint connections between the backend and TCON across
+ * different display pipelines. It will fall back to the old method of
+ * traversing the  of_graph to try and find a matching engine by device
+ * node.
+ *
+ * In the case of single display pipeline device trees, either method
+ * works.
+ */
+static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
+						   struct device_node *node)
+{
+	struct device_node *port;
+	struct sunxi_engine *engine;
+
+	port = of_graph_get_port_by_id(node, 0);
+	if (!port)
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * Is this a corrected device tree with cross pipeline
+	 * connections between the backend and TCON?
+	 */
+	if (of_get_child_count(port) > 1) {
+		/* Get our ID directly from an upstream endpoint */
+		int id = sun4i_tcon_of_get_id_from_port(port);
+
+		/* Get our engine by matching our ID */
+		engine = sun4i_tcon_get_engine_by_id(drv, id);
+
+		of_node_put(port);
+		return engine;
+	}
+
+	/* Fallback to old method by traversing input endpoints */
+	of_node_put(port);
+	return sun4i_tcon_find_engine_traverse(drv, node);
+}
+
 static int sun4i_tcon_bind(struct device *dev, struct device *master,
 			   void *data)
 {
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] drm/sun4i: tcon: Simplify sun4i_tcon_find_engine_traverse for one input

[Chen-Yu Tsai]

Now that sun4i_tcon_find_engine_traverse() usage is restricted to the
single input case, we can remove the for_each_available_child_of_node
loop.

While at it, consolidate all the of_node_put calls into a common exit
path.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 51 +++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 33c20d2f9fb1..e9ab03f0bf6e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -468,7 +468,7 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv,
 				struct device_node *node)
 {
 	struct device_node *port, *ep, *remote;
-	struct sunxi_engine *engine;
+	struct sunxi_engine *engine = ERR_PTR(-EINVAL);
 
 	port = of_graph_get_port_by_id(node, 0);
 	if (!port)
@@ -483,35 +483,34 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv,
 	 *
 	 * Bail out if there are multiple input connections.
 	 */
-	if (of_get_available_child_count(port) != 1) {
-		of_node_put(port);
-		return ERR_PTR(-EINVAL);
-	}
+	if (of_get_available_child_count(port) != 1)
+		goto out_put_port;
 
-	for_each_available_child_of_node(port, ep) {
-		remote = of_graph_get_remote_port_parent(ep);
-		if (!remote)
-			continue;
+	/* Get the first connection without specifying an ID */
+	ep = of_get_next_available_child(port, NULL);
+	if (!ep)
+		goto out_put_port;
 
-		/* does this node match any registered engines? */
-		list_for_each_entry(engine, &drv->engine_list, list) {
-			if (remote == engine->node) {
-				of_node_put(remote);
-				of_node_put(port);
-				return engine;
-			}
-		}
+	remote = of_graph_get_remote_port_parent(ep);
+	if (!remote)
+		goto out_put_ep;
 
-		/* keep looking through upstream ports */
-		engine = sun4i_tcon_find_engine_traverse(drv, remote);
-		if (!IS_ERR(engine)) {
-			of_node_put(remote);
-			of_node_put(port);
-			return engine;
-		}
-	}
+	/* does this node match any registered engines? */
+	list_for_each_entry(engine, &drv->engine_list, list)
+		if (remote == engine->node)
+			goto out_put_remote;
 
-	return ERR_PTR(-EINVAL);
+	/* keep looking through upstream ports */
+	engine = sun4i_tcon_find_engine_traverse(drv, remote);
+
+out_put_remote:
+	of_node_put(remote);
+out_put_ep:
+	of_node_put(ep);
+out_put_port:
+	of_node_put(port);
+
+	return engine;
 }
 
 /*
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/8] drm/sun4i: tcon: Support backend input mux

[Chen-Yu Tsai]

The TCON has a mux to select the source of the data to display.
This mux includes selecting the display backends. On the A31,
which has two display pipelines, this mux can let the TCON
select either backend as its data source. Although the muxing
can be changed on the fly, DRM needs to be able to group a
bunch of layers such that they get switched to another crtc
together. This is because the display backend does the layer
compositing, while the TCON generates the display timings.
This constraint is not supported by DRM.

Here we simply pair up backends and TCONs with the same ID.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 25 +++++++++++++++++++++++--
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  3 +++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index e9ab03f0bf6e..0951e3f0a35e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -701,6 +701,25 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 	if (ret < 0)
 		goto err_free_clocks;
 
+	if (tcon->quirks->needs_de_be_mux) {
+		/*
+		 * We assume there is no dynamic muxing of backends
+		 * and TCONs, so we select the backend with same ID.
+		 *
+		 * While dynamic selection might be interesting, since
+		 * the CRTC is tied to the TCON, while the layers are
+		 * tied to the backends, this means, we will need to
+		 * switch between groups of layers. There might not be
+		 * a way to represent this constraint in DRM.
+		 */
+		regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
+				   SUN4I_TCON0_CTL_SRC_SEL_MASK,
+				   tcon->id);
+		regmap_update_bits(tcon->regs, SUN4I_TCON1_CTL_REG,
+				   SUN4I_TCON1_CTL_SRC_SEL_MASK,
+				   tcon->id);
+	}
+
 	list_add_tail(&tcon->list, &drv->tcon_list);
 
 	return 0;
@@ -756,11 +775,13 @@ static const struct sun4i_tcon_quirks sun5i_a13_quirks = {
 };
 
 static const struct sun4i_tcon_quirks sun6i_a31_quirks = {
-	.has_channel_1	= true,
+	.has_channel_1		= true,
+	.needs_de_be_mux	= true,
 };
 
 static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
-	.has_channel_1	= true,
+	.has_channel_1		= true,
+	.needs_de_be_mux	= true,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index 552c88ec16be..5a219d1ccc26 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -37,6 +37,7 @@
 #define SUN4I_TCON0_CTL_TCON_ENABLE			BIT(31)
 #define SUN4I_TCON0_CTL_CLK_DELAY_MASK			GENMASK(8, 4)
 #define SUN4I_TCON0_CTL_CLK_DELAY(delay)		((delay << 4) & SUN4I_TCON0_CTL_CLK_DELAY_MASK)
+#define SUN4I_TCON0_CTL_SRC_SEL_MASK			GENMASK(2, 0)
 
 #define SUN4I_TCON0_DCLK_REG			0x44
 #define SUN4I_TCON0_DCLK_GATE_BIT			(31)
@@ -85,6 +86,7 @@
 #define SUN4I_TCON1_CTL_INTERLACE_ENABLE		BIT(20)
 #define SUN4I_TCON1_CTL_CLK_DELAY_MASK			GENMASK(8, 4)
 #define SUN4I_TCON1_CTL_CLK_DELAY(delay)		((delay << 4) & SUN4I_TCON1_CTL_CLK_DELAY_MASK)
+#define SUN4I_TCON1_CTL_SRC_SEL_MASK			GENMASK(1, 0)
 
 #define SUN4I_TCON1_BASIC0_REG			0x94
 #define SUN4I_TCON1_BASIC0_X(width)			((((width) - 1) & 0xfff) << 16)
@@ -146,6 +148,7 @@
 struct sun4i_tcon_quirks {
 	bool	has_unknown_mux; /* sun5i has undocumented mux */
 	bool	has_channel_1;	/* a33 does not have channel 1 */
+	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
 };
 
 struct sun4i_tcon {
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] drm/sun4i: call drm_vblank_init with correct number of crtcs

[Chen-Yu Tsai]

If we want to have vblank on both pipelines at the same time, we need
to call drm_vblank_init with num_crtcs = 2.

Instead, since the crtc init calls correctly set mode_config.num_crtc,
we can move the drm_vblank_init call to after the crtc init code is
called, which is the component bind part. Then we can just pass
mode_config.num_crtc in.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 2ff4233fe2da..a2012638d5f7 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -106,11 +106,6 @@ static int sun4i_drv_bind(struct device *dev)
 		goto free_drm;
 	}
 
-	/* drm_vblank_init calls kcalloc, which can fail */
-	ret = drm_vblank_init(drm, 1);
-	if (ret)
-		goto free_mem_region;
-
 	drm_mode_config_init(drm);
 
 	ret = component_bind_all(drm->dev, drm);
@@ -119,6 +114,11 @@ static int sun4i_drv_bind(struct device *dev)
 		goto cleanup_mode_config;
 	}
 
+	/* drm_vblank_init calls kcalloc, which can fail */
+	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
+	if (ret)
+		goto free_mem_region;
+
 	drm->irq_enabled = true;
 
 	/* Remove early framebuffers (ie. simplefb) */
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] ARM: dts: sun6i: Add cross pipeline connections between DRCs and TCONs

[Chen-Yu Tsai]

The TCONs on A31/A31s can select either backend as its input. As there
is no configurable mux in the backend or DRC to redirect their output,
or for the DRC to select an input, the connections are presumably from
the each DRC to each TCON, with the TCON having two input ports, like
the following diagram:

	Backend 0  -------  DRC 0  ------- [0]  TCON 0
				   --   -- [1]
                                     \ /
				      X
				     / \
				   --   -- [0]
	Backend 1  -------  DRC 1  ------- [1]  TCON 1

Add these connection endpoints to the device tree.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 00a4c7614e0a..93209cda28db 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -278,6 +278,11 @@
 						reg = <0>;
 						remote-endpoint = <&drc0_out_tcon0>;
 					};
+
+					tcon0_in_drc1: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&drc1_out_tcon0>;
+					};
 				};
 
 				tcon0_out: port@1 {
@@ -311,6 +316,11 @@
 					#size-cells = <0>;
 					reg = <0>;
 
+					tcon1_in_drc0: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&drc0_out_tcon1>;
+					};
+
 					tcon1_in_drc1: endpoint@1 {
 						reg = <1>;
 						remote-endpoint = <&drc1_out_tcon1>;
@@ -1079,6 +1089,11 @@
 					#size-cells = <0>;
 					reg = <1>;
 
+					drc1_out_tcon0: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&tcon0_in_drc1>;
+					};
+
 					drc1_out_tcon1: endpoint@1 {
 						reg = <1>;
 						remote-endpoint = <&tcon1_in_drc1>;
@@ -1170,6 +1185,11 @@
 						reg = <0>;
 						remote-endpoint = <&tcon0_in_drc0>;
 					};
+
+					drc0_out_tcon1: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&tcon1_in_drc0>;
+					};
 				};
 			};
 		};
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: sun6i: Fix endpoint IDs in second display pipeline

[Maxime Ripard]

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

Hi,

On Fri, Sep 08, 2017 at 03:50:09PM +0800, Chen-Yu Tsai wrote:
> When the second display pipeline device nodes for the A31/A31s were
> added, it was not known that the TCONs could (through either DRCs)
> select either backend as their input. Thus in the endpoints connecting
> these components together, the endpoint IDs were set to 0, while in
> fact they should have been set to 1.
> 
> Fixes: 9a26882a7378 ("ARM: dts: sun6i: Add second display pipeline device
> 		      nodes")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Should we CC stable on this one?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] ARM: dts: sun6i: Fix endpoint IDs in second display pipeline

[Chen-Yu Tsai]

On Fri, Sep 8, 2017 at 9:29 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, Sep 08, 2017 at 03:50:09PM +0800, Chen-Yu Tsai wrote:
>> When the second display pipeline device nodes for the A31/A31s were
>> added, it was not known that the TCONs could (through either DRCs)
>> select either backend as their input. Thus in the endpoints connecting
>> these components together, the endpoint IDs were set to 0, while in
>> fact they should have been set to 1.
>>
>> Fixes: 9a26882a7378 ("ARM: dts: sun6i: Add second display pipeline device
>>                     nodes")
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Should we CC stable on this one?

It wouldn't matter for old kernels, but I suppose we should get it right
everywhere, so we should probably CC stable. Also I forgot to mention
that this patch should go in -fixes, while the rest can go in -next.

Thanks
ChenYu
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/8] drm/sun4i: Make both display pipeline work together

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

Hi,

On Fri, Sep 08, 2017 at 03:50:08PM +0800, Chen-Yu Tsai wrote:
> Hi everyone,
> 
> Previously I added support for two display pipelines for the sun4i-drm
> driver. At the time we did not have support for downstream encoders to
> actually test the second pipeline, nor having both pipelines active at
> the same time.
> 
> With HDMI encoder support now merged and support for it on sun6i in
> progress, we are now able to test both pipelines being active at the
> same time, with LCD output from one and HDMI from the other. Testing
> has revealed some more issues, such as component probing order. Also,
> we found out that muxing between the display backend and TCON was
> possible, and required for the second pipeline to work as intended.
> Last, the number of CRTCs provided to drm_vblank_init needs to be
> increased for vblanks to work properly.
> 
> This patch series does a few things:
> 
>   - Fix endpoint IDs so they conform to what the DT binding stipulates.
> 
>   - Change the order the individual components are queued up.
> 
>   - Fix how the TCON gets its ID when the backend-TCON mux is present.
> 
>   - Support the input mux in the TCON (which selects which backend to
>     use on the A31/A31s).
> 
>   - Fix the drm_vblank_init call and pass the correct number of crtcs.
> 
>   - Add the cross pipeline connections between the DRCs and TCONs on
>     the A31, conforming to the DT binding and what the hardware is
>     capable of.
> 
> More details are available in each individual commit. Please have a look.

I've applied all the commits, with:
patch 1 in our arm-soc fixes branch
patch 2-7 in drm-misc-next
patch 8 in sunxi/dt-for-4.15.

I have a minor comment on one patch that can be fixed in a followup
patch, so please have a look at my other mail.

Thanks for this great work,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH 4/8] drm/sun4i: tcon: get TCON ID and matching engine with remote endpoint ID

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 7589 bytes --]

Hi,

On Fri, Sep 08, 2017 at 03:50:12PM +0800, Chen-Yu Tsai wrote:
> The device tree binding for sun4i-drm says:
> 
>     For all connections between components up to the TCONs in the display
>     pipeline, when there are multiple components of the same type at the
>     same depth, the local endpoint ID must be the same as the remote
>     component's index. For example, if the remote endpoint is Frontend 1,
>     then the local endpoint ID must be 1.
> 
> We should be able to get the TCON's ID directly from any of the remote
> endpoints from its input port. With the ID, we can then go through the
> list of registered engines and find a matching one by ID.
> 
> However the A31 device tree is incorrect. We assumed that there were no
> cross pipeline connections between the backends and TCONs. As a result,
> in all the endpoints between the backends and TCONs of the second
> display pipeline, the endpoint IDs were incorrectly set to 0, when in
> fact they should've been set to 1.
> 
> To maintain compatibility with this incorrect device tree, we first
> check if the TCON's input port has 1 or many endpoints. If there are
> more than 1, then it is likely a fixed version, and we can proceed
> with the new method. If there is only 1 endpoint, then it is possibly
> an incorrect version, or it could be the SoC only has one pipeline.
> In either case we fall back to using the old method of traversing
> the input connections to find a matching engine, and then get its
> ID.
> 
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 121 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 118 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 065654dbfb2c..33c20d2f9fb1 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -463,8 +463,9 @@ static int sun4i_tcon_init_regmap(struct device *dev,
>   * function in fact searches the corresponding engine, and the ID is
>   * requested via the get_id function of the engine.
>   */
> -static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
> -						   struct device_node *node)
> +static struct sunxi_engine *
> +sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv,
> +				struct device_node *node)
>  {
>  	struct device_node *port, *ep, *remote;
>  	struct sunxi_engine *engine;
> @@ -502,7 +503,7 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
>  		}
>  
>  		/* keep looking through upstream ports */
> -		engine = sun4i_tcon_find_engine(drv, remote);
> +		engine = sun4i_tcon_find_engine_traverse(drv, remote);
>  		if (!IS_ERR(engine)) {
>  			of_node_put(remote);
>  			of_node_put(port);
> @@ -513,6 +514,120 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
>  	return ERR_PTR(-EINVAL);
>  }
>  
> +/*
> + * The device tree binding says that the remote endpoint ID of any
> + * connection between components, up to and including the TCON, of
> + * the display pipeline should be equal to the actual ID of the local
> + * component. Thus we can look at any one of the input connections of
> + * the TCONs, and use that connection's remote endpoint ID as our own.
> + *
> + * Since the user of this function already finds the input port,
> + * the port is passed in directly without further checks.
> + */
> +static int sun4i_tcon_of_get_id_from_port(struct device_node *port)
> +{
> +	struct device_node *ep;
> +	int ret = -EINVAL;
> +
> +	/* try finding an upstream endpoint */
> +	for_each_available_child_of_node(port, ep) {
> +		struct device_node *remote;
> +		u32 reg;
> +
> +		remote = of_graph_get_remote_endpoint(ep);
> +		if (!remote)
> +			continue;
> +
> +		ret = of_property_read_u32(remote, "reg", &reg);
> +		if (ret)
> +			continue;
> +
> +		ret = reg;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Once we know the TCON's id, we can look through the list of
> + * engines to find a matching one. We assume all engines have
> + * been probed and added to the list.
> + */
> +static struct sunxi_engine *sun4i_tcon_get_engine_by_id(struct sun4i_drv *drv,
> +							int id)
> +{
> +	struct sunxi_engine *engine;
> +
> +	list_for_each_entry(engine, &drv->engine_list, list)
> +		if (engine->id == id)
> +			return engine;
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +/*
> + * On SoCs with the old display pipeline design (Display Engine 1.0),
> + * we assumed the TCON was always tied to just one backend. However
> + * this proved not to be the case. On the A31, the TCON can select
> + * either backend as its source. On the A20 (and likely on the A10),
> + * the backend can choose which TCON to output to.
> + *
> + * The device tree binding says that the remote endpoint ID of any
> + * connection between components, up to and including the TCON, of
> + * the display pipeline should be equal to the actual ID of the local
> + * component. Thus we should be able to look at any one of the input
> + * connections of the TCONs, and use that connection's remote endpoint
> + * ID as our own.
> + *
> + * However  the connections between the backend and TCON were assumed
> + * to be always singular, and their endpoit IDs were all incorrectly
> + * set to 0. This means for these old device trees, we cannot just look
> + * up the remote endpoint ID of a TCON input endpoint. TCON1 would be
> + * incorrectly identified as TCON0.
> + *
> + * This function first checks if the TCON node has 2 input endpoints.
> + * If so, then the device tree is a corrected version, and it will use
> + * sun4i_tcon_of_get_id() and sun4i_tcon_get_engine_by_id() from above
> + * to fetch the ID and engine directly. If not, then it is likely an
> + * old device trees, where the endpoint IDs were incorrect, but did not
> + * have endpoint connections between the backend and TCON across
> + * different display pipelines. It will fall back to the old method of
> + * traversing the  of_graph to try and find a matching engine by device
> + * node.
> + *
> + * In the case of single display pipeline device trees, either method
> + * works.
> + */
> +static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
> +						   struct device_node *node)
> +{
> +	struct device_node *port;
> +	struct sunxi_engine *engine;
> +
> +	port = of_graph_get_port_by_id(node, 0);
> +	if (!port)
> +		return ERR_PTR(-EINVAL);
> +
> +	/*
> +	 * Is this a corrected device tree with cross pipeline
> +	 * connections between the backend and TCON?
> +	 */
> +	if (of_get_child_count(port) > 1) {
> +		/* Get our ID directly from an upstream endpoint */
> +		int id = sun4i_tcon_of_get_id_from_port(port);
> +
> +		/* Get our engine by matching our ID */
> +		engine = sun4i_tcon_get_engine_by_id(drv, id);
> +
> +		of_node_put(port);
> +		return engine;
> +	}
> +
> +	/* Fallback to old method by traversing input endpoints */
> +	of_node_put(port);
> +	return sun4i_tcon_find_engine_traverse(drv, node);

In the old DT case, I'd like to have a warning displayed in the log
saying that the DT is too old and we won't be able to support the
dual-pipeline properly, so that the users can at least know that they
should update.

Can you add it in a follow-up?

Thanks!


-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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