Re: [RFC PATCH] drm/amd/display: extend HPD debounce filter to DisplayPort SST

[Ivan Lipski]

On 6/25/26 11:04, Nick Haghiri wrote:
> [You don't often get email from nick@xxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Commit c97da4785b3b ("drm/amd/display: Add an HPD filter for HDMI") and
> commit 6a681cd90345 ("drm/amd/display: Add an hdmi_hpd_debounce_delay_ms
> module") added a filter that, on an HDMI disconnect, waits
> hdmi_hpd_debounce_delay_ms and suppresses the hotplug if the sink comes
> back with an unchanged EDID, instead of churning userspace.
>
> DisplayPort SST sinks show the same pattern: some monitors briefly drop
> and re-assert HPD when they enter deep sleep after DPMS-off. On the DP
> path that toggle is forwarded as a real hotplug, so the compositor
> re-probes and re-enables the output and the panel can never stay powered
> off while connected.
>
> Extend the existing filter to DisplayPort SST behind a new
> dp_hpd_debounce_delay_ms module parameter (default 0/off, mirroring the
> HDMI knob). eDP and MST are excluded; are_sinks_equal() and the debounce
> work are reused unchanged.
>
> Signed-off-by: Nick Haghiri <nick@xxxxxxxxxxx>
> ---
>
> RFC notes / open questions (below the --- so they stay out of the commit):
>
> - Near-mechanical port of the HDMI filter to DisplayPort SST. I run it
>    daily on an RX 9070 XT (RDNA4) driving an MSI MPG 274U over DP, which
>    briefly drops and re-asserts HPD on DPMS-off and otherwise keeps the
>    panel from staying asleep; dp_hpd_debounce_delay_ms=1500 fixes it.
> - The DP path reuses the existing debounce work and ->hdmi_prev_sink, so
>    those names are now a little misleading. Happy to rename them to a
>    generic hpd_* if you'd prefer.
> - I added a separate dp_hpd_debounce_delay_ms knob to mirror the HDMI
>    one; folding both into a single hpd_debounce_delay_ms applied by signal
>    type would work too. Let me know which you'd rather have.

Hey Nick, thank you for this patch.

I don't think I've encountered a situation when a monitor with DP can 
wake a system up from DPMS, so that's interesting. What distro and 
desktop environment are you using? I'd like to reproduce this issue and 
help with it.

I think generalizing the hpd_debounce_delay for both DP and HDMI would 
be better. Maybe we can also enable hpd_debounce_delay by default 
for monitors, that are known to have this issue, using apply_edid_quirks


>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 12 +++++++++
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  6 +++++
>   .../display/amdgpu_dm/amdgpu_dm_connector.c   | 20 ++++++++-------
>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 25 ++++++++++++-------
>   5 files changed, 46 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e2d4be3c1..c085a6cc1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -271,6 +271,7 @@ extern int amdgpu_user_queue;
>   extern int amdgpu_ptl;
>
>   extern uint amdgpu_hdmi_hpd_debounce_delay_ms;
> +extern uint amdgpu_dp_hpd_debounce_delay_ms;
>
>   #define AMDGPU_SG_THRESHOLD                    (256*1024*1024)
>   #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS         3000
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 65f2de86f..78df53b8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -246,6 +246,7 @@ int amdgpu_umsch_mm_fwlog;
>   int amdgpu_rebar = -1; /* auto */
>   int amdgpu_user_queue = -1;
>   uint amdgpu_hdmi_hpd_debounce_delay_ms;
> +uint amdgpu_dp_hpd_debounce_delay_ms;
>   int amdgpu_ptl = -1; /* auto */
>
>   DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> @@ -1113,6 +1114,17 @@ module_param_named(user_queue, amdgpu_user_queue, int, 0444);
>   MODULE_PARM_DESC(hdmi_hpd_debounce_delay_ms, "HDMI HPD disconnect debounce delay in milliseconds (0 to disable (by default), 1500 is common)");
>   module_param_named(hdmi_hpd_debounce_delay_ms, amdgpu_hdmi_hpd_debounce_delay_ms, uint, 0644);
>
> +/*
> + * DOC: dp_hpd_debounce_delay_ms (uint)
> + * DisplayPort SST HPD disconnect debounce delay in milliseconds.
> + *
> + * Used to filter short disconnect->reconnect HPD toggles some DisplayPort SST
> + * sinks generate while entering/leaving power save. Set to 0 to disable by
> + * default. eDP and MST are not affected.
> + */
> +MODULE_PARM_DESC(dp_hpd_debounce_delay_ms, "DisplayPort SST HPD disconnect debounce delay in milliseconds (0 to disable (by default), 1500 is common)");
> +module_param_named(dp_hpd_debounce_delay_ms, amdgpu_dp_hpd_debounce_delay_ms, uint, 0644);
> +
>   /**
>    * DOC: ptl (int)
>    * Enable PTL feature at boot time. Possible values:
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 2f4a56741..abc17f547 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -64,6 +64,11 @@ enum amd_vsdb_panel_type {
>    * Maximum HDMI HPD debounce delay in milliseconds
>    */
>   #define AMDGPU_DM_MAX_HDMI_HPD_DEBOUNCE_MS 5000
> +
> +/*
> + * Maximum DisplayPort SST HPD debounce delay in milliseconds
> + */
> +#define AMDGPU_DM_MAX_DP_HPD_DEBOUNCE_MS 5000
>   /*
>   #include "include/amdgpu_dal_power_if.h"
>   #include "amdgpu_dm_irq.h"
> @@ -875,6 +880,7 @@ struct amdgpu_dm_connector {
>
>          /* HDMI HPD debounce support */
>          unsigned int hdmi_hpd_debounce_delay_ms;
> +       unsigned int dp_hpd_debounce_delay_ms;
>          struct delayed_work hdmi_hpd_debounce_work;
>          struct dc_sink *hdmi_prev_sink;
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index 6143cdcf2..c79a8ada8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -1747,8 +1747,8 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
>          if (aconnector->mst_mgr.dev)
>                  drm_dp_mst_topology_mgr_destroy(&aconnector->mst_mgr);
>
> -       /* Cancel and flush any pending HDMI HPD debounce work */
> -       if (aconnector->hdmi_hpd_debounce_delay_ms) {
> +       /* Cancel and flush any pending HPD debounce work */
> +       if (aconnector->hdmi_hpd_debounce_delay_ms || aconnector->dp_hpd_debounce_delay_ms) {
>                  cancel_delayed_work_sync(&aconnector->hdmi_hpd_debounce_work);
>                  if (aconnector->hdmi_prev_sink) {
>                          dc_sink_release(aconnector->hdmi_prev_sink);
> @@ -2829,16 +2829,18 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
>          mutex_init(&aconnector->handle_mst_msg_ready);
>
>          /*
> -        * If HDMI HPD debounce delay is set, use the minimum between selected
> -        * value and AMDGPU_DM_MAX_HDMI_HPD_DEBOUNCE_MS
> +        * If an HPD debounce delay is set, clamp each signal's delay to its
> +        * maximum. The debounce work and cached sink are shared by both the
> +        * HDMI and DisplayPort SST paths.
>           */
> -       if (amdgpu_hdmi_hpd_debounce_delay_ms) {
> -               aconnector->hdmi_hpd_debounce_delay_ms = min(amdgpu_hdmi_hpd_debounce_delay_ms,
> -                                                            AMDGPU_DM_MAX_HDMI_HPD_DEBOUNCE_MS);
> +       aconnector->hdmi_hpd_debounce_delay_ms = amdgpu_hdmi_hpd_debounce_delay_ms ?
> +               min(amdgpu_hdmi_hpd_debounce_delay_ms, AMDGPU_DM_MAX_HDMI_HPD_DEBOUNCE_MS) : 0;
> +       aconnector->dp_hpd_debounce_delay_ms = amdgpu_dp_hpd_debounce_delay_ms ?
> +               min(amdgpu_dp_hpd_debounce_delay_ms, AMDGPU_DM_MAX_DP_HPD_DEBOUNCE_MS) : 0;
> +
> +       if (aconnector->hdmi_hpd_debounce_delay_ms || aconnector->dp_hpd_debounce_delay_ms) {
>                  INIT_DELAYED_WORK(&aconnector->hdmi_hpd_debounce_work, amdgpu_dm_hdmi_hpd_debounce_work);
>                  aconnector->hdmi_prev_sink = NULL;
> -       } else {
> -               aconnector->hdmi_hpd_debounce_delay_ms = 0;
>          }
>
>          dm->hdmi_frl_status_polling_delay_ms = 200;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> index 85711a2f2..2a732d19b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> @@ -1302,6 +1302,7 @@ static void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector,
>          struct dc *dc = aconnector->dc_link->ctx->dc;
>          bool ret = false;
>          bool debounce_required = false;
> +       unsigned int debounce_delay_ms = 0;
>
>          if (adev->dm.disable_hpd_irq)
>                  return;
> @@ -1325,10 +1326,16 @@ static void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector,
>                  drm_err(adev_to_drm(adev), "KMS: Failed to detect connector\n");
>
>          /*
> -        * Check for HDMI disconnect with debounce enabled.
> +        * Check for an HDMI or DisplayPort SST disconnect with debounce
> +        * enabled. eDP and MST are intentionally excluded.
>           */
> -       debounce_required = (aconnector->hdmi_hpd_debounce_delay_ms > 0 &&
> -                             dc_is_hdmi_signal(aconnector->dc_link->connector_signal) &&
> +       if (dc_is_hdmi_signal(aconnector->dc_link->connector_signal))
> +               debounce_delay_ms = aconnector->hdmi_hpd_debounce_delay_ms;
> +       else if (aconnector->dc_link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT &&
> +                aconnector->dc_link->type != dc_connection_mst_branch)
> +               debounce_delay_ms = aconnector->dp_hpd_debounce_delay_ms;
> +
> +       debounce_required = (debounce_delay_ms > 0 &&
>                                new_connection_type == dc_connection_none &&
>                                aconnector->dc_link->local_sink != NULL);
>
> @@ -1344,12 +1351,12 @@ static void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector,
>                          drm_kms_helper_connector_hotplug_event(connector);
>          } else if (debounce_required) {
>                  /*
> -                * HDMI disconnect detected - schedule delayed work instead of
> +                * Disconnect detected - schedule delayed work instead of
>                   * processing immediately. This allows us to coalesce spurious
> -                * HDMI signals from physical unplugs.
> +                * HDMI/DP HPD signals from physical unplugs.
>                   */
> -               drm_dbg_kms(dev, "HDMI HPD: Disconnect detected, scheduling debounce work (%u ms)\n",
> -                           aconnector->hdmi_hpd_debounce_delay_ms);
> +               drm_dbg_kms(dev, "HPD: Disconnect detected, scheduling debounce work (%u ms)\n",
> +                           debounce_delay_ms);
>
>                  /* Cache the current sink for later comparison */
>                  if (aconnector->hdmi_prev_sink)
> @@ -1361,8 +1368,8 @@ static void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector,
>                  /* Schedule delayed detection. */
>                  if (mod_delayed_work(system_percpu_wq,
>                                   &aconnector->hdmi_hpd_debounce_work,
> -                                msecs_to_jiffies(aconnector->hdmi_hpd_debounce_delay_ms)))
> -                       drm_dbg_kms(dev, "HDMI HPD: Re-scheduled debounce work\n");
> +                                msecs_to_jiffies(debounce_delay_ms)))
> +                       drm_dbg_kms(dev, "HPD: Re-scheduled debounce work\n");
>
>          } else {
>
> --
> 2.54.0
--
Thanks,
Ivan Lipski