[PATCH] Revert "drm/amd/display: Adjust the MST resume flow"
Mario Limonciello
mario.limonciello at amd.com
Wed Dec 13 01:21:45 UTC 2023
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