Link training succeeding does not mean that the sink has locked onto the video stream that follows. Some sinks, e.g. the XREAL Air 2 Pro, are not ready to do so for a while after HPD. If the stream starts too early they stay black until the stream is restarted, even though link training passes and lane status is fine. They report this through DPCD SINK_STATUS (RECEIVE_PORT_0_STATUS stays clear). With the display manager starting the modeset right after HPD, this happens on most replugs.
After a modeset on a DP SST link, poll SINK_STATUS. If the sink doesn't report being in sync within a second, restart the stream the same way as on link loss, and keep polling. The number of restarts is limited per connection (reset on HPD, not on HPD RX), so a sink that never reports sync isn't restarted forever. No fixed delay is added to the normal path: sinks that lock on in time are never touched. Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/6101 Signed-off-by: Ambareesh Balaji <[email protected]> --- Tested on Strix Halo with an XREAL Air 2 Pro, on 7.3.1 (the patch applies unchanged there). Without it the glasses stay black after nearly every replug. With it, all 30 test replugs recovered, none needed more than one restart. Captures (DPCD, ftrace, HPD/USB timing) are in the issue linked above. In short: HPD, link training and lane/alignment status look the same on good and bad attempts, and nothing in DPCD changes when the glasses become able to lock. Only SINK_STATUS differs afterwards. Skipping detection-time link training (DC_SKIP_DETECTION_LT) doesn't help. Open questions: - Are the 1 s timeout and the limit of 4 restarts acceptable, or would you prefer different values? - Would you rather have this as a per-sink quirk than for all DP SST sinks? - Is there a better place to hook this, e.g. in DMUB or the HPD path? .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 5 ++ .../display/amdgpu_dm/amdgpu_dm_connector.c | 71 +++++++++++++++++++ .../display/amdgpu_dm/amdgpu_dm_connector.h | 2 + .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 6 ++ .../drm/amd/display/dc/core/dc_link_exports.c | 5 ++ drivers/gpu/drm/amd/display/dc/dc.h | 7 ++ .../gpu/drm/amd/display/dc/inc/link_service.h | 1 + .../drm/amd/display/dc/link/link_factory.c | 1 + .../dc/link/protocols/link_dp_irq_handler.c | 16 +++++ .../dc/link/protocols/link_dp_irq_handler.h | 1 + 11 files changed, 124 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 33c73ca9cff1..915399cf0a18 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4759,6 +4759,15 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state, break; } } + if (mode_set_reset_required) { + for (i = 0; i < params.stream_count; i++) { + struct dc_stream_state *stream = params.streams[i]; + + if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT && stream->dm_stream_context) + amdgpu_dm_dp_sink_sync_start(stream->dm_stream_context); + } + } + if (frl_stream_found) { if (queue_delayed_work(dm->hdmi_frl_status_polling_wq, &dm->hdmi_frl_status_polling_work, 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 f8503ebf28f4..112c2781aacf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -931,6 +931,11 @@ struct amdgpu_dm_connector { /* HDMI compliance automation */ bool hdmi_comp_auto; + + /* DP sink sync watchdog, see amdgpu_dm_dp_sink_sync_work() */ + struct delayed_work sink_sync_work; + unsigned int sink_sync_waited_ms; + unsigned int sink_sync_retrains; }; static inline void amdgpu_dm_set_mst_status(uint8_t *status, 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 e8f050feb6e3..f56261978038 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 @@ -2011,6 +2011,8 @@ STATIC_IFN_KUNIT void amdgpu_dm_connector_destroy(struct drm_connector *connecto if (aconnector->mst_mgr.dev) drm_dp_mst_topology_mgr_destroy(&aconnector->mst_mgr); + cancel_delayed_work_sync(&aconnector->sink_sync_work); + /* 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); @@ -3283,6 +3285,73 @@ void hdmi_frl_status_polling_work(struct work_struct *work) } EXPORT_IF_KUNIT(hdmi_frl_status_polling_work); +/* + * DOC: DP sink sync watchdog + * + * Link training succeeding does not mean that the sink has locked onto the + * video stream that follows. Some sinks (e.g. XREAL Air 2 Pro) are not ready + * to do so right after HPD and then stay blank, reporting through DPCD + * SINK_STATUS that they are not in sync, until the stream is restarted. + * + * After a modeset on a DP SST link, poll that status. If the sink doesn't + * report sync within AMDGPU_DM_DP_SINK_SYNC_TIMEOUT_MS, restart the stream the + * same way as on link loss. The number of restarts is limited per connection, + * so sinks that never report sync are not restarted forever. + */ +#define AMDGPU_DM_DP_SINK_SYNC_POLL_MS 100 +#define AMDGPU_DM_DP_SINK_SYNC_TIMEOUT_MS 1000 +#define AMDGPU_DM_DP_SINK_SYNC_MAX_RETRAINS 4 + +void amdgpu_dm_dp_sink_sync_work(struct work_struct *work) +{ + struct amdgpu_dm_connector *aconnector = + container_of(to_delayed_work(work), struct amdgpu_dm_connector, + sink_sync_work); + struct amdgpu_display_manager *dm = &drm_to_adev(aconnector->base.dev)->dm; + struct dc_link *link = aconnector->dc_link; + bool keep_polling = false; + + scoped_guard(mutex, &dm->dc_lock) { + dc_exit_ips_for_hw_access(dm->dc); + + if (!dc_link_dp_is_sink_out_of_sync(link)) + return; + + aconnector->sink_sync_waited_ms += AMDGPU_DM_DP_SINK_SYNC_POLL_MS; + keep_polling = true; + + if (aconnector->sink_sync_waited_ms >= AMDGPU_DM_DP_SINK_SYNC_TIMEOUT_MS) { + aconnector->sink_sync_waited_ms = 0; + + if (aconnector->sink_sync_retrains >= AMDGPU_DM_DP_SINK_SYNC_MAX_RETRAINS) { + drm_dbg_kms(aconnector->base.dev, + "[CONNECTOR:%d:%s] sink not in sync, giving up\n", + aconnector->base.base.id, aconnector->base.name); + return; + } + + aconnector->sink_sync_retrains++; + drm_dbg_kms(aconnector->base.dev, + "[CONNECTOR:%d:%s] sink not in sync, restarting stream (%u)\n", + aconnector->base.base.id, aconnector->base.name, + aconnector->sink_sync_retrains); + dc_link_dp_handle_link_loss(link); + } + } + + if (keep_polling) + queue_delayed_work(system_percpu_wq, &aconnector->sink_sync_work, + msecs_to_jiffies(AMDGPU_DM_DP_SINK_SYNC_POLL_MS)); +} +EXPORT_IF_KUNIT(amdgpu_dm_dp_sink_sync_work); + +void amdgpu_dm_dp_sink_sync_start(struct amdgpu_dm_connector *aconnector) +{ + aconnector->sink_sync_waited_ms = 0; + mod_delayed_work(system_percpu_wq, &aconnector->sink_sync_work, + msecs_to_jiffies(AMDGPU_DM_DP_SINK_SYNC_POLL_MS)); +} + void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, struct amdgpu_dm_connector *aconnector, int connector_type, @@ -3327,6 +3396,8 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, aconnector->hdmi_hpd_debounce_delay_ms = 0; } + INIT_DELAYED_WORK(&aconnector->sink_sync_work, amdgpu_dm_dp_sink_sync_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_connector.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.h index 7bfbb6853833..6f5d60c0eb98 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.h @@ -72,6 +72,8 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, int link_index); void hdmi_frl_status_polling_work(struct work_struct *work); +void amdgpu_dm_dp_sink_sync_work(struct work_struct *work); +void amdgpu_dm_dp_sink_sync_start(struct amdgpu_dm_connector *aconnector); enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector *connector, const struct drm_display_mode *mode); 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 5b4ff079cd55..6b7ac030c6cb 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 @@ -1395,6 +1395,12 @@ STATIC_IFN_KUNIT void handle_hpd_irq_helper(struct amdgpu_dm_connector *aconnect aconnector->timing_changed = false; + if (reason != DETECT_REASON_HPDRX) { + /* New connection, or a replug: start over with a fresh retrain budget */ + cancel_delayed_work(&aconnector->sink_sync_work); + aconnector->sink_sync_retrains = 0; + } + if (!dc_link_detect_connection_type(aconnector->dc_link, &new_connection_type)) drm_err(adev_to_drm(adev), "KMS: Failed to detect connector\n"); diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c index 5926e8a4f95a..f9f065b66757 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c @@ -424,6 +424,11 @@ void dc_link_dp_handle_link_loss(struct dc_link *link) link->dc->link_srv->dp_handle_link_loss(link); } +bool dc_link_dp_is_sink_out_of_sync(struct dc_link *link) +{ + return link->dc->link_srv->dp_is_sink_out_of_sync(link); +} + enum dc_status dc_link_dp_read_hpd_rx_irq_data( struct dc_link *link, union hpd_irq_data *irq_data) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 916fa87d0dbc..05e90ba60e85 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -2641,6 +2641,13 @@ void dc_link_dp_handle_automated_test(struct dc_link *link); */ void dc_link_dp_handle_link_loss(struct dc_link *link); +/* Check whether a DP SST stream is being sent on the link but the sink does + * not report (DPCD SINK_STATUS) being synchronized with it. Some sinks need + * more time than link training takes to lock onto the video stream and then + * stay blank until the stream is restarted. + */ +bool dc_link_dp_is_sink_out_of_sync(struct dc_link *link); + /* Determine if hpd rx irq should be handled or ignored * return true - hpd rx irq should be handled. * return false - it is safe to ignore hpd rx irq event diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_service.h b/drivers/gpu/drm/amd/display/dc/inc/link_service.h index b2f86c51d470..03faca2fa3d7 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/link_service.h +++ b/drivers/gpu/drm/amd/display/dc/inc/link_service.h @@ -242,6 +242,7 @@ struct link_service { union hpd_irq_data *hpd_irq_dpcd_data); bool (*dp_should_allow_hpd_rx_irq)(const struct dc_link *link); void (*dp_handle_link_loss)(struct dc_link *link); + bool (*dp_is_sink_out_of_sync)(struct dc_link *link); enum dc_status (*dp_read_hpd_rx_irq_data)( struct dc_link *link, union hpd_irq_data *irq_data); diff --git a/drivers/gpu/drm/amd/display/dc/link/link_factory.c b/drivers/gpu/drm/amd/display/dc/link/link_factory.c index 3b192c539f8b..829c0b07634b 100644 --- a/drivers/gpu/drm/amd/display/dc/link/link_factory.c +++ b/drivers/gpu/drm/amd/display/dc/link/link_factory.c @@ -195,6 +195,7 @@ static void construct_link_service_dp_irq_handler(struct link_service *link_srv) link_srv->dp_parse_link_loss_status = dp_parse_link_loss_status; link_srv->dp_should_allow_hpd_rx_irq = dp_should_allow_hpd_rx_irq; link_srv->dp_handle_link_loss = dp_handle_link_loss; + link_srv->dp_is_sink_out_of_sync = dp_is_sink_out_of_sync; link_srv->dp_read_hpd_rx_irq_data = dp_read_hpd_rx_irq_data; link_srv->dp_handle_hpd_rx_irq = dp_handle_hpd_rx_irq; } diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c index da679fb7d89c..d26b519fe6c6 100644 --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.c @@ -348,6 +348,22 @@ void dp_handle_link_loss(struct dc_link *link) } } +bool dp_is_sink_out_of_sync(struct dc_link *link) +{ + struct pipe_ctx *pipes[MAX_PIPES]; + uint8_t count; + uint8_t sink_status = 0; + + link_get_master_pipes_with_dpms_on(link, link->dc->current_state, &count, pipes); + if (!count || pipes[0]->stream->signal != SIGNAL_TYPE_DISPLAY_PORT) + return false; + + if (core_link_read_dpcd(link, DP_SINK_STATUS, &sink_status, 1) != DC_OK) + return false; + + return !(sink_status & DP_RECEIVE_PORT_0_STATUS); +} + static void dp_handle_tunneling_irq(struct dc_link *link) { enum dc_status retval; diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.h b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.h index 87516fb3b45a..e4aabf679863 100644 --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.h +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_irq_handler.h @@ -32,6 +32,7 @@ bool dp_parse_link_loss_status( union hpd_irq_data *hpd_irq_dpcd_data); bool dp_should_allow_hpd_rx_irq(const struct dc_link *link); void dp_handle_link_loss(struct dc_link *link); +bool dp_is_sink_out_of_sync(struct dc_link *link); enum dc_status dp_read_hpd_rx_irq_data( struct dc_link *link, union hpd_irq_data *irq_data); base-commit: 178d00b16ddbb5962adbbe2c486204b0b6a4b9f0 -- 2.56.0
