Re: [PATCH v3 07/10] drm/bridge: synopsys: dw-dp: Add Runtime PM support

[Sebastian Reichel <sebastian.reichel@collabora.com>]

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

Hello Andy,

On Tue, Jul 14, 2026 at 05:01:14PM +0800, Andy Yan wrote:
> Hello Sebastian,
> 
> Sebastian Reichel <sebastian.reichel@collabora.com> 于2026年6月13日周六 02:01写道：
> 
> > Add runtime PM stubs to the Synopsys DesignWare DisplayPort bridge
> > driver. Support is not enabled automatically and must be hooked up
> > in the vendor specific glue code.
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-dp.c | 27 +++++++++++++++++++++++++++
> >  include/drm/bridge/dw_dp.h              |  3 +++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> > index 7fa38145e35c..7f4f36c61484 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> > @@ -1465,6 +1465,8 @@ static ssize_t dw_dp_aux_transfer(struct drm_dp_aux
> > *aux,
> >         if (WARN_ON(msg->size > 16))
> >                 return -E2BIG;
> >
> > +       ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> > +
> >         switch (msg->request & ~DP_AUX_I2C_MOT) {
> >         case DP_AUX_NATIVE_WRITE:
> >         case DP_AUX_I2C_WRITE:
> > @@ -1655,6 +1657,8 @@ static void dw_dp_bridge_atomic_enable(struct
> > drm_bridge *bridge,
> >         struct drm_connector_state *conn_state;
> >         int ret;
> >
> > +       pm_runtime_get_sync(dp->dev);
> > +
> >         connector = drm_atomic_get_new_connector_for_encoder(state,
> > bridge->encoder);
> >         if (!connector) {
> >                 dev_err(dp->dev, "failed to get connector\n");
> > @@ -1709,6 +1713,7 @@ static void dw_dp_bridge_atomic_disable(struct
> > drm_bridge *bridge,
> >         dw_dp_link_disable(dp);
> >         bitmap_zero(dp->sdp_reg_bank, SDP_REG_BANK_SIZE);
> >         dw_dp_reset(dp);
> > +       pm_runtime_put_autosuspend(dp->dev);
> >  }
> >
> >  static bool dw_dp_hpd_detect_link(struct dw_dp *dp, struct drm_connector
> > *connector)
> > @@ -1729,6 +1734,8 @@ static enum drm_connector_status
> > dw_dp_bridge_detect(struct drm_bridge *bridge,
> >  {
> >         struct dw_dp *dp = bridge_to_dp(bridge);
> >
> > +       ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> >
> 
> This may not work correctly. Per the DW-DP TRM, the " HPD_HOT_PLUG
> bit is asserted only after the sink is attached and holds HPD high
> for at least 100ms". Consequently, reading this bit within 100ms
> after resuming runtime_pm may fail to detect the status. I've
> verified this behavior on a Rock 5B board—the HPD bit remains
> unreadable at this point.
> 
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1761,12 +1761,17 @@ static enum drm_connector_status
> dw_dp_bridge_detect(struct drm_bridge *bridge,
> 
>         ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> 
> -       if (!dw_dp_hpd_detect(dp))
> +       if (!dw_dp_hpd_detect(dp)) {
> +               printk(KERN_DEBUG "%s no hpd\n", __func__);
>                 return connector_status_disconnected;
> +       }
> 
> -       if (!dw_dp_hpd_detect_link(dp, connector))
> +       if (!dw_dp_hpd_detect_link(dp, connector)) {
> +               printk(KERN_DEBUG "%s no link\n", __func__);
>                 return connector_status_disconnected;
> +       }
> 
> +       printk(KERN_DEBUG "%s connected\n", __func__);
>         return connector_status_connected;
>  }
> 
> @@ -2149,6 +2154,7 @@ static irqreturn_t dw_dp_irq(int irq, void *data)
>         u32 value;
> 
>         regmap_read(dp->regmap, DW_DP_GENERAL_INTERRUPT, &value);
> +       printk(KERN_DEBUG "%s value: %d\n", __func__, value);
>         if (!value)
>                 return IRQ_NONE;
> 
> bootup
> [    5.218035] panthor fb000000.gpu: [drm] Using Transparent Hugepage
> [    5.219073] [drm] Initialized panthor 1.9.0 for fb000000.gpu on minor 1
> [    7.953973] r8169 0004:41:00.0 enP4p65s0: Link is Up - 1Gbps/Full - flow
> control off
> [    8.601273] dw_dp_rockchip_runtime_resume ret: 0
> [    8.702748] dw_dp_irq value: 1        // (HPD irq 100 100ms  after
> runtime_resume)
> [    8.714236] dw_dp_irq value: 2
> [    8.714775] dw_dp_irq value: 2
> [    8.715201] dw_dp_irq value: 2
> [    8.715710] dw_dp_irq value: 2
> [    8.715744] dw_dp_bridge_detect connected
> [    9.226752] dw_dp_rockchip_runtime_suspend
> 
> 
> Then run modetest:
> 
> # modetest
> trying to open device '/dev/dri/card1'... is not a KMS device
> trying to open device '/dev/dri/card0'... done
> opened device `RockChip Soc DRM` on driver `rockchip` (version 1.0.0 at 0)
> Encoders:
> id      crtc    type    possible crtcs  possible clones
> 86      0       TMDS    0x00000004      0x00000001
> 88      0       TMDS    0x00000001      0x00000002
> 92      0       TMDS    0x00000002      0x00000004
> 
> Connectors:
> id      encoder status          name            size (mm)       modes
> encoders
> 87      0       disconnected    DP-1            0x0             0       86
>   props:
>         1 EDID:
> 
> [   13.809398] rockchip-pm-domain
> fd8d8000.power-management:power-controller: sync_state() pending due to
> fdee0000.hdmi_receiver
> [   29.225506] dw_dp_rockchip_runtime_resume ret: 0
> [   29.225940] dw_dp_bridge_detect no hpd         // no HPD read here
> [   29.326986] dw_dp_irq value: 1                         // HPD irq ater
> 100ms
> [   29.346609] dw_dp_irq value: 2
> [   29.347150] dw_dp_irq value: 2
> [   29.347578] dw_dp_irq value: 2
> [   29.348092] dw_dp_irq value: 2
> 
> 
> if I add a mdelay(110) after  ACQUIRE pm_runtime,  the hdp can be read.
> index 02ca7fe725876..5959e79eb3e2b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1760,7 +1760,7 @@ static enum drm_connector_status
> dw_dp_bridge_detect(struct drm_bridge *bridge,
>         struct dw_dp *dp = bridge_to_dp(bridge);
> 
>         ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> -
> +       mdelay(110);

Interesting, I did not notice this issue with the Sige 5 (RK3576).
Thanks for not just reporting it to not work, but also figuring out
what is going on :) I will add an msleep(110); in the next version.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]