Thread (64 messages) 64 messages, 12 authors, 2016-09-29

[PATCH 1/5] drm/imx-ldb: Add support to drm-bridge

From: p.zabel@pengutronix.de (Philipp Zabel)
Date: 2016-06-02 13:09:28
Also in: dri-devel, linux-devicetree, lkml

Am Montag, den 30.05.2016, 18:39 +0200 schrieb Peter Senna Tschudin:
quoted hunk ↗ jump to hunk
Add support to attach a drm_bridge to imx-ldb in addition to
existing support to attach a LVDS panel.

Signed-off-by: Peter Senna Tschudin <redacted>
---
 drivers/gpu/drm/imx/imx-ldb.c | 75 +++++++++++++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index a58eee5..7233a81 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -57,7 +57,11 @@ struct imx_ldb_channel {
 	struct imx_ldb *ldb;
 	struct drm_connector connector;
 	struct drm_encoder encoder;
+
+	/* Defines what is connected to the ldb, only one at a time */
 	struct drm_panel *panel;
+	struct drm_bridge *ext_bridge;
+
Just bridge should be clear enough.
quoted hunk ↗ jump to hunk
 	struct device_node *child;
 	int chno;
 	void *edid;
@@ -295,6 +299,10 @@ static void imx_ldb_encoder_mode_set(struct drm_encoder *encoder,
 	}
 }
 
+static void imx_ldb_encoder_enable(struct drm_encoder *encoder)
+{
+}
+
Why?

Note that Liu Ying's atomic series touches this also, and after
"drm/imx: atomic phase 3 step 3: Legacy callback fixups" the enable
callback exists. Depending on how either series proceeds, one will have
to be rebased onto the other.
quoted hunk ↗ jump to hunk
 static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
 {
 	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
@@ -373,6 +381,7 @@ static const struct drm_encoder_helper_funcs imx_ldb_encoder_helper_funcs = {
 	.prepare = imx_ldb_encoder_prepare,
 	.commit = imx_ldb_encoder_commit,
 	.mode_set = imx_ldb_encoder_mode_set,
+	.enable = imx_ldb_encoder_enable,
 	.disable = imx_ldb_encoder_disable,
 };
 
@@ -417,16 +426,28 @@ static int imx_ldb_register(struct drm_device *drm,
 	drm_encoder_init(drm, &imx_ldb_ch->encoder, &imx_ldb_encoder_funcs,
 			 DRM_MODE_ENCODER_LVDS, NULL);
 
-	drm_connector_helper_add(&imx_ldb_ch->connector,
-			&imx_ldb_connector_helper_funcs);
-	drm_connector_init(drm, &imx_ldb_ch->connector,
-			   &imx_ldb_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
-
-	if (imx_ldb_ch->panel)
+	if (imx_ldb_ch->panel) {
	if (!imx_ldb_ch->bridge) {
quoted hunk ↗ jump to hunk
+		drm_connector_helper_add(&imx_ldb_ch->connector,
+				&imx_ldb_connector_helper_funcs);
+		drm_connector_init(drm, &imx_ldb_ch->connector,
+				&imx_ldb_connector_funcs,
+				DRM_MODE_CONNECTOR_LVDS);
 		drm_panel_attach(imx_ldb_ch->panel, &imx_ldb_ch->connector);
 
-	drm_mode_connector_attach_encoder(&imx_ldb_ch->connector,
-			&imx_ldb_ch->encoder);
+		drm_mode_connector_attach_encoder(&imx_ldb_ch->connector,
+				&imx_ldb_ch->encoder);
+	}
+
+	if (imx_ldb_ch->ext_bridge) {
+		imx_ldb_ch->ext_bridge->encoder = &imx_ldb_ch->encoder;
+
+		imx_ldb_ch->encoder.bridge = imx_ldb_ch->ext_bridge;
+		ret = drm_bridge_attach(drm, imx_ldb_ch->ext_bridge);
+		if (ret) {
+			DRM_ERROR("Failed to initialize bridge with drm\n");
+			return ret;
+		}
+	}
 
 	return 0;
 }
@@ -583,23 +604,35 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
 			endpoint = of_get_child_by_name(port, "endpoint");
 			if (endpoint) {
 				remote = of_graph_get_remote_port_parent(endpoint);
-				if (remote)
-					channel->panel = of_drm_find_panel(remote);
-				else
-					return -EPROBE_DEFER;
-				if (!channel->panel) {
-					dev_err(dev, "panel not found: %s\n",
-						remote->full_name);
Let's keep this message, at least as dev_dbg.
-					return -EPROBE_DEFER;
+				if (remote) {
+					/* Only one of these two will succeed */
+					channel->panel =
+						of_drm_find_panel(remote);
+
+					channel->ext_bridge =
+						of_drm_find_bridge(remote);
+
+					/*
+					 * If the bridge is compiled as a
+					 * module, it may take some time until
+					 * the bridge driver is available.
+					 * Defer until the bridge driver is
+					 * ready.
+					 */
+					if (!channel->panel &&
+							!channel->ext_bridge)
+						return -EPROBE_DEFER;
 				}
What happens if (!remote)?
 			}
 		}
 
-		edidp = of_get_property(child, "edid", &channel->edid_len);
-		if (edidp) {
-			channel->edid = kmemdup(edidp, channel->edid_len,
-						GFP_KERNEL);
-		} else if (!channel->panel) {
+		if (channel->panel) {
+			edidp = of_get_property(child, "edid",
+					&channel->edid_len);
+			if (edidp)
+				channel->edid = kmemdup(edidp,
+						channel->edid_len, GFP_KERNEL);
+		} else {
Any reason to disallow overriding EDID from DT if there is no panel? I'd
leave this part as is.

regards
Philipp
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help