[PATCH] Revert "drm/amd/display: Adjust the MST resume flow"

[Mario Limonciello]

Ping on this one.

On 12/5/2023 13:54, Mario Limonciello wrote:
> This reverts commit ec5fa9fcdeca69edf7dab5ca3b2e0ceb1c08fe9a.
> 
> Reports are that this causes problems with external monitors after wake up
> from suspend, which is something it was directly supposed to help.
> 
> Cc: Linux Regressions <regressions at lists.linux.dev>
> Cc: stable at vger.kernel.org
> Cc: Daniel Wheeler <daniel.wheeler at amd.com>
> Cc: Wayne Lin <wayne.lin at amd.com>
> Reported-by: Oliver Schmidt <oliver at luced.de>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218211
> Link: https://forum.manjaro.org/t/problems-with-external-monitor-wake-up-after-suspend/151840
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3023
> Signed-off-by: Mario Limonciello <mario.limonciello <mario.limonciello at amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 93 +++----------------
>   1 file changed, 13 insertions(+), 80 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 c146dc9cba92..1ba58e4ecab3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2363,62 +2363,14 @@ 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) {
> @@ -2444,15 +2396,18 @@ 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);
>   
> -			/* 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);
> +			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;
> +			}
>   		}
>   	}
>   	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)
> @@ -2849,8 +2804,7 @@ static int dm_resume(void *handle)
>   	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, ret;
> -	bool need_hotplug = false;
> +	int i, r, j;
>   
>   	if (dm->dc->caps.ips_support) {
>   		dc_dmub_srv_exit_low_power_state(dm->dc);
> @@ -2957,7 +2911,7 @@ static int dm_resume(void *handle)
>   			continue;
>   
>   		/*
> -		 * this is the case when traversing through already created end sink
> +		 * this is the case when traversing through already created
>   		 * MST connectors, should be skipped
>   		 */
>   		if (aconnector && aconnector->mst_root)
> @@ -3017,27 +2971,6 @@ static int dm_resume(void *handle)
>   
>   	dm->cached_state = NULL;
>   
> -	/* Do mst topology probing after resuming cached state*/
> -	drm_connector_list_iter_begin(ddev, &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;
> -
> -		ret = drm_dp_mst_topology_mgr_resume(&aconnector->mst_mgr, true);
> -
> -		if (ret < 0) {
> -			dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx,
> -					aconnector->dc_link);
> -			need_hotplug = true;
> -		}
> -	}
> -	drm_connector_list_iter_end(&iter);
> -
> -	if (need_hotplug)
> -		drm_kms_helper_hotplug_event(ddev);
> -
>   	amdgpu_dm_irq_resume_late(adev);
>   
>   	amdgpu_dm_smu_write_watermarks_table(adev);