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)
> >
> >
> >
> >
> >
>
>
