From: Wayne Lin wayne.lin@amd.com
[Why] In current dm_resume, dm->cached_state is restored after link get detected and updated which will cause problems especially for MST case.
In drm_dp_mst_topology_mgr_resume() today, it will resume the mst branch to be ready handling mst mode and also consecutively do the mst topology probing. Which will cause the dirver have chance to fire hotplug event before restoring the old state. Then Userspace will react to the hotplug event based on a wrong state.
[How] Adjust the resume flow as: 1. restore old state first 2. link detect/topology probing and notify userspace 3. userspace commits new state
For drm_dp_mst_topology_mgr_resume(), it's better to adjust it to pull out topology probing work into a 2nd part procedure of the mst resume. Will have a follow up patch in drm.
Reviewed-by: Stylon Wang stylon.wang@amd.com Cc: Mario Limonciello mario.limonciello@amd.com Cc: Alex Deucher alexander.deucher@amd.com Cc: stable@vger.kernel.org Acked-by: Stylon Wang stylon.wang@amd.com Signed-off-by: Wayne Lin wayne.lin@amd.com --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 185 +++++++++++++----- 1 file changed, 131 insertions(+), 54 deletions(-)
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 a4d57289d07a..2a6ffe11be72 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2334,14 +2334,62 @@ static int dm_late_init(void *handle) return detect_mst_link_for_all_connectors(adev_to_drm(adev)); }
+static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr) +{ + int ret; + u8 guid[16]; + u64 tmp64; + + mutex_lock(&mgr->lock); + if (!mgr->mst_primary) + goto out_fail; + + if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) { + drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n"); + goto out_fail; + } + + ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, + DP_MST_EN | + DP_UP_REQ_EN | + DP_UPSTREAM_IS_SRC); + if (ret < 0) { + drm_dbg_kms(mgr->dev, "mst write failed - undocked during suspend?\n"); + goto out_fail; + } + + /* Some hubs forget their guids after they resume */ + ret = drm_dp_dpcd_read(mgr->aux, DP_GUID, guid, 16); + if (ret != 16) { + drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during suspend?\n"); + goto out_fail; + } + + if (memchr_inv(guid, 0, 16) == NULL) { + tmp64 = get_jiffies_64(); + memcpy(&guid[0], &tmp64, sizeof(u64)); + memcpy(&guid[8], &tmp64, sizeof(u64)); + + ret = drm_dp_dpcd_write(mgr->aux, DP_GUID, guid, 16); + + if (ret != 16) { + drm_dbg_kms(mgr->dev, "check mstb guid failed - undocked during suspend?\n"); + goto out_fail; + } + } + + memcpy(mgr->mst_primary->guid, guid, 16); + +out_fail: + mutex_unlock(&mgr->lock); +} + static void s3_handle_mst(struct drm_device *dev, bool suspend) { struct amdgpu_dm_connector *aconnector; struct drm_connector *connector; struct drm_connector_list_iter iter; struct drm_dp_mst_topology_mgr *mgr; - int ret; - bool need_hotplug = false;
drm_connector_list_iter_begin(dev, &iter); drm_for_each_connector_iter(connector, &iter) { @@ -2363,18 +2411,15 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend) if (!dp_is_lttpr_present(aconnector->dc_link)) try_to_configure_aux_timeout(aconnector->dc_link->ddc, LINK_AUX_DEFAULT_TIMEOUT_PERIOD);
- ret = drm_dp_mst_topology_mgr_resume(mgr, true); - if (ret < 0) { - dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx, - aconnector->dc_link); - need_hotplug = true; - } + /* TODO: move resume_mst_branch_status() into drm mst resume again + * once topology probing work is pulled out from mst resume into mst + * resume 2nd step. mst resume 2nd step should be called after old + * state getting restored (i.e. drm_atomic_helper_resume()). + */ + resume_mst_branch_status(mgr); } } drm_connector_list_iter_end(&iter); - - if (need_hotplug) - drm_kms_helper_hotplug_event(dev); }
static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device *adev) @@ -2751,14 +2796,80 @@ static void dm_gpureset_commit_state(struct dc_state *dc_state, kfree(bundle); }
+static void link_detect_and_update_connectors(struct amdgpu_device *adev) +{ + struct drm_device *dev = adev_to_drm(adev); + struct amdgpu_dm_connector *aconnector; + struct drm_connector *connector; + struct drm_connector_list_iter iter; + struct drm_dp_mst_topology_mgr *mgr; + struct amdgpu_display_manager *dm = &adev->dm; + enum dc_connection_type new_connection_type = dc_connection_none; + int ret; + + /* link detection and update. Skip those MST end sink connectors but do + * detection for mst root connectors + */ + drm_connector_list_iter_begin(dev, &iter); + drm_for_each_connector_iter(connector, &iter) { + aconnector = to_amdgpu_dm_connector(connector); + + if (!aconnector->dc_link) + continue; + + /* + * this is the case when traversing through created end sink + * MST connectors, should be skipped + */ + if (aconnector && aconnector->mst_root) + continue; + + mutex_lock(&aconnector->hpd_lock); + if (!dc_link_detect_connection_type(aconnector->dc_link, &new_connection_type)) + DRM_ERROR("KMS: Failed to detect connector\n"); + + if (aconnector->base.force && new_connection_type == dc_connection_none) { + emulated_link_detect(aconnector->dc_link); + } else { + mutex_lock(&dm->dc_lock); + dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD); + mutex_unlock(&dm->dc_lock); + } + + if (aconnector->fake_enable && aconnector->dc_link->local_sink) + aconnector->fake_enable = false; + + if (aconnector->dc_sink) + dc_sink_release(aconnector->dc_sink); + aconnector->dc_sink = NULL; + amdgpu_dm_update_connector_after_detect(aconnector); + mutex_unlock(&aconnector->hpd_lock); + } + drm_connector_list_iter_end(&iter); + + /* MST topology probing*/ + drm_connector_list_iter_begin(dev, &iter); + drm_for_each_connector_iter(connector, &iter) { + aconnector = to_amdgpu_dm_connector(connector); + if (aconnector->dc_link->type != dc_connection_mst_branch || + aconnector->mst_root) + continue; + + mgr = &aconnector->mst_mgr; + + ret = drm_dp_mst_topology_mgr_resume(mgr, true); + if (ret < 0) + dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx, + aconnector->dc_link); + } + drm_connector_list_iter_end(&iter); +} + static int dm_resume(void *handle) { struct amdgpu_device *adev = handle; struct drm_device *ddev = adev_to_drm(adev); struct amdgpu_display_manager *dm = &adev->dm; - struct amdgpu_dm_connector *aconnector; - struct drm_connector *connector; - struct drm_connector_list_iter iter; struct drm_crtc *crtc; struct drm_crtc_state *new_crtc_state; struct dm_crtc_state *dm_new_crtc_state; @@ -2766,7 +2877,6 @@ static int dm_resume(void *handle) struct drm_plane_state *new_plane_state; struct dm_plane_state *dm_new_plane_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(dm->atomic_obj.state); - enum dc_connection_type new_connection_type = dc_connection_none; struct dc_state *dc_state; int i, r, j;
@@ -2857,44 +2967,6 @@ static int dm_resume(void *handle) /* On resume we need to rewrite the MSTM control bits to enable MST*/ s3_handle_mst(ddev, false);
- /* Do detection*/ - drm_connector_list_iter_begin(ddev, &iter); - drm_for_each_connector_iter(connector, &iter) { - aconnector = to_amdgpu_dm_connector(connector); - - if (!aconnector->dc_link) - continue; - - /* - * this is the case when traversing through already created - * MST connectors, should be skipped - */ - if (aconnector && aconnector->mst_root) - continue; - - mutex_lock(&aconnector->hpd_lock); - if (!dc_link_detect_connection_type(aconnector->dc_link, &new_connection_type)) - DRM_ERROR("KMS: Failed to detect connector\n"); - - if (aconnector->base.force && new_connection_type == dc_connection_none) { - emulated_link_detect(aconnector->dc_link); - } else { - mutex_lock(&dm->dc_lock); - dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD); - mutex_unlock(&dm->dc_lock); - } - - if (aconnector->fake_enable && aconnector->dc_link->local_sink) - aconnector->fake_enable = false; - - if (aconnector->dc_sink) - dc_sink_release(aconnector->dc_sink); - aconnector->dc_sink = NULL; - amdgpu_dm_update_connector_after_detect(aconnector); - mutex_unlock(&aconnector->hpd_lock); - } - drm_connector_list_iter_end(&iter); - /* Force mode set in atomic commit */ for_each_new_crtc_in_state(dm->cached_state, crtc, new_crtc_state, i) new_crtc_state->active_changed = true; @@ -2926,10 +2998,15 @@ static int dm_resume(void *handle)
dm->cached_state = NULL;
+ /* Do link detection and update to upper layer */ + link_detect_and_update_connectors(adev); + + /* Fire hotplug event anyway */ + drm_kms_helper_hotplug_event(ddev); + amdgpu_dm_irq_resume_late(adev);
amdgpu_dm_smu_write_watermarks_table(adev); - return 0; }