From: Ivan Lipski <[email protected]> [Why] Some sinks perform rapid "autoscan" HPD re-assertions right after a disconnect or when entering a power saving mode. These appear as a quick disconnect->reconnect with an identical EDID. Since there is no HPD filter, these quick reconnects are seen as hotplug events, which can unintentionally wake a system with DPMS off.
An example: https://gitlab.freedesktop.org/drm/amd/-/issues/2876 The existing debounce only covered HDMI and used a single connector-wide delay driven by the module parameter. However, a monitor that needs a debounce delay on HDMI may not need one on DisplayPort (and vice versa), so the delay should be differentiated per connector type. Well-behaved monitors should also carry no debounce cost or regression risk by default. [How] - Extend HPD debounce eligibility to HDMI, DVI, and DisplayPort SST, and rename the shared connector fields, work, and cached sink from hdmi_* to generic hpd_*. eDP and MST remain intentionally excluded. - Move the per-monitor delay into struct dc_panel_patch as two fields, hdmi_hpd_debounce_delay_ms and dp_hpd_debounce_delay_ms, so a delay can be set for a specific monitor and specific connector type via apply_edid_quirks(). - Add a quirk for the MSI MPG 274U (drm_edid_encode_panel_id('M', 'S', 'I', 0x3CF0)) that enables a 1500ms DP debounce (experimentally chosen a while ago), leaving HDMI untouched. - Rework the amdgpu_hpd_debounce_delay_ms module parameter into a tristate policy: - -1 = auto (default): debounce only known-problematic panels via EDID quirks, using the per-panel, per-connector-type delay - 0 = force off on all connectors (ignores EDID quirks) - >0 = force on for all eligible connectors, using this delay in ms Auto keeps debounce targeted to quirked panels; explicit enable/disable to be used for triaging. - Factor the eligibility and policy decision into a hpd_debounce_delay_ms() helper that returns the resolved delay (or 0 to skip), keeping handle_hpd_irq_helper() readable. Signed-off-by: Nick Haghiri <[email protected]> Signed-off-by: Ivan Lipski <[email protected]> --- Initially Nick proposed expanding HPD Debounce to non-HDMI connectors, so adding Nick's SOB. --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 22 ++-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 12 +- .../display/amdgpu_dm/amdgpu_dm_connector.c | 31 +++--- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 + .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 104 ++++++++++++------ .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.h | 2 +- drivers/gpu/drm/amd/display/dc/dc_types.h | 2 + 8 files changed, 116 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index f5d65bd0ac25..5119b5188770 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -266,7 +266,7 @@ extern int amdgpu_wbrf; extern int amdgpu_user_queue; extern int amdgpu_ptl; -extern uint amdgpu_hdmi_hpd_debounce_delay_ms; +extern int amdgpu_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 5362705143bc..a7873a3191cd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -245,7 +245,7 @@ int amdgpu_damage_clips = -1; /* auto */ int amdgpu_umsch_mm_fwlog; int amdgpu_rebar = -1; /* auto */ int amdgpu_user_queue = -1; -uint amdgpu_hdmi_hpd_debounce_delay_ms; +int amdgpu_hpd_debounce_delay_ms = -1; /* auto */ int amdgpu_ptl = -1; /* auto */ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0, @@ -1104,14 +1104,22 @@ MODULE_PARM_DESC(user_queue, "Enable user queues (-1 = auto (default), 0 = disab module_param_named(user_queue, amdgpu_user_queue, int, 0444); /* - * DOC: hdmi_hpd_debounce_delay_ms (uint) - * HDMI HPD disconnect debounce delay in milliseconds. + * DOC: hpd_debounce_delay_ms (int) + * HDMI/DVI/DP SST HPD disconnect debounce policy. eDP and MST are not affected. * - * Used to filter short disconnect->reconnect HPD toggles some HDMI sinks - * generate while entering/leaving power save. Set to 0 to disable by default. + * Some sinks briefly toggle HPD (a quick disconnect->reconnect with an + * identical EDID) when entering/leaving power save. Since there is no HPD + * filter, these are seen as hotplug events and can wake a DPMS-off system. + * Debounce coalesces them by delaying disconnect processing and comparing the + * EDID after the delay. + * + * - -1 = auto (default): debounce only known-problematic panels via EDID + * quirks, using a per-panel, per-connector-type delay + * - 0 = force off on all connectors (ignores EDID quirks) + * - >0 = force on for all eligible connectors, using this delay in milliseconds */ -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); +MODULE_PARM_DESC(hpd_debounce_delay_ms, "HDMI/DVI/DP SST HPD disconnect debounce delay in milliseconds (-1 = auto/per-EDID-quirk (default), 0 = force off, >0 = force on with this delay, 1500 is common)"); +module_param_named(hpd_debounce_delay_ms, amdgpu_hpd_debounce_delay_ms, int, 0644); /** * DOC: ptl (int) 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 f396803d1485..e6b13ee82a8c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -61,9 +61,9 @@ enum amd_vsdb_panel_type { #define AMDGPU_HDR_MULT_DEFAULT (0x100000000LL) /* - * Maximum HDMI HPD debounce delay in milliseconds + * Maximum HDMI/DVI/DP SST HPD debounce delay in milliseconds */ -#define AMDGPU_DM_MAX_HDMI_HPD_DEBOUNCE_MS 5000 +#define AMDGPU_DM_MAX_HPD_DEBOUNCE_MS 5000 /* #include "include/amdgpu_dal_power_if.h" #include "amdgpu_dm_irq.h" @@ -884,10 +884,10 @@ struct amdgpu_dm_connector { enum adaptive_sync_type as_type; struct amdgpu_hdmi_vsdb_info vsdb_info; - /* HDMI HPD debounce support */ - unsigned int hdmi_hpd_debounce_delay_ms; - struct delayed_work hdmi_hpd_debounce_work; - struct dc_sink *hdmi_prev_sink; + /* HDMI/DVI/DP SST HPD debounce support */ + int hpd_debounce_override; + struct delayed_work hpd_debounce_work; + struct dc_sink *hpd_prev_sink; /* HDMI compliance automation */ bool hdmi_comp_auto; 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 16e157f4bd10..ff5a33991d35 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 @@ -1806,13 +1806,11 @@ 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_delayed_work_sync(&aconnector->hdmi_hpd_debounce_work); - if (aconnector->hdmi_prev_sink) { - dc_sink_release(aconnector->hdmi_prev_sink); - aconnector->hdmi_prev_sink = NULL; - } + /* Cancel and flush any pending HPD debounce work */ + cancel_delayed_work_sync(&aconnector->hpd_debounce_work); + if (aconnector->hpd_prev_sink) { + dc_sink_release(aconnector->hpd_prev_sink); + aconnector->hpd_prev_sink = NULL; } if (aconnector->bl_idx != -1) { @@ -2896,17 +2894,16 @@ 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 + * Global HPD debounce policy from the module parameter: + * < 0 : auto -> per-panel decision via EDID quirks + * = 0 : force off on all connectors (ignores quirks) + * > 0 : force on for all eligible connectors (clamped to the maximum) */ - 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); - 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; - } + aconnector->hpd_debounce_override = amdgpu_hpd_debounce_delay_ms > 0 ? + min(amdgpu_hpd_debounce_delay_ms, AMDGPU_DM_MAX_HPD_DEBOUNCE_MS) : + amdgpu_hpd_debounce_delay_ms; + aconnector->hpd_prev_sink = NULL; + INIT_DELAYED_WORK(&aconnector->hpd_debounce_work, amdgpu_dm_hpd_debounce_work); dm->hdmi_frl_status_polling_delay_ms = 200; INIT_DELAYED_WORK(&dm->hdmi_frl_status_polling_work, hdmi_frl_status_polling_work); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 30ac1f32fad5..3596766bb843 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -137,6 +137,12 @@ STATIC_IFN_KUNIT void apply_edid_quirks(struct dc_link *link, struct edid *edid, drm_dbg_driver(dev, "Skip PHY SSC reduction on panel id %X\n", panel_id); link->wa_flags.skip_phy_ssc_reduction = true; break; + /* Workaround for monitors that toggle HPD on DisplayPort entering deep sleep */ + case drm_edid_encode_panel_id('M', 'S', 'I', 0x3CF0): + drm_dbg_driver(dev, "Enabling DP HPD debounce on panel id %X\n", panel_id); + edid_caps->panel_patch.dp_hpd_debounce_delay_ms = + min(1500U, AMDGPU_DM_MAX_HPD_DEBOUNCE_MS); + break; default: return; } 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 ae3f81f53074..f6eece130987 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 @@ -1258,13 +1258,13 @@ EXPORT_IF_KUNIT(are_sinks_equal); /** - * DOC: amdgpu_dm_hdmi_hpd_debounce_work + * DOC: amdgpu_dm_hpd_debounce_work * - * HDMI HPD debounce delay in milliseconds. When an HDMI display toggles HPD + * HDMI/DVI/DP SST HPD debounce delay in milliseconds. When a display toggles HPD * (such as during power save transitions), this delay determines how long to * wait before processing the HPD event. This allows distinguishing between a - * physical unplug (>hdmi_hpd_debounce_delay) - * and a spontaneous RX HPD toggle (<hdmi_hpd_debounce_delay). + * physical unplug (>hpd_debounce_delay) + * and a spontaneous RX HPD toggle (<hpd_debounce_delay). * * If the toggle is less than this delay, the driver compares sink capabilities * and permits a hotplug event if they changed. @@ -1272,11 +1272,11 @@ EXPORT_IF_KUNIT(are_sinks_equal); * The default value of 1500ms was chosen based on experimental testing with * various monitors that exhibit spontaneous HPD toggling behavior. */ -void amdgpu_dm_hdmi_hpd_debounce_work(struct work_struct *work) +void amdgpu_dm_hpd_debounce_work(struct work_struct *work) { struct amdgpu_dm_connector *aconnector = container_of(to_delayed_work(work), struct amdgpu_dm_connector, - hdmi_hpd_debounce_work); + hpd_debounce_work); struct drm_connector *connector = &aconnector->base; struct drm_device *dev = connector->dev; struct amdgpu_device *adev = drm_to_adev(dev); @@ -1300,11 +1300,12 @@ void amdgpu_dm_hdmi_hpd_debounce_work(struct work_struct *work) /* Apply workaround delay for certain panels */ amdgpu_dm_apply_delay_after_dpcd_poweroff(adev, aconnector->dc_sink); /* Compare sinks to determine if this was a spontaneous HPD toggle */ - if (are_sinks_equal(aconnector->dc_link->local_sink, aconnector->hdmi_prev_sink)) { + if (are_sinks_equal(aconnector->dc_link->local_sink, aconnector->hpd_prev_sink)) { /* - * Sinks match - this was a spontaneous HDMI HPD toggle. + * Sinks match - this was a spontaneous HPD toggle. */ - drm_dbg_kms(dev, "HDMI HPD: Sink unchanged after debounce, internal re-enable\n"); + drm_dbg_kms(dev, + "HPD: Sink unchanged after debounce, internal re-enable\n"); fake_reconnect = true; } @@ -1321,9 +1322,9 @@ void amdgpu_dm_hdmi_hpd_debounce_work(struct work_struct *work) } /* Release the cached sink reference */ - if (aconnector->hdmi_prev_sink) { - dc_sink_release(aconnector->hdmi_prev_sink); - aconnector->hdmi_prev_sink = NULL; + if (aconnector->hpd_prev_sink) { + dc_sink_release(aconnector->hpd_prev_sink); + aconnector->hpd_prev_sink = NULL; } scoped_guard(mutex, &adev->dm.dc_lock) { @@ -1331,7 +1332,46 @@ void amdgpu_dm_hdmi_hpd_debounce_work(struct work_struct *work) dc_allow_idle_optimizations(dc, true); } } -EXPORT_IF_KUNIT(amdgpu_dm_hdmi_hpd_debounce_work); +EXPORT_IF_KUNIT(amdgpu_dm_hpd_debounce_work); + +/** + * hpd_debounce_delay_ms - resolve the HPD debounce delay for a connector + * @aconnector: connector whose current link is being evaluated + * + * HDMI, DVI, and DisplayPort SST use a per-monitor, per-connector-type delay + * set via EDID quirks (dc_panel_patch). eDP and MST are intentionally excluded. + * The hpd_debounce_override policy (from the module parameter) is then applied: + * + * < 0 : auto -> use the per-panel quirk delay + * = 0 : force off on all connectors (ignores the quirk) + * > 0 : force on for all eligible connectors, using this delay + * + * Return: the debounce delay in milliseconds, or 0 to process HPD immediately. + */ +static unsigned int hpd_debounce_delay_ms(struct amdgpu_dm_connector *aconnector) +{ + struct dc_link *link = aconnector->dc_link; + struct dc_sink *local_sink = link->local_sink; + unsigned int quirk_delay_ms; + + if (dc_is_hdmi_signal(link->connector_signal) || + dc_is_dvi_signal(link->connector_signal)) + quirk_delay_ms = local_sink ? + local_sink->edid_caps.panel_patch.hdmi_hpd_debounce_delay_ms : 0; + else if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT && + link->type != dc_connection_mst_branch) + quirk_delay_ms = local_sink ? + local_sink->edid_caps.panel_patch.dp_hpd_debounce_delay_ms : 0; + else + return 0; /* not an eligible connector type */ + + if (aconnector->hpd_debounce_override == 0) + return 0; /* force off */ + if (aconnector->hpd_debounce_override > 0) + return aconnector->hpd_debounce_override; /* force on */ + + return quirk_delay_ms; /* auto: per-EDID-quirk */ +} STATIC_IFN_KUNIT void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnector, enum dc_detect_reason reason) @@ -1342,8 +1382,9 @@ STATIC_IFN_KUNIT void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnect struct amdgpu_device *adev = drm_to_adev(dev); struct dm_connector_state *dm_con_state = to_dm_connector_state(connector->state); struct dc *dc = aconnector->dc_link->ctx->dc; - bool ret = false; + unsigned int debounce_delay_ms; bool debounce_required = false; + bool ret = false; if (adev->dm.disable_hpd_irq) return; @@ -1366,11 +1407,8 @@ STATIC_IFN_KUNIT void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnect if (!dc_link_detect_connection_type(aconnector->dc_link, &new_connection_type)) drm_err(adev_to_drm(adev), "KMS: Failed to detect connector\n"); - /* - * Check for HDMI disconnect with debounce enabled. - */ - debounce_required = (aconnector->hdmi_hpd_debounce_delay_ms > 0 && - dc_is_hdmi_signal(aconnector->dc_link->connector_signal) && + debounce_delay_ms = hpd_debounce_delay_ms(aconnector); + debounce_required = (debounce_delay_ms > 0 && new_connection_type == dc_connection_none && aconnector->dc_link->local_sink != NULL); @@ -1386,30 +1424,30 @@ STATIC_IFN_KUNIT void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnect 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/DVI/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) - dc_sink_release(aconnector->hdmi_prev_sink); - aconnector->hdmi_prev_sink = aconnector->dc_link->local_sink; - if (aconnector->hdmi_prev_sink) - dc_sink_retain(aconnector->hdmi_prev_sink); + if (aconnector->hpd_prev_sink) + dc_sink_release(aconnector->hpd_prev_sink); + aconnector->hpd_prev_sink = aconnector->dc_link->local_sink; + if (aconnector->hpd_prev_sink) + dc_sink_retain(aconnector->hpd_prev_sink); /* 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"); + &aconnector->hpd_debounce_work, + msecs_to_jiffies(debounce_delay_ms))) + drm_dbg_kms(dev, "HPD: Re-scheduled debounce work\n"); } else { - /* If the aconnector->hdmi_hpd_debounce_work is scheduled, exit early */ - if (delayed_work_pending(&aconnector->hdmi_hpd_debounce_work)) + /* If the aconnector->hpd_debounce_work is scheduled, exit early */ + if (delayed_work_pending(&aconnector->hpd_debounce_work)) return; scoped_guard(mutex, &adev->dm.dc_lock) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.h index 4c200a9614a7..407456ea1f19 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.h @@ -113,7 +113,7 @@ void amdgpu_dm_irq_resume_late(struct amdgpu_device *adev); struct hpd_rx_irq_offload_work_queue *amdgpu_dm_hpd_rx_irq_create_workqueue(struct amdgpu_device *adev); void amdgpu_dm_hpd_rx_irq_work_suspend(struct amdgpu_display_manager *dm); int amdgpu_dm_register_hpd_handlers(struct amdgpu_device *adev); -void amdgpu_dm_hdmi_hpd_debounce_work(struct work_struct *work); +void amdgpu_dm_hpd_debounce_work(struct work_struct *work); /* IRQ handlers */ struct amdgpu_crtc *amdgpu_dm_get_crtc_by_otg_inst(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h index 5ba7a2fffcf0..335b1b72935d 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_types.h +++ b/drivers/gpu/drm/amd/display/dc/dc_types.h @@ -193,6 +193,8 @@ struct dc_panel_patch { bool oled_optimize_display_on; unsigned int force_mst_blocked_discovery; unsigned int wait_after_dpcd_poweroff_ms; + unsigned int hdmi_hpd_debounce_delay_ms; + unsigned int dp_hpd_debounce_delay_ms; }; /** -- 2.43.0
