[Public] Hi Mario, Thanks for the help. My feeling is like this problem probably relates to specific dock. Need time to take further look. Since reverting solves the issue now, feel free to add: Acked-by: Wayne Lin <wayne.lin@xxxxxxx> Thanks, Wayne > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > Sent: Tuesday, December 12, 2023 12:15 AM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Wentland, Harry > <Harry.Wentland@xxxxxxx> > Cc: Linux Regressions <regressions@xxxxxxxxxxxxxxx>; stable@xxxxxxxxxxxxxxx; > Wheeler, Daniel <Daniel.Wheeler@xxxxxxx>; Lin, Wayne > <Wayne.Lin@xxxxxxx>; Oliver Schmidt <oliver@xxxxxxxx> > Subject: Re: [PATCH] Revert "drm/amd/display: Adjust the MST resume flow" > > 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@xxxxxxxxxxxxxxx> > > Cc: stable@xxxxxxxxxxxxxxx > > Cc: Daniel Wheeler <daniel.wheeler@xxxxxxx> > > Cc: Wayne Lin <wayne.lin@xxxxxxx> > > Reported-by: Oliver Schmidt <oliver@xxxxxxxx> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=218211 > > Link: > > https://forum.manjaro.org/t/problems-with-external-monitor-wake-up-aft > > er-suspend/151840 > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3023 > > Signed-off-by: Mario Limonciello <mario.limonciello > > <mario.limonciello@xxxxxxx> > > --- > > .../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);
