[v2,45/49] drm/omap: Add support for drm_bridge

Submitted by Laurent Pinchart on Jan. 11, 2019, 3:51 a.m.

Details

Message ID 20190111035120.20668-46-laurent.pinchart@ideasonboard.com
State New
Series "omapdrm: drm_bridge and drm_panel support"
Headers show

Commit Message

Laurent Pinchart Jan. 11, 2019, 3:51 a.m.
Hook up drm_bridge support in the omapdrm driver. Despite the recent
extensive preparation work, this is a rather intrusive change, as the
management of outputs needs to be adapted through the driver to handle
both omap_dss_device and drm_bridge.

Connector creation is skipped when using a drm_bridge, as the bridge
creates the connector internally. This creates issues with systems that
split connector operations (such as modes retrieval and hot-plug
detection) across different bridges. These systems can't be supported
using drm_bridge for now (their support through the omap_dss_device
infrastructure is not affected), this will be fixed in subsequent
changes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Changes since v1:

- Fix typo in function name (updata -> update)
---
 drivers/gpu/drm/omapdrm/dss/base.c       | 27 ++++++++--
 drivers/gpu/drm/omapdrm/dss/omapdss.h    |  1 +
 drivers/gpu/drm/omapdrm/dss/output.c     | 21 +++++---
 drivers/gpu/drm/omapdrm/omap_connector.c | 16 ++++--
 drivers/gpu/drm/omapdrm/omap_connector.h |  1 -
 drivers/gpu/drm/omapdrm/omap_crtc.c      |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c       | 69 +++++++++++++++++-------
 drivers/gpu/drm/omapdrm/omap_encoder.c   | 69 ++++++++++++++----------
 8 files changed, 145 insertions(+), 61 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/omapdrm/dss/base.c b/drivers/gpu/drm/omapdrm/dss/base.c
index 81ea0f55cd75..09c9f2971aa2 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -19,6 +19,7 @@ 
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
+#include <linux/platform_device.h>
 
 #include "dss.h"
 #include "omapdss.h"
@@ -156,7 +157,7 @@  struct omap_dss_device *omapdss_device_next_output(struct omap_dss_device *from)
 			goto done;
 		}
 
-		if (dssdev->id && dssdev->next)
+		if (dssdev->id && (dssdev->next || dssdev->bridge))
 			goto done;
 	}
 
@@ -184,7 +185,18 @@  int omapdss_device_connect(struct dss_device *dss,
 {
 	int ret;
 
-	dev_dbg(dst->dev, "connect\n");
+	dev_dbg(&dss->pdev->dev, "connect(%s, %s)\n",
+		src ? dev_name(src->dev) : "NULL",
+		dst ? dev_name(dst->dev) : "NULL");
+
+	if (!dst) {
+		/*
+		 * The destination is NULL when the source is connected to a
+		 * bridge instead of a DSS device. Stop here, we will attach the
+		 * bridge later when we will have a DRM encoder.
+		 */
+		return src && src->bridge ? 0 : -EINVAL;
+	}
 
 	if (omapdss_device_is_connected(dst))
 		return -EBUSY;
@@ -204,7 +216,16 @@  EXPORT_SYMBOL_GPL(omapdss_device_connect);
 void omapdss_device_disconnect(struct omap_dss_device *src,
 			       struct omap_dss_device *dst)
 {
-	dev_dbg(dst->dev, "disconnect\n");
+	struct dss_device *dss = src ? src->dss : dst->dss;
+
+	dev_dbg(&dss->pdev->dev, "disconnect(%s, %s)\n",
+		src ? dev_name(src->dev) : "NULL",
+		dst ? dev_name(dst->dev) : "NULL");
+
+	if (!dst) {
+		WARN_ON(!src->bridge);
+		return;
+	}
 
 	if (!dst->id && !omapdss_device_is_connected(dst)) {
 		WARN_ON(!dst->display);
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index ab5467a1e92c..f47e9b94288f 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -410,6 +410,7 @@  struct omap_dss_device {
 
 	struct dss_device *dss;
 	struct omap_dss_device *next;
+	struct drm_bridge *bridge;
 
 	struct list_head list;
 
diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
index f25ecfd26534..2a53025c2fde 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -20,25 +20,34 @@ 
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 
 #include "dss.h"
 #include "omapdss.h"
 
 int omapdss_device_init_output(struct omap_dss_device *out)
 {
-	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
-	if (IS_ERR(out->next)) {
-		if (PTR_ERR(out->next) != -EPROBE_DEFER)
-			dev_err(out->dev, "failed to find video sink\n");
-		return PTR_ERR(out->next);
+	struct device_node *remote_node;
+
+	remote_node = of_graph_get_remote_node(out->dev->of_node, 0, 0);
+	if (!remote_node) {
+		dev_dbg(out->dev, "failed to find video sink\n");
+		return 0;
 	}
 
+	out->next = omapdss_find_device_by_node(remote_node);
+	out->bridge = of_drm_find_bridge(remote_node);
+
+	of_node_put(remote_node);
+
 	if (out->next && out->type != out->next->type) {
 		dev_err(out->dev, "output type and display type don't match\n");
+		omapdss_device_put(out->next);
+		out->next = NULL;
 		return -EINVAL;
 	}
 
-	return 0;
+	return out->next || out->bridge ? 0 : -EPROBE_DEFER;
 }
 EXPORT_SYMBOL(omapdss_device_init_output);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 487603c56b08..f528baa80114 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -304,9 +304,16 @@  static const struct drm_connector_helper_funcs omap_connector_helper_funcs = {
 	.mode_valid = omap_connector_mode_valid,
 };
 
-static int omap_connector_get_type(struct omap_dss_device *display)
+static int omap_connector_get_type(struct omap_dss_device *output)
 {
-	switch (display->type) {
+	struct omap_dss_device *display;
+	enum omap_display_type type;
+
+	display = omapdss_display_get(output);
+	type = display->type;
+	omapdss_device_put(display);
+
+	switch (type) {
 	case OMAP_DISPLAY_TYPE_HDMI:
 		return DRM_MODE_CONNECTOR_HDMIA;
 	case OMAP_DISPLAY_TYPE_DVI:
@@ -329,14 +336,13 @@  static int omap_connector_get_type(struct omap_dss_device *display)
 /* initialize connector */
 struct drm_connector *omap_connector_init(struct drm_device *dev,
 					  struct omap_dss_device *output,
-					  struct omap_dss_device *display,
 					  struct drm_encoder *encoder)
 {
 	struct drm_connector *connector = NULL;
 	struct omap_connector *omap_connector;
 	struct omap_dss_device *dssdev;
 
-	DBG("%s", display->name);
+	DBG("%s", output->name);
 
 	omap_connector = kzalloc(sizeof(*omap_connector), GFP_KERNEL);
 	if (!omap_connector)
@@ -349,7 +355,7 @@  struct drm_connector *omap_connector_init(struct drm_device *dev,
 	connector->doublescan_allowed = 0;
 
 	drm_connector_init(dev, connector, &omap_connector_funcs,
-			   omap_connector_get_type(display));
+			   omap_connector_get_type(output));
 	drm_connector_helper_add(connector, &omap_connector_helper_funcs);
 
 	/*
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.h b/drivers/gpu/drm/omapdrm/omap_connector.h
index 6b7d4d95e32b..608085219336 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.h
+++ b/drivers/gpu/drm/omapdrm/omap_connector.h
@@ -31,7 +31,6 @@  struct omap_dss_device;
 
 struct drm_connector *omap_connector_init(struct drm_device *dev,
 					  struct omap_dss_device *output,
-					  struct omap_dss_device *display,
 					  struct drm_encoder *encoder);
 bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
 void omap_connector_enable_hpd(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 0cea3824d3a6..4486152fd6cc 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -671,7 +671,7 @@  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 					&omap_crtc_funcs, NULL);
 	if (ret < 0) {
 		dev_err(dev->dev, "%s(): could not init crtc for: %s\n",
-			__func__, pipe->display->name);
+			__func__, pipe->output->name);
 		kfree(omap_crtc);
 		return ERR_PTR(ret);
 	}
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 6b91f44e5a60..35c4669dc69d 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -140,9 +140,7 @@  static void omap_disconnect_pipelines(struct drm_device *ddev)
 		omapdss_device_disconnect(NULL, pipe->output);
 
 		omapdss_device_put(pipe->output);
-		omapdss_device_put(pipe->display);
 		pipe->output = NULL;
-		pipe->display = NULL;
 	}
 
 	memset(&priv->channels, 0, sizeof(priv->channels));
@@ -169,7 +167,6 @@  static int omap_connect_pipelines(struct drm_device *ddev)
 
 			pipe = &priv->pipes[priv->num_pipes++];
 			pipe->output = omapdss_device_get(output);
-			pipe->display = omapdss_display_get(output);
 
 			if (priv->num_pipes == ARRAY_SIZE(priv->pipes)) {
 				/* To balance the 'for_each_dss_output' loop */
@@ -207,6 +204,28 @@  static int omap_modeset_init_properties(struct drm_device *dev)
 	return 0;
 }
 
+static int omap_display_id(struct omap_dss_device *output)
+{
+	struct device_node *node = NULL;
+
+	if (output->next) {
+		struct omap_dss_device *display;
+
+		display = omapdss_display_get(output);
+		node = display->dev->of_node;
+		omapdss_device_put(display);
+	} else {
+		struct drm_bridge *bridge = output->bridge;
+
+		while (bridge->next)
+			bridge = bridge->next;
+
+		node = bridge->of_node;
+	}
+
+	return node ? of_alias_get_id(node, "display") : -ENODEV;
+}
+
 static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
@@ -262,7 +281,10 @@  static int omap_modeset_init(struct drm_device *dev)
 		priv->planes[priv->num_planes++] = plane;
 	}
 
-	/* Create the encoders and get the pipelines aliases. */
+	/*
+	 * Create the encoders, attach the bridges and get the pipeline alias
+	 * IDs.
+	 */
 	for (i = 0; i < priv->num_pipes; i++) {
 		struct omap_drm_pipeline *pipe = &priv->pipes[i];
 		int id;
@@ -271,7 +293,14 @@  static int omap_modeset_init(struct drm_device *dev)
 		if (!pipe->encoder)
 			return -ENOMEM;
 
-		id = of_alias_get_id(pipe->display->dev->of_node, "display");
+		if (pipe->output->bridge) {
+			ret = drm_bridge_attach(pipe->encoder,
+						pipe->output->bridge, NULL);
+			if (ret < 0)
+				return ret;
+		}
+
+		id = omap_display_id(pipe->output);
 		pipe->alias_id = id >= 0 ? id : i;
 	}
 
@@ -297,16 +326,16 @@  static int omap_modeset_init(struct drm_device *dev)
 	for (i = 0; i < priv->num_pipes; i++) {
 		struct omap_drm_pipeline *pipe = &priv->pipes[i];
 		struct drm_encoder *encoder = pipe->encoder;
-		struct drm_connector *connector;
 		struct drm_crtc *crtc;
 
-		connector = omap_connector_init(dev, pipe->output,
-						pipe->display, encoder);
-		if (!connector)
-			return -ENOMEM;
+		if (!pipe->output->bridge) {
+			pipe->connector = omap_connector_init(dev, pipe->output,
+							      encoder);
+			if (!pipe->connector)
+				return -ENOMEM;
 
-		drm_connector_attach_encoder(connector, encoder);
-		pipe->connector = connector;
+			drm_connector_attach_encoder(pipe->connector, encoder);
+		}
 
 		crtc = omap_crtc_init(dev, pipe, priv->planes[i]);
 		if (IS_ERR(crtc))
@@ -350,10 +379,12 @@  static int omap_modeset_init(struct drm_device *dev)
 static void omap_modeset_enable_external_hpd(struct drm_device *ddev)
 {
 	struct omap_drm_private *priv = ddev->dev_private;
-	int i;
+	unsigned int i;
 
-	for (i = 0; i < priv->num_pipes; i++)
-		omap_connector_enable_hpd(priv->pipes[i].connector);
+	for (i = 0; i < priv->num_pipes; i++) {
+		if (priv->pipes[i].connector)
+			omap_connector_enable_hpd(priv->pipes[i].connector);
+	}
 }
 
 /*
@@ -362,10 +393,12 @@  static void omap_modeset_enable_external_hpd(struct drm_device *ddev)
 static void omap_modeset_disable_external_hpd(struct drm_device *ddev)
 {
 	struct omap_drm_private *priv = ddev->dev_private;
-	int i;
+	unsigned int i;
 
-	for (i = 0; i < priv->num_pipes; i++)
-		omap_connector_disable_hpd(priv->pipes[i].connector);
+	for (i = 0; i < priv->num_pipes; i++) {
+		if (priv->pipes[i].connector)
+			omap_connector_disable_hpd(priv->pipes[i].connector);
+	}
 }
 
 /*
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index e71d359a8f07..76f94cc0c0cf 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -51,6 +51,34 @@  static const struct drm_encoder_funcs omap_encoder_funcs = {
 	.destroy = omap_encoder_destroy,
 };
 
+static void omap_encoder_update_videomode_flags(struct videomode *vm,
+						u32 bus_flags)
+{
+	if (!(vm->flags & (DISPLAY_FLAGS_DE_LOW |
+			   DISPLAY_FLAGS_DE_HIGH))) {
+		if (bus_flags & DRM_BUS_FLAG_DE_LOW)
+			vm->flags |= DISPLAY_FLAGS_DE_LOW;
+		else if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
+			vm->flags |= DISPLAY_FLAGS_DE_HIGH;
+	}
+
+	if (!(vm->flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
+			   DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
+		if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
+			vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
+		else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
+			vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+	}
+
+	if (!(vm->flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
+			   DISPLAY_FLAGS_SYNC_NEGEDGE))) {
+		if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
+			vm->flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
+		else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
+			vm->flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
+	}
+}
+
 static void omap_encoder_hdmi_mode_set(struct drm_encoder *encoder,
 				       struct drm_display_mode *adjusted_mode)
 {
@@ -87,7 +115,9 @@  static void omap_encoder_mode_set(struct drm_encoder *encoder,
 				  struct drm_display_mode *adjusted_mode)
 {
 	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
+	struct omap_dss_device *output = omap_encoder->output;
 	struct omap_dss_device *dssdev;
+	struct drm_bridge *bridge;
 	struct videomode vm = { 0 };
 
 	drm_display_mode_to_videomode(adjusted_mode, &vm);
@@ -101,44 +131,29 @@  static void omap_encoder_mode_set(struct drm_encoder *encoder,
 	 *
 	 * A better solution is to use DRM's bus-flags through the whole driver.
 	 */
-	for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) {
-		unsigned long bus_flags = dssdev->bus_flags;
-
-		if (!(vm.flags & (DISPLAY_FLAGS_DE_LOW |
-				  DISPLAY_FLAGS_DE_HIGH))) {
-			if (bus_flags & DRM_BUS_FLAG_DE_LOW)
-				vm.flags |= DISPLAY_FLAGS_DE_LOW;
-			else if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
-				vm.flags |= DISPLAY_FLAGS_DE_HIGH;
-		}
+	for (dssdev = output; dssdev; dssdev = dssdev->next)
+		omap_encoder_update_videomode_flags(&vm, dssdev->bus_flags);
 
-		if (!(vm.flags & (DISPLAY_FLAGS_PIXDATA_POSEDGE |
-				  DISPLAY_FLAGS_PIXDATA_NEGEDGE))) {
-			if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
-				vm.flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
-			else if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
-				vm.flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
-		}
+	for (bridge = output->bridge; bridge; bridge = bridge->next) {
+		u32 bus_flags;
 
-		if (!(vm.flags & (DISPLAY_FLAGS_SYNC_POSEDGE |
-				  DISPLAY_FLAGS_SYNC_NEGEDGE))) {
-			if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE)
-				vm.flags |= DISPLAY_FLAGS_SYNC_POSEDGE;
-			else if (bus_flags & DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE)
-				vm.flags |= DISPLAY_FLAGS_SYNC_NEGEDGE;
-		}
+		if (!bridge->timings)
+			continue;
+
+		bus_flags = bridge->timings->input_bus_flags;
+		omap_encoder_update_videomode_flags(&vm, bus_flags);
 	}
 
 	/* Set timings for all devices in the display pipeline. */
-	dss_mgr_set_timings(omap_encoder->output, &vm);
+	dss_mgr_set_timings(output, &vm);
 
-	for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) {
+	for (dssdev = output; dssdev; dssdev = dssdev->next) {
 		if (dssdev->ops->set_timings)
 			dssdev->ops->set_timings(dssdev, adjusted_mode);
 	}
 
 	/* Set the HDMI mode and HDMI infoframe if applicable. */
-	if (omap_encoder->output->type == OMAP_DISPLAY_TYPE_HDMI)
+	if (output->type == OMAP_DISPLAY_TYPE_HDMI)
 		omap_encoder_hdmi_mode_set(encoder, adjusted_mode);
 }
 

Comments

Tomi Valkeinen Jan. 18, 2019, 10:33 a.m.
Hi Laurent,

On 11/01/19 05:51, Laurent Pinchart wrote:
> Hook up drm_bridge support in the omapdrm driver. Despite the recent
> extensive preparation work, this is a rather intrusive change, as the
> management of outputs needs to be adapted through the driver to handle
> both omap_dss_device and drm_bridge.
> 
> Connector creation is skipped when using a drm_bridge, as the bridge
> creates the connector internally. This creates issues with systems that
> split connector operations (such as modes retrieval and hot-plug
> detection) across different bridges. These systems can't be supported
> using drm_bridge for now (their support through the omap_dss_device
> infrastructure is not affected), this will be fixed in subsequent
> changes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

<snip>

> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c
> index f25ecfd26534..2a53025c2fde 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -20,25 +20,34 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <linux/of_graph.h>
>  
>  #include "dss.h"
>  #include "omapdss.h"
>  
>  int omapdss_device_init_output(struct omap_dss_device *out)
>  {
> -	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
> -	if (IS_ERR(out->next)) {
> -		if (PTR_ERR(out->next) != -EPROBE_DEFER)
> -			dev_err(out->dev, "failed to find video sink\n");
> -		return PTR_ERR(out->next);
> +	struct device_node *remote_node;
> +
> +	remote_node = of_graph_get_remote_node(out->dev->of_node, 0, 0);
> +	if (!remote_node) {
> +		dev_dbg(out->dev, "failed to find video sink\n");
> +		return 0;
>  	}

This breaks boards that have displays/bridges connected to ports above
0. For example, DPI output has 3 ports on some SoCs.

I made a quick fix like so:

+       port_num = __ffs(out->of_ports);
+
+       remote_node = of_graph_get_remote_node(out->dev->of_node,
port_num, 0);

Which I think works for all our outputs as we only set a single bit in
of_ports for outputs. But I don't think that's quite correct. Should we
have another field which tells which port is to be used? Then again,
maybe __ffs() is good enough here, as we can guarantee that there's ever
a single port in of_ports.

 Tomi