[PATCH v2 18/21] drm/omap: Don't call EDID read operation recursively

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Aug 6 20:23:12 UTC 2018


Instead of calling the EDID read operation (.read_edid()) recursively
from the display device back to the first device that provides EDID read
support, iterate over the devices manually in the DRM connector code.
This moves the complexity to a single central location and simplifies
the logic in omap_dss_device drivers.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel at collabora.co.uk>
---
 drivers/gpu/drm/omapdrm/displays/connector-dvi.c   |  15 +--
 drivers/gpu/drm/omapdrm/displays/connector-hdmi.c  |  11 ---
 .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c   |  13 ---
 drivers/gpu/drm/omapdrm/dss/hdmi4.c                |   1 +
 drivers/gpu/drm/omapdrm/dss/hdmi5.c                |   1 +
 drivers/gpu/drm/omapdrm/omap_connector.c           | 101 ++++++++++++---------
 6 files changed, 65 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
index 6be260ff6458..eae4108330f1 100644
--- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
+++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
@@ -166,12 +166,6 @@ static int dvic_read_edid(struct omap_dss_device *dssdev,
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
 	int r, l, bytes_read;
 
-	if (ddata->hpd_gpio && !gpiod_get_value_cansleep(ddata->hpd_gpio))
-		return -ENODEV;
-
-	if (!ddata->i2c_adapter)
-		return -ENODEV;
-
 	l = min(EDID_LENGTH, len);
 	r = dvic_ddc_read(ddata->i2c_adapter, edid, l, 0);
 	if (r)
@@ -341,10 +335,11 @@ static int dvic_probe(struct platform_device *pdev)
 	dssdev->of_ports = BIT(0);
 
 	if (ddata->hpd_gpio)
-		dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT
-				  | OMAP_DSS_DEVICE_OP_HPD;
-	else if (ddata->i2c_adapter)
-		dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT;
+		dssdev->ops_flags |= OMAP_DSS_DEVICE_OP_DETECT
+				  |  OMAP_DSS_DEVICE_OP_HPD;
+	if (ddata->i2c_adapter)
+		dssdev->ops_flags |= OMAP_DSS_DEVICE_OP_DETECT
+				  |  OMAP_DSS_DEVICE_OP_EDID;
 
 	omapdss_display_init(dssdev);
 	omapdss_device_register(dssdev);
diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
index 6f2364afb14a..16dc22edcb8e 100644
--- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
+++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
@@ -15,8 +15,6 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-#include <drm/drm_edid.h>
-
 #include "../dss/omapdss.h"
 
 static const struct videomode hdmic_default_vm = {
@@ -126,14 +124,6 @@ static int hdmic_check_timings(struct omap_dss_device *dssdev,
 	return src->ops->check_timings(src, vm);
 }
 
-static int hdmic_read_edid(struct omap_dss_device *dssdev,
-		u8 *edid, int len)
-{
-	struct omap_dss_device *src = dssdev->src;
-
-	return src->ops->read_edid(src, edid, len);
-}
-
 static bool hdmic_detect(struct omap_dss_device *dssdev)
 {
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
@@ -190,7 +180,6 @@ static const struct omap_dss_device_ops hdmic_ops = {
 	.get_timings		= hdmic_get_timings,
 	.check_timings		= hdmic_check_timings,
 
-	.read_edid		= hdmic_read_edid,
 	.detect			= hdmic_detect,
 	.register_hpd_cb	= hdmic_register_hpd_cb,
 	.unregister_hpd_cb	= hdmic_unregister_hpd_cb,
diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
index da97d357bde7..3ce1c935a48c 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
@@ -115,18 +115,6 @@ static int tpd_check_timings(struct omap_dss_device *dssdev,
 	return src->ops->check_timings(src, vm);
 }
 
-static int tpd_read_edid(struct omap_dss_device *dssdev,
-		u8 *edid, int len)
-{
-	struct panel_drv_data *ddata = to_panel_data(dssdev);
-	struct omap_dss_device *src = dssdev->src;
-
-	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
-		return -ENODEV;
-
-	return src->ops->read_edid(src, edid, len);
-}
-
 static bool tpd_detect(struct omap_dss_device *dssdev)
 {
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
@@ -180,7 +168,6 @@ static const struct omap_dss_device_ops tpd_ops = {
 	.disable		= tpd_disable,
 	.check_timings		= tpd_check_timings,
 	.set_timings		= tpd_set_timings,
-	.read_edid		= tpd_read_edid,
 	.detect			= tpd_detect,
 	.register_hpd_cb	= tpd_register_hpd_cb,
 	.unregister_hpd_cb	= tpd_unregister_hpd_cb,
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index bebce93fed3e..c92564300446 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -711,6 +711,7 @@ static int hdmi4_init_output(struct omap_hdmi *hdmi)
 	out->ops = &hdmi_ops;
 	out->owner = THIS_MODULE;
 	out->of_ports = BIT(0);
+	out->ops_flags = OMAP_DSS_DEVICE_OP_EDID;
 
 	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
 	if (IS_ERR(out->next)) {
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index 7c07e0208107..2aaa8ee61662 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -703,6 +703,7 @@ static int hdmi5_init_output(struct omap_hdmi *hdmi)
 	out->ops = &hdmi_ops;
 	out->owner = THIS_MODULE;
 	out->of_ports = BIT(0);
+	out->ops_flags = OMAP_DSS_DEVICE_OP_EDID;
 
 	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
 	if (IS_ERR(out->next)) {
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 344414ef3962..5091991363d6 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -170,65 +170,80 @@ static void omap_connector_destroy(struct drm_connector *connector)
 
 #define MAX_EDID  512
 
+static int omap_connector_get_modes_edid(struct drm_connector *connector,
+					 struct omap_dss_device *dssdev)
+{
+	struct omap_connector *omap_connector = to_omap_connector(connector);
+	enum drm_connector_status status;
+	void *edid;
+	int n;
+
+	status = omap_connector_detect(connector, false);
+	if (status != connector_status_connected)
+		goto no_edid;
+
+	edid = kzalloc(MAX_EDID, GFP_KERNEL);
+	if (!edid)
+		goto no_edid;
+
+	if (dssdev->ops->read_edid(dssdev, edid, MAX_EDID) <= 0 ||
+	    !drm_edid_is_valid(edid)) {
+		kfree(edid);
+		goto no_edid;
+	}
+
+	drm_connector_update_edid_property(connector, edid);
+	n = drm_add_edid_modes(connector, edid);
+
+	omap_connector->hdmi_mode = drm_detect_hdmi_monitor(edid);
+
+	kfree(edid);
+	return n;
+
+no_edid:
+	drm_connector_update_edid_property(connector, NULL);
+	return 0;
+}
+
 static int omap_connector_get_modes(struct drm_connector *connector)
 {
 	struct omap_connector *omap_connector = to_omap_connector(connector);
-	struct omap_dss_device *dssdev = omap_connector->dssdev;
-	struct drm_device *dev = connector->dev;
-	int n = 0;
+	struct omap_dss_device *dssdev;
+	struct drm_display_mode *mode;
+	struct videomode vm = {0};
 
 	DBG("%s", omap_connector->dssdev->name);
 
-	/* if display exposes EDID, then we parse that in the normal way to
-	 * build table of supported modes.. otherwise (ie. fixed resolution
+	/*
+	 * If display exposes EDID, then we parse that in the normal way to
+	 * build table of supported modes. Otherwise (ie. fixed resolution
 	 * LCD panels) we just return a single mode corresponding to the
-	 * currently configured timings:
+	 * currently configured timings.
 	 */
-	if (dssdev->ops->read_edid) {
-		void *edid = kzalloc(MAX_EDID, GFP_KERNEL);
-
-		if (!edid)
-			return 0;
-
-		if ((dssdev->ops->read_edid(dssdev, edid, MAX_EDID) > 0) &&
-				drm_edid_is_valid(edid)) {
-			drm_connector_update_edid_property(
-					connector, edid);
-			n = drm_add_edid_modes(connector, edid);
-
-			omap_connector->hdmi_mode =
-				drm_detect_hdmi_monitor(edid);
-		} else {
-			drm_connector_update_edid_property(
-					connector, NULL);
-		}
-
-		kfree(edid);
-	} else {
-		struct drm_display_mode *mode = drm_mode_create(dev);
-		struct videomode vm = {0};
+	dssdev = omap_connector_find_device(connector,
+					    OMAP_DSS_DEVICE_OP_EDID);
+	if (dssdev)
+		return omap_connector_get_modes_edid(connector, dssdev);
 
-		if (!mode)
-			return 0;
+	mode = drm_mode_create(connector->dev);
+	if (!mode)
+		return 0;
 
-		dssdev->ops->get_timings(dssdev, &vm);
+	dssdev = omap_connector->dssdev;
+	dssdev->ops->get_timings(dssdev, &vm);
 
-		drm_display_mode_from_videomode(&vm, mode);
+	drm_display_mode_from_videomode(&vm, mode);
 
-		mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
-		drm_mode_set_name(mode);
-		drm_mode_probed_add(connector, mode);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
 
-		if (dssdev->driver && dssdev->driver->get_size) {
-			dssdev->driver->get_size(dssdev,
+	if (dssdev->driver && dssdev->driver->get_size)
+		dssdev->driver->get_size(dssdev,
 					 &connector->display_info.width_mm,
 					 &connector->display_info.height_mm);
-		}
 
-		n = 1;
-	}
-
-	return n;
+	return 1;
 }
 
 static int omap_connector_mode_valid(struct drm_connector *connector,
-- 
Regards,

Laurent Pinchart



More information about the dri-devel mailing list