Re: [PATCH v2 00/10] Add HPD support for Rockchip Analogix DP

["Heiko Stübner" <heiko@sntech.de>]

Hi Damon,

Am Dienstag, 4. August 2026, 10:17:07 Mitteleuropäische Sommerzeit schrieb Damon Ding:
> Display-connector mode (DP connector without HPD GPIO):
> 
>   &edp_out_conn {
>       remote-endpoint = <&dp_con_in>;
>   };
> 
>   dp-con {
>       compatible = "dp-connector";
>       label = "DP OUT";
>       type = "full-size";
> 
>       port {
>           dp_con_in: endpoint {
>               remote-endpoint = <&edp_out_conn>;
>           };
>       };
>   };
> 
> Display-connector mode (DP connector with HPD GPIO):
> 
>   dp-con {
>       compatible = "dp-connector";
>       label = "DP OUT";
>       type = "full-size";
>       pinctrl-0 = <&edp0_hpd>;
>       pinctrl-names = "default";
>       hpd-gpios = <&gpio4 RK_PC1 GPIO_ACTIVE_HIGH>;
> 
>       port {
>           dp_con_in: endpoint {
>               remote-endpoint = <&edp_out_conn>;
>           };
>       };
>   };
> 
> All four configurations detect cable plug/unplug events correctly.

hmm, it wasn't working entirely for me though and was still running into
issues when the display was unplugged on boot.

I wiggled around a bit like in the diff below and am now getting correct
plug and unplug events.

But of course, as the dp-variant of the connector does not provide
a "detect" and just the "hpd" functionality, it's missing the initial state.

I'm currently not sure how to find out _if_ a panel is connected on boot.


In other review comments:
- the bridge could use devm_drm_of_get_bridge() as suggested in
  the documentation of drm_of_find_panel_or_bridge(), as that would
  remove the separate panel_bridge creation
- instead of using plat_data->next_bridge _inside_ the driver
  struct drm_bridge has a field next_bridge already.


Heiko


------- 8< -------
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 877e1b3ca7525..1388640a27de7 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -43,7 +43,7 @@ static const bool verify_fast_training;
 static bool analogix_dp_require_pm_for_hpd_irq(struct analogix_dp_device *dp)
 {
 	return analogix_dp_is_rockchip(dp->plat_data->dev_type) && !dp->hpd_gpiod &&
-	       !dp->force_hpd;
+	       !dp->hpd_bridge && !dp->force_hpd;
 }
 
 static void analogix_dp_init_dp(struct analogix_dp_device *dp)
@@ -72,7 +72,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
 	 * Trust connection status from downstream bridge (e.g.,
 	 * display-connector with hpd-gpios).
 	 */
-	if (dp->plat_data->next_bridge && dp->connection_notified)
+	if (dp->hpd_bridge && dp->connection_notified)
 		return 0;
 
 	while (timeout_loop < DP_TIMEOUT_LOOP_COUNT) {
@@ -926,10 +926,10 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
 	 */
 	if (dp->plat_data->next_bridge && dp->last_bridge_is_panel)
 		status = connector_status_connected;
-
-	if (!analogix_dp_detect_hpd(dp))
+	else if (!analogix_dp_detect_hpd(dp))
 		status = connector_status_connected;
 
+printk("---> %s status %d\n", __func__, status);
 	return status;
 }
 
@@ -1044,7 +1044,7 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
 		goto out_dp_init;
 	}
 
-	if (!analogix_dp_require_pm_for_hpd_irq(dp))
+	if (!analogix_dp_require_pm_for_hpd_irq(dp) && !dp->hpd_bridge)
 		enable_irq(dp->irq);
 	return 0;
 
@@ -1187,7 +1187,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
 	if (dp->dpms_mode != DRM_MODE_DPMS_ON)
 		return;
 
-	if (!analogix_dp_require_pm_for_hpd_irq(dp))
+	if (!analogix_dp_require_pm_for_hpd_irq(dp) && !dp->hpd_bridge)
 		disable_irq(dp->irq);
 
 	analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
@@ -1264,6 +1264,7 @@ static void analogix_dp_bridge_notify(struct drm_bridge *bridge, struct drm_conn
 	struct analogix_dp_device *dp = to_dp(bridge);
 
 	dp->connection_notified = (status == connector_status_connected);
+printk("---> %s connection_notified %d\n", __func__, dp->connection_notified);
 }
 
 static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
@@ -1641,6 +1642,13 @@ static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
 	if (ret && ret != -ENODEV)
 		return ret;
 
+	/*
+	 * There is a next link in the chain which is not a panel, we should
+	 * expect hotplug-information coming from there.
+	 */
+	if (plat_data->next_bridge && !drm_bridge_is_panel(plat_data->next_bridge))
+		dp->hpd_bridge = true;
+
 	return component_add(dp->dev, plat_data->ops);
 }
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index d0fb25e543ea0..ecca3b87b4456 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -169,6 +169,7 @@ struct analogix_dp_device {
 	bool			fast_train_enable;
 	bool			psr_supported;
 	bool			last_bridge_is_panel;
+	bool			hpd_bridge;
 	bool			connection_notified;
 
 	u8 dpcd[DP_RECEIVER_CAP_SIZE];
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index ec5950066f838..6f0d642739ffd 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -182,7 +182,7 @@ void analogix_dp_config_interrupt(struct analogix_dp_device *dp)
 	writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_2);
 	writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_3);
 
-	if (dp->hpd_gpiod) {
+	if (dp->hpd_gpiod || dp->hpd_bridge) {
 		analogix_dp_mute_hpd_interrupt(dp, HPD_IRQ);
 	} else {
 		/*
@@ -438,7 +438,7 @@ void analogix_dp_init_hpd(struct analogix_dp_device *dp)
 {
 	u32 reg;
 
-	if (dp->hpd_gpiod)
+	if (dp->hpd_gpiod || dp->hpd_bridge)
 		return;
 
 	analogix_dp_clear_hotplug_interrupts(dp, HPD_IRQ);
@@ -539,6 +539,9 @@ int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp)
 	if (dp->hpd_gpiod) {
 		if (gpiod_get_value(dp->hpd_gpiod))
 			return 0;
+	} else if (dp->hpd_bridge) {
+		if (dp->connection_notified)
+			return 0;
 	} else {
 		reg = readl(dp->reg_base + ANALOGIX_DP_SYS_CTL_3);
 		if (reg & HPD_STATUS)