.../drm/bridge/analogix/analogix_dp_core.c | 87 +++++++--- .../drm/bridge/analogix/analogix_dp_core.h | 12 +- .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 152 ++++++++++++------ 3 files changed, 175 insertions(+), 76 deletions(-)
This series improves the HPD (Hotplug Detect) interrupt handling in
the Analogix DP driver to enable reliable native HPD pin detection on
Rockchip platforms, and introduces platform-specific HPD detection
schemes with fine-grained interrupt control.
On Rockchip platforms, the Analogix DP native HPD pin IRQ requires the
DP controller to remain powered, clocked and initialized to generate
plug/unplug interrupts. The previous driver enabled/disabled IRQ during
bridge enable/disable, which left no HPD detection when the display
pipeline was inactive. Additionally, the interrupt mute/unmute/clear
routines operated on all HPD interrupt bits unconditionally, lacking
the granularity needed for per-event control.
The series reorganizes IRQ and pm_runtime management into bind/unbind,
adds IRQF_ONESHOT to eliminate read-modify-write races on interrupt
mask registers between hardirq and threaded handlers, converts the
interrupt type detection to a bitmask-based scheme for fine-grained
mute/unmute/clear operations, and configures Rockchip platforms to use
the HOTPLUG_CHG interrupt with a 2ms HPD deglitch setting for better
stability.
Patch 1: Move enable_irq()/disable_irq() to bind/unbind and hold a
pm_runtime reference for Rockchip native HPD pin mode.
Patch 2: Convert analogix_dp_get_irq_type() to return a u32 bitmask
instead of an enum, accumulating all pending interrupt flags.
Patch 3: Add IRQF_ONESHOT to prevent hardirq from preempting the
threaded handler; remove redundant mute/unmute from runtime
IRQ path; move status clearing before event handling.
Patch 4: Extend clear_hotplug_interrupts() to accept an irq_type
bitmask for per-event pending interrupt clearing.
Patch 5: Extend mute/unmute helpers to accept an irq_type bitmask for
init-time platform-specific interrupt mask configuration.
Patch 6: Simplify analogix_dp_config_interrupt() by removing redundant
local macros and leveraging the unmute helper.
Patch 7: Configure Rockchip platforms to use HOTPLUG_CHG interrupt with
2ms HPD deglitch; other platforms keep PLUG + HPD_LOST pair.
Patch 8: Skip native HPD interrupt register operations for GPIO HPD
mode, where hotplug is detected through an external GPIO.
Patch 9: Restrict the forced connected-status shortcut to panel
endpoints only, so DP connector bridges rely on HPD detection.
Patch 10: Handle HPD notification from downstream bridges (e.g.,
display-connector with hpd-gpios) to short-circuit
analogix_dp_detect_hpd() when connection is already confirmed.
Tested on RK3576 with both native HPD pin and GPIO HPD configurations.
Native HPD pin mode:
&edp {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&edp_txm0_pins>;
};
GPIO HPD mode:
&edp {
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&edp0_hpd>;
hpd-gpios = <&gpio4 RK_PC1 GPIO_ACTIVE_HIGH>;
};
&pinctrl {
edp {
edp0_hpd: edp0-hpd {
rockchip,pins = <4 RK_PC1 0 &pcfg_pull_none>;
};
};
};
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.
Damon Ding (10):
drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin
detection
drm/bridge: analogix_dp: Return bitmask from
analogix_dp_get_irq_type()
drm/bridge: analogix_dp: Add IRQF_ONESHOT and simplify IRQ handling
drm/bridge: analogix_dp: Extend clear_hotplug_interrupts to accept IRQ
bitmask
drm/bridge: analogix_dp: Extend mute/unmute HPD interrupts to accept
irq bitmask
drm/bridge: analogix_dp: Simplify analogix_dp_config_interrupt()
drm/bridge: analogix_dp: Use platform-specific HPD detection scheme
drm/bridge: analogix_dp: Skip native HPD interrupt ops for GPIO HPD
drm/bridge: analogix_dp: Restrict forced connected status only for
panel endpoint
drm/bridge: analogix_dp: Handle HPD notification from downstream
bridge
.../drm/bridge/analogix/analogix_dp_core.c | 87 +++++++---
.../drm/bridge/analogix/analogix_dp_core.h | 12 +-
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 152 ++++++++++++------
3 files changed, 175 insertions(+), 76 deletions(-)
---
Changes in v2:
- Split IRQ enable/disable logic, handle native HPD pin and
GPIO/force-HPD modes separately to avoid unbalanced enable_irq()
calls.(Sashiko)
- Add separate patch for IRQF_ONESHOT to resolve interrupt mask issues
triggered by interrupt preemption.(Sashiko)
- Update commit messages to align with newly added IRQF_ONESHOT related
commit.
- Move ANALOGIX_DP_HPD_DEGLITCH_L/ANALOGIX_DP_HPD_DEGLITCH_H configs to
analogix_dp_reset().
- Add new patch to restrict the forced connected-status shortcut to
panel endpoints only, allowing DP connector bridges to rely on HPD
detection. (Reported by Heiko Stuebner)
- Add new patch to handle HPD notification from downstream bridges
(e.g., display-connector with hpd-gpios).
--
2.34.1
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)
Hi Heiko,
On 8/5/2026 6:39 AM, Heiko Stübner wrote:
> 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.
Thanks a lot for your testing and feedback. :-)
It seems my test setup gave me the false impression that those cases
worked well.
>
> hmm, it wasn't working entirely for me though and was still running into
> issues when the display was unplugged on boot.
>
Are you seeing this boot‑unplug issue for both scenarios: DP‑connector
with HPD‑gpio paired with eDP without HPD‑gpio, and DP‑connector without
HPD‑gpio paired with eDP with HPD‑gpio? I.e. it wrongly reports
connected even with no display plugged in and proceeds into DRM
.atomic_enable()?
> I wiggled around a bit like in the diff below and am now getting correct
> plug and unplug events.
Oh, I see. For the GPIO HPD case, the IRQ needs to be enabled early so
that plug‑in interrupts can be properly responded to. However GPIO HPD
does not require a runtime PM get. I will better separate these two
scenarios in the next version.
>
> 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.
>
Based on Dmitry's commit cb640b2ca546 ("drm/bridge: display‑connector:
don't set OP_DETECT for DisplayPorts"), HPD events from DP‑variant
connector should be handled by the upstream DP controller. Hence I added
analogix_dp_bridge_notify() to retrieve HPD status coming from downstream.
As expected, under the bridge‑connector framework, the detect result
from Analogix DP should in theory reflect the actual connection status.
Let's dig into this boot‑time initial‑state issue together.
>
> 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.
Great suggestions, I will incorporate them for the next version
alongside Sashiko's comments.
Best regards,
Damon
>
>
> 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)
>
>
>
>
>
Am Mittwoch, 5. August 2026, 06:06:48 Mitteleuropäische Sommerzeit schrieb Damon Ding:
> Hi Heiko,
>
> On 8/5/2026 6:39 AM, Heiko Stübner wrote:
> > 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.
>
> Thanks a lot for your testing and feedback. :-)
>
> It seems my test setup gave me the false impression that those cases
> worked well.
>
> >
> > hmm, it wasn't working entirely for me though and was still running into
> > issues when the display was unplugged on boot.
> >
>
> Are you seeing this boot‑unplug issue for both scenarios: DP‑connector
> with HPD‑gpio paired with eDP without HPD‑gpio, and DP‑connector without
> HPD‑gpio paired with eDP with HPD‑gpio? I.e. it wrongly reports
> connected even with no display plugged in and proceeds into DRM
> .atomic_enable()?
Yep it's different. The analogix-dp gpio-hpd works correctly, because the
analogix driver can do gpiod_get_value() in analogix_dp_get_plug_in_status()
The dp-connector does not, as with the patch you pointed to, it lost
that ability. So on boot a plugged in display is not detected correctly
because there won't be a hotplug interrupt.
>
> > I wiggled around a bit like in the diff below and am now getting correct
> > plug and unplug events.
>
> Oh, I see. For the GPIO HPD case, the IRQ needs to be enabled early so
> that plug‑in interrupts can be properly responded to. However GPIO HPD
> does not require a runtime PM get. I will better separate these two
> scenarios in the next version.
>
> >
> > 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.
> >
>
> Based on Dmitry's commit cb640b2ca546 ("drm/bridge: display‑connector:
> don't set OP_DETECT for DisplayPorts"), HPD events from DP‑variant
> connector should be handled by the upstream DP controller. Hence I added
> analogix_dp_bridge_notify() to retrieve HPD status coming from downstream.
>
> As expected, under the bridge‑connector framework, the detect result
> from Analogix DP should in theory reflect the actual connection status.
> Let's dig into this boot‑time initial‑state issue together.
One idea I had was, is it possible to do a drm_dp_read_dpcd_caps()
read during bind/... to see if something answers?
Thanks for all your work
Heiko
> > 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.
>
> Great suggestions, I will incorporate them for the next version
> alongside Sashiko's comments.
>
> Best regards,
> Damon
>
> >
> >
> > 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)
> >
> >
> >
> >
> >
>
>
Hi Heiko,
On 8/6/2026 7:42 AM, Heiko Stübner wrote:
> Am Mittwoch, 5. August 2026, 06:06:48 Mitteleuropäische Sommerzeit schrieb Damon Ding:
>> Hi Heiko,
>>
>> On 8/5/2026 6:39 AM, Heiko Stübner wrote:
>>> 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.
>>
>> Thanks a lot for your testing and feedback. :-)
>>
>> It seems my test setup gave me the false impression that those cases
>> worked well.
>>
>>>
>>> hmm, it wasn't working entirely for me though and was still running into
>>> issues when the display was unplugged on boot.
>>>
>>
>> Are you seeing this boot‑unplug issue for both scenarios: DP‑connector
>> with HPD‑gpio paired with eDP without HPD‑gpio, and DP‑connector without
>> HPD‑gpio paired with eDP with HPD‑gpio? I.e. it wrongly reports
>> connected even with no display plugged in and proceeds into DRM
>> .atomic_enable()?
>
> Yep it's different. The analogix-dp gpio-hpd works correctly, because the
> analogix driver can do gpiod_get_value() in analogix_dp_get_plug_in_status()
>
> The dp-connector does not, as with the patch you pointed to, it lost
> that ability. So on boot a plugged in display is not detected correctly
> because there won't be a hotplug interrupt.
>
>>
>>> I wiggled around a bit like in the diff below and am now getting correct
>>> plug and unplug events.
>>
>> Oh, I see. For the GPIO HPD case, the IRQ needs to be enabled early so
>> that plug‑in interrupts can be properly responded to. However GPIO HPD
>> does not require a runtime PM get. I will better separate these two
>> scenarios in the next version.
>>
>>>
>>> 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.
>>>
>>
>> Based on Dmitry's commit cb640b2ca546 ("drm/bridge: display‑connector:
>> don't set OP_DETECT for DisplayPorts"), HPD events from DP‑variant
>> connector should be handled by the upstream DP controller. Hence I added
>> analogix_dp_bridge_notify() to retrieve HPD status coming from downstream.
>>
>> As expected, under the bridge‑connector framework, the detect result
>> from Analogix DP should in theory reflect the actual connection status.
>> Let's dig into this boot‑time initial‑state issue together.
>
> One idea I had was, is it possible to do a drm_dp_read_dpcd_caps()
> read during bind/... to see if something answers?
>
Sorry for the long delay to get back to this series. I was stuck on some
tricky local issues.
I'll test your suggestion and try to send out v3 within next week.
Best regards,
Damon
>
>>> 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.
>>
>> Great suggestions, I will incorporate them for the next version
>> alongside Sashiko's comments.
>>
>> Best regards,
>> Damon
>>
>>>
>>>
>>> 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)
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
>
>
>
© 2016 - 2026 Red Hat, Inc.