[PATCH] Revert "drm/amd/display: Adjust the MST resume flow"
Mario Limonciello
mario.limonciello at amd.com
Tue Dec 12 16:06:32 UTC 2023
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