[RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[Dmitry Baryshkov]

I noticed that commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
enable and disable") conflicts with the panel-edp (panel bridge)
support. Both bridges will try to attach directly to the drm encoder
itself. I started writing lengthy letter describing what is broken and
how it should be fixed. Then at some point I stopped and quickly coded
this RFC (which is compile-tested only).

Comments and tests (on both DP and eDP setups) are more than welcome.

The following changes since commit 6ed95285382d6f90a3c3a11d5806a5eb7db715c3:

  drm/msm/a5xx: Fix missing CP_PROTECT for SMMU on A540 (2021-12-17 15:09:46 -0800)

are available in the Git repository at:

  https://git.linaro.org/people/dmitry.baryshkov/kernel.git msm-dp-bridges

for you to fetch changes up to 7eff304d50ba520e9193a293a8e42bbd9d7aa0e1:

  drm/msm/dp: stop carying about the connector type (2022-01-07 04:56:06 +0300)

----------------------------------------------------------------
Dmitry Baryshkov (7):
      drm/msm/dp: fix panel bridge attachment
      drm/msm/dp: support attaching bridges to the DP encoder
      drm/msm/dp: replace dp_connector with drm_bridge_connector
      drm/msm/dp: remove extra wrappers and public functions
      drm/msm/dp: remove unused stubs
      drm/msm/dp: remove dp_display_en/disable prototypes and data argument
      drm/msm/dp: stop carying about the connector type

 drivers/gpu/drm/msm/Makefile        |   1 -
 drivers/gpu/drm/msm/dp/dp_display.c | 263 ++++++++++++++++++++++++++----------
 drivers/gpu/drm/msm/dp/dp_display.h |   5 +-
 drivers/gpu/drm/msm/dp/dp_drm.c     | 250 ----------------------------------
 drivers/gpu/drm/msm/dp/dp_parser.c  |  28 ++--
 drivers/gpu/drm/msm/dp/dp_parser.h  |   4 +-
 drivers/gpu/drm/msm/msm_drv.h       |  32 +----
 7 files changed, 203 insertions(+), 380 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c

[RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[Dmitry Baryshkov]

I noticed that commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
enable and disable") conflicts with the panel-edp (panel bridge)
support. Both bridges will try to attach directly to the drm encoder
itself. I started writing lengthy letter describing what is broken and
how it should be fixed. Then at some point I stopped and quickly coded
this RFC (which is compile-tested only).

Comments and tests (on both DP and eDP setups) are more than welcome.

The following changes since commit 6ed95285382d6f90a3c3a11d5806a5eb7db715c3:

  drm/msm/a5xx: Fix missing CP_PROTECT for SMMU on A540 (2021-12-17 15:09:46 -0800)

are available in the Git repository at:

  https://git.linaro.org/people/dmitry.baryshkov/kernel.git msm-dp-bridges

for you to fetch changes up to 7eff304d50ba520e9193a293a8e42bbd9d7aa0e1:

  drm/msm/dp: stop carying about the connector type (2022-01-07 04:56:06 +0300)

----------------------------------------------------------------
Dmitry Baryshkov (7):
      drm/msm/dp: fix panel bridge attachment
      drm/msm/dp: support attaching bridges to the DP encoder
      drm/msm/dp: replace dp_connector with drm_bridge_connector
      drm/msm/dp: remove extra wrappers and public functions
      drm/msm/dp: remove unused stubs
      drm/msm/dp: remove dp_display_en/disable prototypes and data argument
      drm/msm/dp: stop carying about the connector type

 drivers/gpu/drm/msm/Makefile        |   1 -
 drivers/gpu/drm/msm/dp/dp_display.c | 263 ++++++++++++++++++++++++++----------
 drivers/gpu/drm/msm/dp/dp_display.h |   5 +-
 drivers/gpu/drm/msm/dp/dp_drm.c     | 250 ----------------------------------
 drivers/gpu/drm/msm/dp/dp_parser.c  |  28 ++--
 drivers/gpu/drm/msm/dp/dp_parser.h  |   4 +-
 drivers/gpu/drm/msm/msm_drv.h       |  32 +----
 7 files changed, 203 insertions(+), 380 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c

[RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
enable and disable") the DP driver received a drm_bridge instance, which
is always attached to the encoder as a root bridge. However it conflicts
with the panel_bridge support for eDP panels. Change panel_bridge
attachment to come after dp_bridge attachment.

Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index d4d360d19eba..26ef41a4c1b6 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -169,16 +169,6 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
 
 	drm_connector_attach_encoder(connector, dp_display->encoder);
 
-	if (dp_display->panel_bridge) {
-		ret = drm_bridge_attach(dp_display->encoder,
-					dp_display->panel_bridge, NULL,
-					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-		if (ret < 0) {
-			DRM_ERROR("failed to attach panel bridge: %d\n", ret);
-			return ERR_PTR(ret);
-		}
-	}
-
 	return connector;
 }
 
@@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 		return ERR_PTR(rc);
 	}
 
+	if (dp_display->panel_bridge) {
+		rc = drm_bridge_attach(dp_display->encoder,
+					dp_display->panel_bridge, bridge,
+					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+		if (rc < 0) {
+			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
+			drm_bridge_remove(bridge);
+			return ERR_PTR(rc);
+		}
+	}
+
 	return bridge;
 }
-- 
2.34.1

[RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
enable and disable") the DP driver received a drm_bridge instance, which
is always attached to the encoder as a root bridge. However it conflicts
with the panel_bridge support for eDP panels. Change panel_bridge
attachment to come after dp_bridge attachment.

Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index d4d360d19eba..26ef41a4c1b6 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -169,16 +169,6 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
 
 	drm_connector_attach_encoder(connector, dp_display->encoder);
 
-	if (dp_display->panel_bridge) {
-		ret = drm_bridge_attach(dp_display->encoder,
-					dp_display->panel_bridge, NULL,
-					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-		if (ret < 0) {
-			DRM_ERROR("failed to attach panel bridge: %d\n", ret);
-			return ERR_PTR(ret);
-		}
-	}
-
 	return connector;
 }
 
@@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 		return ERR_PTR(rc);
 	}
 
+	if (dp_display->panel_bridge) {
+		rc = drm_bridge_attach(dp_display->encoder,
+					dp_display->panel_bridge, bridge,
+					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+		if (rc < 0) {
+			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
+			drm_bridge_remove(bridge);
+			return ERR_PTR(rc);
+		}
+	}
+
 	return bridge;
 }
-- 
2.34.1

[RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

Currently DP driver will allocate panel bridge for eDP panels.
Simplify this code to just check if there is any next bridge in the
chain (be it a panel bridge or regular bridge). Rename panel_bridge
field to next_bridge accordingly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
 drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
 drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
 drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
 5 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 7cc4d21f2091..184a1d1470e6 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -246,7 +246,7 @@ static int dp_display_bind(struct device *dev, struct device *master,
 		goto end;
 	}
 
-	dp->dp_display.panel_bridge = dp->parser->panel_bridge;
+	dp->dp_display.next_bridge = dp->parser->next_bridge;
 
 	dp->aux->drm_dev = drm;
 	rc = dp_aux_register(dp->aux);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index e3adcd578a90..7af2b186d2d9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -16,7 +16,7 @@ struct msm_dp {
 	struct drm_bridge *bridge;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_bridge *panel_bridge;
+	struct drm_bridge *next_bridge;
 	bool is_connected;
 	bool audio_enabled;
 	bool power_on;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 26ef41a4c1b6..80f59cf99089 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -236,9 +236,9 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 		return ERR_PTR(rc);
 	}
 
-	if (dp_display->panel_bridge) {
+	if (dp_display->next_bridge) {
 		rc = drm_bridge_attach(dp_display->encoder,
-					dp_display->panel_bridge, bridge,
+					dp_display->next_bridge, bridge,
 					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 		if (rc < 0) {
 			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index a7acc23f742b..5de21f3d0812 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -265,22 +265,14 @@ static int dp_parser_clock(struct dp_parser *parser)
 	return 0;
 }
 
-static int dp_parser_find_panel(struct dp_parser *parser)
+static int dp_parser_find_next_bridge(struct dp_parser *parser)
 {
 	struct device *dev = &parser->pdev->dev;
-	struct drm_panel *panel;
-	int rc;
 
-	rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
-	if (rc) {
-		DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
-		return rc;
-	}
-
-	parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
-	if (IS_ERR(parser->panel_bridge)) {
-		DRM_ERROR("failed to create panel bridge\n");
-		return PTR_ERR(parser->panel_bridge);
+	parser->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(parser->next_bridge)) {
+		DRM_ERROR("failed to create next bridge\n");
+		return PTR_ERR(parser->next_bridge);
 	}
 
 	return 0;
@@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
 	if (rc)
 		return rc;
 
-	if (connector_type == DRM_MODE_CONNECTOR_eDP) {
-		rc = dp_parser_find_panel(parser);
-		if (rc)
-			return rc;
-	}
+	rc = dp_parser_find_next_bridge(parser);
+	if (rc)
+		return rc;
 
 	/* Map the corresponding regulator information according to
 	 * version. Currently, since we only have one supported platform,
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3172da089421..4cec851e38d9 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -123,7 +123,7 @@ struct dp_parser {
 	struct dp_display_data disp_data;
 	const struct dp_regulator_cfg *regulator_cfg;
 	u32 max_dp_lanes;
-	struct drm_bridge *panel_bridge;
+	struct drm_bridge *next_bridge;
 
 	int (*parse)(struct dp_parser *parser, int connector_type);
 };
-- 
2.34.1

[RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

Currently DP driver will allocate panel bridge for eDP panels.
Simplify this code to just check if there is any next bridge in the
chain (be it a panel bridge or regular bridge). Rename panel_bridge
field to next_bridge accordingly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
 drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
 drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
 drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
 5 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 7cc4d21f2091..184a1d1470e6 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -246,7 +246,7 @@ static int dp_display_bind(struct device *dev, struct device *master,
 		goto end;
 	}
 
-	dp->dp_display.panel_bridge = dp->parser->panel_bridge;
+	dp->dp_display.next_bridge = dp->parser->next_bridge;
 
 	dp->aux->drm_dev = drm;
 	rc = dp_aux_register(dp->aux);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index e3adcd578a90..7af2b186d2d9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -16,7 +16,7 @@ struct msm_dp {
 	struct drm_bridge *bridge;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_bridge *panel_bridge;
+	struct drm_bridge *next_bridge;
 	bool is_connected;
 	bool audio_enabled;
 	bool power_on;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 26ef41a4c1b6..80f59cf99089 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -236,9 +236,9 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 		return ERR_PTR(rc);
 	}
 
-	if (dp_display->panel_bridge) {
+	if (dp_display->next_bridge) {
 		rc = drm_bridge_attach(dp_display->encoder,
-					dp_display->panel_bridge, bridge,
+					dp_display->next_bridge, bridge,
 					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 		if (rc < 0) {
 			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index a7acc23f742b..5de21f3d0812 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -265,22 +265,14 @@ static int dp_parser_clock(struct dp_parser *parser)
 	return 0;
 }
 
-static int dp_parser_find_panel(struct dp_parser *parser)
+static int dp_parser_find_next_bridge(struct dp_parser *parser)
 {
 	struct device *dev = &parser->pdev->dev;
-	struct drm_panel *panel;
-	int rc;
 
-	rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
-	if (rc) {
-		DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
-		return rc;
-	}
-
-	parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
-	if (IS_ERR(parser->panel_bridge)) {
-		DRM_ERROR("failed to create panel bridge\n");
-		return PTR_ERR(parser->panel_bridge);
+	parser->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(parser->next_bridge)) {
+		DRM_ERROR("failed to create next bridge\n");
+		return PTR_ERR(parser->next_bridge);
 	}
 
 	return 0;
@@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
 	if (rc)
 		return rc;
 
-	if (connector_type == DRM_MODE_CONNECTOR_eDP) {
-		rc = dp_parser_find_panel(parser);
-		if (rc)
-			return rc;
-	}
+	rc = dp_parser_find_next_bridge(parser);
+	if (rc)
+		return rc;
 
 	/* Map the corresponding regulator information according to
 	 * version. Currently, since we only have one supported platform,
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3172da089421..4cec851e38d9 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -123,7 +123,7 @@ struct dp_parser {
 	struct dp_display_data disp_data;
 	const struct dp_regulator_cfg *regulator_cfg;
 	u32 max_dp_lanes;
-	struct drm_bridge *panel_bridge;
+	struct drm_bridge *next_bridge;
 
 	int (*parse)(struct dp_parser *parser, int connector_type);
 };
-- 
2.34.1

[RFC PATCH 3/7] drm/msm/dp: replace dp_connector with drm_bridge_connector

[Dmitry Baryshkov]

There is little point in having both connector and root bridge
implementation in the same driver. Move connector's functionality to the
bridge to let next bridge in chain to override it.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c |  22 +++---
 drivers/gpu/drm/msm/dp/dp_drm.c     | 111 ++++++++++------------------
 2 files changed, 51 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 184a1d1470e6..539aac117184 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1476,17 +1476,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 
 	dp_display->encoder = encoder;
 
-	dp_display->connector = dp_drm_connector_init(dp_display);
-	if (IS_ERR(dp_display->connector)) {
-		ret = PTR_ERR(dp_display->connector);
-		DRM_DEV_ERROR(dev->dev,
-			"failed to create dp connector: %d\n", ret);
-		dp_display->connector = NULL;
-		return ret;
-	}
-
-	priv->connectors[priv->num_connectors++] = dp_display->connector;
-
 	dp_display->bridge = msm_dp_bridge_init(dp_display, dev, encoder);
 	if (IS_ERR(dp_display->bridge)) {
 		ret = PTR_ERR(dp_display->bridge);
@@ -1498,6 +1487,17 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 
 	priv->bridges[priv->num_bridges++] = dp_display->bridge;
 
+	dp_display->connector = dp_drm_connector_init(dp_display);
+	if (IS_ERR(dp_display->connector)) {
+		ret = PTR_ERR(dp_display->connector);
+		DRM_DEV_ERROR(dev->dev,
+			"failed to create dp connector: %d\n", ret);
+		dp_display->connector = NULL;
+		return ret;
+	}
+
+	priv->connectors[priv->num_connectors++] = dp_display->connector;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..5f093d90d227 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -6,6 +6,7 @@
 #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"
@@ -20,24 +21,16 @@ struct msm_dp_bridge {
 
 #define to_dp_display(x)     container_of((x), struct msm_dp_bridge, bridge)
 
-struct dp_connector {
-	struct drm_connector base;
-	struct msm_dp *dp_display;
-};
-#define to_dp_connector(x) container_of(x, struct dp_connector, base)
-
 /**
- * dp_connector_detect - callback to determine if connector is connected
- * @conn: Pointer to drm connector structure
- * @force: Force detect setting from drm framework
- * Returns: Connector 'is connected' status
+ * dp_bridge_detect - callback to determine if connector is connected
+ * @bridge: Pointer to drm bridge structure
+ * Returns: Bridge's 'is connected' status
  */
-static enum drm_connector_status dp_connector_detect(struct drm_connector *conn,
-		bool force)
+static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
 {
 	struct msm_dp *dp;
 
-	dp = to_dp_connector(conn)->dp_display;
+	dp = to_dp_display(bridge)->dp_display;
 
 	DRM_DEBUG_DP("is_connected = %s\n",
 		(dp->is_connected) ? "true" : "false");
@@ -48,10 +41,11 @@ static enum drm_connector_status dp_connector_detect(struct drm_connector *conn,
 
 /**
  * dp_connector_get_modes - callback to add drm modes via drm_mode_probed_add()
+ * @bridge: Poiner to drm bridge
  * @connector: Pointer to drm connector structure
  * Returns: Number of modes added
  */
-static int dp_connector_get_modes(struct drm_connector *connector)
+static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
 {
 	int rc = 0;
 	struct msm_dp *dp;
@@ -61,7 +55,7 @@ static int dp_connector_get_modes(struct drm_connector *connector)
 	if (!connector)
 		return 0;
 
-	dp = to_dp_connector(connector)->dp_display;
+	dp = to_dp_display(bridge)->dp_display;
 
 	dp_mode = kzalloc(sizeof(*dp_mode),  GFP_KERNEL);
 	if (!dp_mode)
@@ -102,18 +96,20 @@ static int dp_connector_get_modes(struct drm_connector *connector)
 }
 
 /**
- * dp_connector_mode_valid - callback to determine if specified mode is valid
- * @connector: Pointer to drm connector structure
+ * dp_bridge_mode_valid - callback to determine if specified mode is valid
+ * @bridge: Pointer to drm bridge structure
+ * @info: display info
  * @mode: Pointer to drm mode structure
  * Returns: Validity status for specified mode
  */
-static enum drm_mode_status dp_connector_mode_valid(
-		struct drm_connector *connector,
-		struct drm_display_mode *mode)
+static enum drm_mode_status dp_bridge_mode_valid(
+		struct drm_bridge *bridge,
+		const struct drm_display_info *info,
+		const struct drm_display_mode *mode)
 {
 	struct msm_dp *dp_disp;
 
-	dp_disp = to_dp_connector(connector)->dp_display;
+	dp_disp = to_dp_display(bridge)->dp_display;
 
 	if ((dp_disp->max_pclk_khz <= 0) ||
 			(dp_disp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
@@ -123,55 +119,6 @@ static enum drm_mode_status dp_connector_mode_valid(
 	return dp_display_validate_mode(dp_disp, mode->clock);
 }
 
-static const struct drm_connector_funcs dp_connector_funcs = {
-	.detect = dp_connector_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
-	.reset = drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static const struct drm_connector_helper_funcs dp_connector_helper_funcs = {
-	.get_modes = dp_connector_get_modes,
-	.mode_valid = dp_connector_mode_valid,
-};
-
-/* connector initialization */
-struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
-{
-	struct drm_connector *connector = NULL;
-	struct dp_connector *dp_connector;
-	int ret;
-
-	dp_connector = devm_kzalloc(dp_display->drm_dev->dev,
-					sizeof(*dp_connector),
-					GFP_KERNEL);
-	if (!dp_connector)
-		return ERR_PTR(-ENOMEM);
-
-	dp_connector->dp_display = dp_display;
-
-	connector = &dp_connector->base;
-
-	ret = drm_connector_init(dp_display->drm_dev, connector,
-			&dp_connector_funcs,
-			dp_display->connector_type);
-	if (ret)
-		return ERR_PTR(ret);
-
-	drm_connector_helper_add(connector, &dp_connector_helper_funcs);
-
-	/*
-	 * Enable HPD to let hpd event is handled when cable is connected.
-	 */
-	connector->polled = DRM_CONNECTOR_POLL_HPD;
-
-	drm_connector_attach_encoder(connector, dp_display->encoder);
-
-	return connector;
-}
-
 static void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
 				const struct drm_display_mode *mode,
 				const struct drm_display_mode *adjusted_mode)
@@ -211,6 +158,9 @@ static const struct drm_bridge_funcs dp_bridge_ops = {
 	.disable      = dp_bridge_disable,
 	.post_disable = dp_bridge_post_disable,
 	.mode_set     = dp_bridge_mode_set,
+	.mode_valid   = dp_bridge_mode_valid,
+	.get_modes    = dp_bridge_get_modes,
+	.detect       = dp_bridge_detect,
 };
 
 struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
@@ -228,7 +178,12 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 
 	bridge = &dp_bridge->bridge;
 	bridge->funcs = &dp_bridge_ops;
-	bridge->encoder = encoder;
+	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+
+	bridge->ops =
+		DRM_BRIDGE_OP_DETECT |
+		DRM_BRIDGE_OP_HPD |
+		DRM_BRIDGE_OP_MODES;
 
 	rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (rc) {
@@ -249,3 +204,17 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 
 	return bridge;
 }
+
+/* connector initialization */
+struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
+{
+	struct drm_connector *connector = NULL;
+
+	connector = drm_bridge_connector_init(dp_display->drm_dev,  dp_display->encoder);
+	if (IS_ERR(connector))
+		return connector;
+
+	drm_connector_attach_encoder(connector, dp_display->encoder);
+
+	return connector;
+}
-- 
2.34.1

[RFC PATCH 3/7] drm/msm/dp: replace dp_connector with drm_bridge_connector

[Dmitry Baryshkov]

There is little point in having both connector and root bridge
implementation in the same driver. Move connector's functionality to the
bridge to let next bridge in chain to override it.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c |  22 +++---
 drivers/gpu/drm/msm/dp/dp_drm.c     | 111 ++++++++++------------------
 2 files changed, 51 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 184a1d1470e6..539aac117184 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1476,17 +1476,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 
 	dp_display->encoder = encoder;
 
-	dp_display->connector = dp_drm_connector_init(dp_display);
-	if (IS_ERR(dp_display->connector)) {
-		ret = PTR_ERR(dp_display->connector);
-		DRM_DEV_ERROR(dev->dev,
-			"failed to create dp connector: %d\n", ret);
-		dp_display->connector = NULL;
-		return ret;
-	}
-
-	priv->connectors[priv->num_connectors++] = dp_display->connector;
-
 	dp_display->bridge = msm_dp_bridge_init(dp_display, dev, encoder);
 	if (IS_ERR(dp_display->bridge)) {
 		ret = PTR_ERR(dp_display->bridge);
@@ -1498,6 +1487,17 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 
 	priv->bridges[priv->num_bridges++] = dp_display->bridge;
 
+	dp_display->connector = dp_drm_connector_init(dp_display);
+	if (IS_ERR(dp_display->connector)) {
+		ret = PTR_ERR(dp_display->connector);
+		DRM_DEV_ERROR(dev->dev,
+			"failed to create dp connector: %d\n", ret);
+		dp_display->connector = NULL;
+		return ret;
+	}
+
+	priv->connectors[priv->num_connectors++] = dp_display->connector;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..5f093d90d227 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -6,6 +6,7 @@
 #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"
@@ -20,24 +21,16 @@ struct msm_dp_bridge {
 
 #define to_dp_display(x)     container_of((x), struct msm_dp_bridge, bridge)
 
-struct dp_connector {
-	struct drm_connector base;
-	struct msm_dp *dp_display;
-};
-#define to_dp_connector(x) container_of(x, struct dp_connector, base)
-
 /**
- * dp_connector_detect - callback to determine if connector is connected
- * @conn: Pointer to drm connector structure
- * @force: Force detect setting from drm framework
- * Returns: Connector 'is connected' status
+ * dp_bridge_detect - callback to determine if connector is connected
+ * @bridge: Pointer to drm bridge structure
+ * Returns: Bridge's 'is connected' status
  */
-static enum drm_connector_status dp_connector_detect(struct drm_connector *conn,
-		bool force)
+static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
 {
 	struct msm_dp *dp;
 
-	dp = to_dp_connector(conn)->dp_display;
+	dp = to_dp_display(bridge)->dp_display;
 
 	DRM_DEBUG_DP("is_connected = %s\n",
 		(dp->is_connected) ? "true" : "false");
@@ -48,10 +41,11 @@ static enum drm_connector_status dp_connector_detect(struct drm_connector *conn,
 
 /**
  * dp_connector_get_modes - callback to add drm modes via drm_mode_probed_add()
+ * @bridge: Poiner to drm bridge
  * @connector: Pointer to drm connector structure
  * Returns: Number of modes added
  */
-static int dp_connector_get_modes(struct drm_connector *connector)
+static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
 {
 	int rc = 0;
 	struct msm_dp *dp;
@@ -61,7 +55,7 @@ static int dp_connector_get_modes(struct drm_connector *connector)
 	if (!connector)
 		return 0;
 
-	dp = to_dp_connector(connector)->dp_display;
+	dp = to_dp_display(bridge)->dp_display;
 
 	dp_mode = kzalloc(sizeof(*dp_mode),  GFP_KERNEL);
 	if (!dp_mode)
@@ -102,18 +96,20 @@ static int dp_connector_get_modes(struct drm_connector *connector)
 }
 
 /**
- * dp_connector_mode_valid - callback to determine if specified mode is valid
- * @connector: Pointer to drm connector structure
+ * dp_bridge_mode_valid - callback to determine if specified mode is valid
+ * @bridge: Pointer to drm bridge structure
+ * @info: display info
  * @mode: Pointer to drm mode structure
  * Returns: Validity status for specified mode
  */
-static enum drm_mode_status dp_connector_mode_valid(
-		struct drm_connector *connector,
-		struct drm_display_mode *mode)
+static enum drm_mode_status dp_bridge_mode_valid(
+		struct drm_bridge *bridge,
+		const struct drm_display_info *info,
+		const struct drm_display_mode *mode)
 {
 	struct msm_dp *dp_disp;
 
-	dp_disp = to_dp_connector(connector)->dp_display;
+	dp_disp = to_dp_display(bridge)->dp_display;
 
 	if ((dp_disp->max_pclk_khz <= 0) ||
 			(dp_disp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
@@ -123,55 +119,6 @@ static enum drm_mode_status dp_connector_mode_valid(
 	return dp_display_validate_mode(dp_disp, mode->clock);
 }
 
-static const struct drm_connector_funcs dp_connector_funcs = {
-	.detect = dp_connector_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
-	.reset = drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static const struct drm_connector_helper_funcs dp_connector_helper_funcs = {
-	.get_modes = dp_connector_get_modes,
-	.mode_valid = dp_connector_mode_valid,
-};
-
-/* connector initialization */
-struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
-{
-	struct drm_connector *connector = NULL;
-	struct dp_connector *dp_connector;
-	int ret;
-
-	dp_connector = devm_kzalloc(dp_display->drm_dev->dev,
-					sizeof(*dp_connector),
-					GFP_KERNEL);
-	if (!dp_connector)
-		return ERR_PTR(-ENOMEM);
-
-	dp_connector->dp_display = dp_display;
-
-	connector = &dp_connector->base;
-
-	ret = drm_connector_init(dp_display->drm_dev, connector,
-			&dp_connector_funcs,
-			dp_display->connector_type);
-	if (ret)
-		return ERR_PTR(ret);
-
-	drm_connector_helper_add(connector, &dp_connector_helper_funcs);
-
-	/*
-	 * Enable HPD to let hpd event is handled when cable is connected.
-	 */
-	connector->polled = DRM_CONNECTOR_POLL_HPD;
-
-	drm_connector_attach_encoder(connector, dp_display->encoder);
-
-	return connector;
-}
-
 static void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
 				const struct drm_display_mode *mode,
 				const struct drm_display_mode *adjusted_mode)
@@ -211,6 +158,9 @@ static const struct drm_bridge_funcs dp_bridge_ops = {
 	.disable      = dp_bridge_disable,
 	.post_disable = dp_bridge_post_disable,
 	.mode_set     = dp_bridge_mode_set,
+	.mode_valid   = dp_bridge_mode_valid,
+	.get_modes    = dp_bridge_get_modes,
+	.detect       = dp_bridge_detect,
 };
 
 struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
@@ -228,7 +178,12 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 
 	bridge = &dp_bridge->bridge;
 	bridge->funcs = &dp_bridge_ops;
-	bridge->encoder = encoder;
+	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+
+	bridge->ops =
+		DRM_BRIDGE_OP_DETECT |
+		DRM_BRIDGE_OP_HPD |
+		DRM_BRIDGE_OP_MODES;
 
 	rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (rc) {
@@ -249,3 +204,17 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
 
 	return bridge;
 }
+
+/* connector initialization */
+struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
+{
+	struct drm_connector *connector = NULL;
+
+	connector = drm_bridge_connector_init(dp_display->drm_dev,  dp_display->encoder);
+	if (IS_ERR(connector))
+		return connector;
+
+	drm_connector_attach_encoder(connector, dp_display->encoder);
+
+	return connector;
+}
-- 
2.34.1

[RFC PATCH 4/7] drm/msm/dp: remove extra wrappers and public functions

[Dmitry Baryshkov]

dp_bridge's functions are thin wrappers around the msm_dp_display_*
family. Squash dp_bridge callbacks into respective msm_dp_display
functions, removing the latter functions from public space.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/Makefile        |   1 -
 drivers/gpu/drm/msm/dp/dp_display.c | 184 +++++++++++++++++++++--
 drivers/gpu/drm/msm/dp/dp_display.h |   3 -
 drivers/gpu/drm/msm/dp/dp_drm.c     | 220 ----------------------------
 drivers/gpu/drm/msm/msm_drv.h       |  32 +---
 5 files changed, 171 insertions(+), 269 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 03ab55c37beb..354ccf793f4a 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -104,7 +104,6 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
 	dp/dp_catalog.o \
 	dp/dp_ctrl.o \
 	dp/dp_display.o \
-	dp/dp_drm.o \
 	dp/dp_hpd.o \
 	dp/dp_link.o \
 	dp/dp_panel.o \
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 539aac117184..7b4f40cb9a58 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,7 +10,9 @@
 #include <linux/component.h>
 #include <linux/of_irq.h>
 #include <linux/delay.h>
-#include <drm/drm_panel.h>
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 
 #include "msm_drv.h"
 #include "msm_kms.h"
@@ -30,6 +32,13 @@
 
 #define HPD_STRING_SIZE 30
 
+struct msm_dp_bridge {
+	struct drm_bridge bridge;
+	struct msm_dp *dp_display;
+};
+
+#define to_dp_display(x)     container_of((x), struct msm_dp_bridge, bridge)
+
 enum {
 	ISR_DISCONNECTED,
 	ISR_CONNECT_PENDING,
@@ -924,18 +933,30 @@ int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 	return 0;
 }
 
-int dp_display_validate_mode(struct msm_dp *dp, u32 mode_pclk_khz)
+static enum drm_mode_status dp_bridge_mode_valid(
+		struct drm_bridge *bridge,
+		const struct drm_display_info *info,
+		const struct drm_display_mode *mode)
 {
 	const u32 num_components = 3, default_bpp = 24;
 	struct dp_display_private *dp_display;
 	struct dp_link_info *link_info;
 	u32 mode_rate_khz = 0, supported_rate_khz = 0, mode_bpp = 0;
+	struct msm_dp *dp;
+	int mode_pclk_khz = mode->clock;
+
+	dp = to_dp_display(bridge)->dp_display;
 
 	if (!dp || !mode_pclk_khz || !dp->connector) {
 		DRM_ERROR("invalid params\n");
 		return -EINVAL;
 	}
 
+	if ((dp->max_pclk_khz <= 0) ||
+			(dp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
+			(mode->clock > dp->max_pclk_khz))
+		return MODE_BAD;
+
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 	link_info = &dp_display->panel->link_info;
 
@@ -1456,6 +1477,23 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
 	}
 }
 
+/* connector initialization */
+struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
+{
+	struct drm_connector *connector = NULL;
+
+	connector = drm_bridge_connector_init(dp_display->drm_dev,  dp_display->encoder);
+	if (IS_ERR(connector))
+		return connector;
+
+	drm_connector_attach_encoder(connector, dp_display->encoder);
+
+	return connector;
+}
+
+static struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
+			struct drm_encoder *encoder);
+
 int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 			struct drm_encoder *encoder)
 {
@@ -1501,8 +1539,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 	return 0;
 }
 
-int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
+static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	int rc = 0;
 	struct dp_display_private *dp_display;
 	u32 state;
@@ -1510,7 +1550,7 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 	if (!dp_display->dp_mode.drm_mode.clock) {
 		DRM_ERROR("invalid params\n");
-		return -EINVAL;
+		return;
 	}
 
 	mutex_lock(&dp_display->event_mutex);
@@ -1522,14 +1562,14 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
 	if (rc) {
 		DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc);
 		mutex_unlock(&dp_display->event_mutex);
-		return rc;
+		return;
 	}
 
 	rc = dp_display_prepare(dp);
 	if (rc) {
 		DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
 		mutex_unlock(&dp_display->event_mutex);
-		return rc;
+		return;
 	}
 
 	state =  dp_display->hpd_state;
@@ -1554,23 +1594,23 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
 	dp_display->hpd_state = ST_CONNECTED;
 
 	mutex_unlock(&dp_display->event_mutex);
-
-	return rc;
 }
 
-int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder)
+static void dp_bridge_disable(struct drm_bridge *drm_bridge)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	struct dp_display_private *dp_display;
 
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 
 	dp_ctrl_push_idle(dp_display->ctrl);
-
-	return 0;
 }
 
-int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder)
+static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	int rc = 0;
 	u32 state;
 	struct dp_display_private *dp_display;
@@ -1597,13 +1637,14 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder)
 	}
 
 	mutex_unlock(&dp_display->event_mutex);
-	return rc;
 }
 
-void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
+static void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
 				const struct drm_display_mode *mode,
 				const struct drm_display_mode *adjusted_mode)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	struct dp_display_private *dp_display;
 
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
@@ -1626,3 +1667,118 @@ void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
 	dp_display->dp_mode.h_active_low =
 		!!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
 }
+
+static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
+{
+	struct msm_dp *dp;
+
+	dp = to_dp_display(bridge)->dp_display;
+
+	DRM_DEBUG_DP("is_connected = %s\n",
+		(dp->is_connected) ? "true" : "false");
+
+	return (dp->is_connected) ? connector_status_connected :
+					connector_status_disconnected;
+}
+
+static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
+{
+	int rc = 0;
+	struct msm_dp *dp;
+	struct dp_display_mode *dp_mode = NULL;
+	struct drm_display_mode *m, drm_mode;
+
+	if (!connector)
+		return 0;
+
+	dp = to_dp_display(bridge)->dp_display;
+
+	dp_mode = kzalloc(sizeof(*dp_mode),  GFP_KERNEL);
+	if (!dp_mode)
+		return 0;
+
+	/* pluggable case assumes EDID is read when HPD */
+	if (dp->is_connected) {
+		/*
+		 *The get_modes() function might return one mode that is stored
+		 * in dp_mode when compliance test is in progress. If not, the
+		 * return value is equal to the total number of modes supported
+		 * by the sink
+		 */
+		rc = dp_display_get_modes(dp, dp_mode);
+		if (rc <= 0) {
+			DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
+			kfree(dp_mode);
+			return rc;
+		}
+		if (dp_mode->drm_mode.clock) { /* valid DP mode */
+			memset(&drm_mode, 0x0, sizeof(drm_mode));
+			drm_mode_copy(&drm_mode, &dp_mode->drm_mode);
+			m = drm_mode_duplicate(connector->dev, &drm_mode);
+			if (!m) {
+				DRM_ERROR("failed to add mode %ux%u\n",
+				       drm_mode.hdisplay,
+				       drm_mode.vdisplay);
+				kfree(dp_mode);
+				return 0;
+			}
+			drm_mode_probed_add(connector, m);
+		}
+	} else {
+		DRM_DEBUG_DP("No sink connected\n");
+	}
+	kfree(dp_mode);
+	return rc;
+}
+
+static const struct drm_bridge_funcs dp_bridge_ops = {
+	.enable       = dp_bridge_enable,
+	.disable      = dp_bridge_disable,
+	.post_disable = dp_bridge_post_disable,
+	.mode_set     = dp_bridge_mode_set,
+	.mode_valid   = dp_bridge_mode_valid,
+	.get_modes    = dp_bridge_get_modes,
+	.detect       = dp_bridge_detect,
+};
+
+static struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
+			struct drm_encoder *encoder)
+{
+	int rc;
+	struct msm_dp_bridge *dp_bridge;
+	struct drm_bridge *bridge;
+
+	dp_bridge = devm_kzalloc(dev->dev, sizeof(*dp_bridge), GFP_KERNEL);
+	if (!dp_bridge)
+		return ERR_PTR(-ENOMEM);
+
+	dp_bridge->dp_display = dp_display;
+
+	bridge = &dp_bridge->bridge;
+	bridge->funcs = &dp_bridge_ops;
+	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+
+	bridge->ops =
+		DRM_BRIDGE_OP_DETECT |
+		DRM_BRIDGE_OP_HPD |
+		DRM_BRIDGE_OP_MODES;
+
+	rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	if (rc) {
+		DRM_ERROR("failed to attach bridge, rc=%d\n", rc);
+		return ERR_PTR(rc);
+	}
+
+	if (dp_display->next_bridge) {
+		rc = drm_bridge_attach(dp_display->encoder,
+					dp_display->next_bridge, bridge,
+					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+		if (rc < 0) {
+			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
+			drm_bridge_remove(bridge);
+			return ERR_PTR(rc);
+		}
+	}
+
+	return bridge;
+}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 7af2b186d2d9..cabf439af9ee 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -32,9 +32,6 @@ struct msm_dp {
 
 int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 		hdmi_codec_plugged_cb fn, struct device *codec_dev);
-int dp_display_validate_mode(struct msm_dp *dp_display, u32 mode_pclk_khz);
-int dp_display_get_modes(struct msm_dp *dp_display,
-		struct dp_display_mode *dp_mode);
 int dp_display_request_irq(struct msm_dp *dp_display);
 bool dp_display_check_video_test(struct msm_dp *dp_display);
 int dp_display_get_test_bpp(struct msm_dp *dp_display);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
deleted file mode 100644
index 5f093d90d227..000000000000
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ /dev/null
@@ -1,220 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved.
- */
-
-#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"
-#include "msm_kms.h"
-#include "dp_drm.h"
-
-
-struct msm_dp_bridge {
-	struct drm_bridge bridge;
-	struct msm_dp *dp_display;
-};
-
-#define to_dp_display(x)     container_of((x), struct msm_dp_bridge, bridge)
-
-/**
- * dp_bridge_detect - callback to determine if connector is connected
- * @bridge: Pointer to drm bridge structure
- * Returns: Bridge's 'is connected' status
- */
-static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
-{
-	struct msm_dp *dp;
-
-	dp = to_dp_display(bridge)->dp_display;
-
-	DRM_DEBUG_DP("is_connected = %s\n",
-		(dp->is_connected) ? "true" : "false");
-
-	return (dp->is_connected) ? connector_status_connected :
-					connector_status_disconnected;
-}
-
-/**
- * dp_connector_get_modes - callback to add drm modes via drm_mode_probed_add()
- * @bridge: Poiner to drm bridge
- * @connector: Pointer to drm connector structure
- * Returns: Number of modes added
- */
-static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
-{
-	int rc = 0;
-	struct msm_dp *dp;
-	struct dp_display_mode *dp_mode = NULL;
-	struct drm_display_mode *m, drm_mode;
-
-	if (!connector)
-		return 0;
-
-	dp = to_dp_display(bridge)->dp_display;
-
-	dp_mode = kzalloc(sizeof(*dp_mode),  GFP_KERNEL);
-	if (!dp_mode)
-		return 0;
-
-	/* pluggable case assumes EDID is read when HPD */
-	if (dp->is_connected) {
-		/*
-		 *The get_modes() function might return one mode that is stored
-		 * in dp_mode when compliance test is in progress. If not, the
-		 * return value is equal to the total number of modes supported
-		 * by the sink
-		 */
-		rc = dp_display_get_modes(dp, dp_mode);
-		if (rc <= 0) {
-			DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
-			kfree(dp_mode);
-			return rc;
-		}
-		if (dp_mode->drm_mode.clock) { /* valid DP mode */
-			memset(&drm_mode, 0x0, sizeof(drm_mode));
-			drm_mode_copy(&drm_mode, &dp_mode->drm_mode);
-			m = drm_mode_duplicate(connector->dev, &drm_mode);
-			if (!m) {
-				DRM_ERROR("failed to add mode %ux%u\n",
-				       drm_mode.hdisplay,
-				       drm_mode.vdisplay);
-				kfree(dp_mode);
-				return 0;
-			}
-			drm_mode_probed_add(connector, m);
-		}
-	} else {
-		DRM_DEBUG_DP("No sink connected\n");
-	}
-	kfree(dp_mode);
-	return rc;
-}
-
-/**
- * dp_bridge_mode_valid - callback to determine if specified mode is valid
- * @bridge: Pointer to drm bridge structure
- * @info: display info
- * @mode: Pointer to drm mode structure
- * Returns: Validity status for specified mode
- */
-static enum drm_mode_status dp_bridge_mode_valid(
-		struct drm_bridge *bridge,
-		const struct drm_display_info *info,
-		const struct drm_display_mode *mode)
-{
-	struct msm_dp *dp_disp;
-
-	dp_disp = to_dp_display(bridge)->dp_display;
-
-	if ((dp_disp->max_pclk_khz <= 0) ||
-			(dp_disp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
-			(mode->clock > dp_disp->max_pclk_khz))
-		return MODE_BAD;
-
-	return dp_display_validate_mode(dp_disp, mode->clock);
-}
-
-static void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
-				const struct drm_display_mode *mode,
-				const struct drm_display_mode *adjusted_mode)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_mode_set(dp_display, drm_bridge->encoder, mode, adjusted_mode);
-}
-
-static void dp_bridge_enable(struct drm_bridge *drm_bridge)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_enable(dp_display, drm_bridge->encoder);
-}
-
-static void dp_bridge_disable(struct drm_bridge *drm_bridge)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_pre_disable(dp_display, drm_bridge->encoder);
-}
-
-static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_disable(dp_display, drm_bridge->encoder);
-}
-
-static const struct drm_bridge_funcs dp_bridge_ops = {
-	.enable       = dp_bridge_enable,
-	.disable      = dp_bridge_disable,
-	.post_disable = dp_bridge_post_disable,
-	.mode_set     = dp_bridge_mode_set,
-	.mode_valid   = dp_bridge_mode_valid,
-	.get_modes    = dp_bridge_get_modes,
-	.detect       = dp_bridge_detect,
-};
-
-struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
-			struct drm_encoder *encoder)
-{
-	int rc;
-	struct msm_dp_bridge *dp_bridge;
-	struct drm_bridge *bridge;
-
-	dp_bridge = devm_kzalloc(dev->dev, sizeof(*dp_bridge), GFP_KERNEL);
-	if (!dp_bridge)
-		return ERR_PTR(-ENOMEM);
-
-	dp_bridge->dp_display = dp_display;
-
-	bridge = &dp_bridge->bridge;
-	bridge->funcs = &dp_bridge_ops;
-	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
-
-	bridge->ops =
-		DRM_BRIDGE_OP_DETECT |
-		DRM_BRIDGE_OP_HPD |
-		DRM_BRIDGE_OP_MODES;
-
-	rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-	if (rc) {
-		DRM_ERROR("failed to attach bridge, rc=%d\n", rc);
-		return ERR_PTR(rc);
-	}
-
-	if (dp_display->next_bridge) {
-		rc = drm_bridge_attach(dp_display->encoder,
-					dp_display->next_bridge, bridge,
-					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-		if (rc < 0) {
-			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
-			drm_bridge_remove(bridge);
-			return ERR_PTR(rc);
-		}
-	}
-
-	return bridge;
-}
-
-/* connector initialization */
-struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
-{
-	struct drm_connector *connector = NULL;
-
-	connector = drm_bridge_connector_init(dp_display->drm_dev,  dp_display->encoder);
-	if (IS_ERR(connector))
-		return connector;
-
-	drm_connector_attach_encoder(connector, dp_display->encoder);
-
-	return connector;
-}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index d7574e6bd4e4..a48e0737692c 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -385,16 +385,7 @@ int __init msm_dp_register(void);
 void __exit msm_dp_unregister(void);
 int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 			 struct drm_encoder *encoder);
-int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder);
-int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder);
-int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder);
-void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
-				const struct drm_display_mode *mode,
-				const struct drm_display_mode *adjusted_mode);
-
-struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display,
-					struct drm_device *dev,
-					struct drm_encoder *encoder);
+
 void msm_dp_irq_postinstall(struct msm_dp *dp_display);
 void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp_display);
 
@@ -414,27 +405,6 @@ static inline int msm_dp_modeset_init(struct msm_dp *dp_display,
 {
 	return -EINVAL;
 }
-static inline int msm_dp_display_enable(struct msm_dp *dp,
-					struct drm_encoder *encoder)
-{
-	return -EINVAL;
-}
-static inline int msm_dp_display_disable(struct msm_dp *dp,
-					struct drm_encoder *encoder)
-{
-	return -EINVAL;
-}
-static inline int msm_dp_display_pre_disable(struct msm_dp *dp,
-					struct drm_encoder *encoder)
-{
-	return -EINVAL;
-}
-static inline void msm_dp_display_mode_set(struct msm_dp *dp,
-				struct drm_encoder *encoder,
-				const struct drm_display_mode *mode,
-				const struct drm_display_mode *adjusted_mode)
-{
-}
 
 static inline void msm_dp_irq_postinstall(struct msm_dp *dp_display)
 {
-- 
2.34.1

[RFC PATCH 4/7] drm/msm/dp: remove extra wrappers and public functions

[Dmitry Baryshkov]

dp_bridge's functions are thin wrappers around the msm_dp_display_*
family. Squash dp_bridge callbacks into respective msm_dp_display
functions, removing the latter functions from public space.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/Makefile        |   1 -
 drivers/gpu/drm/msm/dp/dp_display.c | 184 +++++++++++++++++++++--
 drivers/gpu/drm/msm/dp/dp_display.h |   3 -
 drivers/gpu/drm/msm/dp/dp_drm.c     | 220 ----------------------------
 drivers/gpu/drm/msm/msm_drv.h       |  32 +---
 5 files changed, 171 insertions(+), 269 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 03ab55c37beb..354ccf793f4a 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -104,7 +104,6 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
 	dp/dp_catalog.o \
 	dp/dp_ctrl.o \
 	dp/dp_display.o \
-	dp/dp_drm.o \
 	dp/dp_hpd.o \
 	dp/dp_link.o \
 	dp/dp_panel.o \
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 539aac117184..7b4f40cb9a58 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,7 +10,9 @@
 #include <linux/component.h>
 #include <linux/of_irq.h>
 #include <linux/delay.h>
-#include <drm/drm_panel.h>
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 
 #include "msm_drv.h"
 #include "msm_kms.h"
@@ -30,6 +32,13 @@
 
 #define HPD_STRING_SIZE 30
 
+struct msm_dp_bridge {
+	struct drm_bridge bridge;
+	struct msm_dp *dp_display;
+};
+
+#define to_dp_display(x)     container_of((x), struct msm_dp_bridge, bridge)
+
 enum {
 	ISR_DISCONNECTED,
 	ISR_CONNECT_PENDING,
@@ -924,18 +933,30 @@ int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 	return 0;
 }
 
-int dp_display_validate_mode(struct msm_dp *dp, u32 mode_pclk_khz)
+static enum drm_mode_status dp_bridge_mode_valid(
+		struct drm_bridge *bridge,
+		const struct drm_display_info *info,
+		const struct drm_display_mode *mode)
 {
 	const u32 num_components = 3, default_bpp = 24;
 	struct dp_display_private *dp_display;
 	struct dp_link_info *link_info;
 	u32 mode_rate_khz = 0, supported_rate_khz = 0, mode_bpp = 0;
+	struct msm_dp *dp;
+	int mode_pclk_khz = mode->clock;
+
+	dp = to_dp_display(bridge)->dp_display;
 
 	if (!dp || !mode_pclk_khz || !dp->connector) {
 		DRM_ERROR("invalid params\n");
 		return -EINVAL;
 	}
 
+	if ((dp->max_pclk_khz <= 0) ||
+			(dp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
+			(mode->clock > dp->max_pclk_khz))
+		return MODE_BAD;
+
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 	link_info = &dp_display->panel->link_info;
 
@@ -1456,6 +1477,23 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
 	}
 }
 
+/* connector initialization */
+struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
+{
+	struct drm_connector *connector = NULL;
+
+	connector = drm_bridge_connector_init(dp_display->drm_dev,  dp_display->encoder);
+	if (IS_ERR(connector))
+		return connector;
+
+	drm_connector_attach_encoder(connector, dp_display->encoder);
+
+	return connector;
+}
+
+static struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
+			struct drm_encoder *encoder);
+
 int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 			struct drm_encoder *encoder)
 {
@@ -1501,8 +1539,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 	return 0;
 }
 
-int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
+static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	int rc = 0;
 	struct dp_display_private *dp_display;
 	u32 state;
@@ -1510,7 +1550,7 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 	if (!dp_display->dp_mode.drm_mode.clock) {
 		DRM_ERROR("invalid params\n");
-		return -EINVAL;
+		return;
 	}
 
 	mutex_lock(&dp_display->event_mutex);
@@ -1522,14 +1562,14 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
 	if (rc) {
 		DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc);
 		mutex_unlock(&dp_display->event_mutex);
-		return rc;
+		return;
 	}
 
 	rc = dp_display_prepare(dp);
 	if (rc) {
 		DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
 		mutex_unlock(&dp_display->event_mutex);
-		return rc;
+		return;
 	}
 
 	state =  dp_display->hpd_state;
@@ -1554,23 +1594,23 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
 	dp_display->hpd_state = ST_CONNECTED;
 
 	mutex_unlock(&dp_display->event_mutex);
-
-	return rc;
 }
 
-int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder)
+static void dp_bridge_disable(struct drm_bridge *drm_bridge)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	struct dp_display_private *dp_display;
 
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
 
 	dp_ctrl_push_idle(dp_display->ctrl);
-
-	return 0;
 }
 
-int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder)
+static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	int rc = 0;
 	u32 state;
 	struct dp_display_private *dp_display;
@@ -1597,13 +1637,14 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder)
 	}
 
 	mutex_unlock(&dp_display->event_mutex);
-	return rc;
 }
 
-void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
+static void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
 				const struct drm_display_mode *mode,
 				const struct drm_display_mode *adjusted_mode)
 {
+	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
+	struct msm_dp *dp = dp_bridge->dp_display;
 	struct dp_display_private *dp_display;
 
 	dp_display = container_of(dp, struct dp_display_private, dp_display);
@@ -1626,3 +1667,118 @@ void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
 	dp_display->dp_mode.h_active_low =
 		!!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
 }
+
+static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
+{
+	struct msm_dp *dp;
+
+	dp = to_dp_display(bridge)->dp_display;
+
+	DRM_DEBUG_DP("is_connected = %s\n",
+		(dp->is_connected) ? "true" : "false");
+
+	return (dp->is_connected) ? connector_status_connected :
+					connector_status_disconnected;
+}
+
+static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
+{
+	int rc = 0;
+	struct msm_dp *dp;
+	struct dp_display_mode *dp_mode = NULL;
+	struct drm_display_mode *m, drm_mode;
+
+	if (!connector)
+		return 0;
+
+	dp = to_dp_display(bridge)->dp_display;
+
+	dp_mode = kzalloc(sizeof(*dp_mode),  GFP_KERNEL);
+	if (!dp_mode)
+		return 0;
+
+	/* pluggable case assumes EDID is read when HPD */
+	if (dp->is_connected) {
+		/*
+		 *The get_modes() function might return one mode that is stored
+		 * in dp_mode when compliance test is in progress. If not, the
+		 * return value is equal to the total number of modes supported
+		 * by the sink
+		 */
+		rc = dp_display_get_modes(dp, dp_mode);
+		if (rc <= 0) {
+			DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
+			kfree(dp_mode);
+			return rc;
+		}
+		if (dp_mode->drm_mode.clock) { /* valid DP mode */
+			memset(&drm_mode, 0x0, sizeof(drm_mode));
+			drm_mode_copy(&drm_mode, &dp_mode->drm_mode);
+			m = drm_mode_duplicate(connector->dev, &drm_mode);
+			if (!m) {
+				DRM_ERROR("failed to add mode %ux%u\n",
+				       drm_mode.hdisplay,
+				       drm_mode.vdisplay);
+				kfree(dp_mode);
+				return 0;
+			}
+			drm_mode_probed_add(connector, m);
+		}
+	} else {
+		DRM_DEBUG_DP("No sink connected\n");
+	}
+	kfree(dp_mode);
+	return rc;
+}
+
+static const struct drm_bridge_funcs dp_bridge_ops = {
+	.enable       = dp_bridge_enable,
+	.disable      = dp_bridge_disable,
+	.post_disable = dp_bridge_post_disable,
+	.mode_set     = dp_bridge_mode_set,
+	.mode_valid   = dp_bridge_mode_valid,
+	.get_modes    = dp_bridge_get_modes,
+	.detect       = dp_bridge_detect,
+};
+
+static struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
+			struct drm_encoder *encoder)
+{
+	int rc;
+	struct msm_dp_bridge *dp_bridge;
+	struct drm_bridge *bridge;
+
+	dp_bridge = devm_kzalloc(dev->dev, sizeof(*dp_bridge), GFP_KERNEL);
+	if (!dp_bridge)
+		return ERR_PTR(-ENOMEM);
+
+	dp_bridge->dp_display = dp_display;
+
+	bridge = &dp_bridge->bridge;
+	bridge->funcs = &dp_bridge_ops;
+	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+
+	bridge->ops =
+		DRM_BRIDGE_OP_DETECT |
+		DRM_BRIDGE_OP_HPD |
+		DRM_BRIDGE_OP_MODES;
+
+	rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	if (rc) {
+		DRM_ERROR("failed to attach bridge, rc=%d\n", rc);
+		return ERR_PTR(rc);
+	}
+
+	if (dp_display->next_bridge) {
+		rc = drm_bridge_attach(dp_display->encoder,
+					dp_display->next_bridge, bridge,
+					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+		if (rc < 0) {
+			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
+			drm_bridge_remove(bridge);
+			return ERR_PTR(rc);
+		}
+	}
+
+	return bridge;
+}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 7af2b186d2d9..cabf439af9ee 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -32,9 +32,6 @@ struct msm_dp {
 
 int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 		hdmi_codec_plugged_cb fn, struct device *codec_dev);
-int dp_display_validate_mode(struct msm_dp *dp_display, u32 mode_pclk_khz);
-int dp_display_get_modes(struct msm_dp *dp_display,
-		struct dp_display_mode *dp_mode);
 int dp_display_request_irq(struct msm_dp *dp_display);
 bool dp_display_check_video_test(struct msm_dp *dp_display);
 int dp_display_get_test_bpp(struct msm_dp *dp_display);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
deleted file mode 100644
index 5f093d90d227..000000000000
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ /dev/null
@@ -1,220 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved.
- */
-
-#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"
-#include "msm_kms.h"
-#include "dp_drm.h"
-
-
-struct msm_dp_bridge {
-	struct drm_bridge bridge;
-	struct msm_dp *dp_display;
-};
-
-#define to_dp_display(x)     container_of((x), struct msm_dp_bridge, bridge)
-
-/**
- * dp_bridge_detect - callback to determine if connector is connected
- * @bridge: Pointer to drm bridge structure
- * Returns: Bridge's 'is connected' status
- */
-static enum drm_connector_status dp_bridge_detect(struct drm_bridge *bridge)
-{
-	struct msm_dp *dp;
-
-	dp = to_dp_display(bridge)->dp_display;
-
-	DRM_DEBUG_DP("is_connected = %s\n",
-		(dp->is_connected) ? "true" : "false");
-
-	return (dp->is_connected) ? connector_status_connected :
-					connector_status_disconnected;
-}
-
-/**
- * dp_connector_get_modes - callback to add drm modes via drm_mode_probed_add()
- * @bridge: Poiner to drm bridge
- * @connector: Pointer to drm connector structure
- * Returns: Number of modes added
- */
-static int dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
-{
-	int rc = 0;
-	struct msm_dp *dp;
-	struct dp_display_mode *dp_mode = NULL;
-	struct drm_display_mode *m, drm_mode;
-
-	if (!connector)
-		return 0;
-
-	dp = to_dp_display(bridge)->dp_display;
-
-	dp_mode = kzalloc(sizeof(*dp_mode),  GFP_KERNEL);
-	if (!dp_mode)
-		return 0;
-
-	/* pluggable case assumes EDID is read when HPD */
-	if (dp->is_connected) {
-		/*
-		 *The get_modes() function might return one mode that is stored
-		 * in dp_mode when compliance test is in progress. If not, the
-		 * return value is equal to the total number of modes supported
-		 * by the sink
-		 */
-		rc = dp_display_get_modes(dp, dp_mode);
-		if (rc <= 0) {
-			DRM_ERROR("failed to get DP sink modes, rc=%d\n", rc);
-			kfree(dp_mode);
-			return rc;
-		}
-		if (dp_mode->drm_mode.clock) { /* valid DP mode */
-			memset(&drm_mode, 0x0, sizeof(drm_mode));
-			drm_mode_copy(&drm_mode, &dp_mode->drm_mode);
-			m = drm_mode_duplicate(connector->dev, &drm_mode);
-			if (!m) {
-				DRM_ERROR("failed to add mode %ux%u\n",
-				       drm_mode.hdisplay,
-				       drm_mode.vdisplay);
-				kfree(dp_mode);
-				return 0;
-			}
-			drm_mode_probed_add(connector, m);
-		}
-	} else {
-		DRM_DEBUG_DP("No sink connected\n");
-	}
-	kfree(dp_mode);
-	return rc;
-}
-
-/**
- * dp_bridge_mode_valid - callback to determine if specified mode is valid
- * @bridge: Pointer to drm bridge structure
- * @info: display info
- * @mode: Pointer to drm mode structure
- * Returns: Validity status for specified mode
- */
-static enum drm_mode_status dp_bridge_mode_valid(
-		struct drm_bridge *bridge,
-		const struct drm_display_info *info,
-		const struct drm_display_mode *mode)
-{
-	struct msm_dp *dp_disp;
-
-	dp_disp = to_dp_display(bridge)->dp_display;
-
-	if ((dp_disp->max_pclk_khz <= 0) ||
-			(dp_disp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) ||
-			(mode->clock > dp_disp->max_pclk_khz))
-		return MODE_BAD;
-
-	return dp_display_validate_mode(dp_disp, mode->clock);
-}
-
-static void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
-				const struct drm_display_mode *mode,
-				const struct drm_display_mode *adjusted_mode)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_mode_set(dp_display, drm_bridge->encoder, mode, adjusted_mode);
-}
-
-static void dp_bridge_enable(struct drm_bridge *drm_bridge)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_enable(dp_display, drm_bridge->encoder);
-}
-
-static void dp_bridge_disable(struct drm_bridge *drm_bridge)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_pre_disable(dp_display, drm_bridge->encoder);
-}
-
-static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
-{
-	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
-	struct msm_dp *dp_display = dp_bridge->dp_display;
-
-	msm_dp_display_disable(dp_display, drm_bridge->encoder);
-}
-
-static const struct drm_bridge_funcs dp_bridge_ops = {
-	.enable       = dp_bridge_enable,
-	.disable      = dp_bridge_disable,
-	.post_disable = dp_bridge_post_disable,
-	.mode_set     = dp_bridge_mode_set,
-	.mode_valid   = dp_bridge_mode_valid,
-	.get_modes    = dp_bridge_get_modes,
-	.detect       = dp_bridge_detect,
-};
-
-struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
-			struct drm_encoder *encoder)
-{
-	int rc;
-	struct msm_dp_bridge *dp_bridge;
-	struct drm_bridge *bridge;
-
-	dp_bridge = devm_kzalloc(dev->dev, sizeof(*dp_bridge), GFP_KERNEL);
-	if (!dp_bridge)
-		return ERR_PTR(-ENOMEM);
-
-	dp_bridge->dp_display = dp_display;
-
-	bridge = &dp_bridge->bridge;
-	bridge->funcs = &dp_bridge_ops;
-	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
-
-	bridge->ops =
-		DRM_BRIDGE_OP_DETECT |
-		DRM_BRIDGE_OP_HPD |
-		DRM_BRIDGE_OP_MODES;
-
-	rc = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-	if (rc) {
-		DRM_ERROR("failed to attach bridge, rc=%d\n", rc);
-		return ERR_PTR(rc);
-	}
-
-	if (dp_display->next_bridge) {
-		rc = drm_bridge_attach(dp_display->encoder,
-					dp_display->next_bridge, bridge,
-					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-		if (rc < 0) {
-			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
-			drm_bridge_remove(bridge);
-			return ERR_PTR(rc);
-		}
-	}
-
-	return bridge;
-}
-
-/* connector initialization */
-struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
-{
-	struct drm_connector *connector = NULL;
-
-	connector = drm_bridge_connector_init(dp_display->drm_dev,  dp_display->encoder);
-	if (IS_ERR(connector))
-		return connector;
-
-	drm_connector_attach_encoder(connector, dp_display->encoder);
-
-	return connector;
-}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index d7574e6bd4e4..a48e0737692c 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -385,16 +385,7 @@ int __init msm_dp_register(void);
 void __exit msm_dp_unregister(void);
 int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 			 struct drm_encoder *encoder);
-int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder);
-int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder);
-int msm_dp_display_pre_disable(struct msm_dp *dp, struct drm_encoder *encoder);
-void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder,
-				const struct drm_display_mode *mode,
-				const struct drm_display_mode *adjusted_mode);
-
-struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display,
-					struct drm_device *dev,
-					struct drm_encoder *encoder);
+
 void msm_dp_irq_postinstall(struct msm_dp *dp_display);
 void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp_display);
 
@@ -414,27 +405,6 @@ static inline int msm_dp_modeset_init(struct msm_dp *dp_display,
 {
 	return -EINVAL;
 }
-static inline int msm_dp_display_enable(struct msm_dp *dp,
-					struct drm_encoder *encoder)
-{
-	return -EINVAL;
-}
-static inline int msm_dp_display_disable(struct msm_dp *dp,
-					struct drm_encoder *encoder)
-{
-	return -EINVAL;
-}
-static inline int msm_dp_display_pre_disable(struct msm_dp *dp,
-					struct drm_encoder *encoder)
-{
-	return -EINVAL;
-}
-static inline void msm_dp_display_mode_set(struct msm_dp *dp,
-				struct drm_encoder *encoder,
-				const struct drm_display_mode *mode,
-				const struct drm_display_mode *adjusted_mode)
-{
-}
 
 static inline void msm_dp_irq_postinstall(struct msm_dp *dp_display)
 {
-- 
2.34.1

[RFC PATCH 5/7] drm/msm/dp: remove unused stubs

[Dmitry Baryshkov]

Refactoring DP code transformed several functions into empty stubs.
Remove them.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 35 -----------------------------
 1 file changed, 35 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 7b4f40cb9a58..e63d6056e39d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -841,11 +841,6 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
 	return 0;
 }
 
-static int dp_display_prepare(struct msm_dp *dp_display)
-{
-	return 0;
-}
-
 static int dp_display_enable(struct dp_display_private *dp, u32 data)
 {
 	int rc = 0;
@@ -915,11 +910,6 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data)
 	return 0;
 }
 
-static int dp_display_unprepare(struct msm_dp *dp_display)
-{
-	return 0;
-}
-
 int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 		hdmi_codec_plugged_cb fn, struct device *codec_dev)
 {
@@ -1400,21 +1390,9 @@ static int dp_pm_suspend(struct device *dev)
 	return 0;
 }
 
-static int dp_pm_prepare(struct device *dev)
-{
-	return 0;
-}
-
-static void dp_pm_complete(struct device *dev)
-{
-
-}
-
 static const struct dev_pm_ops dp_pm_ops = {
 	.suspend = dp_pm_suspend,
 	.resume =  dp_pm_resume,
-	.prepare = dp_pm_prepare,
-	.complete = dp_pm_complete,
 };
 
 static struct platform_driver dp_display_driver = {
@@ -1565,13 +1543,6 @@ static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 		return;
 	}
 
-	rc = dp_display_prepare(dp);
-	if (rc) {
-		DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
-		mutex_unlock(&dp_display->event_mutex);
-		return;
-	}
-
 	state =  dp_display->hpd_state;
 
 	if (state == ST_DISPLAY_OFF)
@@ -1583,7 +1554,6 @@ static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 	if (rc) {
 		DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
 		dp_display_disable(dp_display, 0);
-		dp_display_unprepare(dp);
 	}
 
 	/* manual kick off plug event to train link */
@@ -1611,7 +1581,6 @@ static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 {
 	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
 	struct msm_dp *dp = dp_bridge->dp_display;
-	int rc = 0;
 	u32 state;
 	struct dp_display_private *dp_display;
 
@@ -1624,10 +1593,6 @@ static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 
 	dp_display_disable(dp_display, 0);
 
-	rc = dp_display_unprepare(dp);
-	if (rc)
-		DRM_ERROR("DP display unprepare failed, rc=%d\n", rc);
-
 	state =  dp_display->hpd_state;
 	if (state == ST_DISCONNECT_PENDING) {
 		/* completed disconnection */
-- 
2.34.1

[RFC PATCH 5/7] drm/msm/dp: remove unused stubs

[Dmitry Baryshkov]

Refactoring DP code transformed several functions into empty stubs.
Remove them.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 35 -----------------------------
 1 file changed, 35 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 7b4f40cb9a58..e63d6056e39d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -841,11 +841,6 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
 	return 0;
 }
 
-static int dp_display_prepare(struct msm_dp *dp_display)
-{
-	return 0;
-}
-
 static int dp_display_enable(struct dp_display_private *dp, u32 data)
 {
 	int rc = 0;
@@ -915,11 +910,6 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data)
 	return 0;
 }
 
-static int dp_display_unprepare(struct msm_dp *dp_display)
-{
-	return 0;
-}
-
 int dp_display_set_plugged_cb(struct msm_dp *dp_display,
 		hdmi_codec_plugged_cb fn, struct device *codec_dev)
 {
@@ -1400,21 +1390,9 @@ static int dp_pm_suspend(struct device *dev)
 	return 0;
 }
 
-static int dp_pm_prepare(struct device *dev)
-{
-	return 0;
-}
-
-static void dp_pm_complete(struct device *dev)
-{
-
-}
-
 static const struct dev_pm_ops dp_pm_ops = {
 	.suspend = dp_pm_suspend,
 	.resume =  dp_pm_resume,
-	.prepare = dp_pm_prepare,
-	.complete = dp_pm_complete,
 };
 
 static struct platform_driver dp_display_driver = {
@@ -1565,13 +1543,6 @@ static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 		return;
 	}
 
-	rc = dp_display_prepare(dp);
-	if (rc) {
-		DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
-		mutex_unlock(&dp_display->event_mutex);
-		return;
-	}
-
 	state =  dp_display->hpd_state;
 
 	if (state == ST_DISPLAY_OFF)
@@ -1583,7 +1554,6 @@ static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 	if (rc) {
 		DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
 		dp_display_disable(dp_display, 0);
-		dp_display_unprepare(dp);
 	}
 
 	/* manual kick off plug event to train link */
@@ -1611,7 +1581,6 @@ static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 {
 	struct msm_dp_bridge *dp_bridge = to_dp_display(drm_bridge);
 	struct msm_dp *dp = dp_bridge->dp_display;
-	int rc = 0;
 	u32 state;
 	struct dp_display_private *dp_display;
 
@@ -1624,10 +1593,6 @@ static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 
 	dp_display_disable(dp_display, 0);
 
-	rc = dp_display_unprepare(dp);
-	if (rc)
-		DRM_ERROR("DP display unprepare failed, rc=%d\n", rc);
-
 	state =  dp_display->hpd_state;
 	if (state == ST_DISCONNECT_PENDING) {
 		/* completed disconnection */
-- 
2.34.1

[RFC PATCH 6/7] drm/msm/dp: remove dp_display_en/disable prototypes and data argument

[Dmitry Baryshkov]

Remove unused dp_display_en/disable prototypes. While we are at it,
remove extra 'data' argument that is unused.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e63d6056e39d..720e80ea99cb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -559,9 +559,6 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
 	return 0;
 };
 
-static int dp_display_enable(struct dp_display_private *dp, u32 data);
-static int dp_display_disable(struct dp_display_private *dp, u32 data);
-
 static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data)
 {
 	u32 state;
@@ -841,7 +838,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
 	return 0;
 }
 
-static int dp_display_enable(struct dp_display_private *dp, u32 data)
+static int dp_display_enable(struct dp_display_private *dp)
 {
 	int rc = 0;
 	struct msm_dp *dp_display = &dp->dp_display;
@@ -878,7 +875,7 @@ static int dp_display_post_enable(struct msm_dp *dp_display)
 	return 0;
 }
 
-static int dp_display_disable(struct dp_display_private *dp, u32 data)
+static int dp_display_disable(struct dp_display_private *dp)
 {
 	struct msm_dp *dp_display = &dp->dp_display;
 
@@ -1548,12 +1545,12 @@ static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 	if (state == ST_DISPLAY_OFF)
 		dp_display_host_init(dp_display, true);
 
-	dp_display_enable(dp_display, 0);
+	dp_display_enable(dp_display);
 
 	rc = dp_display_post_enable(dp);
 	if (rc) {
 		DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
-		dp_display_disable(dp_display, 0);
+		dp_display_disable(dp_display);
 	}
 
 	/* manual kick off plug event to train link */
@@ -1591,7 +1588,7 @@ static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 	/* stop sentinel checking */
 	dp_del_event(dp_display, EV_DISCONNECT_PENDING_TIMEOUT);
 
-	dp_display_disable(dp_display, 0);
+	dp_display_disable(dp_display);
 
 	state =  dp_display->hpd_state;
 	if (state == ST_DISCONNECT_PENDING) {
-- 
2.34.1

[RFC PATCH 6/7] drm/msm/dp: remove dp_display_en/disable prototypes and data argument

[Dmitry Baryshkov]

Remove unused dp_display_en/disable prototypes. While we are at it,
remove extra 'data' argument that is unused.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e63d6056e39d..720e80ea99cb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -559,9 +559,6 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
 	return 0;
 };
 
-static int dp_display_enable(struct dp_display_private *dp, u32 data);
-static int dp_display_disable(struct dp_display_private *dp, u32 data);
-
 static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data)
 {
 	u32 state;
@@ -841,7 +838,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
 	return 0;
 }
 
-static int dp_display_enable(struct dp_display_private *dp, u32 data)
+static int dp_display_enable(struct dp_display_private *dp)
 {
 	int rc = 0;
 	struct msm_dp *dp_display = &dp->dp_display;
@@ -878,7 +875,7 @@ static int dp_display_post_enable(struct msm_dp *dp_display)
 	return 0;
 }
 
-static int dp_display_disable(struct dp_display_private *dp, u32 data)
+static int dp_display_disable(struct dp_display_private *dp)
 {
 	struct msm_dp *dp_display = &dp->dp_display;
 
@@ -1548,12 +1545,12 @@ static void dp_bridge_enable(struct drm_bridge *drm_bridge)
 	if (state == ST_DISPLAY_OFF)
 		dp_display_host_init(dp_display, true);
 
-	dp_display_enable(dp_display, 0);
+	dp_display_enable(dp_display);
 
 	rc = dp_display_post_enable(dp);
 	if (rc) {
 		DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
-		dp_display_disable(dp_display, 0);
+		dp_display_disable(dp_display);
 	}
 
 	/* manual kick off plug event to train link */
@@ -1591,7 +1588,7 @@ static void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
 	/* stop sentinel checking */
 	dp_del_event(dp_display, EV_DISCONNECT_PENDING_TIMEOUT);
 
-	dp_display_disable(dp_display, 0);
+	dp_display_disable(dp_display);
 
 	state =  dp_display->hpd_state;
 	if (state == ST_DISCONNECT_PENDING) {
-- 
2.34.1

[RFC PATCH 7/7] drm/msm/dp: stop carying about the connector type

[Dmitry Baryshkov]

After changing dp_parser code to always check for the next bridge, it
does not check connector type anymore. Remove connector type from the
dp_paser_parse() arguments list and from the struct msm_dp_desc fields
list.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 9 ++++-----
 drivers/gpu/drm/msm/dp/dp_parser.c  | 2 +-
 drivers/gpu/drm/msm/dp/dp_parser.h  | 2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 720e80ea99cb..0a71a17975b0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -129,7 +129,6 @@ struct dp_display_private {
 
 struct msm_dp_desc {
 	phys_addr_t io_start;
-	unsigned int connector_type;
 };
 
 struct msm_dp_config {
@@ -139,15 +138,15 @@ struct msm_dp_config {
 
 static const struct msm_dp_config sc7180_dp_cfg = {
 	.descs = (const struct msm_dp_desc[]) {
-		[MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
+		[MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000 },
 	},
 	.num_descs = 1,
 };
 
 static const struct msm_dp_config sc7280_dp_cfg = {
 	.descs = (const struct msm_dp_desc[]) {
-		[MSM_DP_CONTROLLER_0] =	{ .io_start = 0x0ae90000, .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
-		[MSM_DP_CONTROLLER_1] =	{ .io_start = 0x0aea0000, .connector_type = DRM_MODE_CONNECTOR_eDP },
+		[MSM_DP_CONTROLLER_0] =	{ .io_start = 0x0ae90000 },
+		[MSM_DP_CONTROLLER_1] =	{ .io_start = 0x0aea0000 },
 	},
 	.num_descs = 2,
 };
@@ -249,7 +248,7 @@ static int dp_display_bind(struct device *dev, struct device *master,
 	dp->dp_display.drm_dev = drm;
 	priv->dp[dp->id] = &dp->dp_display;
 
-	rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
+	rc = dp->parser->parse(dp->parser);
 	if (rc) {
 		DRM_ERROR("device tree parsing failed\n");
 		goto end;
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index 5de21f3d0812..044ab0b63b14 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -278,7 +278,7 @@ static int dp_parser_find_next_bridge(struct dp_parser *parser)
 	return 0;
 }
 
-static int dp_parser_parse(struct dp_parser *parser, int connector_type)
+static int dp_parser_parse(struct dp_parser *parser)
 {
 	int rc = 0;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 4cec851e38d9..1f036dd3e224 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -125,7 +125,7 @@ struct dp_parser {
 	u32 max_dp_lanes;
 	struct drm_bridge *next_bridge;
 
-	int (*parse)(struct dp_parser *parser, int connector_type);
+	int (*parse)(struct dp_parser *parser);
 };
 
 /**
-- 
2.34.1

[RFC PATCH 7/7] drm/msm/dp: stop carying about the connector type

[Dmitry Baryshkov]

After changing dp_parser code to always check for the next bridge, it
does not check connector type anymore. Remove connector type from the
dp_paser_parse() arguments list and from the struct msm_dp_desc fields
list.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 9 ++++-----
 drivers/gpu/drm/msm/dp/dp_parser.c  | 2 +-
 drivers/gpu/drm/msm/dp/dp_parser.h  | 2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 720e80ea99cb..0a71a17975b0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -129,7 +129,6 @@ struct dp_display_private {
 
 struct msm_dp_desc {
 	phys_addr_t io_start;
-	unsigned int connector_type;
 };
 
 struct msm_dp_config {
@@ -139,15 +138,15 @@ struct msm_dp_config {
 
 static const struct msm_dp_config sc7180_dp_cfg = {
 	.descs = (const struct msm_dp_desc[]) {
-		[MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
+		[MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000 },
 	},
 	.num_descs = 1,
 };
 
 static const struct msm_dp_config sc7280_dp_cfg = {
 	.descs = (const struct msm_dp_desc[]) {
-		[MSM_DP_CONTROLLER_0] =	{ .io_start = 0x0ae90000, .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
-		[MSM_DP_CONTROLLER_1] =	{ .io_start = 0x0aea0000, .connector_type = DRM_MODE_CONNECTOR_eDP },
+		[MSM_DP_CONTROLLER_0] =	{ .io_start = 0x0ae90000 },
+		[MSM_DP_CONTROLLER_1] =	{ .io_start = 0x0aea0000 },
 	},
 	.num_descs = 2,
 };
@@ -249,7 +248,7 @@ static int dp_display_bind(struct device *dev, struct device *master,
 	dp->dp_display.drm_dev = drm;
 	priv->dp[dp->id] = &dp->dp_display;
 
-	rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
+	rc = dp->parser->parse(dp->parser);
 	if (rc) {
 		DRM_ERROR("device tree parsing failed\n");
 		goto end;
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index 5de21f3d0812..044ab0b63b14 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -278,7 +278,7 @@ static int dp_parser_find_next_bridge(struct dp_parser *parser)
 	return 0;
 }
 
-static int dp_parser_parse(struct dp_parser *parser, int connector_type)
+static int dp_parser_parse(struct dp_parser *parser)
 {
 	int rc = 0;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 4cec851e38d9..1f036dd3e224 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -125,7 +125,7 @@ struct dp_parser {
 	u32 max_dp_lanes;
 	struct drm_bridge *next_bridge;
 
-	int (*parse)(struct dp_parser *parser, int connector_type);
+	int (*parse)(struct dp_parser *parser);
 };
 
 /**
-- 
2.34.1

Re: [RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-06 18:01:25)
> I noticed that commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> enable and disable") conflicts with the panel-edp (panel bridge)
> support. Both bridges will try to attach directly to the drm encoder
> itself. I started writing lengthy letter describing what is broken and
> how it should be fixed. Then at some point I stopped and quickly coded
> this RFC (which is compile-tested only).
>
> Comments and tests (on both DP and eDP setups) are more than welcome.

There are some DP patches dribbling in every day or so and it's really
hard to follow along. I asked Kuogee to resend all outstanding patches
as a single series but that hasn't happened. I'm not super interested in
reviewing/testing out these patches until the outstanding patches for DP
on the list are reviewed and landed. Have you looked at those patches?
See [1] for an example.

>
> The following changes since commit 6ed95285382d6f90a3c3a11d5806a5eb7db715c3:
>
>   drm/msm/a5xx: Fix missing CP_PROTECT for SMMU on A540 (2021-12-17 15:09:46 -0800)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/dmitry.baryshkov/kernel.git msm-dp-bridges
>
> for you to fetch changes up to 7eff304d50ba520e9193a293a8e42bbd9d7aa0e1:
>
>   drm/msm/dp: stop carying about the connector type (2022-01-07 04:56:06 +0300)
>
> ----------------------------------------------------------------
> Dmitry Baryshkov (7):
>       drm/msm/dp: fix panel bridge attachment
>       drm/msm/dp: support attaching bridges to the DP encoder
>       drm/msm/dp: replace dp_connector with drm_bridge_connector
>       drm/msm/dp: remove extra wrappers and public functions
>       drm/msm/dp: remove unused stubs
>       drm/msm/dp: remove dp_display_en/disable prototypes and data argument
>       drm/msm/dp: stop carying about the connector type
>
>  drivers/gpu/drm/msm/Makefile        |   1 -
>  drivers/gpu/drm/msm/dp/dp_display.c | 263 ++++++++++++++++++++++++++----------
>  drivers/gpu/drm/msm/dp/dp_display.h |   5 +-
>  drivers/gpu/drm/msm/dp/dp_drm.c     | 250 ----------------------------------
>  drivers/gpu/drm/msm/dp/dp_parser.c  |  28 ++--
>  drivers/gpu/drm/msm/dp/dp_parser.h  |   4 +-
>  drivers/gpu/drm/msm/msm_drv.h       |  32 +----
>  7 files changed, 203 insertions(+), 380 deletions(-)
>  delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
>

[1] https://lore.kernel.org/r/1640220845-25266-1-git-send-email-quic_khsieh@quicinc.com

Re: [RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-06 18:01:25)
> I noticed that commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> enable and disable") conflicts with the panel-edp (panel bridge)
> support. Both bridges will try to attach directly to the drm encoder
> itself. I started writing lengthy letter describing what is broken and
> how it should be fixed. Then at some point I stopped and quickly coded
> this RFC (which is compile-tested only).
>
> Comments and tests (on both DP and eDP setups) are more than welcome.

There are some DP patches dribbling in every day or so and it's really
hard to follow along. I asked Kuogee to resend all outstanding patches
as a single series but that hasn't happened. I'm not super interested in
reviewing/testing out these patches until the outstanding patches for DP
on the list are reviewed and landed. Have you looked at those patches?
See [1] for an example.

>
> The following changes since commit 6ed95285382d6f90a3c3a11d5806a5eb7db715c3:
>
>   drm/msm/a5xx: Fix missing CP_PROTECT for SMMU on A540 (2021-12-17 15:09:46 -0800)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/dmitry.baryshkov/kernel.git msm-dp-bridges
>
> for you to fetch changes up to 7eff304d50ba520e9193a293a8e42bbd9d7aa0e1:
>
>   drm/msm/dp: stop carying about the connector type (2022-01-07 04:56:06 +0300)
>
> ----------------------------------------------------------------
> Dmitry Baryshkov (7):
>       drm/msm/dp: fix panel bridge attachment
>       drm/msm/dp: support attaching bridges to the DP encoder
>       drm/msm/dp: replace dp_connector with drm_bridge_connector
>       drm/msm/dp: remove extra wrappers and public functions
>       drm/msm/dp: remove unused stubs
>       drm/msm/dp: remove dp_display_en/disable prototypes and data argument
>       drm/msm/dp: stop carying about the connector type
>
>  drivers/gpu/drm/msm/Makefile        |   1 -
>  drivers/gpu/drm/msm/dp/dp_display.c | 263 ++++++++++++++++++++++++++----------
>  drivers/gpu/drm/msm/dp/dp_display.h |   5 +-
>  drivers/gpu/drm/msm/dp/dp_drm.c     | 250 ----------------------------------
>  drivers/gpu/drm/msm/dp/dp_parser.c  |  28 ++--
>  drivers/gpu/drm/msm/dp/dp_parser.h  |   4 +-
>  drivers/gpu/drm/msm/msm_drv.h       |  32 +----
>  7 files changed, 203 insertions(+), 380 deletions(-)
>  delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
>

[1] https://lore.kernel.org/r/1640220845-25266-1-git-send-email-quic_khsieh@quicinc.com

Re: [RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[Dmitry Baryshkov]

On 07/01/2022 05:16, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:25)
>> I noticed that commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
>> enable and disable") conflicts with the panel-edp (panel bridge)
>> support. Both bridges will try to attach directly to the drm encoder
>> itself. I started writing lengthy letter describing what is broken and
>> how it should be fixed. Then at some point I stopped and quickly coded
>> this RFC (which is compile-tested only).
>>
>> Comments and tests (on both DP and eDP setups) are more than welcome.
> 
> There are some DP patches dribbling in every day or so and it's really
> hard to follow along. I asked Kuogee to resend all outstanding patches
> as a single series but that hasn't happened. I'm not super interested in
> reviewing/testing out these patches until the outstanding patches for DP
> on the list are reviewed and landed. Have you looked at those patches?

I haven't been following the DP patches. Well, in fact I was mostly 
stopping myself from looking onto the DP driver and getting elbow deep 
in it. Partially because some of the patches circulating on the list 
were clear hacks (e.g. PHY timeouts). Some would be too complex to 
review them without deep diving into DP. Most of my attention (and spare 
time) goes to the DPU/DSI/MDP5 (and to lesser extent MDP4/HDMI) drives.

With regards to this patch series, the patch 1 is probably most 
important (and might warrant sending it separately), as it should fix 
eDP support for Bjorn.

So, initially I wrote just patch 1. And then the surrounding code 
immediately prompted me to update the rest of the drm glue code. Elbow 
deep, as I said. Patch 7 might be a bit advantageous (and maybe I should 
remove it in future),

> See [1] for an example.

I think most of the patches circulating through the list are irrelevant 
to this patch series, as they do not touch the drm glue code.

>> The following changes since commit 6ed95285382d6f90a3c3a11d5806a5eb7db715c3:
>>
>>    drm/msm/a5xx: Fix missing CP_PROTECT for SMMU on A540 (2021-12-17 15:09:46 -0800)
>>
>> are available in the Git repository at:
>>
>>    https://git.linaro.org/people/dmitry.baryshkov/kernel.git msm-dp-bridges
>>
>> for you to fetch changes up to 7eff304d50ba520e9193a293a8e42bbd9d7aa0e1:
>>
>>    drm/msm/dp: stop carying about the connector type (2022-01-07 04:56:06 +0300)
>>
>> ----------------------------------------------------------------
>> Dmitry Baryshkov (7):
>>        drm/msm/dp: fix panel bridge attachment
>>        drm/msm/dp: support attaching bridges to the DP encoder
>>        drm/msm/dp: replace dp_connector with drm_bridge_connector
>>        drm/msm/dp: remove extra wrappers and public functions
>>        drm/msm/dp: remove unused stubs
>>        drm/msm/dp: remove dp_display_en/disable prototypes and data argument
>>        drm/msm/dp: stop carying about the connector type
>>
>>   drivers/gpu/drm/msm/Makefile        |   1 -
>>   drivers/gpu/drm/msm/dp/dp_display.c | 263 ++++++++++++++++++++++++++----------
>>   drivers/gpu/drm/msm/dp/dp_display.h |   5 +-
>>   drivers/gpu/drm/msm/dp/dp_drm.c     | 250 ----------------------------------
>>   drivers/gpu/drm/msm/dp/dp_parser.c  |  28 ++--
>>   drivers/gpu/drm/msm/dp/dp_parser.h  |   4 +-
>>   drivers/gpu/drm/msm/msm_drv.h       |  32 +----
>>   7 files changed, 203 insertions(+), 380 deletions(-)
>>   delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
>>
> 
> [1] https://lore.kernel.org/r/1640220845-25266-1-git-send-email-quic_khsieh@quicinc.com


-- 
With best wishes
Dmitry

Re: [RFC PATCH 0/7] Simplify and correct msm/dp bridge implementation

[Dmitry Baryshkov]

On 07/01/2022 05:16, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:25)
>> I noticed that commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
>> enable and disable") conflicts with the panel-edp (panel bridge)
>> support. Both bridges will try to attach directly to the drm encoder
>> itself. I started writing lengthy letter describing what is broken and
>> how it should be fixed. Then at some point I stopped and quickly coded
>> this RFC (which is compile-tested only).
>>
>> Comments and tests (on both DP and eDP setups) are more than welcome.
> 
> There are some DP patches dribbling in every day or so and it's really
> hard to follow along. I asked Kuogee to resend all outstanding patches
> as a single series but that hasn't happened. I'm not super interested in
> reviewing/testing out these patches until the outstanding patches for DP
> on the list are reviewed and landed. Have you looked at those patches?

I haven't been following the DP patches. Well, in fact I was mostly 
stopping myself from looking onto the DP driver and getting elbow deep 
in it. Partially because some of the patches circulating on the list 
were clear hacks (e.g. PHY timeouts). Some would be too complex to 
review them without deep diving into DP. Most of my attention (and spare 
time) goes to the DPU/DSI/MDP5 (and to lesser extent MDP4/HDMI) drives.

With regards to this patch series, the patch 1 is probably most 
important (and might warrant sending it separately), as it should fix 
eDP support for Bjorn.

So, initially I wrote just patch 1. And then the surrounding code 
immediately prompted me to update the rest of the drm glue code. Elbow 
deep, as I said. Patch 7 might be a bit advantageous (and maybe I should 
remove it in future),

> See [1] for an example.

I think most of the patches circulating through the list are irrelevant 
to this patch series, as they do not touch the drm glue code.

>> The following changes since commit 6ed95285382d6f90a3c3a11d5806a5eb7db715c3:
>>
>>    drm/msm/a5xx: Fix missing CP_PROTECT for SMMU on A540 (2021-12-17 15:09:46 -0800)
>>
>> are available in the Git repository at:
>>
>>    https://git.linaro.org/people/dmitry.baryshkov/kernel.git msm-dp-bridges
>>
>> for you to fetch changes up to 7eff304d50ba520e9193a293a8e42bbd9d7aa0e1:
>>
>>    drm/msm/dp: stop carying about the connector type (2022-01-07 04:56:06 +0300)
>>
>> ----------------------------------------------------------------
>> Dmitry Baryshkov (7):
>>        drm/msm/dp: fix panel bridge attachment
>>        drm/msm/dp: support attaching bridges to the DP encoder
>>        drm/msm/dp: replace dp_connector with drm_bridge_connector
>>        drm/msm/dp: remove extra wrappers and public functions
>>        drm/msm/dp: remove unused stubs
>>        drm/msm/dp: remove dp_display_en/disable prototypes and data argument
>>        drm/msm/dp: stop carying about the connector type
>>
>>   drivers/gpu/drm/msm/Makefile        |   1 -
>>   drivers/gpu/drm/msm/dp/dp_display.c | 263 ++++++++++++++++++++++++++----------
>>   drivers/gpu/drm/msm/dp/dp_display.h |   5 +-
>>   drivers/gpu/drm/msm/dp/dp_drm.c     | 250 ----------------------------------
>>   drivers/gpu/drm/msm/dp/dp_parser.c  |  28 ++--
>>   drivers/gpu/drm/msm/dp/dp_parser.h  |   4 +-
>>   drivers/gpu/drm/msm/msm_drv.h       |  32 +----
>>   7 files changed, 203 insertions(+), 380 deletions(-)
>>   delete mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
>>
> 
> [1] https://lore.kernel.org/r/1640220845-25266-1-git-send-email-quic_khsieh@quicinc.com


-- 
With best wishes
Dmitry

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-06 18:01:26)
> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> enable and disable") the DP driver received a drm_bridge instance, which
> is always attached to the encoder as a root bridge. However it conflicts
> with the panel_bridge support for eDP panels.

Can you elaborate here? How does it conflict? Could be as simple as "it
attaches before the panel bridge can attach to the root" or something
like that.

> Change panel_bridge
> attachment to come after dp_bridge attachment.
>

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-06 18:01:26)
> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> enable and disable") the DP driver received a drm_bridge instance, which
> is always attached to the encoder as a root bridge. However it conflicts
> with the panel_bridge support for eDP panels.

Can you elaborate here? How does it conflict? Could be as simple as "it
attaches before the panel bridge can attach to the root" or something
like that.

> Change panel_bridge
> attachment to come after dp_bridge attachment.
>

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
> Currently DP driver will allocate panel bridge for eDP panels.
> Simplify this code to just check if there is any next bridge in the
> chain (be it a panel bridge or regular bridge). Rename panel_bridge
> field to next_bridge accordingly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>  drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>  drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>  drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>  5 files changed, 13 insertions(+), 23 deletions(-)

I like this one, it certainly makes it easier to understand.

> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index a7acc23f742b..5de21f3d0812 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
>         if (rc)
>                 return rc;
>
> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {

It feels like this is on purpose, but I don't see any comment so I have
no idea. I think qcom folks are concerned about changing how not eDP
works. I'll have to test it out locally.

> -               rc = dp_parser_find_panel(parser);
> -               if (rc)
> -                       return rc;
> -       }
> +       rc = dp_parser_find_next_bridge(parser);
> +       if (rc)
> +               return rc;
>
>         /* Map the corresponding regulator information according to
>          * version. Currently, since we only have one supported platform,

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
> Currently DP driver will allocate panel bridge for eDP panels.
> Simplify this code to just check if there is any next bridge in the
> chain (be it a panel bridge or regular bridge). Rename panel_bridge
> field to next_bridge accordingly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>  drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>  drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>  drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>  5 files changed, 13 insertions(+), 23 deletions(-)

I like this one, it certainly makes it easier to understand.

> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index a7acc23f742b..5de21f3d0812 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
>         if (rc)
>                 return rc;
>
> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {

It feels like this is on purpose, but I don't see any comment so I have
no idea. I think qcom folks are concerned about changing how not eDP
works. I'll have to test it out locally.

> -               rc = dp_parser_find_panel(parser);
> -               if (rc)
> -                       return rc;
> -       }
> +       rc = dp_parser_find_next_bridge(parser);
> +       if (rc)
> +               return rc;
>
>         /* Map the corresponding regulator information according to
>          * version. Currently, since we only have one supported platform,

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

On 07/01/2022 06:37, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:26)
>> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
>> enable and disable") the DP driver received a drm_bridge instance, which
>> is always attached to the encoder as a root bridge. However it conflicts
>> with the panel_bridge support for eDP panels.
> 
> Can you elaborate here? How does it conflict? Could be as simple as "it
> attaches before the panel bridge can attach to the root" or something
> like that.

Actually it would be the other way around: panel bridge attaching before 
the "dp" one. But yes, you got the idea. I'll extend the patch's 
description.

>> Change panel_bridge
>> attachment to come after dp_bridge attachment.
>>


-- 
With best wishes
Dmitry

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

On 07/01/2022 06:37, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:26)
>> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
>> enable and disable") the DP driver received a drm_bridge instance, which
>> is always attached to the encoder as a root bridge. However it conflicts
>> with the panel_bridge support for eDP panels.
> 
> Can you elaborate here? How does it conflict? Could be as simple as "it
> attaches before the panel bridge can attach to the root" or something
> like that.

Actually it would be the other way around: panel bridge attaching before 
the "dp" one. But yes, you got the idea. I'll extend the patch's 
description.

>> Change panel_bridge
>> attachment to come after dp_bridge attachment.
>>


-- 
With best wishes
Dmitry

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

On 07/01/2022 06:42, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
>> Currently DP driver will allocate panel bridge for eDP panels.
>> Simplify this code to just check if there is any next bridge in the
>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
>> field to next_bridge accordingly.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>>   5 files changed, 13 insertions(+), 23 deletions(-)
> 
> I like this one, it certainly makes it easier to understand.
> 
>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
>> index a7acc23f742b..5de21f3d0812 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
>>          if (rc)
>>                  return rc;
>>
>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> 
> It feels like this is on purpose, but I don't see any comment so I have
> no idea. I think qcom folks are concerned about changing how not eDP
> works. I'll have to test it out locally.

Ah, another thing that should go into the commit message.

Current situation:
- DP: no external bridges supported.
- eDP: only a drm_panel wrapped into the panel bridge

After this patch:
- both DP and eDP support any chain of bridges attached.


While the change means nothing for the DP (IIUC, it will not have any 
bridges), it simplifies the code path, lowering the amount of checks.

And for eDP this means that we can attach any eDP-to-something bridges 
(e.g. NXP PTN3460).


Well... After re-checking the source code for 
devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably 
revert removal of the check. The function will return -ENODEV if neither 
bridge nor panel are specified.

> 
>> -               rc = dp_parser_find_panel(parser);
>> -               if (rc)
>> -                       return rc;
>> -       }
>> +       rc = dp_parser_find_next_bridge(parser);
>> +       if (rc)
>> +               return rc;
>>
>>          /* Map the corresponding regulator information according to
>>           * version. Currently, since we only have one supported platform,


-- 
With best wishes
Dmitry

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

On 07/01/2022 06:42, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
>> Currently DP driver will allocate panel bridge for eDP panels.
>> Simplify this code to just check if there is any next bridge in the
>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
>> field to next_bridge accordingly.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>>   5 files changed, 13 insertions(+), 23 deletions(-)
> 
> I like this one, it certainly makes it easier to understand.
> 
>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
>> index a7acc23f742b..5de21f3d0812 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
>>          if (rc)
>>                  return rc;
>>
>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> 
> It feels like this is on purpose, but I don't see any comment so I have
> no idea. I think qcom folks are concerned about changing how not eDP
> works. I'll have to test it out locally.

Ah, another thing that should go into the commit message.

Current situation:
- DP: no external bridges supported.
- eDP: only a drm_panel wrapped into the panel bridge

After this patch:
- both DP and eDP support any chain of bridges attached.


While the change means nothing for the DP (IIUC, it will not have any 
bridges), it simplifies the code path, lowering the amount of checks.

And for eDP this means that we can attach any eDP-to-something bridges 
(e.g. NXP PTN3460).


Well... After re-checking the source code for 
devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably 
revert removal of the check. The function will return -ENODEV if neither 
bridge nor panel are specified.

> 
>> -               rc = dp_parser_find_panel(parser);
>> -               if (rc)
>> -                       return rc;
>> -       }
>> +       rc = dp_parser_find_next_bridge(parser);
>> +       if (rc)
>> +               return rc;
>>
>>          /* Map the corresponding regulator information according to
>>           * version. Currently, since we only have one supported platform,


-- 
With best wishes
Dmitry

Re: [RFC PATCH 4/7] drm/msm/dp: remove extra wrappers and public functions

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3538 bytes --]

Hi Dmitry,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on drm/drm-next]
[also build test WARNING on next-20220106]
[cannot apply to v5.16-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/Simplify-and-correct-msm-dp-bridge-implementation/20220107-100217
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220107/202201071518.3kbZwrrp-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/86a525a49f84d97357e16cd34ea18d25d8488994
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/Simplify-and-correct-msm-dp-bridge-implementation/20220107-100217
        git checkout 86a525a49f84d97357e16cd34ea18d25d8488994
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/msm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/msm/dp/dp_display.c:979:5: warning: no previous prototype for 'dp_display_get_modes' [-Wmissing-prototypes]
     979 | int dp_display_get_modes(struct msm_dp *dp,
         |     ^~~~~~~~~~~~~~~~~~~~


vim +/dp_display_get_modes +979 drivers/gpu/drm/msm/dp/dp_display.c

c943b4948b5848f Chandan Uddaraju 2020-08-27  978  
c943b4948b5848f Chandan Uddaraju 2020-08-27 @979  int dp_display_get_modes(struct msm_dp *dp,
c943b4948b5848f Chandan Uddaraju 2020-08-27  980  				struct dp_display_mode *dp_mode)
c943b4948b5848f Chandan Uddaraju 2020-08-27  981  {
c943b4948b5848f Chandan Uddaraju 2020-08-27  982  	struct dp_display_private *dp_display;
c943b4948b5848f Chandan Uddaraju 2020-08-27  983  	int ret = 0;
c943b4948b5848f Chandan Uddaraju 2020-08-27  984  
c943b4948b5848f Chandan Uddaraju 2020-08-27  985  	if (!dp) {
c943b4948b5848f Chandan Uddaraju 2020-08-27  986  		DRM_ERROR("invalid params\n");
c943b4948b5848f Chandan Uddaraju 2020-08-27  987  		return 0;
c943b4948b5848f Chandan Uddaraju 2020-08-27  988  	}
c943b4948b5848f Chandan Uddaraju 2020-08-27  989  
c943b4948b5848f Chandan Uddaraju 2020-08-27  990  	dp_display = container_of(dp, struct dp_display_private, dp_display);
c943b4948b5848f Chandan Uddaraju 2020-08-27  991  
c943b4948b5848f Chandan Uddaraju 2020-08-27  992  	ret = dp_panel_get_modes(dp_display->panel,
c943b4948b5848f Chandan Uddaraju 2020-08-27  993  		dp->connector, dp_mode);
c943b4948b5848f Chandan Uddaraju 2020-08-27  994  	if (dp_mode->drm_mode.clock)
c943b4948b5848f Chandan Uddaraju 2020-08-27  995  		dp->max_pclk_khz = dp_mode->drm_mode.clock;
c943b4948b5848f Chandan Uddaraju 2020-08-27  996  	return ret;
c943b4948b5848f Chandan Uddaraju 2020-08-27  997  }
c943b4948b5848f Chandan Uddaraju 2020-08-27  998  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [RFC PATCH 7/7] drm/msm/dp: stop carying about the connector type

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5728 bytes --]

Hi Dmitry,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20220106]
[cannot apply to v5.16-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/Simplify-and-correct-msm-dp-bridge-implementation/20220107-100217
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220108/202201080314.Ul2HrR54-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5e6a356796584c6309d4947f4a07e45eb4988e46
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/Simplify-and-correct-msm-dp-bridge-implementation/20220107-100217
        git checkout 5e6a356796584c6309d4947f4a07e45eb4988e46
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/msm/dp/dp_display.c:965:5: warning: no previous prototype for 'dp_display_get_modes' [-Wmissing-prototypes]
     965 | int dp_display_get_modes(struct msm_dp *dp,
         |     ^~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/msm/dp/dp_display.c: In function 'dp_display_probe':
>> drivers/gpu/drm/msm/dp/dp_display.c:1260:45: error: 'const struct msm_dp_desc' has no member named 'connector_type'
    1260 |         dp->dp_display.connector_type = desc->connector_type;
         |                                             ^~


vim +1260 drivers/gpu/drm/msm/dp/dp_display.c

269e92d84cd223 Bjorn Andersson  2021-10-16  1238  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1239  static int dp_display_probe(struct platform_device *pdev)
c943b4948b5848 Chandan Uddaraju 2020-08-27  1240  {
c943b4948b5848 Chandan Uddaraju 2020-08-27  1241  	int rc = 0;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1242  	struct dp_display_private *dp;
269e92d84cd223 Bjorn Andersson  2021-10-16  1243  	const struct msm_dp_desc *desc;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1244  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1245  	if (!pdev || !pdev->dev.of_node) {
c943b4948b5848 Chandan Uddaraju 2020-08-27  1246  		DRM_ERROR("pdev not found\n");
c943b4948b5848 Chandan Uddaraju 2020-08-27  1247  		return -ENODEV;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1248  	}
c943b4948b5848 Chandan Uddaraju 2020-08-27  1249  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1250  	dp = devm_kzalloc(&pdev->dev, sizeof(*dp), GFP_KERNEL);
c943b4948b5848 Chandan Uddaraju 2020-08-27  1251  	if (!dp)
c943b4948b5848 Chandan Uddaraju 2020-08-27  1252  		return -ENOMEM;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1253  
bb3de286d9921c Bjorn Andersson  2021-10-16  1254  	desc = dp_display_get_desc(pdev, &dp->id);
269e92d84cd223 Bjorn Andersson  2021-10-16  1255  	if (!desc)
269e92d84cd223 Bjorn Andersson  2021-10-16  1256  		return -EINVAL;
269e92d84cd223 Bjorn Andersson  2021-10-16  1257  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1258  	dp->pdev = pdev;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1259  	dp->name = "drm_dp";
269e92d84cd223 Bjorn Andersson  2021-10-16 @1260  	dp->dp_display.connector_type = desc->connector_type;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1261  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1262  	rc = dp_init_sub_modules(dp);
c943b4948b5848 Chandan Uddaraju 2020-08-27  1263  	if (rc) {
c943b4948b5848 Chandan Uddaraju 2020-08-27  1264  		DRM_ERROR("init sub module failed\n");
c943b4948b5848 Chandan Uddaraju 2020-08-27  1265  		return -EPROBE_DEFER;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1266  	}
c943b4948b5848 Chandan Uddaraju 2020-08-27  1267  
8ede2ecc3e5ee3 Kuogee Hsieh     2020-09-11  1268  	mutex_init(&dp->event_mutex);
c943b4948b5848 Chandan Uddaraju 2020-08-27  1269  
d13e36d7d2227e Abhinav Kumar    2020-09-12  1270  	/* Store DP audio handle inside DP display */
d624e50aa3c162 Bjorn Andersson  2021-10-16  1271  	dp->dp_display.dp_audio = dp->audio;
d13e36d7d2227e Abhinav Kumar    2020-09-12  1272  
158b9aa74479c9 Abhinav Kumar    2020-09-12  1273  	init_completion(&dp->audio_comp);
158b9aa74479c9 Abhinav Kumar    2020-09-12  1274  
d624e50aa3c162 Bjorn Andersson  2021-10-16  1275  	platform_set_drvdata(pdev, &dp->dp_display);
061eb621fc2780 Abhinav Kumar    2020-09-12  1276  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1277  	rc = component_add(&pdev->dev, &dp_display_comp_ops);
c943b4948b5848 Chandan Uddaraju 2020-08-27  1278  	if (rc) {
c943b4948b5848 Chandan Uddaraju 2020-08-27  1279  		DRM_ERROR("component add failed, rc=%d\n", rc);
c943b4948b5848 Chandan Uddaraju 2020-08-27  1280  		dp_display_deinit_sub_modules(dp);
c943b4948b5848 Chandan Uddaraju 2020-08-27  1281  	}
c943b4948b5848 Chandan Uddaraju 2020-08-27  1282  
c943b4948b5848 Chandan Uddaraju 2020-08-27  1283  	return rc;
c943b4948b5848 Chandan Uddaraju 2020-08-27  1284  }
c943b4948b5848 Chandan Uddaraju 2020-08-27  1285  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Kuogee Hsieh]

On 1/6/2022 9:26 PM, Dmitry Baryshkov wrote:
> On 07/01/2022 06:42, Stephen Boyd wrote:
>> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
>>> Currently DP driver will allocate panel bridge for eDP panels.
>>> Simplify this code to just check if there is any next bridge in the
>>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
>>> field to next_bridge accordingly.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>>>   5 files changed, 13 insertions(+), 23 deletions(-)
>>
>> I like this one, it certainly makes it easier to understand.
>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
>>> b/drivers/gpu/drm/msm/dp/dp_parser.c
>>> index a7acc23f742b..5de21f3d0812 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
>>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser 
>>> *parser, int connector_type)
>>>          if (rc)
>>>                  return rc;
>>>
>>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
>>
>> It feels like this is on purpose, but I don't see any comment so I have
>> no idea. I think qcom folks are concerned about changing how not eDP
>> works. I'll have to test it out locally.
>
> Ah, another thing that should go into the commit message.
>
> Current situation:
> - DP: no external bridges supported.
> - eDP: only a drm_panel wrapped into the panel bridge
>
> After this patch:
> - both DP and eDP support any chain of bridges attached.
>
>
> While the change means nothing for the DP (IIUC, it will not have any 
> bridges), it simplifies the code path, lowering the amount of checks.
>
> And for eDP this means that we can attach any eDP-to-something bridges 
> (e.g. NXP PTN3460).
>
>
> Well... After re-checking the source code for 
> devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably 
> revert removal of the check. The function will return -ENODEV if 
> neither bridge nor panel are specified.
>
I am new to drm and  confusing with bridge here.

Isn't bridge used to bridging two different kind of interface together?

for example, dsi <--> bridge <--> dp.

why edp need bridge here?

Can you give me more info regrading what bridge try to do here.



>>
>>> -               rc = dp_parser_find_panel(parser);
>>> -               if (rc)
>>> -                       return rc;
>>> -       }
>>> +       rc = dp_parser_find_next_bridge(parser);
>>> +       if (rc)
>>> +               return rc;
>>>
>>>          /* Map the corresponding regulator information according to
>>>           * version. Currently, since we only have one supported 
>>> platform,
>
>

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Kuogee Hsieh]

On 1/6/2022 9:26 PM, Dmitry Baryshkov wrote:
> On 07/01/2022 06:42, Stephen Boyd wrote:
>> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
>>> Currently DP driver will allocate panel bridge for eDP panels.
>>> Simplify this code to just check if there is any next bridge in the
>>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
>>> field to next_bridge accordingly.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>>>   5 files changed, 13 insertions(+), 23 deletions(-)
>>
>> I like this one, it certainly makes it easier to understand.
>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
>>> b/drivers/gpu/drm/msm/dp/dp_parser.c
>>> index a7acc23f742b..5de21f3d0812 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
>>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser 
>>> *parser, int connector_type)
>>>          if (rc)
>>>                  return rc;
>>>
>>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
>>
>> It feels like this is on purpose, but I don't see any comment so I have
>> no idea. I think qcom folks are concerned about changing how not eDP
>> works. I'll have to test it out locally.
>
> Ah, another thing that should go into the commit message.
>
> Current situation:
> - DP: no external bridges supported.
> - eDP: only a drm_panel wrapped into the panel bridge
>
> After this patch:
> - both DP and eDP support any chain of bridges attached.
>
>
> While the change means nothing for the DP (IIUC, it will not have any 
> bridges), it simplifies the code path, lowering the amount of checks.
>
> And for eDP this means that we can attach any eDP-to-something bridges 
> (e.g. NXP PTN3460).
>
>
> Well... After re-checking the source code for 
> devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably 
> revert removal of the check. The function will return -ENODEV if 
> neither bridge nor panel are specified.
>
I am new to drm and  confusing with bridge here.

Isn't bridge used to bridging two different kind of interface together?

for example, dsi <--> bridge <--> dp.

why edp need bridge here?

Can you give me more info regrading what bridge try to do here.



>>
>>> -               rc = dp_parser_find_panel(parser);
>>> -               if (rc)
>>> -                       return rc;
>>> -       }
>>> +       rc = dp_parser_find_next_bridge(parser);
>>> +       if (rc)
>>> +               return rc;
>>>
>>>          /* Map the corresponding regulator information according to
>>>           * version. Currently, since we only have one supported 
>>> platform,
>
>

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

On Wed, 12 Jan 2022 at 02:12, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
>
> On 1/6/2022 9:26 PM, Dmitry Baryshkov wrote:
> > On 07/01/2022 06:42, Stephen Boyd wrote:
> >> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
> >>> Currently DP driver will allocate panel bridge for eDP panels.
> >>> Simplify this code to just check if there is any next bridge in the
> >>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
> >>> field to next_bridge accordingly.
> >>>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
> >>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
> >>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
> >>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
> >>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
> >>>   5 files changed, 13 insertions(+), 23 deletions(-)
> >>
> >> I like this one, it certainly makes it easier to understand.
> >>
> >>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> b/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> index a7acc23f742b..5de21f3d0812 100644
> >>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser
> >>> *parser, int connector_type)
> >>>          if (rc)
> >>>                  return rc;
> >>>
> >>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> >>
> >> It feels like this is on purpose, but I don't see any comment so I have
> >> no idea. I think qcom folks are concerned about changing how not eDP
> >> works. I'll have to test it out locally.
> >
> > Ah, another thing that should go into the commit message.
> >
> > Current situation:
> > - DP: no external bridges supported.
> > - eDP: only a drm_panel wrapped into the panel bridge
> >
> > After this patch:
> > - both DP and eDP support any chain of bridges attached.
> >
> >
> > While the change means nothing for the DP (IIUC, it will not have any
> > bridges), it simplifies the code path, lowering the amount of checks.
> >
> > And for eDP this means that we can attach any eDP-to-something bridges
> > (e.g. NXP PTN3460).
> >
> >
> > Well... After re-checking the source code for
> > devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably
> > revert removal of the check. The function will return -ENODEV if
> > neither bridge nor panel are specified.
> >
> I am new to drm and  confusing with bridge here.
>
> Isn't bridge used to bridging two different kind of interface together?
>
> for example, dsi <--> bridge <--> dp.
>
> why edp need bridge here?
>
> Can you give me more info regrading what bridge try to do here.

First, there are bridges converting the eDP interface to another
interface. The mentioned NXP PTN3460 converts (embedded) DisplayPort
to LVDS.

Second (and this is the case here) drm_bridge can be used to wrap
drm_panel (panel-bridge), so that the driver doesn't have to care
about the drm_panel interface.

Last, but not least, external display connectors can also be
abstracted as bridges (see display-connector.c).

This becomes even more appealing as the driver can then switch to
drm_bridge_connector, supporting any kinds of pipelines attached to
the encoder, supporting any kind of converters, panel or external
connector pipelines.Think about the following (sometimes crazy, but
possible) examples. With
drm_bridge/panel-bridge/display-connector/drm_bridge_connector there
is no difference for the driver at all:
- DP encoder ⇒ DP port ⇒ DP monitor
- DP encoder ⇒ DP connector supporting generic GPIO as HPD ⇒ DP port ⇒
DP monitor
- eDP encoder ⇒ eDP panel
- eDP encoder ⇒ ptn3460 ⇒ fixed LVDS panel
- eDP encoder ⇒ ptn3460 ⇒ LVDS connector with EDID lines for panel autodetect
- eDP encoder ⇒ ptn3460 ⇒ THC63LVD1024 ⇒ DPI panel.
- eDP encoder ⇒ LT8912 ⇒ DSI panel

> >>
> >>> -               rc = dp_parser_find_panel(parser);
> >>> -               if (rc)
> >>> -                       return rc;
> >>> -       }
> >>> +       rc = dp_parser_find_next_bridge(parser);
> >>> +       if (rc)
> >>> +               return rc;
> >>>
> >>>          /* Map the corresponding regulator information according to
> >>>           * version. Currently, since we only have one supported
> >>> platform,
> >
> >



-- 
With best wishes
Dmitry

Re: [RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

On Wed, 12 Jan 2022 at 02:12, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
>
> On 1/6/2022 9:26 PM, Dmitry Baryshkov wrote:
> > On 07/01/2022 06:42, Stephen Boyd wrote:
> >> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
> >>> Currently DP driver will allocate panel bridge for eDP panels.
> >>> Simplify this code to just check if there is any next bridge in the
> >>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
> >>> field to next_bridge accordingly.
> >>>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
> >>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
> >>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
> >>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
> >>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
> >>>   5 files changed, 13 insertions(+), 23 deletions(-)
> >>
> >> I like this one, it certainly makes it easier to understand.
> >>
> >>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> b/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> index a7acc23f742b..5de21f3d0812 100644
> >>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> >>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser
> >>> *parser, int connector_type)
> >>>          if (rc)
> >>>                  return rc;
> >>>
> >>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> >>
> >> It feels like this is on purpose, but I don't see any comment so I have
> >> no idea. I think qcom folks are concerned about changing how not eDP
> >> works. I'll have to test it out locally.
> >
> > Ah, another thing that should go into the commit message.
> >
> > Current situation:
> > - DP: no external bridges supported.
> > - eDP: only a drm_panel wrapped into the panel bridge
> >
> > After this patch:
> > - both DP and eDP support any chain of bridges attached.
> >
> >
> > While the change means nothing for the DP (IIUC, it will not have any
> > bridges), it simplifies the code path, lowering the amount of checks.
> >
> > And for eDP this means that we can attach any eDP-to-something bridges
> > (e.g. NXP PTN3460).
> >
> >
> > Well... After re-checking the source code for
> > devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably
> > revert removal of the check. The function will return -ENODEV if
> > neither bridge nor panel are specified.
> >
> I am new to drm and  confusing with bridge here.
>
> Isn't bridge used to bridging two different kind of interface together?
>
> for example, dsi <--> bridge <--> dp.
>
> why edp need bridge here?
>
> Can you give me more info regrading what bridge try to do here.

First, there are bridges converting the eDP interface to another
interface. The mentioned NXP PTN3460 converts (embedded) DisplayPort
to LVDS.

Second (and this is the case here) drm_bridge can be used to wrap
drm_panel (panel-bridge), so that the driver doesn't have to care
about the drm_panel interface.

Last, but not least, external display connectors can also be
abstracted as bridges (see display-connector.c).

This becomes even more appealing as the driver can then switch to
drm_bridge_connector, supporting any kinds of pipelines attached to
the encoder, supporting any kind of converters, panel or external
connector pipelines.Think about the following (sometimes crazy, but
possible) examples. With
drm_bridge/panel-bridge/display-connector/drm_bridge_connector there
is no difference for the driver at all:
- DP encoder ⇒ DP port ⇒ DP monitor
- DP encoder ⇒ DP connector supporting generic GPIO as HPD ⇒ DP port ⇒
DP monitor
- eDP encoder ⇒ eDP panel
- eDP encoder ⇒ ptn3460 ⇒ fixed LVDS panel
- eDP encoder ⇒ ptn3460 ⇒ LVDS connector with EDID lines for panel autodetect
- eDP encoder ⇒ ptn3460 ⇒ THC63LVD1024 ⇒ DPI panel.
- eDP encoder ⇒ LT8912 ⇒ DSI panel

> >>
> >>> -               rc = dp_parser_find_panel(parser);
> >>> -               if (rc)
> >>> -                       return rc;
> >>> -       }
> >>> +       rc = dp_parser_find_next_bridge(parser);
> >>> +       if (rc)
> >>> +               return rc;
> >>>
> >>>          /* Map the corresponding regulator information according to
> >>>           * version. Currently, since we only have one supported
> >>> platform,
> >
> >



-- 
With best wishes
Dmitry

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Kuogee Hsieh]

On 1/6/2022 6:01 PM, Dmitry Baryshkov wrote:
> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> enable and disable") the DP driver received a drm_bridge instance, which
> is always attached to the encoder as a root bridge. However it conflicts
> with the panel_bridge support for eDP panels. Change panel_bridge
> attachment to come after dp_bridge attachment.
>
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> index d4d360d19eba..26ef41a4c1b6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -169,16 +169,6 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
>   
>   	drm_connector_attach_encoder(connector, dp_display->encoder);
>   
> -	if (dp_display->panel_bridge) {
> -		ret = drm_bridge_attach(dp_display->encoder,
> -					dp_display->panel_bridge, NULL,
> -					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> -		if (ret < 0) {
> -			DRM_ERROR("failed to attach panel bridge: %d\n", ret);
> -			return ERR_PTR(ret);
> -		}
> -	}
> -
>   	return connector;
>   }
>   
> @@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
>   		return ERR_PTR(rc);
>   	}
>   

can check connector_type here and if connector_type == 
DRM_MODE_CONNECTOR_eDP then no drm_bridge  add to eDP?  So that eDP only 
has panel_bridge and DP only has drm_bridge?

is this fix all your concerns?


> +	if (dp_display->panel_bridge) {
> +		rc = drm_bridge_attach(dp_display->encoder,
> +					dp_display->panel_bridge, bridge,
> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +		if (rc < 0) {
> +			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
> +			drm_bridge_remove(bridge);
> +			return ERR_PTR(rc);
> +		}
> +	}
> +
>   	return bridge;
>   }

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Kuogee Hsieh]

On 1/6/2022 6:01 PM, Dmitry Baryshkov wrote:
> In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> enable and disable") the DP driver received a drm_bridge instance, which
> is always attached to the encoder as a root bridge. However it conflicts
> with the panel_bridge support for eDP panels. Change panel_bridge
> attachment to come after dp_bridge attachment.
>
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> index d4d360d19eba..26ef41a4c1b6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> @@ -169,16 +169,6 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
>   
>   	drm_connector_attach_encoder(connector, dp_display->encoder);
>   
> -	if (dp_display->panel_bridge) {
> -		ret = drm_bridge_attach(dp_display->encoder,
> -					dp_display->panel_bridge, NULL,
> -					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> -		if (ret < 0) {
> -			DRM_ERROR("failed to attach panel bridge: %d\n", ret);
> -			return ERR_PTR(ret);
> -		}
> -	}
> -
>   	return connector;
>   }
>   
> @@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
>   		return ERR_PTR(rc);
>   	}
>   

can check connector_type here and if connector_type == 
DRM_MODE_CONNECTOR_eDP then no drm_bridge  add to eDP?  So that eDP only 
has panel_bridge and DP only has drm_bridge?

is this fix all your concerns?


> +	if (dp_display->panel_bridge) {
> +		rc = drm_bridge_attach(dp_display->encoder,
> +					dp_display->panel_bridge, bridge,
> +					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +		if (rc < 0) {
> +			DRM_ERROR("failed to attach panel bridge: %d\n", rc);
> +			drm_bridge_remove(bridge);
> +			return ERR_PTR(rc);
> +		}
> +	}
> +
>   	return bridge;
>   }

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

On Wed, 12 Jan 2022 at 20:41, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
>
> On 1/6/2022 6:01 PM, Dmitry Baryshkov wrote:
> > In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> > enable and disable") the DP driver received a drm_bridge instance, which
> > is always attached to the encoder as a root bridge. However it conflicts
> > with the panel_bridge support for eDP panels. Change panel_bridge
> > attachment to come after dp_bridge attachment.
> >
> > Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> > Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> > index d4d360d19eba..26ef41a4c1b6 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> > @@ -169,16 +169,6 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
> >
> >       drm_connector_attach_encoder(connector, dp_display->encoder);
> >
> > -     if (dp_display->panel_bridge) {
> > -             ret = drm_bridge_attach(dp_display->encoder,
> > -                                     dp_display->panel_bridge, NULL,
> > -                                     DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > -             if (ret < 0) {
> > -                     DRM_ERROR("failed to attach panel bridge: %d\n", ret);
> > -                     return ERR_PTR(ret);
> > -             }
> > -     }
> > -
> >       return connector;
> >   }
> >
> > @@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
> >               return ERR_PTR(rc);
> >       }
> >
>
> can check connector_type here and if connector_type ==
> DRM_MODE_CONNECTOR_eDP then no drm_bridge  add to eDP?  So that eDP only
> has panel_bridge and DP only has drm_bridge?

No, we still need the DP bridge for the eDP. It handles modesetting,
enabling and disabling of the DP controller, etc.

>
> is this fix all your concerns?
>
>
> > +     if (dp_display->panel_bridge) {
> > +             rc = drm_bridge_attach(dp_display->encoder,
> > +                                     dp_display->panel_bridge, bridge,
> > +                                     DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > +             if (rc < 0) {
> > +                     DRM_ERROR("failed to attach panel bridge: %d\n", rc);
> > +                     drm_bridge_remove(bridge);
> > +                     return ERR_PTR(rc);
> > +             }
> > +     }
> > +
> >       return bridge;
> >   }



-- 
With best wishes
Dmitry

Re: [RFC PATCH 1/7] drm/msm/dp: fix panel bridge attachment

[Dmitry Baryshkov]

On Wed, 12 Jan 2022 at 20:41, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
>
> On 1/6/2022 6:01 PM, Dmitry Baryshkov wrote:
> > In commit 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display
> > enable and disable") the DP driver received a drm_bridge instance, which
> > is always attached to the encoder as a root bridge. However it conflicts
> > with the panel_bridge support for eDP panels. Change panel_bridge
> > attachment to come after dp_bridge attachment.
> >
> > Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> > Cc: Kuogee Hsieh <quic_khsieh@quicinc.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/dp/dp_drm.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> > index d4d360d19eba..26ef41a4c1b6 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> > @@ -169,16 +169,6 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
> >
> >       drm_connector_attach_encoder(connector, dp_display->encoder);
> >
> > -     if (dp_display->panel_bridge) {
> > -             ret = drm_bridge_attach(dp_display->encoder,
> > -                                     dp_display->panel_bridge, NULL,
> > -                                     DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > -             if (ret < 0) {
> > -                     DRM_ERROR("failed to attach panel bridge: %d\n", ret);
> > -                     return ERR_PTR(ret);
> > -             }
> > -     }
> > -
> >       return connector;
> >   }
> >
> > @@ -246,5 +236,16 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp *dp_display, struct drm_devi
> >               return ERR_PTR(rc);
> >       }
> >
>
> can check connector_type here and if connector_type ==
> DRM_MODE_CONNECTOR_eDP then no drm_bridge  add to eDP?  So that eDP only
> has panel_bridge and DP only has drm_bridge?

No, we still need the DP bridge for the eDP. It handles modesetting,
enabling and disabling of the DP controller, etc.

>
> is this fix all your concerns?
>
>
> > +     if (dp_display->panel_bridge) {
> > +             rc = drm_bridge_attach(dp_display->encoder,
> > +                                     dp_display->panel_bridge, bridge,
> > +                                     DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > +             if (rc < 0) {
> > +                     DRM_ERROR("failed to attach panel bridge: %d\n", rc);
> > +                     drm_bridge_remove(bridge);
> > +                     return ERR_PTR(rc);
> > +             }
> > +     }
> > +
> >       return bridge;
> >   }



-- 
With best wishes
Dmitry