[PATCH 23/24] drm/msm: create the bridge connectors from common code

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

Each display sub-block (DSI, DP, HDMI) and the MDP4 LVDS path open-coded
the same drm_bridge_connector_init() call in its own modeset path,
scattering connector creation across the backends and duplicating the
boilerplate.

Now that every sub-block attaches its bridge chain to the encoder created
by the backend, and no sub-block keeps its own reference to the resulting
connector, the connectors can be built entirely from common code. Have
each ->modeset_init() only set up the bridge chain, and add a new
msm_kms_init_connectors() that walks every encoder once the bridges are
in place and creates a bridge connector for each. Encoders without a
bridge chain - such as the virtual writeback encoder, which brings its
own drm_writeback_connector - are skipped.

The only sub-block specific step left is attaching the DP subconnector
property, which is keyed off the connector type and so is applied
generically in the loop; no per-sub-block callback is needed.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c |  8 ------
 drivers/gpu/drm/msm/dp/dp_display.c      |  9 ------
 drivers/gpu/drm/msm/dp/dp_drm.c          | 17 -----------
 drivers/gpu/drm/msm/dp/dp_drm.h          |  2 --
 drivers/gpu/drm/msm/dsi/dsi.c            |  6 ++--
 drivers/gpu/drm/msm/dsi/dsi.h            |  4 +--
 drivers/gpu/drm/msm/dsi/dsi_manager.c    | 13 ++-------
 drivers/gpu/drm/msm/hdmi/hdmi.c          | 22 ++-------------
 drivers/gpu/drm/msm/msm_kms.c            | 48 ++++++++++++++++++++++++++++----
 9 files changed, 52 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index cd566a287f6f..93f1f0e81ac4 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -7,7 +7,6 @@
 #include <linux/delay.h>
 
 #include <drm/drm_bridge.h>
-#include <drm/drm_bridge_connector.h>
 #include <drm/drm_vblank.h>
 
 #include "msm_drv.h"
@@ -191,7 +190,6 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms,
 	struct drm_device *dev = mdp4_kms->dev;
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_encoder *encoder;
-	struct drm_connector *connector;
 	struct drm_bridge *next_bridge;
 	int dsi_id;
 	int ret;
@@ -226,12 +224,6 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms,
 			return ret;
 		}
 
-		connector = drm_bridge_connector_init(dev, encoder);
-		if (IS_ERR(connector)) {
-			DRM_DEV_ERROR(dev->dev, "failed to initialize LVDS connector\n");
-			return PTR_ERR(connector);
-		}
-
 		break;
 	case DRM_MODE_ENCODER_TMDS:
 		encoder = mdp4_dtv_encoder_init(dev);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 44503789d454..c03878183b9a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1380,7 +1380,6 @@ static int msm_dp_modeset_init(struct msm_display *display,
 			       struct drm_device *dev, struct drm_encoder *encoder)
 {
 	struct msm_dp *msm_dp_display = container_of(display, struct msm_dp, display);
-	struct drm_connector *connector;
 	int ret;
 
 	msm_dp_display->drm_dev = dev;
@@ -1392,14 +1391,6 @@ static int msm_dp_modeset_init(struct msm_display *display,
 		return ret;
 	}
 
-	connector = msm_dp_drm_connector_init(msm_dp_display, encoder);
-	if (IS_ERR(connector)) {
-		ret = PTR_ERR(connector);
-		DRM_DEV_ERROR(dev->dev,
-			"failed to create dp connector: %d\n", ret);
-		return ret;
-	}
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 00653b0364dc..01f8320f9c32 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -7,7 +7,6 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_bridge.h>
-#include <drm/drm_bridge_connector.h>
 #include <drm/drm_crtc.h>
 
 #include "msm_drv.h"
@@ -312,19 +311,3 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
 
 	return 0;
 }
-
-/* connector initialization */
-struct drm_connector *msm_dp_drm_connector_init(struct msm_dp *msm_dp_display,
-					    struct drm_encoder *encoder)
-{
-	struct drm_connector *connector = NULL;
-
-	connector = drm_bridge_connector_init(msm_dp_display->drm_dev, encoder);
-	if (IS_ERR(connector))
-		return connector;
-
-	if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
-		drm_connector_attach_dp_subconnector_property(connector);
-
-	return connector;
-}
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 4c6b14dd52ac..4d642de4a50b 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -19,8 +19,6 @@ struct msm_dp_bridge {
 
 #define to_dp_bridge(x)     container_of((x), struct msm_dp_bridge, bridge)
 
-struct drm_connector *msm_dp_drm_connector_init(struct msm_dp *msm_dp_display,
-					    struct drm_encoder *encoder);
 int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
 		   struct drm_encoder *encoder);
 
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 3bcea5f5b5a3..19a3ff0d27ba 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -283,15 +283,15 @@ static int msm_dsi_modeset_init(struct msm_display *display,
 	    !msm_dsi_is_master_dsi(msm_dsi)) {
 		/*
 		 * Do not return an eror here,
-		 * Just skip creating encoder/connector for the slave-DSI.
+		 * Just skip creating the bridge for the slave-DSI.
 		 */
 		return 0;
 	}
 
-	ret = msm_dsi_manager_connector_init(msm_dsi, encoder);
+	ret = msm_dsi_manager_bridge_init(msm_dsi, encoder);
 	if (ret) {
 		DRM_DEV_ERROR(dev->dev,
-			"failed to create dsi connector: %d\n", ret);
+			"failed to create dsi bridge: %d\n", ret);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index d594a7399f54..87a7be955c5f 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -52,8 +52,8 @@ struct msm_dsi {
 /* dsi manager */
 bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
 bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
-int msm_dsi_manager_connector_init(struct msm_dsi *msm_dsi,
-				   struct drm_encoder *encoder);
+int msm_dsi_manager_bridge_init(struct msm_dsi *msm_dsi,
+				struct drm_encoder *encoder);
 int msm_dsi_manager_cmd_xfer(int id, const struct mipi_dsi_msg *msg);
 bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len);
 int msm_dsi_manager_register(struct msm_dsi *msm_dsi);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 2a5326f1fa8b..9edace2b396d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2015, The Linux Foundation. All rights reserved.
  */
 
-#include "drm/drm_bridge_connector.h"
 
 #include "msm_kms.h"
 #include "dsi.h"
@@ -458,13 +457,11 @@ static const struct drm_bridge_funcs dsi_mgr_bridge_funcs = {
 };
 
 /* initialize bridge */
-int msm_dsi_manager_connector_init(struct msm_dsi *msm_dsi,
-				   struct drm_encoder *encoder)
+int msm_dsi_manager_bridge_init(struct msm_dsi *msm_dsi,
+				struct drm_encoder *encoder)
 {
-	struct drm_device *dev = msm_dsi->dev;
 	struct drm_bridge *bridge;
 	struct dsi_bridge *dsi_bridge;
-	struct drm_connector *connector;
 	int ret;
 
 	dsi_bridge = devm_drm_bridge_alloc(msm_dsi->dev->dev, struct dsi_bridge, base,
@@ -484,12 +481,6 @@ int msm_dsi_manager_connector_init(struct msm_dsi *msm_dsi,
 	if (ret)
 		return ret;
 
-	connector = drm_bridge_connector_init(dev, encoder);
-	if (IS_ERR(connector)) {
-		DRM_ERROR("Unable to create bridge connector\n");
-		return PTR_ERR(connector);
-	}
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 62961dddd8f5..33474e90af53 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -11,7 +11,6 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 
-#include <drm/drm_bridge_connector.h>
 #include <drm/drm_of.h>
 #include <drm/display/drm_hdmi_state_helper.h>
 
@@ -164,7 +163,6 @@ static int msm_hdmi_modeset_init(struct msm_display *display,
 		struct drm_device *dev, struct drm_encoder *encoder)
 {
 	struct hdmi *hdmi = container_of(display, struct hdmi, display);
-	struct drm_connector *connector = NULL;
 	int ret;
 
 	hdmi->dev = dev;
@@ -173,7 +171,7 @@ static int msm_hdmi_modeset_init(struct msm_display *display,
 	ret = msm_hdmi_bridge_init(hdmi);
 	if (ret) {
 		DRM_DEV_ERROR(dev->dev, "failed to create HDMI bridge: %d\n", ret);
-		goto fail;
+		return ret;
 	}
 
 	if (hdmi->next_bridge) {
@@ -181,34 +179,20 @@ static int msm_hdmi_modeset_init(struct msm_display *display,
 					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 		if (ret) {
 			DRM_DEV_ERROR(dev->dev, "failed to attach next HDMI bridge: %d\n", ret);
-			goto fail;
+			return ret;
 		}
 	}
 
-	connector = drm_bridge_connector_init(hdmi->dev, encoder);
-	if (IS_ERR(connector)) {
-		ret = PTR_ERR(connector);
-		DRM_DEV_ERROR(dev->dev, "failed to create HDMI connector: %d\n", ret);
-		connector = NULL;
-		goto fail;
-	}
-
 	ret = devm_request_irq(dev->dev, hdmi->irq,
 			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
 			"hdmi_isr", hdmi);
 	if (ret < 0) {
 		DRM_DEV_ERROR(dev->dev, "failed to request IRQ%u: %d\n",
 				hdmi->irq, ret);
-		goto fail;
+		return ret;
 	}
 
 	return 0;
-
-fail:
-	if (connector)
-		connector->funcs->destroy(connector);
-
-	return ret;
 }
 
 static void msm_hdmi_snapshot(struct msm_display *display,
diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index 1a081e15a0a8..71af3780b7b1 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -10,7 +10,10 @@
 #include <linux/sched/mm.h>
 #include <uapi/linux/sched/types.h>
 
+#include <drm/drm_bridge_connector.h>
+#include <drm/drm_connector.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_encoder.h>
 #include <drm/drm_mode_config.h>
 #include <drm/drm_vblank.h>
 #include <drm/clients/drm_client_setup.h>
@@ -271,15 +274,15 @@ static int msm_display_modeset_init(struct msm_display *display,
 }
 
 /*
- * Set up the bridges and connectors for the display sub-blocks, dispatching
- * through the common display interface using the encoders the backend created
- * in ->kms_init().
+ * Set up the bridge chains for the display sub-blocks, dispatching through the
+ * common display interface using the encoders the backend created in
+ * ->kms_init().
  *
- * The slave link of a bonded DSI pair shares its master's encoder and creates
- * no connector of its own; its ->modeset_init() handles that internally, so it
+ * The slave link of a bonded DSI pair shares its master's encoder and sets up
+ * no bridge of its own; its ->modeset_init() handles that internally, so it
  * needs no special-casing here.
  */
-static int msm_kms_init_connectors(struct drm_device *ddev)
+static int msm_kms_modeset_init(struct drm_device *ddev)
 {
 	struct msm_drm_private *priv = ddev->dev_private;
 	struct msm_kms *kms = priv->kms;
@@ -310,6 +313,35 @@ static int msm_kms_init_connectors(struct drm_device *ddev)
 	return ret;
 }
 
+/* Build a bridge connector for each encoder. */
+static int msm_kms_init_connectors(struct drm_device *ddev)
+{
+	struct drm_encoder *encoder;
+
+	drm_for_each_encoder(encoder, ddev) {
+		struct drm_connector *connector;
+		int ret;
+
+		/* the virtual writeback encoder brings its own connector */
+		if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
+			continue;
+
+		connector = drm_bridge_connector_init(ddev, encoder);
+		if (IS_ERR(connector)) {
+			DRM_DEV_ERROR(ddev->dev,
+				      "failed to init connector for %s: %ld\n",
+				      encoder->name, PTR_ERR(connector));
+			return PTR_ERR(connector);
+		}
+
+		/* The DP subconnector property applies to DisplayPort only. */
+		if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
+			drm_connector_attach_dp_subconnector_property(connector);
+	}
+
+	return 0;
+}
+
 int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
 {
 	struct msm_drm_private *priv = dev_get_drvdata(dev);
@@ -353,6 +385,10 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
 	 * ->hw_init(); now set up the bridges and connectors for them from
 	 * common code.
 	 */
+	ret = msm_kms_modeset_init(ddev);
+	if (ret)
+		goto err_msm_uninit;
+
 	ret = msm_kms_init_connectors(ddev);
 	if (ret)
 		goto err_msm_uninit;

-- 
2.47.3