[PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Damon Ding]

PATCH 1 is a small format optimization for struct analogid_dp_device.
PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
PATCH 3-6 are preparations for apply drm_bridge_connector helper.
PATCH 7 is to apply the drm_bridge_connector helper.
PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
PATCH 11-12 are preparations for apply panel_bridge helper.
PATCH 13 is to apply the panel_bridge helper.

Damon Ding (13):
  drm/bridge: analogix_dp: Formalize the struct analogix_dp_device
  drm/bridge: analogix_dp: Move &drm_bridge_funcs.mode_set to
    &drm_bridge_funcs.atomic_enable
  drm/bridge: analogix_dp: Add &analogix_dp_plat_data.next_bridge
  drm/exynos: exynos_dp: Remove &exynos_dp_device.ptn_bridge
  drm/bridge: exynos_dp: Remove unused &exynos_dp_device.connector
  drm/bridge: analogix_dp: Remove redundant
    &analogix_dp_plat_data.skip_connector
  drm/bridge: analogix_dp: Apply drm_bridge_connector helper
  drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
  drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()
  drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()
  drm/bridge: analogix_dp: Remove panel disabling and enabling in
    analogix_dp_set_bridge()
  drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing
    in analogix_dp_unbind()
  drm/bridge: analogix_dp: Apply panel_bridge helper

 .../drm/bridge/analogix/analogix_dp_core.c    | 384 ++++++++++--------
 .../drm/bridge/analogix/analogix_dp_core.h    |   5 +-
 drivers/gpu/drm/exynos/exynos_dp.c            |  48 +--
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  49 +--
 include/drm/bridge/analogix_dp.h              |   7 +-
 5 files changed, 248 insertions(+), 245 deletions(-)

---

Changes in v2:
- Update Exynos DP driver synchronously.
- Move the panel/bridge parsing to the Analogix side.

Changes in v3:
- Rebase for the existing devm_drm_bridge_alloc() applying commit.
- Fix the typographical error of panel/bridge check in exynos_dp_bind().
- Squash all commits related to skip_connector deletion in both Exynos and
  Analogix code into one.
- Apply panel_bridge helper to make the codes more concise.
- Fix the handing of bridge in analogix_dp_bridge_get_modes().
- Remove unnecessary parameter struct drm_connector* for callback
  &analogix_dp_plat_data.attach().
- In order to decouple the connector driver and the bridge driver, move
  the bridge connector initilization to the Rockchip and Exynos sides.

Changes in v4:
- Rebase for the applied &drm_bridge_funcs.detect() modification commit.
- Rename analogix_dp_find_panel_or_bridge() to analogix_dp_finish_probe().
- Drop the drmm_encoder_init() modification commit.
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge.

-- 
2.34.1

[PATCH v4 01/13] drm/bridge: analogix_dp: Formalize the struct analogix_dp_device

[Damon Ding]

Use the tap instead of the space for &analogix_dp_device.aux and
&analogix_dp_device.force_hpd.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index b86e93f30ed6..91b215c6a0cf 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -156,7 +156,7 @@ struct analogix_dp_device {
 	struct drm_device	*drm_dev;
 	struct drm_connector	connector;
 	struct drm_bridge	bridge;
-	struct drm_dp_aux       aux;
+	struct drm_dp_aux	aux;
 	struct clk		*clock;
 	unsigned int		irq;
 	void __iomem		*reg_base;
@@ -166,7 +166,7 @@ struct analogix_dp_device {
 	struct phy		*phy;
 	int			dpms_mode;
 	struct gpio_desc	*hpd_gpiod;
-	bool                    force_hpd;
+	bool			force_hpd;
 	bool			fast_train_enable;
 	bool			psr_supported;
 
-- 
2.34.1

[PATCH v4 02/13] drm/bridge: analogix_dp: Move &drm_bridge_funcs.mode_set to &drm_bridge_funcs.atomic_enable

[Damon Ding]

According to the include/drm/drm_bridge.h, the callback
&drm_bridge_funcs.mode_set is deprecated and it should be better to
include the mode setting in the &drm_bridge_funcs.atomic_enable instead.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 161 +++++++++---------
 1 file changed, 82 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index ed35e567d117..0106e7e0f093 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1177,12 +1177,88 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
 	return ret;
 }
 
+static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
+					const struct drm_display_mode *mode)
+{
+	struct analogix_dp_device *dp = to_dp(bridge);
+	struct drm_display_info *display_info = &dp->connector.display_info;
+	struct video_info *video = &dp->video_info;
+	struct device_node *dp_node = dp->dev->of_node;
+	int vic;
+
+	/* Input video interlaces & hsync pol & vsync pol */
+	video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
+	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
+	video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
+
+	/* Input video dynamic_range & colorimetry */
+	vic = drm_match_cea_mode(mode);
+	if ((vic == 6) || (vic == 7) || (vic == 21) || (vic == 22) ||
+	    (vic == 2) || (vic == 3) || (vic == 17) || (vic == 18)) {
+		video->dynamic_range = CEA;
+		video->ycbcr_coeff = COLOR_YCBCR601;
+	} else if (vic) {
+		video->dynamic_range = CEA;
+		video->ycbcr_coeff = COLOR_YCBCR709;
+	} else {
+		video->dynamic_range = VESA;
+		video->ycbcr_coeff = COLOR_YCBCR709;
+	}
+
+	/* Input vide bpc and color_formats */
+	switch (display_info->bpc) {
+	case 12:
+		video->color_depth = COLOR_12;
+		break;
+	case 10:
+		video->color_depth = COLOR_10;
+		break;
+	case 8:
+		video->color_depth = COLOR_8;
+		break;
+	case 6:
+		video->color_depth = COLOR_6;
+		break;
+	default:
+		video->color_depth = COLOR_8;
+		break;
+	}
+	if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+		video->color_space = COLOR_YCBCR444;
+	else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+		video->color_space = COLOR_YCBCR422;
+	else
+		video->color_space = COLOR_RGB;
+
+	/*
+	 * NOTE: those property parsing code is used for providing backward
+	 * compatibility for samsung platform.
+	 * Due to we used the "of_property_read_u32" interfaces, when this
+	 * property isn't present, the "video_info" can keep the original
+	 * values and wouldn't be modified.
+	 */
+	of_property_read_u32(dp_node, "samsung,color-space",
+			     &video->color_space);
+	of_property_read_u32(dp_node, "samsung,dynamic-range",
+			     &video->dynamic_range);
+	of_property_read_u32(dp_node, "samsung,ycbcr-coeff",
+			     &video->ycbcr_coeff);
+	of_property_read_u32(dp_node, "samsung,color-depth",
+			     &video->color_depth);
+	if (of_property_read_bool(dp_node, "hsync-active-high"))
+		video->h_sync_polarity = true;
+	if (of_property_read_bool(dp_node, "vsync-active-high"))
+		video->v_sync_polarity = true;
+	if (of_property_read_bool(dp_node, "interlaced"))
+		video->interlaced = true;
+}
+
 static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 					     struct drm_atomic_state *old_state)
 {
 	struct analogix_dp_device *dp = to_dp(bridge);
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	int timeout_loop = 0;
 	int ret;
 
@@ -1190,6 +1266,11 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 	if (!crtc)
 		return;
 
+	new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc);
+	if (!new_crtc_state)
+		return;
+	analogix_dp_bridge_mode_set(bridge, &new_crtc_state->adjusted_mode);
+
 	old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc);
 	/* Not a full enable, just disable PSR and continue */
 	if (old_crtc_state && old_crtc_state->self_refresh_active) {
@@ -1296,83 +1377,6 @@ static void analogix_dp_bridge_atomic_post_disable(struct drm_bridge *bridge,
 		DRM_ERROR("Failed to enable psr (%d)\n", ret);
 }
 
-static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
-				const struct drm_display_mode *orig_mode,
-				const struct drm_display_mode *mode)
-{
-	struct analogix_dp_device *dp = to_dp(bridge);
-	struct drm_display_info *display_info = &dp->connector.display_info;
-	struct video_info *video = &dp->video_info;
-	struct device_node *dp_node = dp->dev->of_node;
-	int vic;
-
-	/* Input video interlaces & hsync pol & vsync pol */
-	video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
-	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
-	video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
-
-	/* Input video dynamic_range & colorimetry */
-	vic = drm_match_cea_mode(mode);
-	if ((vic == 6) || (vic == 7) || (vic == 21) || (vic == 22) ||
-	    (vic == 2) || (vic == 3) || (vic == 17) || (vic == 18)) {
-		video->dynamic_range = CEA;
-		video->ycbcr_coeff = COLOR_YCBCR601;
-	} else if (vic) {
-		video->dynamic_range = CEA;
-		video->ycbcr_coeff = COLOR_YCBCR709;
-	} else {
-		video->dynamic_range = VESA;
-		video->ycbcr_coeff = COLOR_YCBCR709;
-	}
-
-	/* Input vide bpc and color_formats */
-	switch (display_info->bpc) {
-	case 12:
-		video->color_depth = COLOR_12;
-		break;
-	case 10:
-		video->color_depth = COLOR_10;
-		break;
-	case 8:
-		video->color_depth = COLOR_8;
-		break;
-	case 6:
-		video->color_depth = COLOR_6;
-		break;
-	default:
-		video->color_depth = COLOR_8;
-		break;
-	}
-	if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
-		video->color_space = COLOR_YCBCR444;
-	else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
-		video->color_space = COLOR_YCBCR422;
-	else
-		video->color_space = COLOR_RGB;
-
-	/*
-	 * NOTE: those property parsing code is used for providing backward
-	 * compatibility for samsung platform.
-	 * Due to we used the "of_property_read_u32" interfaces, when this
-	 * property isn't present, the "video_info" can keep the original
-	 * values and wouldn't be modified.
-	 */
-	of_property_read_u32(dp_node, "samsung,color-space",
-			     &video->color_space);
-	of_property_read_u32(dp_node, "samsung,dynamic-range",
-			     &video->dynamic_range);
-	of_property_read_u32(dp_node, "samsung,ycbcr-coeff",
-			     &video->ycbcr_coeff);
-	of_property_read_u32(dp_node, "samsung,color-depth",
-			     &video->color_depth);
-	if (of_property_read_bool(dp_node, "hsync-active-high"))
-		video->h_sync_polarity = true;
-	if (of_property_read_bool(dp_node, "vsync-active-high"))
-		video->v_sync_polarity = true;
-	if (of_property_read_bool(dp_node, "interlaced"))
-		video->interlaced = true;
-}
-
 static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
@@ -1381,7 +1385,6 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
 	.atomic_enable = analogix_dp_bridge_atomic_enable,
 	.atomic_disable = analogix_dp_bridge_atomic_disable,
 	.atomic_post_disable = analogix_dp_bridge_atomic_post_disable,
-	.mode_set = analogix_dp_bridge_mode_set,
 	.attach = analogix_dp_bridge_attach,
 };
 
-- 
2.34.1

[PATCH v4 03/13] drm/bridge: analogix_dp: Add &analogix_dp_plat_data.next_bridge

[Damon Ding]

In order to move the panel/bridge parsing and attachmenet to the
Analogix side, add component struct drm_bridge *next_bridge to
platform data struct analogix_dp_plat_data.

The movement makes sense because the panel/bridge should logically
be positioned behind the Analogix bridge in the display pipeline.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

---

Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge
---
 include/drm/bridge/analogix_dp.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index cf17646c1310..582357c20640 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -27,6 +27,7 @@ static inline bool is_rockchip(enum analogix_dp_devtype type)
 struct analogix_dp_plat_data {
 	enum analogix_dp_devtype dev_type;
 	struct drm_panel *panel;
+	struct drm_bridge *next_bridge;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 	bool skip_connector;
-- 
2.34.1

[PATCH v4 04/13] drm/exynos: exynos_dp: Remove &exynos_dp_device.ptn_bridge

[Damon Ding]

Use &analogix_dp_plat_data.bridge instead of &exynos_dp_device.ptn_bridge
directly.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

------

Changes in v3:
- Fix the typographical error for &dp->plat_data.bridge.

Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge.
---
 drivers/gpu/drm/exynos/exynos_dp.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 5bcf41e0bd04..f469ac5b3c2a 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -36,7 +36,6 @@
 struct exynos_dp_device {
 	struct drm_encoder         encoder;
 	struct drm_connector       *connector;
-	struct drm_bridge          *ptn_bridge;
 	struct drm_device          *drm_dev;
 	struct device              *dev;
 
@@ -106,8 +105,8 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
 	dp->connector = connector;
 
 	/* Pre-empt DP connector creation if there's a bridge */
-	if (dp->ptn_bridge) {
-		ret = drm_bridge_attach(&dp->encoder, dp->ptn_bridge, bridge,
+	if (plat_data->next_bridge) {
+		ret = drm_bridge_attach(&dp->encoder, plat_data->next_bridge, bridge,
 					0);
 		if (ret)
 			return ret;
@@ -155,7 +154,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 
 	dp->drm_dev = drm_dev;
 
-	if (!dp->plat_data.panel && !dp->ptn_bridge) {
+	if (!dp->plat_data.panel && !dp->plat_data.next_bridge) {
 		ret = exynos_dp_dt_parse_panel(dp);
 		if (ret)
 			return ret;
@@ -232,6 +231,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
 
 	/* The remote port can be either a panel or a bridge */
 	dp->plat_data.panel = panel;
+	dp->plat_data.next_bridge = bridge;
 	dp->plat_data.dev_type = EXYNOS_DP;
 	dp->plat_data.power_on = exynos_dp_poweron;
 	dp->plat_data.power_off = exynos_dp_poweroff;
@@ -239,8 +239,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
 	dp->plat_data.get_modes = exynos_dp_get_modes;
 	dp->plat_data.skip_connector = !!bridge;
 
-	dp->ptn_bridge = bridge;
-
 out:
 	dp->adp = analogix_dp_probe(dev, &dp->plat_data);
 	if (IS_ERR(dp->adp))
-- 
2.34.1

[PATCH v4 05/13] drm/bridge: exynos_dp: Remove unused &exynos_dp_device.connector

[Damon Ding]

The &exynos_dp_device.connector is assigned in exynos_dp_bridge_attach()
but never used. It should make sense to remove it.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/exynos/exynos_dp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index f469ac5b3c2a..e20513164032 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -35,7 +35,6 @@
 
 struct exynos_dp_device {
 	struct drm_encoder         encoder;
-	struct drm_connector       *connector;
 	struct drm_device          *drm_dev;
 	struct device              *dev;
 
@@ -102,8 +101,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
 	struct exynos_dp_device *dp = to_dp(plat_data);
 	int ret;
 
-	dp->connector = connector;
-
 	/* Pre-empt DP connector creation if there's a bridge */
 	if (plat_data->next_bridge) {
 		ret = drm_bridge_attach(&dp->encoder, plat_data->next_bridge, bridge,
-- 
2.34.1

[PATCH v4 06/13] drm/bridge: analogix_dp: Remove redundant &analogix_dp_plat_data.skip_connector

[Damon Ding]

The &analogix_dp_plat_data.skip_connector related check can be replaced
by &analogix_dp_plat_data.bridge.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

------

Changes in v3:
- Squash the Exynos side commit and the Analogix side commit together.

Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge.
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 +-
 drivers/gpu/drm/exynos/exynos_dp.c                 | 1 -
 include/drm/bridge/analogix_dp.h                   | 1 -
 3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 0106e7e0f093..7876b310aaed 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1050,7 +1050,7 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge,
 		return -EINVAL;
 	}
 
-	if (!dp->plat_data->skip_connector) {
+	if (!dp->plat_data->next_bridge) {
 		connector = &dp->connector;
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
 
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index e20513164032..702128d76ae3 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -234,7 +234,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
 	dp->plat_data.power_off = exynos_dp_poweroff;
 	dp->plat_data.attach = exynos_dp_bridge_attach;
 	dp->plat_data.get_modes = exynos_dp_get_modes;
-	dp->plat_data.skip_connector = !!bridge;
 
 out:
 	dp->adp = analogix_dp_probe(dev, &dp->plat_data);
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 582357c20640..f06da105d8f2 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -30,7 +30,6 @@ struct analogix_dp_plat_data {
 	struct drm_bridge *next_bridge;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
-	bool skip_connector;
 
 	int (*power_on)(struct analogix_dp_plat_data *);
 	int (*power_off)(struct analogix_dp_plat_data *);
-- 
2.34.1

[PATCH v4 07/13] drm/bridge: analogix_dp: Apply drm_bridge_connector helper

[Damon Ding]

Apply drm_bridge_connector helper for Analogix DP driver.

The following changes have been made:
- Apply drm_bridge_connector helper to get rid of &drm_connector_funcs
  and &drm_connector_helper_funcs.
- Remove unnecessary parameter struct drm_connector* for callback
  &analogix_dp_plat_data.attach.
- Remove &analogix_dp_device.connector.
- Convert analogix_dp_atomic_check()/analogix_dp_detect() to
  &drm_bridge_funcs.atomic_check()/&drm_bridge_funcs.detect().
- Split analogix_dp_get_modes() into &drm_bridge_funcs.get_modes() and
  &drm_bridge_funcs.edid_read().

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

------

Changes in v2:
- For &drm_bridge.ops, remove DRM_BRIDGE_OP_HPD and add
  DRM_BRIDGE_OP_EDID.
- Add analogix_dp_bridge_edid_read().
- Move &analogix_dp_plat_data.skip_connector deletion to the previous
  patches.

Changes in v3:
- Rebase with the new devm_drm_bridge_alloc() related commit
  48f05c3b4b70 ("drm/bridge: analogix_dp: Use devm_drm_bridge_alloc()
  API").
- Expand the commit message.
- Call drm_bridge_get_modes() in analogix_dp_bridge_get_modes() if the
  bridge is available.
- Remove unnecessary parameter struct drm_connector* for callback
  &analogix_dp_plat_data.attach.
- In order to decouple the connector driver and the bridge driver, move
  the bridge connector initilization to the Rockchip and Exynos sides.

Changes in v4:
- Expand analogix_dp_bridge_detect() parameters to &drm_bridge and
  &drm_connector.
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge.
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 145 ++++++++----------
 .../drm/bridge/analogix/analogix_dp_core.h    |   1 -
 drivers/gpu/drm/exynos/exynos_dp.c            |  18 ++-
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  11 +-
 include/drm/bridge/analogix_dp.h              |   3 +-
 5 files changed, 88 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 7876b310aaed..a8ed44ec8ef5 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -947,24 +947,16 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
 	return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
 }
 
-static int analogix_dp_get_modes(struct drm_connector *connector)
+static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
 {
-	struct analogix_dp_device *dp = to_dp(connector);
-	const struct drm_edid *drm_edid;
+	struct analogix_dp_device *dp = to_dp(bridge);
 	int num_modes = 0;
 
-	if (dp->plat_data->panel) {
+	if (dp->plat_data->panel)
 		num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
-	} else {
-		drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
 
-		drm_edid_connector_update(&dp->connector, drm_edid);
-
-		if (drm_edid) {
-			num_modes += drm_edid_connector_add_modes(&dp->connector);
-			drm_edid_free(drm_edid);
-		}
-	}
+	if (dp->plat_data->next_bridge)
+		num_modes += drm_bridge_get_modes(dp->plat_data->next_bridge, connector);
 
 	if (dp->plat_data->get_modes)
 		num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
@@ -972,51 +964,39 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
 	return num_modes;
 }
 
-static struct drm_encoder *
-analogix_dp_best_encoder(struct drm_connector *connector)
+static const struct drm_edid *analogix_dp_bridge_edid_read(struct drm_bridge *bridge,
+							   struct drm_connector *connector)
 {
-	struct analogix_dp_device *dp = to_dp(connector);
+	struct analogix_dp_device *dp = to_dp(bridge);
+	const struct drm_edid *drm_edid = NULL;
 
-	return dp->encoder;
-}
+	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
 
+	if (dp->plat_data->get_modes)
+		dp->plat_data->get_modes(dp->plat_data, connector);
 
-static int analogix_dp_atomic_check(struct drm_connector *connector,
-				    struct drm_atomic_state *state)
-{
-	struct analogix_dp_device *dp = to_dp(connector);
-	struct drm_connector_state *conn_state;
-	struct drm_crtc_state *crtc_state;
+	return drm_edid;
+}
 
-	conn_state = drm_atomic_get_new_connector_state(state, connector);
-	if (WARN_ON(!conn_state))
-		return -ENODEV;
+static int analogix_dp_bridge_atomic_check(struct drm_bridge *bridge,
+					   struct drm_bridge_state *bridge_state,
+					   struct drm_crtc_state *crtc_state,
+					   struct drm_connector_state *conn_state)
+{
+	struct analogix_dp_device *dp = to_dp(bridge);
 
 	conn_state->self_refresh_aware = true;
 
-	if (!conn_state->crtc)
-		return 0;
-
-	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
-	if (!crtc_state)
-		return 0;
-
 	if (crtc_state->self_refresh_active && !dp->psr_supported)
 		return -EINVAL;
 
 	return 0;
 }
 
-static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
-	.get_modes = analogix_dp_get_modes,
-	.best_encoder = analogix_dp_best_encoder,
-	.atomic_check = analogix_dp_atomic_check,
-};
-
 static enum drm_connector_status
-analogix_dp_detect(struct drm_connector *connector, bool force)
+analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 {
-	struct analogix_dp_device *dp = to_dp(connector);
+	struct analogix_dp_device *dp = to_dp(bridge);
 	enum drm_connector_status status = connector_status_disconnected;
 
 	if (dp->plat_data->panel)
@@ -1028,21 +1008,11 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
 	return status;
 }
 
-static const struct drm_connector_funcs analogix_dp_connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.detect = analogix_dp_detect,
-	.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 int analogix_dp_bridge_attach(struct drm_bridge *bridge,
 				     struct drm_encoder *encoder,
 				     enum drm_bridge_attach_flags flags)
 {
 	struct analogix_dp_device *dp = to_dp(bridge);
-	struct drm_connector *connector = NULL;
 	int ret = 0;
 
 	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
@@ -1050,31 +1020,8 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge,
 		return -EINVAL;
 	}
 
-	if (!dp->plat_data->next_bridge) {
-		connector = &dp->connector;
-		connector->polled = DRM_CONNECTOR_POLL_HPD;
-
-		ret = drm_connector_init(dp->drm_dev, connector,
-					 &analogix_dp_connector_funcs,
-					 DRM_MODE_CONNECTOR_eDP);
-		if (ret) {
-			DRM_ERROR("Failed to initialize connector with drm\n");
-			return ret;
-		}
-
-		drm_connector_helper_add(connector,
-					 &analogix_dp_connector_helper_funcs);
-		drm_connector_attach_encoder(connector, encoder);
-	}
-
-	/*
-	 * NOTE: the connector registration is implemented in analogix
-	 * platform driver, that to say connector would be exist after
-	 * plat_data->attch return, that's why we record the connector
-	 * point after plat attached.
-	 */
 	if (dp->plat_data->attach) {
-		ret = dp->plat_data->attach(dp->plat_data, bridge, connector);
+		ret = dp->plat_data->attach(dp->plat_data, bridge);
 		if (ret) {
 			DRM_ERROR("Failed at platform attach func\n");
 			return ret;
@@ -1178,14 +1125,21 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
 }
 
 static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
+					struct drm_atomic_state *state,
 					const struct drm_display_mode *mode)
 {
 	struct analogix_dp_device *dp = to_dp(bridge);
-	struct drm_display_info *display_info = &dp->connector.display_info;
 	struct video_info *video = &dp->video_info;
 	struct device_node *dp_node = dp->dev->of_node;
+	struct drm_connector *connector;
+	struct drm_display_info *display_info;
 	int vic;
 
+	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	if (!connector)
+		return;
+	display_info = &connector->display_info;
+
 	/* Input video interlaces & hsync pol & vsync pol */
 	video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
 	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
@@ -1269,7 +1223,7 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 	new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc);
 	if (!new_crtc_state)
 		return;
-	analogix_dp_bridge_mode_set(bridge, &new_crtc_state->adjusted_mode);
+	analogix_dp_bridge_mode_set(bridge, old_state, &new_crtc_state->adjusted_mode);
 
 	old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc);
 	/* Not a full enable, just disable PSR and continue */
@@ -1385,7 +1339,11 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
 	.atomic_enable = analogix_dp_bridge_atomic_enable,
 	.atomic_disable = analogix_dp_bridge_atomic_disable,
 	.atomic_post_disable = analogix_dp_bridge_atomic_post_disable,
+	.atomic_check = analogix_dp_bridge_atomic_check,
 	.attach = analogix_dp_bridge_attach,
+	.get_modes = analogix_dp_bridge_get_modes,
+	.edid_read = analogix_dp_bridge_edid_read,
+	.detect = analogix_dp_bridge_detect,
 };
 
 static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
@@ -1615,6 +1573,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_resume);
 
 int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 {
+	struct drm_bridge *bridge = &dp->bridge;
 	int ret;
 
 	dp->drm_dev = drm_dev;
@@ -1628,7 +1587,16 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 		return ret;
 	}
 
-	ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);
+	bridge->ops = DRM_BRIDGE_OP_DETECT |
+		      DRM_BRIDGE_OP_EDID |
+		      DRM_BRIDGE_OP_MODES;
+	bridge->of_node = dp->dev->of_node;
+	bridge->type = DRM_MODE_CONNECTOR_eDP;
+	ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
+	if (ret)
+		goto err_unregister_aux;
+
+	ret = drm_bridge_attach(dp->encoder, bridge, NULL, 0);
 	if (ret) {
 		DRM_ERROR("failed to create bridge (%d)\n", ret);
 		goto err_unregister_aux;
@@ -1646,7 +1614,6 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
 void analogix_dp_unbind(struct analogix_dp_device *dp)
 {
 	analogix_dp_bridge_disable(&dp->bridge);
-	dp->connector.funcs->destroy(&dp->connector);
 
 	drm_panel_unprepare(dp->plat_data->panel);
 
@@ -1656,7 +1623,8 @@ EXPORT_SYMBOL_GPL(analogix_dp_unbind);
 
 int analogix_dp_start_crc(struct drm_connector *connector)
 {
-	struct analogix_dp_device *dp = to_dp(connector);
+	struct analogix_dp_device *dp;
+	struct drm_bridge *bridge;
 
 	if (!connector->state->crtc) {
 		DRM_ERROR("Connector %s doesn't currently have a CRTC.\n",
@@ -1664,13 +1632,26 @@ int analogix_dp_start_crc(struct drm_connector *connector)
 		return -EINVAL;
 	}
 
+	bridge = drm_bridge_chain_get_first_bridge(connector->encoder);
+	if (bridge->type != DRM_MODE_CONNECTOR_eDP)
+		return -EINVAL;
+
+	dp = to_dp(bridge);
+
 	return drm_dp_start_crc(&dp->aux, connector->state->crtc);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_start_crc);
 
 int analogix_dp_stop_crc(struct drm_connector *connector)
 {
-	struct analogix_dp_device *dp = to_dp(connector);
+	struct analogix_dp_device *dp;
+	struct drm_bridge *bridge;
+
+	bridge = drm_bridge_chain_get_first_bridge(connector->encoder);
+	if (bridge->type != DRM_MODE_CONNECTOR_eDP)
+		return -EINVAL;
+
+	dp = to_dp(bridge);
 
 	return drm_dp_stop_crc(&dp->aux);
 }
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 91b215c6a0cf..17347448c6b0 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -154,7 +154,6 @@ struct analogix_dp_device {
 	struct drm_encoder	*encoder;
 	struct device		*dev;
 	struct drm_device	*drm_dev;
-	struct drm_connector	connector;
 	struct drm_bridge	bridge;
 	struct drm_dp_aux	aux;
 	struct clk		*clock;
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 702128d76ae3..65579873ceea 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -21,6 +21,7 @@
 #include <drm/bridge/analogix_dp.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
@@ -95,8 +96,7 @@ static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data,
 }
 
 static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
-				   struct drm_bridge *bridge,
-				   struct drm_connector *connector)
+				   struct drm_bridge *bridge)
 {
 	struct exynos_dp_device *dp = to_dp(plat_data);
 	int ret;
@@ -147,6 +147,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
 	struct drm_encoder *encoder = &dp->encoder;
 	struct drm_device *drm_dev = data;
+	struct drm_connector *connector;
 	int ret;
 
 	dp->drm_dev = drm_dev;
@@ -168,10 +169,19 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 	dp->plat_data.encoder = encoder;
 
 	ret = analogix_dp_bind(dp->adp, dp->drm_dev);
-	if (ret)
+	if (ret) {
 		dp->encoder.funcs->destroy(&dp->encoder);
+		return ret;
+	}
+
+	connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
+	if (IS_ERR(connector)) {
+		ret = PTR_ERR(connector);
+		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
+		return ret;
+	}
 
-	return ret;
+	return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
 }
 
 static void exynos_dp_unbind(struct device *dev, struct device *master,
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index d30f0983a53a..87dfb48206db 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -25,6 +25,7 @@
 #include <drm/display/drm_dp_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/bridge/analogix_dp.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
@@ -394,6 +395,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
 {
 	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
 	struct drm_device *drm_dev = data;
+	struct drm_connector *connector;
 	int ret;
 
 	dp->drm_dev = drm_dev;
@@ -413,7 +415,14 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
 	if (ret)
 		goto err_cleanup_encoder;
 
-	return 0;
+	connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
+	if (IS_ERR(connector)) {
+		ret = PTR_ERR(connector);
+		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
+		goto err_cleanup_encoder;
+	}
+
+	return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
 err_cleanup_encoder:
 	dp->encoder.encoder.funcs->destroy(&dp->encoder.encoder);
 	return ret;
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index f06da105d8f2..ffc05f3de232 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -33,8 +33,7 @@ struct analogix_dp_plat_data {
 
 	int (*power_on)(struct analogix_dp_plat_data *);
 	int (*power_off)(struct analogix_dp_plat_data *);
-	int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *,
-		      struct drm_connector *);
+	int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *);
 	int (*get_modes)(struct analogix_dp_plat_data *,
 			 struct drm_connector *);
 };
-- 
2.34.1

[PATCH v4 08/13] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()

[Damon Ding]

Since the panel/bridge should logically be positioned behind the
Analogix bridge in the display pipeline, it makes sense to handle
the panel/bridge parsing on the Analogix side. Therefore, we add
a new API analogix_dp_finish_probe(), which combines the panel/bridge
parsing with component addition, to do it.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge.
- Remame API analogix_dp_find_panel_or_bridge() to
  analogix_dp_finish_probe().
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 48 +++++++++++++++++++
 include/drm/bridge/analogix_dp.h              |  2 +
 2 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index a8ed44ec8ef5..3e76a7b7d227 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -20,12 +20,14 @@
 #include <linux/platform_device.h>
 
 #include <drm/bridge/analogix_dp.h>
+#include <drm/display/drm_dp_aux_bus.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
@@ -1671,6 +1673,52 @@ struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp)
 }
 EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
 
+static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
+{
+	struct analogix_dp_device *dp = to_dp(aux);
+	struct analogix_dp_plat_data *plat_data = dp->plat_data;
+	int port = plat_data->dev_type == EXYNOS_DP ? 0 : 1;
+	int ret;
+
+	/*
+	 * If drm_of_find_panel_or_bridge() returns -ENODEV, there may be no valid panel
+	 * or bridge nodes. The driver should go on for the driver-free bridge or the DP
+	 * mode applications.
+	 */
+	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, port, 0,
+					  &plat_data->panel, &plat_data->next_bridge);
+	if (ret && ret != -ENODEV)
+		return ret;
+
+	return component_add(dp->dev, plat_data->ops);
+}
+
+int analogix_dp_finish_probe(struct analogix_dp_device *dp)
+{
+	int ret;
+
+	ret = devm_of_dp_aux_populate_bus(&dp->aux, analogix_dp_aux_done_probing);
+	if (ret) {
+		/*
+		 * If devm_of_dp_aux_populate_bus() returns -ENODEV, the done_probing() will
+		 * not be called because there are no EP devices. Then the callback function
+		 * analogix_dp_aux_done_probing() will be called directly in order to support
+		 * the other valid DT configurations.
+		 *
+		 * NOTE: The devm_of_dp_aux_populate_bus() is allowed to return -EPROBE_DEFER.
+		 */
+		if (ret != -ENODEV) {
+			dev_err(dp->dev, "failed to populate aux bus\n");
+			return ret;
+		}
+
+		return analogix_dp_aux_done_probing(&dp->aux);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
+
 MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
 MODULE_DESCRIPTION("Analogix DP Core Driver");
 MODULE_LICENSE("GPL v2");
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index ffc05f3de232..afd0cc33b54e 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -30,6 +30,7 @@ struct analogix_dp_plat_data {
 	struct drm_bridge *next_bridge;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
+	const struct component_ops *ops;
 
 	int (*power_on)(struct analogix_dp_plat_data *);
 	int (*power_off)(struct analogix_dp_plat_data *);
@@ -51,5 +52,6 @@ int analogix_dp_stop_crc(struct drm_connector *connector);
 
 struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct drm_dp_aux *aux);
 struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
+int analogix_dp_finish_probe(struct analogix_dp_device *dp);
 
 #endif /* _ANALOGIX_DP_H_ */
-- 
2.34.1

[PATCH v4 09/13] drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()

[Damon Ding]

Apply analogix_dp_finish_probe() in order to move the panel/bridge
parsing from Rockchip side to the Analogix side.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

---

Changes in v4:
- Rename analogix_dp_find_panel_or_bridge() to
  analogix_dp_finish_probe().
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 38 +------------------
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 87dfb48206db..0862b09a8be2 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -21,14 +21,12 @@
 #include <video/of_videomode.h>
 #include <video/videomode.h>
 
-#include <drm/display/drm_dp_aux_bus.h>
 #include <drm/display/drm_dp_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge_connector.h>
 #include <drm/bridge/analogix_dp.h>
 #include <drm/drm_of.h>
-#include <drm/drm_panel.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
@@ -442,24 +440,6 @@ static const struct component_ops rockchip_dp_component_ops = {
 	.unbind = rockchip_dp_unbind,
 };
 
-static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
-{
-	struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
-	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
-	int ret;
-
-	/*
-	 * If drm_of_find_panel_or_bridge() returns -ENODEV, there may be no valid panel
-	 * or bridge nodes. The driver should go on for the driver-free bridge or the DP
-	 * mode applications.
-	 */
-	ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
-	if (ret && ret != -ENODEV)
-		return ret;
-
-	return component_add(dp->dev, &rockchip_dp_component_ops);
-}
-
 static int rockchip_dp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -499,6 +479,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 	dp->plat_data.power_on = rockchip_dp_poweron;
 	dp->plat_data.power_off = rockchip_dp_powerdown;
 	dp->plat_data.get_modes = rockchip_dp_get_modes;
+	dp->plat_data.ops = &rockchip_dp_component_ops;
 
 	ret = rockchip_dp_of_probe(dp);
 	if (ret < 0)
@@ -510,22 +491,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
 	if (IS_ERR(dp->adp))
 		return PTR_ERR(dp->adp);
 
-	ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
-	if (ret) {
-		/*
-		 * If devm_of_dp_aux_populate_bus() returns -ENODEV, the done_probing() will not
-		 * be called because there are no EP devices. Then the rockchip_dp_link_panel()
-		 * will be called directly in order to support the other valid DT configurations.
-		 *
-		 * NOTE: The devm_of_dp_aux_populate_bus() is allowed to return -EPROBE_DEFER.
-		 */
-		if (ret != -ENODEV)
-			return dev_err_probe(dp->dev, ret, "failed to populate aux bus\n");
-
-		return rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
-	}
-
-	return 0;
+	return analogix_dp_finish_probe(dp->adp);
 }
 
 static void rockchip_dp_remove(struct platform_device *pdev)
-- 
2.34.1

[PATCH v4 10/13] drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()

[Damon Ding]

Apply analogix_dp_finish_probe() in order to move the panel/bridge
parsing from Exynos side to the Analogix side.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

---

Changes in v4:
- Rename analogix_dp_find_panel_or_bridge() to
  analogix_dp_finish_probe().
---
 drivers/gpu/drm/exynos/exynos_dp.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 65579873ceea..80ba700d2964 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -203,9 +203,6 @@ static int exynos_dp_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np;
 	struct exynos_dp_device *dp;
-	struct drm_panel *panel;
-	struct drm_bridge *bridge;
-	int ret;
 
 	dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
 			  GFP_KERNEL);
@@ -225,32 +222,31 @@ static int exynos_dp_probe(struct platform_device *pdev)
 	if (np) {
 		dp->plat_data.panel = of_drm_find_panel(np);
 
-		of_node_put(np);
 		if (IS_ERR(dp->plat_data.panel))
 			return PTR_ERR(dp->plat_data.panel);
 
 		goto out;
 	}
 
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, &bridge);
-	if (ret)
-		return ret;
-
 	/* The remote port can be either a panel or a bridge */
-	dp->plat_data.panel = panel;
-	dp->plat_data.next_bridge = bridge;
 	dp->plat_data.dev_type = EXYNOS_DP;
 	dp->plat_data.power_on = exynos_dp_poweron;
 	dp->plat_data.power_off = exynos_dp_poweroff;
 	dp->plat_data.attach = exynos_dp_bridge_attach;
 	dp->plat_data.get_modes = exynos_dp_get_modes;
+	dp->plat_data.ops = &exynos_dp_ops;
 
 out:
 	dp->adp = analogix_dp_probe(dev, &dp->plat_data);
 	if (IS_ERR(dp->adp))
 		return PTR_ERR(dp->adp);
 
-	return component_add(&pdev->dev, &exynos_dp_ops);
+	if (np) {
+		of_node_put(np);
+		return component_add(&pdev->dev, &exynos_dp_ops);
+	} else {
+		return analogix_dp_finish_probe(dp->adp);
+	}
 }
 
 static void exynos_dp_remove(struct platform_device *pdev)
-- 
2.34.1

[PATCH v4 11/13] drm/bridge: analogix_dp: Remove panel disabling and enabling in analogix_dp_set_bridge()

[Damon Ding]

The &drm_panel_funcs.enable() and &drm_panel_funcs.disable() mainly
help turn on/off the backlight to make the image visible, and the
backlight operations are even needless if drm_panel_of_backlight() or
drm_panel_dp_aux_backlight() is applied, in which case the enabling
and disabling process just add necessary delays.

Therefore, it should make sense to remove panel disabling and move
panel enabling after analogix_dp_set_bridge() finished.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 3e76a7b7d227..3c518106b896 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -840,9 +840,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
 {
 	int ret;
 
-	/* Keep the panel disabled while we configure video */
-	drm_panel_disable(dp->plat_data->panel);
-
 	ret = analogix_dp_train_link(dp);
 	if (ret) {
 		dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
@@ -862,9 +859,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
 		return ret;
 	}
 
-	/* Safe to enable the panel now */
-	drm_panel_enable(dp->plat_data->panel);
-
 	/* Check whether panel supports fast training */
 	ret = analogix_dp_fast_link_train_detection(dp);
 	if (ret)
@@ -1242,6 +1236,7 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 	while (timeout_loop < MAX_PLL_LOCK_LOOP) {
 		if (analogix_dp_set_bridge(dp) == 0) {
 			dp->dpms_mode = DRM_MODE_DPMS_ON;
+			drm_panel_enable(dp->plat_data->panel);
 			return;
 		}
 		dev_err(dp->dev, "failed to set bridge, retry: %d\n",
-- 
2.34.1

[PATCH v4 12/13] drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing in analogix_dp_unbind()

[Damon Ding]

The analogix_dp_unbind() should be balanced with analogix_dp_bind().
There are no bridge enabling and panel preparing in analogix_dp_bind(),
so it should be reasonable to remove the bridge disabing and panel
unpreparing in analogix_dp_unbind().

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 3c518106b896..a14c065254fa 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1610,10 +1610,6 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
 
 void analogix_dp_unbind(struct analogix_dp_device *dp)
 {
-	analogix_dp_bridge_disable(&dp->bridge);
-
-	drm_panel_unprepare(dp->plat_data->panel);
-
 	drm_dp_aux_unregister(&dp->aux);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_unbind);
-- 
2.34.1

[PATCH v4 13/13] drm/bridge: analogix_dp: Apply panel_bridge helper

[Damon Ding]

In order to unify the handling of the panel and bridge, apply
panel_bridge helpers for Analogix DP driver. With this patch, the
bridge support will also become available.

The following changes have ben made:
- Apply plane_bridge helper to wrap the panel as the bridge.
- Remove the explicit panel APIs calls, which can be replaced with
  the automic bridge APIs calls wrapped by the panel.
- Unify the API of getting modes to drm_bridge_get_modes().

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Rename the &analogix_dp_plat_data.bridge to
  &analogix_dp_plat_data.next_bridge.
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 31 +++++++++++--------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index a14c065254fa..0529bfb02884 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -948,11 +948,7 @@ static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_co
 	struct analogix_dp_device *dp = to_dp(bridge);
 	int num_modes = 0;
 
-	if (dp->plat_data->panel)
-		num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
-
-	if (dp->plat_data->next_bridge)
-		num_modes += drm_bridge_get_modes(dp->plat_data->next_bridge, connector);
+	num_modes += drm_bridge_get_modes(dp->plat_data->next_bridge, connector);
 
 	if (dp->plat_data->get_modes)
 		num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
@@ -995,7 +991,7 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
 	struct analogix_dp_device *dp = to_dp(bridge);
 	enum drm_connector_status status = connector_status_disconnected;
 
-	if (dp->plat_data->panel)
+	if (drm_bridge_is_panel(dp->plat_data->next_bridge))
 		return connector_status_connected;
 
 	if (!analogix_dp_detect_hpd(dp))
@@ -1080,8 +1076,6 @@ static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,
 	/* Don't touch the panel if we're coming back from PSR */
 	if (old_crtc_state && old_crtc_state->self_refresh_active)
 		return;
-
-	drm_panel_prepare(dp->plat_data->panel);
 }
 
 static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
@@ -1236,7 +1230,6 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 	while (timeout_loop < MAX_PLL_LOCK_LOOP) {
 		if (analogix_dp_set_bridge(dp) == 0) {
 			dp->dpms_mode = DRM_MODE_DPMS_ON;
-			drm_panel_enable(dp->plat_data->panel);
 			return;
 		}
 		dev_err(dp->dev, "failed to set bridge, retry: %d\n",
@@ -1254,16 +1247,12 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
 	if (dp->dpms_mode != DRM_MODE_DPMS_ON)
 		return;
 
-	drm_panel_disable(dp->plat_data->panel);
-
 	disable_irq(dp->irq);
 
 	analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
 
 	pm_runtime_put_sync(dp->dev);
 
-	drm_panel_unprepare(dp->plat_data->panel);
-
 	dp->fast_train_enable = false;
 	dp->psr_supported = false;
 	dp->dpms_mode = DRM_MODE_DPMS_OFF;
@@ -1599,6 +1588,22 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 		goto err_unregister_aux;
 	}
 
+	if (dp->plat_data->panel) {
+		dp->plat_data->next_bridge = devm_drm_panel_bridge_add(dp->dev,
+								       dp->plat_data->panel);
+		if (IS_ERR(dp->plat_data->next_bridge)) {
+			ret = PTR_ERR(bridge);
+			goto err_unregister_aux;
+		}
+	}
+
+	ret = drm_bridge_attach(dp->encoder, dp->plat_data->next_bridge, bridge,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	if (ret) {
+		dev_err(dp->dev, "failed to attach following panel or bridge (%d)\n", ret);
+		goto err_unregister_aux;
+	}
+
 	return 0;
 
 err_unregister_aux:
-- 
2.34.1

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Marek Szyprowski]

On 14.08.2025 12:47, Damon Ding wrote:
> PATCH 1 is a small format optimization for struct analogid_dp_device.
> PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
> PATCH 7 is to apply the drm_bridge_connector helper.
> PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
> PATCH 11-12 are preparations for apply panel_bridge helper.
> PATCH 13 is to apply the panel_bridge helper.

This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's Kconfig, 
so it causes build break:

drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to 
`drm_bridge_connector_init'
make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1

After adding this dependency, the Exynos DP driver stops working. On 
Samsung Snow Chromebook I observed following issue:

[    4.534220] exynos-dp 145b0000.dp-controller: failed to attach 
following panel or bridge (-16)
[    4.543428] exynos-drm exynos-drm: failed to bind 
145b0000.dp-controller (ops exynos_dp_ops): -16
[    4.551775] exynos-drm exynos-drm: adev bind failed: -16
[    4.556559] exynos-dp 145b0000.dp-controller: probe with driver 
exynos-dp failed with error -16

I will investigate details later in the evening.

> Damon Ding (13):
>    drm/bridge: analogix_dp: Formalize the struct analogix_dp_device
>    drm/bridge: analogix_dp: Move &drm_bridge_funcs.mode_set to
>      &drm_bridge_funcs.atomic_enable
>    drm/bridge: analogix_dp: Add &analogix_dp_plat_data.next_bridge
>    drm/exynos: exynos_dp: Remove &exynos_dp_device.ptn_bridge
>    drm/bridge: exynos_dp: Remove unused &exynos_dp_device.connector
>    drm/bridge: analogix_dp: Remove redundant
>      &analogix_dp_plat_data.skip_connector
>    drm/bridge: analogix_dp: Apply drm_bridge_connector helper
>    drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
>    drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()
>    drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()
>    drm/bridge: analogix_dp: Remove panel disabling and enabling in
>      analogix_dp_set_bridge()
>    drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing
>      in analogix_dp_unbind()
>    drm/bridge: analogix_dp: Apply panel_bridge helper
>
>   .../drm/bridge/analogix/analogix_dp_core.c    | 384 ++++++++++--------
>   .../drm/bridge/analogix/analogix_dp_core.h    |   5 +-
>   drivers/gpu/drm/exynos/exynos_dp.c            |  48 +--
>   .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  49 +--
>   include/drm/bridge/analogix_dp.h              |   7 +-
>   5 files changed, 248 insertions(+), 245 deletions(-)
>
> ---
>
> Changes in v2:
> - Update Exynos DP driver synchronously.
> - Move the panel/bridge parsing to the Analogix side.
>
> Changes in v3:
> - Rebase for the existing devm_drm_bridge_alloc() applying commit.
> - Fix the typographical error of panel/bridge check in exynos_dp_bind().
> - Squash all commits related to skip_connector deletion in both Exynos and
>    Analogix code into one.
> - Apply panel_bridge helper to make the codes more concise.
> - Fix the handing of bridge in analogix_dp_bridge_get_modes().
> - Remove unnecessary parameter struct drm_connector* for callback
>    &analogix_dp_plat_data.attach().
> - In order to decouple the connector driver and the bridge driver, move
>    the bridge connector initilization to the Rockchip and Exynos sides.
>
> Changes in v4:
> - Rebase for the applied &drm_bridge_funcs.detect() modification commit.
> - Rename analogix_dp_find_panel_or_bridge() to analogix_dp_finish_probe().
> - Drop the drmm_encoder_init() modification commit.
> - Rename the &analogix_dp_plat_data.bridge to
>    &analogix_dp_plat_data.next_bridge.
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Marek Szyprowski]

On 14.08.2025 16:33, Marek Szyprowski wrote:
> On 14.08.2025 12:47, Damon Ding wrote:
>> PATCH 1 is a small format optimization for struct analogid_dp_device.
>> PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>> PATCH 7 is to apply the drm_bridge_connector helper.
>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
>> PATCH 11-12 are preparations for apply panel_bridge helper.
>> PATCH 13 is to apply the panel_bridge helper.
>
> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's Kconfig, 
> so it causes build break:
>
> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to 
> `drm_bridge_connector_init'
> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>
> After adding this dependency, the Exynos DP driver stops working. On 
> Samsung Snow Chromebook I observed following issue:
>
> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach 
> following panel or bridge (-16)
> [    4.543428] exynos-drm exynos-drm: failed to bind 
> 145b0000.dp-controller (ops exynos_dp_ops): -16
> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver 
> exynos-dp failed with error -16
>
> I will investigate details later in the evening.

The failure is caused by trying to add plat_data->next_bridge twice 
(from exynos_dp's .attach callback, and from analogix' ->bind callback).


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Damon Ding]

Hi Marek,

On 2025/8/14 22:33, Marek Szyprowski wrote:
> On 14.08.2025 12:47, Damon Ding wrote:
>> PATCH 1 is a small format optimization for struct analogid_dp_device.
>> PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>> PATCH 7 is to apply the drm_bridge_connector helper.
>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
>> PATCH 11-12 are preparations for apply panel_bridge helper.
>> PATCH 13 is to apply the panel_bridge helper.
> 
> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's Kconfig,
> so it causes build break:
> 

> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
> `drm_bridge_connector_init'
> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
> 
> After adding this dependency, the Exynos DP driver stops working. On
> Samsung Snow Chromebook I observed following issue:
> 
> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
> following panel or bridge (-16)
> [    4.543428] exynos-drm exynos-drm: failed to bind
> 145b0000.dp-controller (ops exynos_dp_ops): -16
> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
> exynos-dp failed with error -16
> 
> I will investigate details later in the evening.
> 

Thanks for your review and test. :-)

I found the Rockchip side also lacks 'select DRM_BRIDGE_CONNECTOR' in 
ROCKCHIP_ANALOGIX_DP's Kconfig. I will add the DRM_BRIDGE_CONNECTOR 
selection for both of them in the next version.

>> Damon Ding (13):
>>     drm/bridge: analogix_dp: Formalize the struct analogix_dp_device
>>     drm/bridge: analogix_dp: Move &drm_bridge_funcs.mode_set to
>>       &drm_bridge_funcs.atomic_enable
>>     drm/bridge: analogix_dp: Add &analogix_dp_plat_data.next_bridge
>>     drm/exynos: exynos_dp: Remove &exynos_dp_device.ptn_bridge
>>     drm/bridge: exynos_dp: Remove unused &exynos_dp_device.connector
>>     drm/bridge: analogix_dp: Remove redundant
>>       &analogix_dp_plat_data.skip_connector
>>     drm/bridge: analogix_dp: Apply drm_bridge_connector helper
>>     drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
>>     drm/rockchip: analogix_dp: Apply analogix_dp_finish_probe()
>>     drm/exynos: exynos_dp: Apply analogix_dp_finish_probe()
>>     drm/bridge: analogix_dp: Remove panel disabling and enabling in
>>       analogix_dp_set_bridge()
>>     drm/bridge: analogix_dp: Remove bridge disabing and panel unpreparing
>>       in analogix_dp_unbind()
>>     drm/bridge: analogix_dp: Apply panel_bridge helper
>>
>>    .../drm/bridge/analogix/analogix_dp_core.c    | 384 ++++++++++--------
>>    .../drm/bridge/analogix/analogix_dp_core.h    |   5 +-
>>    drivers/gpu/drm/exynos/exynos_dp.c            |  48 +--
>>    .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  49 +--
>>    include/drm/bridge/analogix_dp.h              |   7 +-
>>    5 files changed, 248 insertions(+), 245 deletions(-)
>>
>> ---
>>
>> Changes in v2:
>> - Update Exynos DP driver synchronously.
>> - Move the panel/bridge parsing to the Analogix side.
>>
>> Changes in v3:
>> - Rebase for the existing devm_drm_bridge_alloc() applying commit.
>> - Fix the typographical error of panel/bridge check in exynos_dp_bind().
>> - Squash all commits related to skip_connector deletion in both Exynos and
>>     Analogix code into one.
>> - Apply panel_bridge helper to make the codes more concise.
>> - Fix the handing of bridge in analogix_dp_bridge_get_modes().
>> - Remove unnecessary parameter struct drm_connector* for callback
>>     &analogix_dp_plat_data.attach().
>> - In order to decouple the connector driver and the bridge driver, move
>>     the bridge connector initilization to the Rockchip and Exynos sides.
>>
>> Changes in v4:
>> - Rebase for the applied &drm_bridge_funcs.detect() modification commit.
>> - Rename analogix_dp_find_panel_or_bridge() to analogix_dp_finish_probe().
>> - Drop the drmm_encoder_init() modification commit.
>> - Rename the &analogix_dp_plat_data.bridge to
>>     &analogix_dp_plat_data.next_bridge.
>>
> Best regards

Best regards,
Damon

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Damon Ding]

Hi Marek,

On 2025/8/15 5:16, Marek Szyprowski wrote:
> 
> On 14.08.2025 16:33, Marek Szyprowski wrote:
>> On 14.08.2025 12:47, Damon Ding wrote:
>>> PATCH 1 is a small format optimization for struct analogid_dp_device.
>>> PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
>>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>>> PATCH 7 is to apply the drm_bridge_connector helper.
>>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
>>> PATCH 11-12 are preparations for apply panel_bridge helper.
>>> PATCH 13 is to apply the panel_bridge helper.
>>
>> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's Kconfig,
>> so it causes build break:
>>
>> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
>> `drm_bridge_connector_init'
>> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>>
>> After adding this dependency, the Exynos DP driver stops working. On
>> Samsung Snow Chromebook I observed following issue:
>>
>> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
>> following panel or bridge (-16)
>> [    4.543428] exynos-drm exynos-drm: failed to bind
>> 145b0000.dp-controller (ops exynos_dp_ops): -16
>> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
>> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
>> exynos-dp failed with error -16
>>
>> I will investigate details later in the evening.
> 
> The failure is caused by trying to add plat_data->next_bridge twice
> (from exynos_dp's .attach callback, and from analogix' ->bind callback).
> 
> 
> Best regards

I see. The bridge attachment for the next bridge was not well thought 
out. It may be better to move panel_bridge addition a little forward and 
remove next_bridge attachment on the Analogix side. Then, the Rockchip 
side and Exynos side can do their own next_bridge attachment in 
&analogix_dp_plat_data.attach() as they want.

Could you please help test the following modifications(they have been 
tested on my RK3588S EVB1 Board) on the Samsung Snow Chromebook? ;-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 0529bfb02884..8a9ce1f31678 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1573,6 +1573,15 @@ int analogix_dp_bind(struct analogix_dp_device 
*dp, struct drm_device *drm_dev)
                 return ret;
         }

+       if (dp->plat_data->panel) {
+               dp->plat_data->next_bridge = 
devm_drm_panel_bridge_add(dp->dev,
+ 
dp->plat_data->panel);
+               if (IS_ERR(dp->plat_data->next_bridge)) {
+                       ret = PTR_ERR(bridge);
+                       goto err_unregister_aux;
+               }
+       }
+
         bridge->ops = DRM_BRIDGE_OP_DETECT |
                       DRM_BRIDGE_OP_EDID |
                       DRM_BRIDGE_OP_MODES;
@@ -1588,22 +1597,6 @@ int analogix_dp_bind(struct analogix_dp_device 
*dp, struct drm_device *drm_dev)
                 goto err_unregister_aux;
         }

-       if (dp->plat_data->panel) {
-               dp->plat_data->next_bridge = 
devm_drm_panel_bridge_add(dp->dev,
- 
dp->plat_data->panel);
-               if (IS_ERR(dp->plat_data->next_bridge)) {
-                       ret = PTR_ERR(bridge);
-                       goto err_unregister_aux;
-               }
-       }
-
-       ret = drm_bridge_attach(dp->encoder, dp->plat_data->next_bridge, 
bridge,
-                               DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-       if (ret) {
-               dev_err(dp->dev, "failed to attach following panel or 
bridge (%d)\n", ret);
-               goto err_unregister_aux;
-       }
-
         return 0;

  err_unregister_aux:
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c 
b/drivers/gpu/drm/exynos/exynos_dp.c
index 80ba700d2964..d0422f940249 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -104,7 +104,7 @@ static int exynos_dp_bridge_attach(struct 
analogix_dp_plat_data *plat_data,
         /* Pre-empt DP connector creation if there's a bridge */
         if (plat_data->next_bridge) {
                 ret = drm_bridge_attach(&dp->encoder, 
plat_data->next_bridge, bridge,
-                                       0);
+                                       DRM_BRIDGE_ATTACH_NO_CONNECTOR);
                 if (ret)
                         return ret;
         }
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 0862b09a8be2..dfd32a79b94f 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -164,6 +164,24 @@ static int rockchip_dp_powerdown(struct 
analogix_dp_plat_data *plat_data)
         return 0;
  }

+static int rockchip_dp_attach(struct analogix_dp_plat_data *plat_data,
+                                    struct drm_bridge *bridge)
+{
+       struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
+       int ret;
+
+       if (plat_data->next_bridge) {
+               ret = drm_bridge_attach(&dp->encoder.encoder, 
plat_data->next_bridge, bridge,
+                                       DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+               if (ret) {
+                       dev_err(dp->dev, "failed to attach following 
panel or bridge (%d)\n", ret);
+                       return ret;
+               }
+       }
+
+       return 0;
+}
+
  static int rockchip_dp_get_modes(struct analogix_dp_plat_data *plat_data,
                                  struct drm_connector *connector)
  {
@@ -478,6 +496,7 @@ static int rockchip_dp_probe(struct platform_device 
*pdev)
         dp->plat_data.dev_type = dp->data->chip_type;
         dp->plat_data.power_on = rockchip_dp_poweron;
         dp->plat_data.power_off = rockchip_dp_powerdown;
+       dp->plat_data.attach = rockchip_dp_attach;
         dp->plat_data.get_modes = rockchip_dp_get_modes;
         dp->plat_data.ops = &rockchip_dp_component_ops;


Best regards,
Damon

Re: [PATCH v4 08/13] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()

[kernel test robot]

Hi Damon,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on rockchip/for-next linus/master v6.17-rc1 next-20250815]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Damon-Ding/drm-bridge-analogix_dp-Formalize-the-struct-analogix_dp_device/20250814-185009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link:    https://lore.kernel.org/r/20250814104753.195255-9-damon.ding%40rock-chips.com
patch subject: [PATCH v4 08/13] drm/bridge: analogix_dp: Add new API analogix_dp_finish_probe()
config: x86_64-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508160857.yC3oMucJ-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508160857.yC3oMucJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508160857.yC3oMucJ-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `analogix_dp_finish_probe':
>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1700: undefined reference to `devm_of_dp_aux_populate_bus'


vim +1700 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c

  1695	
  1696	int analogix_dp_finish_probe(struct analogix_dp_device *dp)
  1697	{
  1698		int ret;
  1699	
> 1700		ret = devm_of_dp_aux_populate_bus(&dp->aux, analogix_dp_aux_done_probing);
  1701		if (ret) {
  1702			/*
  1703			 * If devm_of_dp_aux_populate_bus() returns -ENODEV, the done_probing() will
  1704			 * not be called because there are no EP devices. Then the callback function
  1705			 * analogix_dp_aux_done_probing() will be called directly in order to support
  1706			 * the other valid DT configurations.
  1707			 *
  1708			 * NOTE: The devm_of_dp_aux_populate_bus() is allowed to return -EPROBE_DEFER.
  1709			 */
  1710			if (ret != -ENODEV) {
  1711				dev_err(dp->dev, "failed to populate aux bus\n");
  1712				return ret;
  1713			}
  1714	
  1715			return analogix_dp_aux_done_probing(&dp->aux);
  1716		}
  1717	
  1718		return 0;
  1719	}
  1720	EXPORT_SYMBOL_GPL(analogix_dp_finish_probe);
  1721	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 13/13] drm/bridge: analogix_dp: Apply panel_bridge helper

[Dan Carpenter]

Hi Damon,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Damon-Ding/drm-bridge-analogix_dp-Formalize-the-struct-analogix_dp_device/20250814-185009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link:    https://lore.kernel.org/r/20250814104753.195255-14-damon.ding%40rock-chips.com
patch subject: [PATCH v4 13/13] drm/bridge: analogix_dp: Apply panel_bridge helper
config: hexagon-randconfig-r072-20250815 (https://download.01.org/0day-ci/archive/20250816/202508161345.sfsLHzY7-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202508161345.sfsLHzY7-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1595 analogix_dp_bind() warn: passing a valid pointer to 'PTR_ERR'

vim +/PTR_ERR +1595 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c

152cce0006abf7e Marek Szyprowski 2020-03-10  1560  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
152cce0006abf7e Marek Szyprowski 2020-03-10  1561  {
41dc71b87962d41 Damon Ding       2025-08-14  1562  	struct drm_bridge *bridge = &dp->bridge;
152cce0006abf7e Marek Szyprowski 2020-03-10  1563  	int ret;
152cce0006abf7e Marek Szyprowski 2020-03-10  1564  
3424e3a4f844c0a Yakir Yang       2016-03-29  1565  	dp->drm_dev = drm_dev;
3424e3a4f844c0a Yakir Yang       2016-03-29  1566  	dp->encoder = dp->plat_data->encoder;
3424e3a4f844c0a Yakir Yang       2016-03-29  1567  
6cba3fe433415b2 Lyude Paul       2021-04-23  1568  	dp->aux.drm_dev = drm_dev;
0d97ad03f4220ca Tomeu Vizoso     2016-08-24  1569  
0d97ad03f4220ca Tomeu Vizoso     2016-08-24  1570  	ret = drm_dp_aux_register(&dp->aux);
2d192f4a3acc1c6 Lucas Stach      2024-06-19  1571  	if (ret) {
2d192f4a3acc1c6 Lucas Stach      2024-06-19  1572  		DRM_ERROR("failed to register AUX (%d)\n", ret);
c71db051142a74b Damon Ding       2025-03-10  1573  		return ret;
2d192f4a3acc1c6 Lucas Stach      2024-06-19  1574  	}
0d97ad03f4220ca Tomeu Vizoso     2016-08-24  1575  
41dc71b87962d41 Damon Ding       2025-08-14  1576  	bridge->ops = DRM_BRIDGE_OP_DETECT |
41dc71b87962d41 Damon Ding       2025-08-14  1577  		      DRM_BRIDGE_OP_EDID |
41dc71b87962d41 Damon Ding       2025-08-14  1578  		      DRM_BRIDGE_OP_MODES;
41dc71b87962d41 Damon Ding       2025-08-14  1579  	bridge->of_node = dp->dev->of_node;
41dc71b87962d41 Damon Ding       2025-08-14  1580  	bridge->type = DRM_MODE_CONNECTOR_eDP;
41dc71b87962d41 Damon Ding       2025-08-14  1581  	ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
41dc71b87962d41 Damon Ding       2025-08-14  1582  	if (ret)
41dc71b87962d41 Damon Ding       2025-08-14  1583  		goto err_unregister_aux;
41dc71b87962d41 Damon Ding       2025-08-14  1584  
41dc71b87962d41 Damon Ding       2025-08-14  1585  	ret = drm_bridge_attach(dp->encoder, bridge, NULL, 0);
3424e3a4f844c0a Yakir Yang       2016-03-29  1586  	if (ret) {
3424e3a4f844c0a Yakir Yang       2016-03-29  1587  		DRM_ERROR("failed to create bridge (%d)\n", ret);
2d192f4a3acc1c6 Lucas Stach      2024-06-19  1588  		goto err_unregister_aux;
3424e3a4f844c0a Yakir Yang       2016-03-29  1589  	}
3424e3a4f844c0a Yakir Yang       2016-03-29  1590  
dbd8040a6d0dedc Damon Ding       2025-08-14  1591  	if (dp->plat_data->panel) {
dbd8040a6d0dedc Damon Ding       2025-08-14  1592  		dp->plat_data->next_bridge = devm_drm_panel_bridge_add(dp->dev,
dbd8040a6d0dedc Damon Ding       2025-08-14  1593  								       dp->plat_data->panel);
dbd8040a6d0dedc Damon Ding       2025-08-14  1594  		if (IS_ERR(dp->plat_data->next_bridge)) {
dbd8040a6d0dedc Damon Ding       2025-08-14 @1595  			ret = PTR_ERR(bridge);

Wrong variable.  s/bridge/dp->plat_data->next_bridge/.

dbd8040a6d0dedc Damon Ding       2025-08-14  1596  			goto err_unregister_aux;
dbd8040a6d0dedc Damon Ding       2025-08-14  1597  		}
dbd8040a6d0dedc Damon Ding       2025-08-14  1598  	}
dbd8040a6d0dedc Damon Ding       2025-08-14  1599  
dbd8040a6d0dedc Damon Ding       2025-08-14  1600  	ret = drm_bridge_attach(dp->encoder, dp->plat_data->next_bridge, bridge,
dbd8040a6d0dedc Damon Ding       2025-08-14  1601  				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
dbd8040a6d0dedc Damon Ding       2025-08-14  1602  	if (ret) {
dbd8040a6d0dedc Damon Ding       2025-08-14  1603  		dev_err(dp->dev, "failed to attach following panel or bridge (%d)\n", ret);
dbd8040a6d0dedc Damon Ding       2025-08-14  1604  		goto err_unregister_aux;
dbd8040a6d0dedc Damon Ding       2025-08-14  1605  	}
dbd8040a6d0dedc Damon Ding       2025-08-14  1606  
152cce0006abf7e Marek Szyprowski 2020-03-10  1607  	return 0;
3424e3a4f844c0a Yakir Yang       2016-03-29  1608  
2d192f4a3acc1c6 Lucas Stach      2024-06-19  1609  err_unregister_aux:
2d192f4a3acc1c6 Lucas Stach      2024-06-19  1610  	drm_dp_aux_unregister(&dp->aux);
f0a8b49c03d22a5 Marek Szyprowski 2016-12-30  1611  
152cce0006abf7e Marek Szyprowski 2020-03-10  1612  	return ret;
3424e3a4f844c0a Yakir Yang       2016-03-29  1613  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 07/13] drm/bridge: analogix_dp: Apply drm_bridge_connector helper

[Dmitry Baryshkov]

On Thu, Aug 14, 2025 at 06:47:47PM +0800, Damon Ding wrote:
> Apply drm_bridge_connector helper for Analogix DP driver.
> 
> The following changes have been made:
> - Apply drm_bridge_connector helper to get rid of &drm_connector_funcs
>   and &drm_connector_helper_funcs.
> - Remove unnecessary parameter struct drm_connector* for callback
>   &analogix_dp_plat_data.attach.
> - Remove &analogix_dp_device.connector.
> - Convert analogix_dp_atomic_check()/analogix_dp_detect() to
>   &drm_bridge_funcs.atomic_check()/&drm_bridge_funcs.detect().
> - Split analogix_dp_get_modes() into &drm_bridge_funcs.get_modes() and
>   &drm_bridge_funcs.edid_read().
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> 
> ------
> 
> Changes in v2:
> - For &drm_bridge.ops, remove DRM_BRIDGE_OP_HPD and add
>   DRM_BRIDGE_OP_EDID.
> - Add analogix_dp_bridge_edid_read().
> - Move &analogix_dp_plat_data.skip_connector deletion to the previous
>   patches.
> 
> Changes in v3:
> - Rebase with the new devm_drm_bridge_alloc() related commit
>   48f05c3b4b70 ("drm/bridge: analogix_dp: Use devm_drm_bridge_alloc()
>   API").
> - Expand the commit message.
> - Call drm_bridge_get_modes() in analogix_dp_bridge_get_modes() if the
>   bridge is available.
> - Remove unnecessary parameter struct drm_connector* for callback
>   &analogix_dp_plat_data.attach.
> - In order to decouple the connector driver and the bridge driver, move
>   the bridge connector initilization to the Rockchip and Exynos sides.
> 
> Changes in v4:
> - Expand analogix_dp_bridge_detect() parameters to &drm_bridge and
>   &drm_connector.
> - Rename the &analogix_dp_plat_data.bridge to
>   &analogix_dp_plat_data.next_bridge.
> ---
>  .../drm/bridge/analogix/analogix_dp_core.c    | 145 ++++++++----------
>  .../drm/bridge/analogix/analogix_dp_core.h    |   1 -
>  drivers/gpu/drm/exynos/exynos_dp.c            |  18 ++-
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  11 +-
>  include/drm/bridge/analogix_dp.h              |   3 +-
>  5 files changed, 88 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 7876b310aaed..a8ed44ec8ef5 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -947,24 +947,16 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
>  	return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
>  }
>  
> -static int analogix_dp_get_modes(struct drm_connector *connector)
> +static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
>  {
> -	struct analogix_dp_device *dp = to_dp(connector);
> -	const struct drm_edid *drm_edid;
> +	struct analogix_dp_device *dp = to_dp(bridge);
>  	int num_modes = 0;
>  
> -	if (dp->plat_data->panel) {
> +	if (dp->plat_data->panel)
>  		num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
> -	} else {
> -		drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>  
> -		drm_edid_connector_update(&dp->connector, drm_edid);
> -
> -		if (drm_edid) {
> -			num_modes += drm_edid_connector_add_modes(&dp->connector);
> -			drm_edid_free(drm_edid);
> -		}
> -	}
> +	if (dp->plat_data->next_bridge)
> +		num_modes += drm_bridge_get_modes(dp->plat_data->next_bridge, connector);

If there is a next bridge which provides OP_MODES, then
drm_bridge_connector will use it for get_modes() and skip this one
completely. I'm not sure what's the value of this call.

>  
>  	if (dp->plat_data->get_modes)
>  		num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
> @@ -972,51 +964,39 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
>  	return num_modes;
>  }
>  
> -static struct drm_encoder *
> -analogix_dp_best_encoder(struct drm_connector *connector)
> +static const struct drm_edid *analogix_dp_bridge_edid_read(struct drm_bridge *bridge,
> +							   struct drm_connector *connector)
>  {
> -	struct analogix_dp_device *dp = to_dp(connector);
> +	struct analogix_dp_device *dp = to_dp(bridge);
> +	const struct drm_edid *drm_edid = NULL;
>  
> -	return dp->encoder;
> -}
> +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>  
> +	if (dp->plat_data->get_modes)
> +		dp->plat_data->get_modes(dp->plat_data, connector);


So, we have DDC, but we still want to return platform modes? What is the
usecase for that?

There might be some, but I think it deserves a comment in the source
file.

>  
> -static int analogix_dp_atomic_check(struct drm_connector *connector,
> -				    struct drm_atomic_state *state)
> -{
> -	struct analogix_dp_device *dp = to_dp(connector);
> -	struct drm_connector_state *conn_state;
> -	struct drm_crtc_state *crtc_state;
> +	return drm_edid;
> +}
>  
> -	conn_state = drm_atomic_get_new_connector_state(state, connector);
> -	if (WARN_ON(!conn_state))
> -		return -ENODEV;
> +static int analogix_dp_bridge_atomic_check(struct drm_bridge *bridge,
> +					   struct drm_bridge_state *bridge_state,
> +					   struct drm_crtc_state *crtc_state,
> +					   struct drm_connector_state *conn_state)
> +{
> +	struct analogix_dp_device *dp = to_dp(bridge);
>  
>  	conn_state->self_refresh_aware = true;
>  
> -	if (!conn_state->crtc)
> -		return 0;
> -
> -	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> -	if (!crtc_state)
> -		return 0;
> -
>  	if (crtc_state->self_refresh_active && !dp->psr_supported)
>  		return -EINVAL;
>  
>  	return 0;
>  }
>  
> -static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
> -	.get_modes = analogix_dp_get_modes,
> -	.best_encoder = analogix_dp_best_encoder,
> -	.atomic_check = analogix_dp_atomic_check,
> -};
> -
>  static enum drm_connector_status
> -analogix_dp_detect(struct drm_connector *connector, bool force)
> +analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>  {
> -	struct analogix_dp_device *dp = to_dp(connector);
> +	struct analogix_dp_device *dp = to_dp(bridge);
>  	enum drm_connector_status status = connector_status_disconnected;
>  
>  	if (dp->plat_data->panel)
> @@ -1028,21 +1008,11 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
>  	return status;
>  }
>  
> -static const struct drm_connector_funcs analogix_dp_connector_funcs = {
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.detect = analogix_dp_detect,
> -	.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 int analogix_dp_bridge_attach(struct drm_bridge *bridge,
>  				     struct drm_encoder *encoder,
>  				     enum drm_bridge_attach_flags flags)
>  {
>  	struct analogix_dp_device *dp = to_dp(bridge);
> -	struct drm_connector *connector = NULL;
>  	int ret = 0;
>  
>  	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> @@ -1050,31 +1020,8 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge,
>  		return -EINVAL;
>  	}
>  
> -	if (!dp->plat_data->next_bridge) {
> -		connector = &dp->connector;
> -		connector->polled = DRM_CONNECTOR_POLL_HPD;
> -
> -		ret = drm_connector_init(dp->drm_dev, connector,
> -					 &analogix_dp_connector_funcs,
> -					 DRM_MODE_CONNECTOR_eDP);
> -		if (ret) {
> -			DRM_ERROR("Failed to initialize connector with drm\n");
> -			return ret;
> -		}
> -
> -		drm_connector_helper_add(connector,
> -					 &analogix_dp_connector_helper_funcs);
> -		drm_connector_attach_encoder(connector, encoder);
> -	}
> -
> -	/*
> -	 * NOTE: the connector registration is implemented in analogix
> -	 * platform driver, that to say connector would be exist after
> -	 * plat_data->attch return, that's why we record the connector
> -	 * point after plat attached.
> -	 */
>  	if (dp->plat_data->attach) {
> -		ret = dp->plat_data->attach(dp->plat_data, bridge, connector);
> +		ret = dp->plat_data->attach(dp->plat_data, bridge);
>  		if (ret) {
>  			DRM_ERROR("Failed at platform attach func\n");
>  			return ret;
> @@ -1178,14 +1125,21 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
>  }
>  
>  static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
> +					struct drm_atomic_state *state,
>  					const struct drm_display_mode *mode)
>  {
>  	struct analogix_dp_device *dp = to_dp(bridge);
> -	struct drm_display_info *display_info = &dp->connector.display_info;
>  	struct video_info *video = &dp->video_info;
>  	struct device_node *dp_node = dp->dev->of_node;
> +	struct drm_connector *connector;
> +	struct drm_display_info *display_info;
>  	int vic;
>  
> +	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> +	if (!connector)
> +		return;
> +	display_info = &connector->display_info;
> +
>  	/* Input video interlaces & hsync pol & vsync pol */
>  	video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
>  	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
> @@ -1269,7 +1223,7 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>  	new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc);
>  	if (!new_crtc_state)
>  		return;
> -	analogix_dp_bridge_mode_set(bridge, &new_crtc_state->adjusted_mode);
> +	analogix_dp_bridge_mode_set(bridge, old_state, &new_crtc_state->adjusted_mode);
>  
>  	old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc);
>  	/* Not a full enable, just disable PSR and continue */
> @@ -1385,7 +1339,11 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
>  	.atomic_enable = analogix_dp_bridge_atomic_enable,
>  	.atomic_disable = analogix_dp_bridge_atomic_disable,
>  	.atomic_post_disable = analogix_dp_bridge_atomic_post_disable,
> +	.atomic_check = analogix_dp_bridge_atomic_check,
>  	.attach = analogix_dp_bridge_attach,
> +	.get_modes = analogix_dp_bridge_get_modes,
> +	.edid_read = analogix_dp_bridge_edid_read,
> +	.detect = analogix_dp_bridge_detect,
>  };
>  
>  static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
> @@ -1615,6 +1573,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_resume);
>  
>  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>  {
> +	struct drm_bridge *bridge = &dp->bridge;
>  	int ret;
>  
>  	dp->drm_dev = drm_dev;
> @@ -1628,7 +1587,16 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>  		return ret;
>  	}
>  
> -	ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);
> +	bridge->ops = DRM_BRIDGE_OP_DETECT |
> +		      DRM_BRIDGE_OP_EDID |

Should this be limited to the !panel cases? Otherwise OP_EDID overrides
OP_MODES and the analogix_dp_bridge_get_modes() will never be called.

> +		      DRM_BRIDGE_OP_MODES;
> +	bridge->of_node = dp->dev->of_node;
> +	bridge->type = DRM_MODE_CONNECTOR_eDP;
> +	ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
> +	if (ret)
> +		goto err_unregister_aux;
> +
> +	ret = drm_bridge_attach(dp->encoder, bridge, NULL, 0);
>  	if (ret) {
>  		DRM_ERROR("failed to create bridge (%d)\n", ret);
>  		goto err_unregister_aux;
> @@ -1646,7 +1614,6 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
>  void analogix_dp_unbind(struct analogix_dp_device *dp)
>  {
>  	analogix_dp_bridge_disable(&dp->bridge);
> -	dp->connector.funcs->destroy(&dp->connector);
>  
>  	drm_panel_unprepare(dp->plat_data->panel);
>  
> @@ -1656,7 +1623,8 @@ EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>  
>  int analogix_dp_start_crc(struct drm_connector *connector)
>  {
> -	struct analogix_dp_device *dp = to_dp(connector);
> +	struct analogix_dp_device *dp;
> +	struct drm_bridge *bridge;
>  
>  	if (!connector->state->crtc) {
>  		DRM_ERROR("Connector %s doesn't currently have a CRTC.\n",
> @@ -1664,13 +1632,26 @@ int analogix_dp_start_crc(struct drm_connector *connector)
>  		return -EINVAL;
>  	}
>  
> +	bridge = drm_bridge_chain_get_first_bridge(connector->encoder);
> +	if (bridge->type != DRM_MODE_CONNECTOR_eDP)
> +		return -EINVAL;
> +
> +	dp = to_dp(bridge);
> +
>  	return drm_dp_start_crc(&dp->aux, connector->state->crtc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_start_crc);
>  
>  int analogix_dp_stop_crc(struct drm_connector *connector)
>  {
> -	struct analogix_dp_device *dp = to_dp(connector);
> +	struct analogix_dp_device *dp;
> +	struct drm_bridge *bridge;
> +
> +	bridge = drm_bridge_chain_get_first_bridge(connector->encoder);
> +	if (bridge->type != DRM_MODE_CONNECTOR_eDP)
> +		return -EINVAL;
> +
> +	dp = to_dp(bridge);
>  
>  	return drm_dp_stop_crc(&dp->aux);
>  }
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 91b215c6a0cf..17347448c6b0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -154,7 +154,6 @@ struct analogix_dp_device {
>  	struct drm_encoder	*encoder;
>  	struct device		*dev;
>  	struct drm_device	*drm_dev;
> -	struct drm_connector	connector;
>  	struct drm_bridge	bridge;
>  	struct drm_dp_aux	aux;
>  	struct clk		*clock;
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index 702128d76ae3..65579873ceea 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -21,6 +21,7 @@
>  #include <drm/bridge/analogix_dp.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
> @@ -95,8 +96,7 @@ static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data,
>  }
>  
>  static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
> -				   struct drm_bridge *bridge,
> -				   struct drm_connector *connector)
> +				   struct drm_bridge *bridge)
>  {
>  	struct exynos_dp_device *dp = to_dp(plat_data);
>  	int ret;
> @@ -147,6 +147,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>  	struct exynos_dp_device *dp = dev_get_drvdata(dev);
>  	struct drm_encoder *encoder = &dp->encoder;
>  	struct drm_device *drm_dev = data;
> +	struct drm_connector *connector;
>  	int ret;
>  
>  	dp->drm_dev = drm_dev;
> @@ -168,10 +169,19 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>  	dp->plat_data.encoder = encoder;
>  
>  	ret = analogix_dp_bind(dp->adp, dp->drm_dev);
> -	if (ret)
> +	if (ret) {
>  		dp->encoder.funcs->destroy(&dp->encoder);
> +		return ret;
> +	}
> +
> +	connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
> +	if (IS_ERR(connector)) {
> +		ret = PTR_ERR(connector);
> +		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
> +		return ret;
> +	}
>  
> -	return ret;
> +	return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
>  }
>  
>  static void exynos_dp_unbind(struct device *dev, struct device *master,
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index d30f0983a53a..87dfb48206db 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -25,6 +25,7 @@
>  #include <drm/display/drm_dp_helper.h>
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/bridge/analogix_dp.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
> @@ -394,6 +395,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>  {
>  	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
>  	struct drm_device *drm_dev = data;
> +	struct drm_connector *connector;
>  	int ret;
>  
>  	dp->drm_dev = drm_dev;
> @@ -413,7 +415,14 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>  	if (ret)
>  		goto err_cleanup_encoder;
>  
> -	return 0;
> +	connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
> +	if (IS_ERR(connector)) {
> +		ret = PTR_ERR(connector);
> +		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
> +		goto err_cleanup_encoder;
> +	}
> +
> +	return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
>  err_cleanup_encoder:
>  	dp->encoder.encoder.funcs->destroy(&dp->encoder.encoder);
>  	return ret;
> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
> index f06da105d8f2..ffc05f3de232 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -33,8 +33,7 @@ struct analogix_dp_plat_data {
>  
>  	int (*power_on)(struct analogix_dp_plat_data *);
>  	int (*power_off)(struct analogix_dp_plat_data *);
> -	int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *,
> -		      struct drm_connector *);
> +	int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *);
>  	int (*get_modes)(struct analogix_dp_plat_data *,
>  			 struct drm_connector *);
>  };
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Marek Szyprowski]

On 15.08.2025 04:59, Damon Ding wrote:
> On 2025/8/15 5:16, Marek Szyprowski wrote:
>> On 14.08.2025 16:33, Marek Szyprowski wrote:
>>> On 14.08.2025 12:47, Damon Ding wrote:
>>>> PATCH 1 is a small format optimization for struct analogid_dp_device.
>>>> PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
>>>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>>>> PATCH 7 is to apply the drm_bridge_connector helper.
>>>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
>>>> PATCH 11-12 are preparations for apply panel_bridge helper.
>>>> PATCH 13 is to apply the panel_bridge helper.
>>>
>>> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's Kconfig,
>>> so it causes build break:
>>>
>>> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
>>> `drm_bridge_connector_init'
>>> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>>>
>>> After adding this dependency, the Exynos DP driver stops working. On
>>> Samsung Snow Chromebook I observed following issue:
>>>
>>> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
>>> following panel or bridge (-16)
>>> [    4.543428] exynos-drm exynos-drm: failed to bind
>>> 145b0000.dp-controller (ops exynos_dp_ops): -16
>>> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
>>> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
>>> exynos-dp failed with error -16
>>>
>>> I will investigate details later in the evening.
>>
>> The failure is caused by trying to add plat_data->next_bridge twice
>> (from exynos_dp's .attach callback, and from analogix' ->bind callback).
>>
>>
>> Best regards
>
> I see. The bridge attachment for the next bridge was not well thought 
> out. It may be better to move panel_bridge addition a little forward 
> and remove next_bridge attachment on the Analogix side. Then, the 
> Rockchip side and Exynos side can do their own next_bridge attachment 
> in &analogix_dp_plat_data.attach() as they want.
>
> Could you please help test the following modifications(they have been 
> tested on my RK3588S EVB1 Board) on the Samsung Snow Chromebook? ;-)

Assuming that I properly applied the malformed diff, it doesn't solve 
all the issues. There are no errors reported though, but the display 
chain doesn't work and no valid mode is reported:

# dmesg | grep drm
[    3.384992] [drm] Initialized panfrost 1.4.0 for 11800000.gpu on minor 0
[    4.487739] [drm] Exynos DRM: using 14400000.fimd device for DMA 
mapping operations
[    4.494202] exynos-drm exynos-drm: bound 14400000.fimd (ops 
fimd_component_ops)
[    4.502374] exynos-drm exynos-drm: bound 14450000.mixer (ops 
mixer_component_ops)
[    4.511930] exynos-drm exynos-drm: bound 145b0000.dp-controller (ops 
exynos_dp_ops)
[    4.518411] exynos-drm exynos-drm: bound 14530000.hdmi (ops 
hdmi_component_ops)
[    4.529628] [drm] Initialized exynos 1.1.0 for exynos-drm on minor 1
[    4.657434] exynos-drm exynos-drm: [drm] Cannot find any crtc or sizes
[    4.925023] exynos-drm exynos-drm: [drm] Cannot find any crtc or sizes

# ./modetest -c -Mexynos
Connectors:
id      encoder status          name            size (mm)       modes 
   encoders
69      0       disconnected    LVDS-1          0x0             0       68
  props:
        1 EDID:
                flags: immutable blob
                blobs:

                value:
        2 DPMS:
                flags: enum
                enums: On=0 Standby=1 Suspend=2 Off=3
                value: 0
        5 link-status:
                flags: enum
                enums: Good=0 Bad=1
                value: 0
        6 non-desktop:
                flags: immutable range
                values: 0 1
                value: 0
        4 TILE:
                flags: immutable blob
                blobs:

                value:
71      0       disconnected    HDMI-A-1        0x0             0       70
  props:
        1 EDID:
                flags: immutable blob
                blobs:

                value:
        2 DPMS:
                flags: enum
                enums: On=0 Standby=1 Suspend=2 Off=3
                value: 0
        5 link-status:
                flags: enum
                enums: Good=0 Bad=1
                value: 0
        6 non-desktop:
                flags: immutable range
                values: 0 1
                value: 0
        4 TILE:
                flags: immutable blob
                blobs:

                value:


I will investigate details later this week.


>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 0529bfb02884..8a9ce1f31678 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1573,6 +1573,15 @@ int analogix_dp_bind(struct analogix_dp_device 
> *dp, struct drm_device *drm_dev)
>                 return ret;
>         }
>
> +       if (dp->plat_data->panel) {
> +               dp->plat_data->next_bridge = 
> devm_drm_panel_bridge_add(dp->dev,
> + dp->plat_data->panel);
> +               if (IS_ERR(dp->plat_data->next_bridge)) {
> +                       ret = PTR_ERR(bridge);
> +                       goto err_unregister_aux;
> +               }
> +       }
> +
>         bridge->ops = DRM_BRIDGE_OP_DETECT |
>                       DRM_BRIDGE_OP_EDID |
>                       DRM_BRIDGE_OP_MODES;
> @@ -1588,22 +1597,6 @@ int analogix_dp_bind(struct analogix_dp_device 
> *dp, struct drm_device *drm_dev)
>                 goto err_unregister_aux;
>         }
>
> -       if (dp->plat_data->panel) {
> -               dp->plat_data->next_bridge = 
> devm_drm_panel_bridge_add(dp->dev,
> - dp->plat_data->panel);
> -               if (IS_ERR(dp->plat_data->next_bridge)) {
> -                       ret = PTR_ERR(bridge);
> -                       goto err_unregister_aux;
> -               }
> -       }
> -
> -       ret = drm_bridge_attach(dp->encoder, 
> dp->plat_data->next_bridge, bridge,
> -                               DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> -       if (ret) {
> -               dev_err(dp->dev, "failed to attach following panel or 
> bridge (%d)\n", ret);
> -               goto err_unregister_aux;
> -       }
> -
>         return 0;
>
>  err_unregister_aux:
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c 
> b/drivers/gpu/drm/exynos/exynos_dp.c
> index 80ba700d2964..d0422f940249 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -104,7 +104,7 @@ static int exynos_dp_bridge_attach(struct 
> analogix_dp_plat_data *plat_data,
>         /* Pre-empt DP connector creation if there's a bridge */
>         if (plat_data->next_bridge) {
>                 ret = drm_bridge_attach(&dp->encoder, 
> plat_data->next_bridge, bridge,
> -                                       0);
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>                 if (ret)
>                         return ret;
>         }
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 0862b09a8be2..dfd32a79b94f 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -164,6 +164,24 @@ static int rockchip_dp_powerdown(struct 
> analogix_dp_plat_data *plat_data)
>         return 0;
>  }
>
> +static int rockchip_dp_attach(struct analogix_dp_plat_data *plat_data,
> +                                    struct drm_bridge *bridge)
> +{
> +       struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
> +       int ret;
> +
> +       if (plat_data->next_bridge) {
> +               ret = drm_bridge_attach(&dp->encoder.encoder, 
> plat_data->next_bridge, bridge,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +               if (ret) {
> +                       dev_err(dp->dev, "failed to attach following 
> panel or bridge (%d)\n", ret);
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static int rockchip_dp_get_modes(struct analogix_dp_plat_data 
> *plat_data,
>                                  struct drm_connector *connector)
>  {
> @@ -478,6 +496,7 @@ static int rockchip_dp_probe(struct 
> platform_device *pdev)
>         dp->plat_data.dev_type = dp->data->chip_type;
>         dp->plat_data.power_on = rockchip_dp_poweron;
>         dp->plat_data.power_off = rockchip_dp_powerdown;
> +       dp->plat_data.attach = rockchip_dp_attach;
>         dp->plat_data.get_modes = rockchip_dp_get_modes;
>         dp->plat_data.ops = &rockchip_dp_component_ops;
>
>
> Best regards,
> Damon
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 07/13] drm/bridge: analogix_dp: Apply drm_bridge_connector helper

[Damon Ding]

Hi Dmitry,

On 8/17/2025 12:43 AM, Dmitry Baryshkov wrote:
> On Thu, Aug 14, 2025 at 06:47:47PM +0800, Damon Ding wrote:
>> Apply drm_bridge_connector helper for Analogix DP driver.
>>
>> The following changes have been made:
>> - Apply drm_bridge_connector helper to get rid of &drm_connector_funcs
>>    and &drm_connector_helper_funcs.
>> - Remove unnecessary parameter struct drm_connector* for callback
>>    &analogix_dp_plat_data.attach.
>> - Remove &analogix_dp_device.connector.
>> - Convert analogix_dp_atomic_check()/analogix_dp_detect() to
>>    &drm_bridge_funcs.atomic_check()/&drm_bridge_funcs.detect().
>> - Split analogix_dp_get_modes() into &drm_bridge_funcs.get_modes() and
>>    &drm_bridge_funcs.edid_read().
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ------
>>
>> Changes in v2:
>> - For &drm_bridge.ops, remove DRM_BRIDGE_OP_HPD and add
>>    DRM_BRIDGE_OP_EDID.
>> - Add analogix_dp_bridge_edid_read().
>> - Move &analogix_dp_plat_data.skip_connector deletion to the previous
>>    patches.
>>
>> Changes in v3:
>> - Rebase with the new devm_drm_bridge_alloc() related commit
>>    48f05c3b4b70 ("drm/bridge: analogix_dp: Use devm_drm_bridge_alloc()
>>    API").
>> - Expand the commit message.
>> - Call drm_bridge_get_modes() in analogix_dp_bridge_get_modes() if the
>>    bridge is available.
>> - Remove unnecessary parameter struct drm_connector* for callback
>>    &analogix_dp_plat_data.attach.
>> - In order to decouple the connector driver and the bridge driver, move
>>    the bridge connector initilization to the Rockchip and Exynos sides.
>>
>> Changes in v4:
>> - Expand analogix_dp_bridge_detect() parameters to &drm_bridge and
>>    &drm_connector.
>> - Rename the &analogix_dp_plat_data.bridge to
>>    &analogix_dp_plat_data.next_bridge.
>> ---
>>   .../drm/bridge/analogix/analogix_dp_core.c    | 145 ++++++++----------
>>   .../drm/bridge/analogix/analogix_dp_core.h    |   1 -
>>   drivers/gpu/drm/exynos/exynos_dp.c            |  18 ++-
>>   .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  11 +-
>>   include/drm/bridge/analogix_dp.h              |   3 +-
>>   5 files changed, 88 insertions(+), 90 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 7876b310aaed..a8ed44ec8ef5 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -947,24 +947,16 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
>>   	return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
>>   }
>>   
>> -static int analogix_dp_get_modes(struct drm_connector *connector)
>> +static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
>>   {
>> -	struct analogix_dp_device *dp = to_dp(connector);
>> -	const struct drm_edid *drm_edid;
>> +	struct analogix_dp_device *dp = to_dp(bridge);
>>   	int num_modes = 0;
>>   
>> -	if (dp->plat_data->panel) {
>> +	if (dp->plat_data->panel)
>>   		num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
>> -	} else {
>> -		drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>>   
>> -		drm_edid_connector_update(&dp->connector, drm_edid);
>> -
>> -		if (drm_edid) {
>> -			num_modes += drm_edid_connector_add_modes(&dp->connector);
>> -			drm_edid_free(drm_edid);
>> -		}
>> -	}
>> +	if (dp->plat_data->next_bridge)
>> +		num_modes += drm_bridge_get_modes(dp->plat_data->next_bridge, connector);
> 
> If there is a next bridge which provides OP_MODES, then
> drm_bridge_connector will use it for get_modes() and skip this one
> completely. I'm not sure what's the value of this call.

Following your advice, it is really a good idea to distinguish the 
drm_bridge_ops between the panel and the bridge. Will add it in v5.

> 
>>   
>>   	if (dp->plat_data->get_modes)
>>   		num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
>> @@ -972,51 +964,39 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
>>   	return num_modes;
>>   }
>>   
>> -static struct drm_encoder *
>> -analogix_dp_best_encoder(struct drm_connector *connector)
>> +static const struct drm_edid *analogix_dp_bridge_edid_read(struct drm_bridge *bridge,
>> +							   struct drm_connector *connector)
>>   {
>> -	struct analogix_dp_device *dp = to_dp(connector);
>> +	struct analogix_dp_device *dp = to_dp(bridge);
>> +	const struct drm_edid *drm_edid = NULL;
>>   
>> -	return dp->encoder;
>> -}
>> +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>>   
>> +	if (dp->plat_data->get_modes)
>> +		dp->plat_data->get_modes(dp->plat_data, connector);
> 
> 
> So, we have DDC, but we still want to return platform modes? What is the
> usecase for that?
> 
> There might be some, but I think it deserves a comment in the source
> file.
> 

For Rockchip side, since RK3588 and RK3576 can support YUV formats while 
the other can not, the &analogix_dp_plat_data.get_modes() help filter 
out YUV formats for some platforms(The YUV feature support may not be 
fit for this patch series and will come later).

For Exynos side, I think &analogix_dp_plat_data.get_modes() can help
parse the video mode set in the eDP DT node when there is no available 
panel or bridge.

I will add some comments about it in the next version.

>>   
>> -static int analogix_dp_atomic_check(struct drm_connector *connector,
>> -				    struct drm_atomic_state *state)
>> -{
>> -	struct analogix_dp_device *dp = to_dp(connector);
>> -	struct drm_connector_state *conn_state;
>> -	struct drm_crtc_state *crtc_state;
>> +	return drm_edid;
>> +}
>>   
>> -	conn_state = drm_atomic_get_new_connector_state(state, connector);
>> -	if (WARN_ON(!conn_state))
>> -		return -ENODEV;
>> +static int analogix_dp_bridge_atomic_check(struct drm_bridge *bridge,
>> +					   struct drm_bridge_state *bridge_state,
>> +					   struct drm_crtc_state *crtc_state,
>> +					   struct drm_connector_state *conn_state)
>> +{
>> +	struct analogix_dp_device *dp = to_dp(bridge);
>>   
>>   	conn_state->self_refresh_aware = true;
>>   
>> -	if (!conn_state->crtc)
>> -		return 0;
>> -
>> -	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
>> -	if (!crtc_state)
>> -		return 0;
>> -
>>   	if (crtc_state->self_refresh_active && !dp->psr_supported)
>>   		return -EINVAL;
>>   
>>   	return 0;
>>   }
>>   
>> -static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
>> -	.get_modes = analogix_dp_get_modes,
>> -	.best_encoder = analogix_dp_best_encoder,
>> -	.atomic_check = analogix_dp_atomic_check,
>> -};
>> -
>>   static enum drm_connector_status
>> -analogix_dp_detect(struct drm_connector *connector, bool force)
>> +analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>>   {
>> -	struct analogix_dp_device *dp = to_dp(connector);
>> +	struct analogix_dp_device *dp = to_dp(bridge);
>>   	enum drm_connector_status status = connector_status_disconnected;
>>   
>>   	if (dp->plat_data->panel)
>> @@ -1028,21 +1008,11 @@ analogix_dp_detect(struct drm_connector *connector, bool force)
>>   	return status;
>>   }
>>   
>> -static const struct drm_connector_funcs analogix_dp_connector_funcs = {
>> -	.fill_modes = drm_helper_probe_single_connector_modes,
>> -	.detect = analogix_dp_detect,
>> -	.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 int analogix_dp_bridge_attach(struct drm_bridge *bridge,
>>   				     struct drm_encoder *encoder,
>>   				     enum drm_bridge_attach_flags flags)
>>   {
>>   	struct analogix_dp_device *dp = to_dp(bridge);
>> -	struct drm_connector *connector = NULL;
>>   	int ret = 0;
>>   
>>   	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
>> @@ -1050,31 +1020,8 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge,
>>   		return -EINVAL;
>>   	}
>>   
>> -	if (!dp->plat_data->next_bridge) {
>> -		connector = &dp->connector;
>> -		connector->polled = DRM_CONNECTOR_POLL_HPD;
>> -
>> -		ret = drm_connector_init(dp->drm_dev, connector,
>> -					 &analogix_dp_connector_funcs,
>> -					 DRM_MODE_CONNECTOR_eDP);
>> -		if (ret) {
>> -			DRM_ERROR("Failed to initialize connector with drm\n");
>> -			return ret;
>> -		}
>> -
>> -		drm_connector_helper_add(connector,
>> -					 &analogix_dp_connector_helper_funcs);
>> -		drm_connector_attach_encoder(connector, encoder);
>> -	}
>> -
>> -	/*
>> -	 * NOTE: the connector registration is implemented in analogix
>> -	 * platform driver, that to say connector would be exist after
>> -	 * plat_data->attch return, that's why we record the connector
>> -	 * point after plat attached.
>> -	 */
>>   	if (dp->plat_data->attach) {
>> -		ret = dp->plat_data->attach(dp->plat_data, bridge, connector);
>> +		ret = dp->plat_data->attach(dp->plat_data, bridge);
>>   		if (ret) {
>>   			DRM_ERROR("Failed at platform attach func\n");
>>   			return ret;
>> @@ -1178,14 +1125,21 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
>>   }
>>   
>>   static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge,
>> +					struct drm_atomic_state *state,
>>   					const struct drm_display_mode *mode)
>>   {
>>   	struct analogix_dp_device *dp = to_dp(bridge);
>> -	struct drm_display_info *display_info = &dp->connector.display_info;
>>   	struct video_info *video = &dp->video_info;
>>   	struct device_node *dp_node = dp->dev->of_node;
>> +	struct drm_connector *connector;
>> +	struct drm_display_info *display_info;
>>   	int vic;
>>   
>> +	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
>> +	if (!connector)
>> +		return;
>> +	display_info = &connector->display_info;
>> +
>>   	/* Input video interlaces & hsync pol & vsync pol */
>>   	video->interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
>>   	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
>> @@ -1269,7 +1223,7 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>>   	new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc);
>>   	if (!new_crtc_state)
>>   		return;
>> -	analogix_dp_bridge_mode_set(bridge, &new_crtc_state->adjusted_mode);
>> +	analogix_dp_bridge_mode_set(bridge, old_state, &new_crtc_state->adjusted_mode);
>>   
>>   	old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc);
>>   	/* Not a full enable, just disable PSR and continue */
>> @@ -1385,7 +1339,11 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
>>   	.atomic_enable = analogix_dp_bridge_atomic_enable,
>>   	.atomic_disable = analogix_dp_bridge_atomic_disable,
>>   	.atomic_post_disable = analogix_dp_bridge_atomic_post_disable,
>> +	.atomic_check = analogix_dp_bridge_atomic_check,
>>   	.attach = analogix_dp_bridge_attach,
>> +	.get_modes = analogix_dp_bridge_get_modes,
>> +	.edid_read = analogix_dp_bridge_edid_read,
>> +	.detect = analogix_dp_bridge_detect,
>>   };
>>   
>>   static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>> @@ -1615,6 +1573,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_resume);
>>   
>>   int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>>   {
>> +	struct drm_bridge *bridge = &dp->bridge;
>>   	int ret;
>>   
>>   	dp->drm_dev = drm_dev;
>> @@ -1628,7 +1587,16 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>>   		return ret;
>>   	}
>>   
>> -	ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);
>> +	bridge->ops = DRM_BRIDGE_OP_DETECT |
>> +		      DRM_BRIDGE_OP_EDID |
> 
> Should this be limited to the !panel cases? Otherwise OP_EDID overrides
> OP_MODES and the analogix_dp_bridge_get_modes() will never be called.
> 
>> +		      DRM_BRIDGE_OP_MODES;
>> +	bridge->of_node = dp->dev->of_node;
>> +	bridge->type = DRM_MODE_CONNECTOR_eDP;
>> +	ret = devm_drm_bridge_add(dp->dev, &dp->bridge);
>> +	if (ret)
>> +		goto err_unregister_aux;
>> +
>> +	ret = drm_bridge_attach(dp->encoder, bridge, NULL, 0);
>>   	if (ret) {
>>   		DRM_ERROR("failed to create bridge (%d)\n", ret);
>>   		goto err_unregister_aux;
>> @@ -1646,7 +1614,6 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind);
>>   void analogix_dp_unbind(struct analogix_dp_device *dp)
>>   {
>>   	analogix_dp_bridge_disable(&dp->bridge);
>> -	dp->connector.funcs->destroy(&dp->connector);
>>   
>>   	drm_panel_unprepare(dp->plat_data->panel);
>>   
>> @@ -1656,7 +1623,8 @@ EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>>   
>>   int analogix_dp_start_crc(struct drm_connector *connector)
>>   {
>> -	struct analogix_dp_device *dp = to_dp(connector);
>> +	struct analogix_dp_device *dp;
>> +	struct drm_bridge *bridge;
>>   
>>   	if (!connector->state->crtc) {
>>   		DRM_ERROR("Connector %s doesn't currently have a CRTC.\n",
>> @@ -1664,13 +1632,26 @@ int analogix_dp_start_crc(struct drm_connector *connector)
>>   		return -EINVAL;
>>   	}
>>   
>> +	bridge = drm_bridge_chain_get_first_bridge(connector->encoder);
>> +	if (bridge->type != DRM_MODE_CONNECTOR_eDP)
>> +		return -EINVAL;
>> +
>> +	dp = to_dp(bridge);
>> +
>>   	return drm_dp_start_crc(&dp->aux, connector->state->crtc);
>>   }
>>   EXPORT_SYMBOL_GPL(analogix_dp_start_crc);
>>   
>>   int analogix_dp_stop_crc(struct drm_connector *connector)
>>   {
>> -	struct analogix_dp_device *dp = to_dp(connector);
>> +	struct analogix_dp_device *dp;
>> +	struct drm_bridge *bridge;
>> +
>> +	bridge = drm_bridge_chain_get_first_bridge(connector->encoder);
>> +	if (bridge->type != DRM_MODE_CONNECTOR_eDP)
>> +		return -EINVAL;
>> +
>> +	dp = to_dp(bridge);
>>   
>>   	return drm_dp_stop_crc(&dp->aux);
>>   }
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index 91b215c6a0cf..17347448c6b0 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -154,7 +154,6 @@ struct analogix_dp_device {
>>   	struct drm_encoder	*encoder;
>>   	struct device		*dev;
>>   	struct drm_device	*drm_dev;
>> -	struct drm_connector	connector;
>>   	struct drm_bridge	bridge;
>>   	struct drm_dp_aux	aux;
>>   	struct clk		*clock;
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
>> index 702128d76ae3..65579873ceea 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
>> @@ -21,6 +21,7 @@
>>   #include <drm/bridge/analogix_dp.h>
>>   #include <drm/drm_atomic_helper.h>
>>   #include <drm/drm_bridge.h>
>> +#include <drm/drm_bridge_connector.h>
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_of.h>
>>   #include <drm/drm_panel.h>
>> @@ -95,8 +96,7 @@ static int exynos_dp_get_modes(struct analogix_dp_plat_data *plat_data,
>>   }
>>   
>>   static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
>> -				   struct drm_bridge *bridge,
>> -				   struct drm_connector *connector)
>> +				   struct drm_bridge *bridge)
>>   {
>>   	struct exynos_dp_device *dp = to_dp(plat_data);
>>   	int ret;
>> @@ -147,6 +147,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>>   	struct exynos_dp_device *dp = dev_get_drvdata(dev);
>>   	struct drm_encoder *encoder = &dp->encoder;
>>   	struct drm_device *drm_dev = data;
>> +	struct drm_connector *connector;
>>   	int ret;
>>   
>>   	dp->drm_dev = drm_dev;
>> @@ -168,10 +169,19 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>>   	dp->plat_data.encoder = encoder;
>>   
>>   	ret = analogix_dp_bind(dp->adp, dp->drm_dev);
>> -	if (ret)
>> +	if (ret) {
>>   		dp->encoder.funcs->destroy(&dp->encoder);
>> +		return ret;
>> +	}
>> +
>> +	connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
>> +	if (IS_ERR(connector)) {
>> +		ret = PTR_ERR(connector);
>> +		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
>> +		return ret;
>> +	}
>>   
>> -	return ret;
>> +	return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
>>   }
>>   
>>   static void exynos_dp_unbind(struct device *dev, struct device *master,
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index d30f0983a53a..87dfb48206db 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -25,6 +25,7 @@
>>   #include <drm/display/drm_dp_helper.h>
>>   #include <drm/drm_atomic.h>
>>   #include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_bridge_connector.h>
>>   #include <drm/bridge/analogix_dp.h>
>>   #include <drm/drm_of.h>
>>   #include <drm/drm_panel.h>
>> @@ -394,6 +395,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>>   {
>>   	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
>>   	struct drm_device *drm_dev = data;
>> +	struct drm_connector *connector;
>>   	int ret;
>>   
>>   	dp->drm_dev = drm_dev;
>> @@ -413,7 +415,14 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>>   	if (ret)
>>   		goto err_cleanup_encoder;
>>   
>> -	return 0;
>> +	connector = drm_bridge_connector_init(dp->drm_dev, dp->plat_data.encoder);
>> +	if (IS_ERR(connector)) {
>> +		ret = PTR_ERR(connector);
>> +		dev_err(dp->dev, "Failed to initialize bridge_connector\n");
>> +		goto err_cleanup_encoder;
>> +	}
>> +
>> +	return drm_connector_attach_encoder(connector, dp->plat_data.encoder);
>>   err_cleanup_encoder:
>>   	dp->encoder.encoder.funcs->destroy(&dp->encoder.encoder);
>>   	return ret;
>> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
>> index f06da105d8f2..ffc05f3de232 100644
>> --- a/include/drm/bridge/analogix_dp.h
>> +++ b/include/drm/bridge/analogix_dp.h
>> @@ -33,8 +33,7 @@ struct analogix_dp_plat_data {
>>   
>>   	int (*power_on)(struct analogix_dp_plat_data *);
>>   	int (*power_off)(struct analogix_dp_plat_data *);
>> -	int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *,
>> -		      struct drm_connector *);
>> +	int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *);
>>   	int (*get_modes)(struct analogix_dp_plat_data *,
>>   			 struct drm_connector *);
>>   };
>> -- 
>> 2.34.1
>>
> 

Best regards,
Damon

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Damon Ding]

Hi Marek,

On 8/20/2025 5:20 AM, Marek Szyprowski wrote:
> On 15.08.2025 04:59, Damon Ding wrote:
>> On 2025/8/15 5:16, Marek Szyprowski wrote:
>>> On 14.08.2025 16:33, Marek Szyprowski wrote:
>>>> On 14.08.2025 12:47, Damon Ding wrote:
>>>>> PATCH 1 is a small format optimization for struct analogid_dp_device.
>>>>> PATCH 2 is to perform mode setting in &drm_bridge_funcs.atomic_enable.
>>>>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>>>>> PATCH 7 is to apply the drm_bridge_connector helper.
>>>>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix side.
>>>>> PATCH 11-12 are preparations for apply panel_bridge helper.
>>>>> PATCH 13 is to apply the panel_bridge helper.
>>>>
>>>> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's Kconfig,
>>>> so it causes build break:
>>>>
>>>> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
>>>> `drm_bridge_connector_init'
>>>> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>>>>
>>>> After adding this dependency, the Exynos DP driver stops working. On
>>>> Samsung Snow Chromebook I observed following issue:
>>>>
>>>> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
>>>> following panel or bridge (-16)
>>>> [    4.543428] exynos-drm exynos-drm: failed to bind
>>>> 145b0000.dp-controller (ops exynos_dp_ops): -16
>>>> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
>>>> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
>>>> exynos-dp failed with error -16
>>>>
>>>> I will investigate details later in the evening.
>>>
>>> The failure is caused by trying to add plat_data->next_bridge twice
>>> (from exynos_dp's .attach callback, and from analogix' ->bind callback).
>>>
>>>
>>> Best regards
>>
>> I see. The bridge attachment for the next bridge was not well thought
>> out. It may be better to move panel_bridge addition a little forward
>> and remove next_bridge attachment on the Analogix side. Then, the
>> Rockchip side and Exynos side can do their own next_bridge attachment
>> in &analogix_dp_plat_data.attach() as they want.
>>
>> Could you please help test the following modifications(they have been
>> tested on my RK3588S EVB1 Board) on the Samsung Snow Chromebook? ;-)
> 
> Assuming that I properly applied the malformed diff, it doesn't solve
> all the issues. There are no errors reported though, but the display
> chain doesn't work and no valid mode is reported:
> 
> # dmesg | grep drm
> [    3.384992] [drm] Initialized panfrost 1.4.0 for 11800000.gpu on minor 0
> [    4.487739] [drm] Exynos DRM: using 14400000.fimd device for DMA
> mapping operations
> [    4.494202] exynos-drm exynos-drm: bound 14400000.fimd (ops
> fimd_component_ops)
> [    4.502374] exynos-drm exynos-drm: bound 14450000.mixer (ops
> mixer_component_ops)
> [    4.511930] exynos-drm exynos-drm: bound 145b0000.dp-controller (ops
> exynos_dp_ops)
> [    4.518411] exynos-drm exynos-drm: bound 14530000.hdmi (ops
> hdmi_component_ops)
> [    4.529628] [drm] Initialized exynos 1.1.0 for exynos-drm on minor 1
> [    4.657434] exynos-drm exynos-drm: [drm] Cannot find any crtc or sizes
> [    4.925023] exynos-drm exynos-drm: [drm] Cannot find any crtc or sizes
> 
> # ./modetest -c -Mexynos
> Connectors:
> id      encoder status          name            size (mm)       modes
>     encoders
> 69      0       disconnected    LVDS-1          0x0             0       68
>    props:
>          1 EDID:
>                  flags: immutable blob
>                  blobs:
> 
>                  value:
>          2 DPMS:
>                  flags: enum
>                  enums: On=0 Standby=1 Suspend=2 Off=3
>                  value: 0
>          5 link-status:
>                  flags: enum
>                  enums: Good=0 Bad=1
>                  value: 0
>          6 non-desktop:
>                  flags: immutable range
>                  values: 0 1
>                  value: 0
>          4 TILE:
>                  flags: immutable blob
>                  blobs:
> 
>                  value:
> 71      0       disconnected    HDMI-A-1        0x0             0       70
>    props:
>          1 EDID:
>                  flags: immutable blob
>                  blobs:
> 
>                  value:
>          2 DPMS:
>                  flags: enum
>                  enums: On=0 Standby=1 Suspend=2 Off=3
>                  value: 0
>          5 link-status:
>                  flags: enum
>                  enums: Good=0 Bad=1
>                  value: 0
>          6 non-desktop:
>                  flags: immutable range
>                  values: 0 1
>                  value: 0
>          4 TILE:
>                  flags: immutable blob
>                  blobs:
> 
>                  value:
> 
> 
> I will investigate details later this week.
> 

Could you please provide the related DTS file for the test? I will also 
try to find out the reason for this unexpected issue. ;-)

> 
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 0529bfb02884..8a9ce1f31678 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1573,6 +1573,15 @@ int analogix_dp_bind(struct analogix_dp_device
>> *dp, struct drm_device *drm_dev)
>>                  return ret;
>>          }
>>
>> +       if (dp->plat_data->panel) {
>> +               dp->plat_data->next_bridge =
>> devm_drm_panel_bridge_add(dp->dev,
>> + dp->plat_data->panel);
>> +               if (IS_ERR(dp->plat_data->next_bridge)) {
>> +                       ret = PTR_ERR(bridge);
>> +                       goto err_unregister_aux;
>> +               }
>> +       }
>> +
>>          bridge->ops = DRM_BRIDGE_OP_DETECT |
>>                        DRM_BRIDGE_OP_EDID |
>>                        DRM_BRIDGE_OP_MODES;
>> @@ -1588,22 +1597,6 @@ int analogix_dp_bind(struct analogix_dp_device
>> *dp, struct drm_device *drm_dev)
>>                  goto err_unregister_aux;
>>          }
>>
>> -       if (dp->plat_data->panel) {
>> -               dp->plat_data->next_bridge =
>> devm_drm_panel_bridge_add(dp->dev,
>> - dp->plat_data->panel);
>> -               if (IS_ERR(dp->plat_data->next_bridge)) {
>> -                       ret = PTR_ERR(bridge);
>> -                       goto err_unregister_aux;
>> -               }
>> -       }
>> -
>> -       ret = drm_bridge_attach(dp->encoder,
>> dp->plat_data->next_bridge, bridge,
>> -                               DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>> -       if (ret) {
>> -               dev_err(dp->dev, "failed to attach following panel or
>> bridge (%d)\n", ret);
>> -               goto err_unregister_aux;
>> -       }
>> -
>>          return 0;
>>
>>   err_unregister_aux:
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
>> b/drivers/gpu/drm/exynos/exynos_dp.c
>> index 80ba700d2964..d0422f940249 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
>> @@ -104,7 +104,7 @@ static int exynos_dp_bridge_attach(struct
>> analogix_dp_plat_data *plat_data,
>>          /* Pre-empt DP connector creation if there's a bridge */
>>          if (plat_data->next_bridge) {
>>                  ret = drm_bridge_attach(&dp->encoder,
>> plat_data->next_bridge, bridge,
>> -                                       0);
>> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>>                  if (ret)
>>                          return ret;
>>          }
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index 0862b09a8be2..dfd32a79b94f 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -164,6 +164,24 @@ static int rockchip_dp_powerdown(struct
>> analogix_dp_plat_data *plat_data)
>>          return 0;
>>   }
>>
>> +static int rockchip_dp_attach(struct analogix_dp_plat_data *plat_data,
>> +                                    struct drm_bridge *bridge)
>> +{
>> +       struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
>> +       int ret;
>> +
>> +       if (plat_data->next_bridge) {
>> +               ret = drm_bridge_attach(&dp->encoder.encoder,
>> plat_data->next_bridge, bridge,
>> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>> +               if (ret) {
>> +                       dev_err(dp->dev, "failed to attach following
>> panel or bridge (%d)\n", ret);
>> +                       return ret;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>   static int rockchip_dp_get_modes(struct analogix_dp_plat_data
>> *plat_data,
>>                                   struct drm_connector *connector)
>>   {
>> @@ -478,6 +496,7 @@ static int rockchip_dp_probe(struct
>> platform_device *pdev)
>>          dp->plat_data.dev_type = dp->data->chip_type;
>>          dp->plat_data.power_on = rockchip_dp_poweron;
>>          dp->plat_data.power_off = rockchip_dp_powerdown;
>> +       dp->plat_data.attach = rockchip_dp_attach;
>>          dp->plat_data.get_modes = rockchip_dp_get_modes;
>>          dp->plat_data.ops = &rockchip_dp_component_ops;
>>
>>

Best regards,
Damon

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Marek Szyprowski]

On 29.08.2025 10:08, Damon Ding wrote:
> On 8/20/2025 5:20 AM, Marek Szyprowski wrote:
>> On 15.08.2025 04:59, Damon Ding wrote:
>>> On 2025/8/15 5:16, Marek Szyprowski wrote:
>>>> On 14.08.2025 16:33, Marek Szyprowski wrote:
>>>>> On 14.08.2025 12:47, Damon Ding wrote:
>>>>>> PATCH 1 is a small format optimization for struct 
>>>>>> analogid_dp_device.
>>>>>> PATCH 2 is to perform mode setting in 
>>>>>> &drm_bridge_funcs.atomic_enable.
>>>>>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>>>>>> PATCH 7 is to apply the drm_bridge_connector helper.
>>>>>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix 
>>>>>> side.
>>>>>> PATCH 11-12 are preparations for apply panel_bridge helper.
>>>>>> PATCH 13 is to apply the panel_bridge helper.
>>>>>
>>>>> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's 
>>>>> Kconfig,
>>>>> so it causes build break:
>>>>>
>>>>> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
>>>>> `drm_bridge_connector_init'
>>>>> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>>>>>
>>>>> After adding this dependency, the Exynos DP driver stops working. On
>>>>> Samsung Snow Chromebook I observed following issue:
>>>>>
>>>>> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
>>>>> following panel or bridge (-16)
>>>>> [    4.543428] exynos-drm exynos-drm: failed to bind
>>>>> 145b0000.dp-controller (ops exynos_dp_ops): -16
>>>>> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
>>>>> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
>>>>> exynos-dp failed with error -16
>>>>>
>>>>> I will investigate details later in the evening.
>>>>
>>>> The failure is caused by trying to add plat_data->next_bridge twice
>>>> (from exynos_dp's .attach callback, and from analogix' ->bind 
>>>> callback).
>>>>
>>>>
>>>> Best regards
>>>
>>> I see. The bridge attachment for the next bridge was not well thought
>>> out. It may be better to move panel_bridge addition a little forward
>>> and remove next_bridge attachment on the Analogix side. Then, the
>>> Rockchip side and Exynos side can do their own next_bridge attachment
>>> in &analogix_dp_plat_data.attach() as they want.
>>>
>>> Could you please help test the following modifications(they have been
>>> tested on my RK3588S EVB1 Board) on the Samsung Snow Chromebook? ;-)
>>
>> Assuming that I properly applied the malformed diff, it doesn't solve
>> all the issues. There are no errors reported though, but the display
>> chain doesn't work and no valid mode is reported:
>>
>> # dmesg | grep drm
>> [    3.384992] [drm] Initialized panfrost 1.4.0 for 11800000.gpu on 
>> minor 0
>> [    4.487739] [drm] Exynos DRM: using 14400000.fimd device for DMA
>> mapping operations
>> [    4.494202] exynos-drm exynos-drm: bound 14400000.fimd (ops
>> fimd_component_ops)
>> [    4.502374] exynos-drm exynos-drm: bound 14450000.mixer (ops
>> mixer_component_ops)
>> [    4.511930] exynos-drm exynos-drm: bound 145b0000.dp-controller (ops
>> exynos_dp_ops)
>> [    4.518411] exynos-drm exynos-drm: bound 14530000.hdmi (ops
>> hdmi_component_ops)
>> [    4.529628] [drm] Initialized exynos 1.1.0 for exynos-drm on minor 1
>> [    4.657434] exynos-drm exynos-drm: [drm] Cannot find any crtc or 
>> sizes
>> [    4.925023] exynos-drm exynos-drm: [drm] Cannot find any crtc or 
>> sizes
>>
>> # ./modetest -c -Mexynos
>> Connectors:
>> id      encoder status          name            size (mm)       modes
>>     encoders
>> 69      0       disconnected    LVDS-1          0x0             0 
>>       68
>>    props:
>>          1 EDID:
>>                  flags: immutable blob
>>                  blobs:
>>
>>                  value:
>>          2 DPMS:
>>                  flags: enum
>>                  enums: On=0 Standby=1 Suspend=2 Off=3
>>                  value: 0
>>          5 link-status:
>>                  flags: enum
>>                  enums: Good=0 Bad=1
>>                  value: 0
>>          6 non-desktop:
>>                  flags: immutable range
>>                  values: 0 1
>>                  value: 0
>>          4 TILE:
>>                  flags: immutable blob
>>                  blobs:
>>
>>                  value:
>> 71      0       disconnected    HDMI-A-1        0x0             0 
>>       70
>>    props:
>>          1 EDID:
>>                  flags: immutable blob
>>                  blobs:
>>
>>                  value:
>>          2 DPMS:
>>                  flags: enum
>>                  enums: On=0 Standby=1 Suspend=2 Off=3
>>                  value: 0
>>          5 link-status:
>>                  flags: enum
>>                  enums: Good=0 Bad=1
>>                  value: 0
>>          6 non-desktop:
>>                  flags: immutable range
>>                  values: 0 1
>>                  value: 0
>>          4 TILE:
>>                  flags: immutable blob
>>                  blobs:
>>
>>                  value:
>>
>>
>> I will investigate details later this week.
>>
>
> Could you please provide the related DTS file for the test? I will 
> also try to find out the reason for this unexpected issue. ;-)

Unfortunately I didn't find enough time to debug this further. The above 
log is from Samsung Snow Chromebook, 
arch/arm/boot/dts/samsung/exynos5250-snow.dts


 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 07/13] drm/bridge: analogix_dp: Apply drm_bridge_connector helper

[Dmitry Baryshkov]

On Wed, Aug 20, 2025 at 05:18:13PM +0800, Damon Ding wrote:
> Hi Dmitry,
> 
> On 8/17/2025 12:43 AM, Dmitry Baryshkov wrote:
> > On Thu, Aug 14, 2025 at 06:47:47PM +0800, Damon Ding wrote:
> > > Apply drm_bridge_connector helper for Analogix DP driver.
> > > 
> > > The following changes have been made:
> > > - Apply drm_bridge_connector helper to get rid of &drm_connector_funcs
> > >    and &drm_connector_helper_funcs.
> > > - Remove unnecessary parameter struct drm_connector* for callback
> > >    &analogix_dp_plat_data.attach.
> > > - Remove &analogix_dp_device.connector.
> > > - Convert analogix_dp_atomic_check()/analogix_dp_detect() to
> > >    &drm_bridge_funcs.atomic_check()/&drm_bridge_funcs.detect().
> > > - Split analogix_dp_get_modes() into &drm_bridge_funcs.get_modes() and
> > >    &drm_bridge_funcs.edid_read().
> > > 
> > > Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> > > 
> > > ------
> > > 
> > > Changes in v2:
> > > - For &drm_bridge.ops, remove DRM_BRIDGE_OP_HPD and add
> > >    DRM_BRIDGE_OP_EDID.
> > > - Add analogix_dp_bridge_edid_read().
> > > - Move &analogix_dp_plat_data.skip_connector deletion to the previous
> > >    patches.
> > > 
> > > Changes in v3:
> > > - Rebase with the new devm_drm_bridge_alloc() related commit
> > >    48f05c3b4b70 ("drm/bridge: analogix_dp: Use devm_drm_bridge_alloc()
> > >    API").
> > > - Expand the commit message.
> > > - Call drm_bridge_get_modes() in analogix_dp_bridge_get_modes() if the
> > >    bridge is available.
> > > - Remove unnecessary parameter struct drm_connector* for callback
> > >    &analogix_dp_plat_data.attach.
> > > - In order to decouple the connector driver and the bridge driver, move
> > >    the bridge connector initilization to the Rockchip and Exynos sides.
> > > 
> > > Changes in v4:
> > > - Expand analogix_dp_bridge_detect() parameters to &drm_bridge and
> > >    &drm_connector.
> > > - Rename the &analogix_dp_plat_data.bridge to
> > >    &analogix_dp_plat_data.next_bridge.
> > > ---
> > >   .../drm/bridge/analogix/analogix_dp_core.c    | 145 ++++++++----------
> > >   .../drm/bridge/analogix/analogix_dp_core.h    |   1 -
> > >   drivers/gpu/drm/exynos/exynos_dp.c            |  18 ++-
> > >   .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  11 +-
> > >   include/drm/bridge/analogix_dp.h              |   3 +-
> > >   5 files changed, 88 insertions(+), 90 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > index 7876b310aaed..a8ed44ec8ef5 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > @@ -947,24 +947,16 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> > >   	return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
> > >   }
> > > -static int analogix_dp_get_modes(struct drm_connector *connector)
> > > +static int analogix_dp_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector)
> > >   {
> > > -	struct analogix_dp_device *dp = to_dp(connector);
> > > -	const struct drm_edid *drm_edid;
> > > +	struct analogix_dp_device *dp = to_dp(bridge);
> > >   	int num_modes = 0;
> > > -	if (dp->plat_data->panel) {
> > > +	if (dp->plat_data->panel)
> > >   		num_modes += drm_panel_get_modes(dp->plat_data->panel, connector);
> > > -	} else {
> > > -		drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
> > > -		drm_edid_connector_update(&dp->connector, drm_edid);
> > > -
> > > -		if (drm_edid) {
> > > -			num_modes += drm_edid_connector_add_modes(&dp->connector);
> > > -			drm_edid_free(drm_edid);
> > > -		}
> > > -	}
> > > +	if (dp->plat_data->next_bridge)
> > > +		num_modes += drm_bridge_get_modes(dp->plat_data->next_bridge, connector);
> > 
> > If there is a next bridge which provides OP_MODES, then
> > drm_bridge_connector will use it for get_modes() and skip this one
> > completely. I'm not sure what's the value of this call.
> 
> Following your advice, it is really a good idea to distinguish the
> drm_bridge_ops between the panel and the bridge. Will add it in v5.
> 
> > 
> > >   	if (dp->plat_data->get_modes)
> > >   		num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
> > > @@ -972,51 +964,39 @@ static int analogix_dp_get_modes(struct drm_connector *connector)
> > >   	return num_modes;
> > >   }
> > > -static struct drm_encoder *
> > > -analogix_dp_best_encoder(struct drm_connector *connector)
> > > +static const struct drm_edid *analogix_dp_bridge_edid_read(struct drm_bridge *bridge,
> > > +							   struct drm_connector *connector)
> > >   {
> > > -	struct analogix_dp_device *dp = to_dp(connector);
> > > +	struct analogix_dp_device *dp = to_dp(bridge);
> > > +	const struct drm_edid *drm_edid = NULL;
> > > -	return dp->encoder;
> > > -}
> > > +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
> > > +	if (dp->plat_data->get_modes)
> > > +		dp->plat_data->get_modes(dp->plat_data, connector);
> > 
> > 
> > So, we have DDC, but we still want to return platform modes? What is the
> > usecase for that?
> > 
> > There might be some, but I think it deserves a comment in the source
> > file.
> > 
> 
> For Rockchip side, since RK3588 and RK3576 can support YUV formats while the
> other can not, the &analogix_dp_plat_data.get_modes() help filter out YUV
> formats for some platforms(The YUV feature support may not be fit for this
> patch series and will come later).

Note, get_modes() here adds modes rather than filtering them. You can
use .mode_valid in order to filter out YUV modes.

> 
> For Exynos side, I think &analogix_dp_plat_data.get_modes() can help
> parse the video mode set in the eDP DT node when there is no available panel
> or bridge.

I think this should be handled by a separate bridge. E.g. see how the
imx-legacy-bridge is implemented.

> 
> I will add some comments about it in the next version.
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Damon Ding]

Hi Marek,

On 8/29/2025 4:23 PM, Marek Szyprowski wrote:
> On 29.08.2025 10:08, Damon Ding wrote:
>> On 8/20/2025 5:20 AM, Marek Szyprowski wrote:
>>> On 15.08.2025 04:59, Damon Ding wrote:
>>>> On 2025/8/15 5:16, Marek Szyprowski wrote:
>>>>> On 14.08.2025 16:33, Marek Szyprowski wrote:
>>>>>> On 14.08.2025 12:47, Damon Ding wrote:
>>>>>>> PATCH 1 is a small format optimization for struct
>>>>>>> analogid_dp_device.
>>>>>>> PATCH 2 is to perform mode setting in
>>>>>>> &drm_bridge_funcs.atomic_enable.
>>>>>>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>>>>>>> PATCH 7 is to apply the drm_bridge_connector helper.
>>>>>>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix
>>>>>>> side.
>>>>>>> PATCH 11-12 are preparations for apply panel_bridge helper.
>>>>>>> PATCH 13 is to apply the panel_bridge helper.
>>>>>>
>>>>>> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's
>>>>>> Kconfig,
>>>>>> so it causes build break:
>>>>>>
>>>>>> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
>>>>>> `drm_bridge_connector_init'
>>>>>> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>>>>>>
>>>>>> After adding this dependency, the Exynos DP driver stops working. On
>>>>>> Samsung Snow Chromebook I observed following issue:
>>>>>>
>>>>>> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
>>>>>> following panel or bridge (-16)
>>>>>> [    4.543428] exynos-drm exynos-drm: failed to bind
>>>>>> 145b0000.dp-controller (ops exynos_dp_ops): -16
>>>>>> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
>>>>>> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
>>>>>> exynos-dp failed with error -16
>>>>>>
>>>>>> I will investigate details later in the evening.
>>>>>
>>>>> The failure is caused by trying to add plat_data->next_bridge twice
>>>>> (from exynos_dp's .attach callback, and from analogix' ->bind
>>>>> callback).
>>>>>
>>>>>
>>>>> Best regards
>>>>
>>>> I see. The bridge attachment for the next bridge was not well thought
>>>> out. It may be better to move panel_bridge addition a little forward
>>>> and remove next_bridge attachment on the Analogix side. Then, the
>>>> Rockchip side and Exynos side can do their own next_bridge attachment
>>>> in &analogix_dp_plat_data.attach() as they want.
>>>>
>>>> Could you please help test the following modifications(they have been
>>>> tested on my RK3588S EVB1 Board) on the Samsung Snow Chromebook? ;-)
>>>
>>> Assuming that I properly applied the malformed diff, it doesn't solve
>>> all the issues. There are no errors reported though, but the display
>>> chain doesn't work and no valid mode is reported:
>>>
>>> # dmesg | grep drm
>>> [    3.384992] [drm] Initialized panfrost 1.4.0 for 11800000.gpu on
>>> minor 0
>>> [    4.487739] [drm] Exynos DRM: using 14400000.fimd device for DMA
>>> mapping operations
>>> [    4.494202] exynos-drm exynos-drm: bound 14400000.fimd (ops
>>> fimd_component_ops)
>>> [    4.502374] exynos-drm exynos-drm: bound 14450000.mixer (ops
>>> mixer_component_ops)
>>> [    4.511930] exynos-drm exynos-drm: bound 145b0000.dp-controller (ops
>>> exynos_dp_ops)
>>> [    4.518411] exynos-drm exynos-drm: bound 14530000.hdmi (ops
>>> hdmi_component_ops)
>>> [    4.529628] [drm] Initialized exynos 1.1.0 for exynos-drm on minor 1
>>> [    4.657434] exynos-drm exynos-drm: [drm] Cannot find any crtc or
>>> sizes
>>> [    4.925023] exynos-drm exynos-drm: [drm] Cannot find any crtc or
>>> sizes
>>>
>>> # ./modetest -c -Mexynos
>>> Connectors:
>>> id      encoder status          name            size (mm)       modes
>>>      encoders
>>> 69      0       disconnected    LVDS-1          0x0             0
>>>        68
>>>     props:
>>>           1 EDID:
>>>                   flags: immutable blob
>>>                   blobs:
>>>
>>>                   value:
>>>           2 DPMS:
>>>                   flags: enum
>>>                   enums: On=0 Standby=1 Suspend=2 Off=3
>>>                   value: 0
>>>           5 link-status:
>>>                   flags: enum
>>>                   enums: Good=0 Bad=1
>>>                   value: 0
>>>           6 non-desktop:
>>>                   flags: immutable range
>>>                   values: 0 1
>>>                   value: 0
>>>           4 TILE:
>>>                   flags: immutable blob
>>>                   blobs:
>>>
>>>                   value:
>>> 71      0       disconnected    HDMI-A-1        0x0             0
>>>        70
>>>     props:
>>>           1 EDID:
>>>                   flags: immutable blob
>>>                   blobs:
>>>
>>>                   value:
>>>           2 DPMS:
>>>                   flags: enum
>>>                   enums: On=0 Standby=1 Suspend=2 Off=3
>>>                   value: 0
>>>           5 link-status:
>>>                   flags: enum
>>>                   enums: Good=0 Bad=1
>>>                   value: 0
>>>           6 non-desktop:
>>>                   flags: immutable range
>>>                   values: 0 1
>>>                   value: 0
>>>           4 TILE:
>>>                   flags: immutable blob
>>>                   blobs:
>>>
>>>                   value:
>>>
>>>
>>> I will investigate details later this week.
>>>
>>
>> Could you please provide the related DTS file for the test? I will
>> also try to find out the reason for this unexpected issue. ;-)
> 
> Unfortunately I didn't find enough time to debug this further. The above
> log is from Samsung Snow Chromebook,
> arch/arm/boot/dts/samsung/exynos5250-snow.dts
> 
> 

I compare the differences in the following display path before and after 
this patch series:

exynos_dp -> nxp-ptn3460 -> panel "auo,b116xw03"

The issue is likely caused by the &drm_connector_funcs.detect() related 
logic. Before this patch series, the nxp-ptn3460 connector is always 
connector_status_connected because there is not available 
&drm_connector_funcs.detect(). After it, the DRM_BRIDGE_OP_DETECT flag 
make the connection status depend on analogix_dp_bridge_detect().

Could you please add the following patches additionally and try again?
(Not the final solution, just validation)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index a93ff8f0a468..355911c47354 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1491,9 +1491,11 @@ int analogix_dp_bind(struct analogix_dp_device 
*dp, struct drm_device *drm_dev)
                 }
         }

-       bridge->ops = DRM_BRIDGE_OP_DETECT |
-                     DRM_BRIDGE_OP_EDID |
+       bridge->ops = DRM_BRIDGE_OP_EDID |
                       DRM_BRIDGE_OP_MODES;
+       if (drm_bridge_is_panel(dp->plat_data->next_bridge))
+               bridge->ops |= DRM_BRIDGE_OP_DETECT;
+
         bridge->of_node = dp->dev->of_node;
         bridge->type = DRM_MODE_CONNECTOR_eDP;
         ret = devm_drm_bridge_add(dp->dev, &dp->bridge);

Best regards,
Damon

Re: [PATCH v4 00/13] Apply drm_bridge_connector and panel_bridge helper for the Analogix DP driver

[Marek Szyprowski]

On 01.09.2025 05:41, Damon Ding wrote:
> On 8/29/2025 4:23 PM, Marek Szyprowski wrote:
>> On 29.08.2025 10:08, Damon Ding wrote:
>>> On 8/20/2025 5:20 AM, Marek Szyprowski wrote:
>>>> On 15.08.2025 04:59, Damon Ding wrote:
>>>>> On 2025/8/15 5:16, Marek Szyprowski wrote:
>>>>>> On 14.08.2025 16:33, Marek Szyprowski wrote:
>>>>>>> On 14.08.2025 12:47, Damon Ding wrote:
>>>>>>>> PATCH 1 is a small format optimization for struct
>>>>>>>> analogid_dp_device.
>>>>>>>> PATCH 2 is to perform mode setting in
>>>>>>>> &drm_bridge_funcs.atomic_enable.
>>>>>>>> PATCH 3-6 are preparations for apply drm_bridge_connector helper.
>>>>>>>> PATCH 7 is to apply the drm_bridge_connector helper.
>>>>>>>> PATCH 8-10 are to move the panel/bridge parsing to the Analogix
>>>>>>>> side.
>>>>>>>> PATCH 11-12 are preparations for apply panel_bridge helper.
>>>>>>>> PATCH 13 is to apply the panel_bridge helper.
>>>>>>>
>>>>>>> This series lacks 'select DRM_BRIDGE_CONNECTOR' in ExynosDP's
>>>>>>> Kconfig,
>>>>>>> so it causes build break:
>>>>>>>
>>>>>>> drivers/gpu/drm/exynos/exynos_dp.c:177: undefined reference to
>>>>>>> `drm_bridge_connector_init'
>>>>>>> make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux] Error 1
>>>>>>>
>>>>>>> After adding this dependency, the Exynos DP driver stops 
>>>>>>> working. On
>>>>>>> Samsung Snow Chromebook I observed following issue:
>>>>>>>
>>>>>>> [    4.534220] exynos-dp 145b0000.dp-controller: failed to attach
>>>>>>> following panel or bridge (-16)
>>>>>>> [    4.543428] exynos-drm exynos-drm: failed to bind
>>>>>>> 145b0000.dp-controller (ops exynos_dp_ops): -16
>>>>>>> [    4.551775] exynos-drm exynos-drm: adev bind failed: -16
>>>>>>> [    4.556559] exynos-dp 145b0000.dp-controller: probe with driver
>>>>>>> exynos-dp failed with error -16
>>>>>>>
>>>>>>> I will investigate details later in the evening.
>>>>>>
>>>>>> The failure is caused by trying to add plat_data->next_bridge twice
>>>>>> (from exynos_dp's .attach callback, and from analogix' ->bind
>>>>>> callback).
>>>>>>
>>>>>>
>>>>>> Best regards
>>>>>
>>>>> I see. The bridge attachment for the next bridge was not well thought
>>>>> out. It may be better to move panel_bridge addition a little forward
>>>>> and remove next_bridge attachment on the Analogix side. Then, the
>>>>> Rockchip side and Exynos side can do their own next_bridge attachment
>>>>> in &analogix_dp_plat_data.attach() as they want.
>>>>>
>>>>> Could you please help test the following modifications(they have been
>>>>> tested on my RK3588S EVB1 Board) on the Samsung Snow Chromebook? ;-)
>>>>
>>>> Assuming that I properly applied the malformed diff, it doesn't solve
>>>> all the issues. There are no errors reported though, but the display
>>>> chain doesn't work and no valid mode is reported:
>>>>
>>>> # dmesg | grep drm
>>>> [    3.384992] [drm] Initialized panfrost 1.4.0 for 11800000.gpu on
>>>> minor 0
>>>> [    4.487739] [drm] Exynos DRM: using 14400000.fimd device for DMA
>>>> mapping operations
>>>> [    4.494202] exynos-drm exynos-drm: bound 14400000.fimd (ops
>>>> fimd_component_ops)
>>>> [    4.502374] exynos-drm exynos-drm: bound 14450000.mixer (ops
>>>> mixer_component_ops)
>>>> [    4.511930] exynos-drm exynos-drm: bound 145b0000.dp-controller 
>>>> (ops
>>>> exynos_dp_ops)
>>>> [    4.518411] exynos-drm exynos-drm: bound 14530000.hdmi (ops
>>>> hdmi_component_ops)
>>>> [    4.529628] [drm] Initialized exynos 1.1.0 for exynos-drm on 
>>>> minor 1
>>>> [    4.657434] exynos-drm exynos-drm: [drm] Cannot find any crtc or
>>>> sizes
>>>> [    4.925023] exynos-drm exynos-drm: [drm] Cannot find any crtc or
>>>> sizes
>>>>
>>>> # ./modetest -c -Mexynos
>>>> Connectors:
>>>> id      encoder status          name            size (mm)       modes
>>>>      encoders
>>>> 69      0       disconnected    LVDS-1          0x0             0
>>>>        68
>>>>     props:
>>>>           1 EDID:
>>>>                   flags: immutable blob
>>>>                   blobs:
>>>>
>>>>                   value:
>>>>           2 DPMS:
>>>>                   flags: enum
>>>>                   enums: On=0 Standby=1 Suspend=2 Off=3
>>>>                   value: 0
>>>>           5 link-status:
>>>>                   flags: enum
>>>>                   enums: Good=0 Bad=1
>>>>                   value: 0
>>>>           6 non-desktop:
>>>>                   flags: immutable range
>>>>                   values: 0 1
>>>>                   value: 0
>>>>           4 TILE:
>>>>                   flags: immutable blob
>>>>                   blobs:
>>>>
>>>>                   value:
>>>> 71      0       disconnected    HDMI-A-1        0x0             0
>>>>        70
>>>>     props:
>>>>           1 EDID:
>>>>                   flags: immutable blob
>>>>                   blobs:
>>>>
>>>>                   value:
>>>>           2 DPMS:
>>>>                   flags: enum
>>>>                   enums: On=0 Standby=1 Suspend=2 Off=3
>>>>                   value: 0
>>>>           5 link-status:
>>>>                   flags: enum
>>>>                   enums: Good=0 Bad=1
>>>>                   value: 0
>>>>           6 non-desktop:
>>>>                   flags: immutable range
>>>>                   values: 0 1
>>>>                   value: 0
>>>>           4 TILE:
>>>>                   flags: immutable blob
>>>>                   blobs:
>>>>
>>>>                   value:
>>>>
>>>>
>>>> I will investigate details later this week.
>>>>
>>>
>>> Could you please provide the related DTS file for the test? I will
>>> also try to find out the reason for this unexpected issue. ;-)
>>
>> Unfortunately I didn't find enough time to debug this further. The above
>> log is from Samsung Snow Chromebook,
>> arch/arm/boot/dts/samsung/exynos5250-snow.dts
>>
>>
>
> I compare the differences in the following display path before and 
> after this patch series:
>
> exynos_dp -> nxp-ptn3460 -> panel "auo,b116xw03"
>
> The issue is likely caused by the &drm_connector_funcs.detect() 
> related logic. Before this patch series, the nxp-ptn3460 connector is 
> always connector_status_connected because there is not available 
> &drm_connector_funcs.detect(). After it, the DRM_BRIDGE_OP_DETECT flag 
> make the connection status depend on analogix_dp_bridge_detect().
>
> Could you please add the following patches additionally and try again?
> (Not the final solution, just validation)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index a93ff8f0a468..355911c47354 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1491,9 +1491,11 @@ int analogix_dp_bind(struct analogix_dp_device 
> *dp, struct drm_device *drm_dev)
>                 }
>         }
>
> -       bridge->ops = DRM_BRIDGE_OP_DETECT |
> -                     DRM_BRIDGE_OP_EDID |
> +       bridge->ops = DRM_BRIDGE_OP_EDID |
>                       DRM_BRIDGE_OP_MODES;
> +       if (drm_bridge_is_panel(dp->plat_data->next_bridge))
> +               bridge->ops |= DRM_BRIDGE_OP_DETECT;
> +
>         bridge->of_node = dp->dev->of_node;
>         bridge->type = DRM_MODE_CONNECTOR_eDP;
>         ret = devm_drm_bridge_add(dp->dev, &dp->bridge);

It is better. Now the display panel is detected and reported to 
userspace, but it looks that something is not properly initialized, 
because there is garbage instead of the proper picture.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland