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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@luced.de Link: https://bugzilla.kernel.org/show_bug.cgi?id=218211 Link: https://forum.manjaro.org/t/problems-with-external-monitor-wake-up-after-sus... Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3023 Signed-off-by: Mario Limonciello <mario.limonciello mario.limonciello@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);
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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@luced.de Link: https://bugzilla.kernel.org/show_bug.cgi?id=218211 Link: https://forum.manjaro.org/t/problems-with-external-monitor-wake-up-after-sus... Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3023 Signed-off-by: Mario Limonciello <mario.limonciello mario.limonciello@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;}- 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*/ if (aconnector && aconnector->mst_root)
- MST connectors, should be skipped
@@ -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);
[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@amd.com
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@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@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 duringsuspend?\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 duringsuspend?\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 duringsuspend?\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() intodrm mst resume again
* once topology probing work is pulled out from mstresume into mst
* resume 2nd step. mst resume 2nd step should becalled 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 endsink
* 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);
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@amd.com
Sure, thanks.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@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@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 duringsuspend?\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 duringsuspend?\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 duringsuspend?\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() intodrm mst resume again
* once topology probing work is pulled out from mstresume into mst
* resume 2nd step. mst resume 2nd step should becalled 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 endsink
* 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);
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-s...
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@amd.com
Sure, thanks.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@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@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);
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-s...
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@amd.com
Sure, thanks.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@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@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);
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-s...
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@amd.com
Sure, thanks.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@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@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);
On Tue, Dec 12, 2023 at 9:00 PM Mario Limonciello mario.limonciello@amd.com 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-s...
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?
IIRC, Wayne's patch was to fix an HP dock, I suspect reverting this will just break one dock to fix another. Wayne, do you have the other problematic dock that this patch was needed to fix or even the thinkpad dock? Would be nice to properly sort this out if possible.
Alex
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@amd.com
Sure, thanks.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev Cc: stable@vger.kernel.org Cc: Daniel Wheeler daniel.wheeler@amd.com Cc: Wayne Lin wayne.lin@amd.com Reported-by: Oliver Schmidt oliver@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@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 duringsuspend?\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 duringsuspend?\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 duringsuspend?\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() intodrm mst resume again
* once topology probing work is pulled out from mstresume into mst
* resume 2nd step. mst resume 2nd step should becalled 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 endsink
* 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);
On 12/13/2023 08:17, Alex Deucher wrote:
On Tue, Dec 12, 2023 at 9:00 PM Mario Limonciello mario.limonciello@amd.com 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-s...
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?
IIRC, Wayne's patch was to fix an HP dock, I suspect reverting this will just break one dock to fix another. Wayne, do you have the other problematic dock that this patch was needed to fix or even the thinkpad dock? Would be nice to properly sort this out if possible.
Oliver responded back that a firmware update for the problematic dock fixed it.
So in that case I'll revert it in amd-staging-drm-next.
Alex
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@amd.com
Sure, thanks.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, December 12, 2023 12:15 AM To: amd-gfx@lists.freedesktop.org; Wentland, Harry Harry.Wentland@amd.com Cc: Linux Regressions regressions@lists.linux.dev; stable@vger.kernel.org; Wheeler, Daniel Daniel.Wheeler@amd.com; Lin, Wayne Wayne.Lin@amd.com; Oliver Schmidt oliver@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@lists.linux.dev > Cc: stable@vger.kernel.org > Cc: Daniel Wheeler daniel.wheeler@amd.com > Cc: Wayne Lin wayne.lin@amd.com > Reported-by: Oliver Schmidt oliver@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@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);
linux-stable-mirror@lists.linaro.org
-
Alex Deucher -
Lin, Wayne -
Mario Limonciello -
Oliver Schmidt