All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wayne Lin <Wayne.Lin@amd.com>
To: <amd-gfx@lists.freedesktop.org>
Cc: Rodrigo.Siqueira@amd.com, jerry.zuo@amd.com,
	Aurabindo.Pillai@amd.com, Wayne Lin <Wayne.Lin@amd.com>,
	Harry.Wentland@amd.com
Subject: [PATCH 3/3] drm/amd/display: Release remote dc_sink under mst scenario
Date: Tue, 10 May 2022 17:57:01 +0800	[thread overview]
Message-ID: <20220510095701.57375-4-Wayne.Lin@amd.com> (raw)
In-Reply-To: <20220510095701.57375-1-Wayne.Lin@amd.com>

[Why]
Observe that we have several problems while releasing remote dc_sink
under mst cases.

- When unplug mst branch device from the source, we now try to free all
  remote dc_sinks in dm_helpers_dp_mst_stop_top_mgr(). However, there are
  bugs while we're releasing dc_sinks here. First of all,
  link->remote_sinks[] array get shuffled within
  dc_link_remove_remote_sink(). As the result, increasing the array index
  within the releasing loop is wrong. Secondly, it tries to call
  dc_sink_release() to release the dc_sink of the same aconnector every
  time in the loop. Which can't release dc_sink of all aconnector in the
  mst topology.
- There is no code path for us to release remote dc_sink for disconnected
  sst monitor which unplug event is notified by CSN sideband message. Which
  means we'll use stale dc_sink data to represent later on connected
  monitor. Also, has chance to break the maximum remote dc_sink number
  constraint.

[How]
Distinguish unplug event of mst scenario into 2 cases.

* Unplug sst/legacy stream sink off the mst topology
- Release related remote dc_sink in detec_ctx().

* Unplug mst branch device off the mst topology
- Release related remote dc_sink in early_unregister()

Signed-off-by: Wayne Lin <Wayne.Lin@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 18 +--------
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 39 +++++++++++++++++--
 3 files changed, 39 insertions(+), 23 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 d81b9ef11dba..f53f8962e6da 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2191,7 +2191,8 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
 		} else {
 			ret = drm_dp_mst_topology_mgr_resume(mgr, true);
 			if (ret < 0) {
-				drm_dp_mst_topology_mgr_set_mst(mgr, false);
+				dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx,
+					aconnector->dc_link);
 				need_hotplug = true;
 			}
 		}
@@ -10359,7 +10360,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
 		 * added MST connectors not found in existing crtc_state in the chained mode
 		 * TODO: need to dig out the root cause of that
 		 */
-		if (!aconnector || (!aconnector->dc_sink && aconnector->mst_port))
+		if (!aconnector)
 			goto skip_modeset;
 
 		if (modereset_required(new_crtc_state))
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 28cf24f6ab32..e4fb19a68ccd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -451,7 +451,6 @@ bool dm_helpers_dp_mst_stop_top_mgr(
 		struct dc_link *link)
 {
 	struct amdgpu_dm_connector *aconnector = link->priv;
-	uint8_t i;
 
 	if (!aconnector) {
 		DRM_ERROR("Failed to find connector for link!");
@@ -463,22 +462,7 @@ bool dm_helpers_dp_mst_stop_top_mgr(
 
 	if (aconnector->mst_mgr.mst_state == true) {
 		drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_mgr, false);
-
-		for (i = 0; i < MAX_SINKS_PER_LINK; i++) {
-			if (link->remote_sinks[i] == NULL)
-				continue;
-
-			if (link->remote_sinks[i]->sink_signal ==
-			    SIGNAL_TYPE_DISPLAY_PORT_MST) {
-				dc_link_remove_remote_sink(link, link->remote_sinks[i]);
-
-				if (aconnector->dc_sink) {
-					dc_sink_release(aconnector->dc_sink);
-					aconnector->dc_sink = NULL;
-					aconnector->dc_link->cur_link_settings.lane_count = 0;
-				}
-			}
-		}
+		link->cur_link_settings.lane_count = 0;
 	}
 
 	return false;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 0542034530b1..e0531a569a14 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -142,11 +142,28 @@ amdgpu_dm_mst_connector_late_register(struct drm_connector *connector)
 static void
 amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
 {
-	struct amdgpu_dm_connector *amdgpu_dm_connector =
+	struct amdgpu_dm_connector *aconnector =
 		to_amdgpu_dm_connector(connector);
-	struct drm_dp_mst_port *port = amdgpu_dm_connector->port;
+	struct drm_dp_mst_port *port = aconnector->port;
+	struct amdgpu_dm_connector *root = aconnector->mst_port;
+	struct dc_link *dc_link = aconnector->dc_link;
+	struct dc_sink *dc_sink = aconnector->dc_sink;
 
 	drm_dp_mst_connector_early_unregister(connector, port);
+
+	/*
+	 * Release dc_sink for connector which its attached port is
+	 * no longer in the mst topology
+	 */
+	drm_modeset_lock(&root->mst_mgr.base.lock, NULL);
+	if (dc_sink) {
+		if (dc_link->sink_count)
+			dc_link_remove_remote_sink(dc_link, dc_sink);
+
+		dc_sink_release(dc_sink);
+		aconnector->dc_sink = NULL;
+	}
+	drm_modeset_unlock(&root->mst_mgr.base.lock);
 }
 
 static const struct drm_connector_funcs dm_dp_mst_connector_funcs = {
@@ -346,12 +363,26 @@ dm_dp_mst_detect(struct drm_connector *connector,
 {
 	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
 	struct amdgpu_dm_connector *master = aconnector->mst_port;
+	int connection_status;
 
 	if (drm_connector_is_unregistered(connector))
 		return connector_status_disconnected;
 
-	return drm_dp_mst_detect_port(connector, ctx, &master->mst_mgr,
-				      aconnector->port);
+	connection_status = drm_dp_mst_detect_port(connector, ctx, &master->mst_mgr,
+							aconnector->port);
+
+	/*
+	 * Release dc_sink for connector which unplug event is notified by CSN msg
+	 */
+	if (connection_status == connector_status_disconnected && aconnector->dc_sink) {
+		if (aconnector->dc_link->sink_count)
+			dc_link_remove_remote_sink(aconnector->dc_link, aconnector->dc_sink);
+
+		dc_sink_release(aconnector->dc_sink);
+		aconnector->dc_sink = NULL;
+	}
+
+	return connection_status;
 }
 
 static int dm_dp_mst_atomic_check(struct drm_connector *connector,
-- 
2.36.1


  parent reply	other threads:[~2022-05-10  9:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10  9:56 [PATCH 0/3] Fix issues when unplung monitor under mst scenario Wayne Lin
2022-05-10  9:56 ` [PATCH 1/3] Revert "drm/amd/display: Add flag to detect dpms force off during HPD" Wayne Lin
2022-05-10  9:57 ` [PATCH 2/3] Revert "drm/amd/display: turn DPMS off on connector unplug" Wayne Lin
2022-05-10  9:57 ` Wayne Lin [this message]
2022-05-17 21:49 ` [PATCH 0/3] Fix issues when unplung monitor under mst scenario Lyude Paul
2022-05-31 19:38 ` Lyude Paul
2022-06-01  6:05   ` Lin, Wayne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220510095701.57375-4-Wayne.Lin@amd.com \
    --to=wayne.lin@amd.com \
    --cc=Aurabindo.Pillai@amd.com \
    --cc=Harry.Wentland@amd.com \
    --cc=Rodrigo.Siqueira@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=jerry.zuo@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.