[PATCH/RFC 00/60] omapdrm: Reverse direction of DSS device (dis)connect operations

[Laurent Pinchart]

Hi Tomi,

On Wednesday, 7 March 2018 16:11:04 EET Tomi Valkeinen wrote:
> On 07/03/18 02:24, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series is a first step towards moving the omapdrm driver away
> > from the custom bridge and panel drivers to drm_bridge and drm_panel.
> > 
> > The main blocker to transition to drm_bridge and drm_panel is the
> > direction of the bridge operations. While the omapdrm driver manages its
> > components from sink to source (panel to DSS output), the drm_bridge API
> > is manages bridges in the source to sink direction. This makes the two
> > models incompatible, and requires reversing the direction of operations
> > inside omapdrm.
> > 
> > Don't rejoice too fast, we're still far from a complete transition, but
> > this first step paves the way by reworking the driver's internals to make
> > source to sink order possible. It then transitions the connect and
> > disconnect operations (the omapdrm equivalent of the drm_bridge attach
> > and detach operations) to the new direction.
> > 
> > I've sent the patches as an RFC as I might not be aware of all the
> > consequences of the numerous changes to the driver's internals, even if I
> > took care to analyze the code flows to the best of my abilities.
> > 
> > The series contains patches previously posted by Jyri and Peter that I
> > have found helpful. Please see the individual patches for changes compared
> > to the original versions (trivial conflict resolutions caused by a rebase
> > are not mentioned).
> > 
> > The patches are based on top of a merge between Tomi's omapdrm-next branch
> > and the drm-misc/drm-misc-next branch for the "drm: fix drm_get_max_iomem
> > type mismatch" compilation fix. They can be found at
> > 
> > 	git://linuxtv.org/pinchartl/media.git omapdrm/bridge
> > 
> > The series has been tested on a Pandaboard with the HDMI and DVI outputs.
> > All patches have been at least compile-tested individually. I'll now go
> > through the process of making sure each of them gets tested on hardware
> > as well.
> 
> Thanks, nice work!
> 
> I did a read-the-descs-review, and looks good to me. My only comments
> are what I already mentioned in the chat: AM5 EVM doesn't work LCD
> (panel-dpi broken, perhaps?) (AM5 EVM was the only board I tested),

I've now tested the AM5 EVM and the LCD panel worked out of the box. I tried 
to load all modules before panel_dpi and the panel then stopped working. After 
a bit of investigation it turns out that the dpi_init_port() and 
sdi_init_port() functions can return probe deferral, but their return value is 
ignored by the caller. The problem pre-exists this series, the patch below 
fixes it. Surprisingly enough given the extent of the rework, it doesn't 
conflict with this series, I'll thus place it towards the start of v2.

>From 82798aa248b30199ab8c6dc3417e6478f1fca8c4 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Date: Wed, 7 Mar 2018 20:34:42 +0200
Subject: [PATCH] drm/omap: dss: Handle DPI and SDI port initialization
 failures

The dpi_init_port() and sdi_init_port() functions can return errors but
their return value is ignored. This prevents both probe failures and
probe deferral from working correctly. Propagate the errors up the call
stack.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/dss/dss.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/
dss.c
index 5f7789cf43c7..841256c34d65 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss.c
@@ -1185,7 +1185,8 @@ static int dss_init_ports(struct dss_device *dss)
 	struct platform_device *pdev = dss->pdev;
 	struct device_node *parent = pdev->dev.of_node;
 	struct device_node *port;
-	int i;
+	unsigned int i;
+	int r;
 
 	for (i = 0; i < dss->feat->num_ports; i++) {
 		port = of_graph_get_port_by_id(parent, i);
@@ -1194,11 +1195,17 @@ static int dss_init_ports(struct dss_device *dss)
 
 		switch (dss->feat->ports[i]) {
 		case OMAP_DISPLAY_TYPE_DPI:
-			dpi_init_port(dss, pdev, port, dss->feat->model);
+			r = dpi_init_port(dss, pdev, port, dss->feat->model);
+			if (r)
+				return r;
 			break;
+
 		case OMAP_DISPLAY_TYPE_SDI:
-			sdi_init_port(dss, pdev, port);
+			r = sdi_init_port(dss, pdev, port);
+			if (r)
+				return r;
 			break;
+
 		default:
 			break;
 		}

> and patch 7 it not correct, as the infoframe is set from omap_encoder.

You're right. I'll drop that patch.

-- 
Regards,

Laurent Pinchart