On 13.12.23 02:21, Mario Limonciello wrote: > By chance do you have access to any other dock or monitor combinations that > you can conclude it only happens on this dock or only a certain monitor, or > only a certain monitor connected to this dock? Mario, that was a good suggestion! I indeed had access to another docking station, although it's the same type. I than tried this docking station with my AMD X13 Thinkpad and it was working there. It turned out, the working docking station had newer firmware. So I updated the firmware and now it works :-) Firmware Update: ThinkPad Docking Station Firmware Utility v3.3.4 (cs18dkfw334_web.exe) from https://pcsupport.lenovo.com/us/en/downloads/DS505699 How to resolve my issues on freedesktop.org and bugzilla.kernel.org? Thank you for your support! On 13.12.23 02:21, Mario Limonciello wrote: > On 12/12/2023 18:08, Oliver Schmidt wrote: >> Hi Wayne, >> >> On 12.12.23 17:06, Mario Limonciello wrote: >>> I looked through your bugs related to this and I didn't see a reference to the >>> specific docking station model. >>> The logs mentioned "Thinkpad dock" but no model. >>> Could you share more about it so that AMD can try to reproduce it? >> >> Yes, it is a ThinkPad Ultra Dockingstation, part number 40AJ0135EU, see also >> https://support.lenovo.com/us/en/solutions/pd500173-thinkpad-ultra-docking-station-overview-and-service-parts >> >> > > By chance do you have access to any other dock or monitor combinations that you > can conclude it only happens on this dock or only a certain monitor, or only a > certain monitor connected to this dock? > >> Best regards, >> Oliver >> >> On 12.12.23 17:06, Mario Limonciello wrote: >>> On 12/12/2023 04:10, Lin, Wayne wrote: >>>> [Public] >>>> >>>> Hi Mario, >>>> >>>> Thanks for the help. >>>> My feeling is like this problem probably relates to specific dock. Need time >>>> to take >>>> further look. >>> >>> Oliver, >>> >>> I looked through your bugs related to this and I didn't see a reference to the >>> specific docking station model. >>> The logs mentioned "Thinkpad dock" but no model. >>> >>> Could you share more about it so that AMD can try to reproduce it? >>> >>>> >>>> Since reverting solves the issue now, feel free to add: >>>> Acked-by: Wayne Lin <wayne.lin@xxxxxxx> >>> >>> Sure, thanks. >>> >>>> >>>> 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); >>>> >>> >
