[PATCH 08/15] drm/amd/display: Adjust the resume flow

Stylon Wang stylon.wang at amd.com
Wed Aug 9 15:04:58 UTC 2023


From: Wayne Lin <wayne.lin at 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 at amd.com>
Cc: Mario Limonciello <mario.limonciello at amd.com>
Cc: Alex Deucher <alexander.deucher at amd.com>
Cc: stable at vger.kernel.org
Acked-by: Stylon Wang <stylon.wang at amd.com>
Signed-off-by: Wayne Lin <wayne.lin at 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;
 }
 
-- 
2.41.0



More information about the amd-gfx mailing list