[PATCH] Revert "drm/amd/display: Adjust the MST resume flow"
Oliver Schmidt
oliver at luced.de
Wed Dec 13 09:10:52 UTC 2023
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 at amd.com>
>>>
>>> Sure, thanks.
>>>
>>>>
>>>> Thanks,
>>>> Wayne
>>>>
>>>>> -----Original Message-----
>>>>> From: Limonciello, Mario <Mario.Limonciello at amd.com>
>>>>> Sent: Tuesday, December 12, 2023 12:15 AM
>>>>> To: amd-gfx at lists.freedesktop.org; Wentland, Harry
>>>>> <Harry.Wentland at amd.com>
>>>>> Cc: Linux Regressions <regressions at lists.linux.dev>; stable at vger.kernel.org;
>>>>> Wheeler, Daniel <Daniel.Wheeler at amd.com>; Lin, Wayne
>>>>> <Wayne.Lin at amd.com>; Oliver Schmidt <oliver at luced.de>
>>>>> 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 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-aft
>>>>>> er-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);
>>>>
>>>
>
More information about the amd-gfx
mailing list