dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Wayne Lin <Wayne.Lin@amd.com>
To: <dri-devel@lists.freedesktop.org>, <amd-gfx@lists.freedesktop.org>
Cc: Harry Wentland <hwentlan@amd.com>,
	jerry.zuo@amd.com, Wayne Lin <Wayne.Lin@amd.com>,
	Nicholas.Kazlauskas@amd.com
Subject: [PATCH 2/2] drm/dp_mst: Handle SST-only branch device case
Date: Wed, 8 Jan 2020 16:44:16 +0800	[thread overview]
Message-ID: <20200108084416.6296-3-Wayne.Lin@amd.com> (raw)
In-Reply-To: <20200108084416.6296-1-Wayne.Lin@amd.com>

[Why]
While handling LINK_ADDRESS reply, current code expects a peer device
can handle sideband message once the peer device type is reported as
DP_PEER_DEVICE_MST_BRANCHING. However, when the connected device is
a SST branch case, it can't handle the sideband message(MST_CAP=0 in
DPCD 00021h).

Current code will try to send LINK_ADDRESS to SST branch device and end
up with message timeout and monitor can't display normally. As the
result of that, we should take SST branch device into account.

[How]
According to DP 1.4 spec, we can use Peer_Device_Type as
DP_PEER_DEVICE_MST_BRANCHING and Message_Capability_Status as 0 to
indicate peer device as a SST-only branch device.

Fix following:
- Take SST-only branch device case into account in
drm_dp_port_set_pdt() and add a new parameter 'new_mcs'. Take sst branch
device case as the same case as DP_PEER_DEVICE_DP_LEGACY_CONV and
DP_PEER_DEVICE_SST_SINK. All original handling logics remain.
- Take SST-only branch device case into account in
drm_dp_mst_port_add_connector().
- Fix some parts in drm_dp_mst_handle_link_address_port() to have SST
branch device case into consideration.
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_mst_handle_conn_stat().
- Have SST branch device also report
connector_status_connected when the ddps is true
in drm_dp_mst_detect_port()
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_delayed_destroy_port()

Fixes: c485e2c97dae ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Wayne Lin <Wayne.Lin@amd.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 131 +++++++++++++-------------
 1 file changed, 68 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 8f54b241db08..4395d5cc0645 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1934,73 +1934,74 @@ static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt, bool mcs)
 	return true;
 }
 
-static int drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt)
+static int
+drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
+		    bool new_mcs)
 {
 	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
 	struct drm_dp_mst_branch *mstb;
 	u8 rad[8], lct;
 	int ret = 0;
 
-	if (port->pdt == new_pdt)
+	if (port->pdt == new_pdt && port->mcs == new_mcs)
 		return 0;
 
 	/* Teardown the old pdt, if there is one */
-	switch (port->pdt) {
-	case DP_PEER_DEVICE_DP_LEGACY_CONV:
-	case DP_PEER_DEVICE_SST_SINK:
-		/*
-		 * If the new PDT would also have an i2c bus, don't bother
-		 * with reregistering it
-		 */
-		if (new_pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
-		    new_pdt == DP_PEER_DEVICE_SST_SINK) {
-			port->pdt = new_pdt;
-			return 0;
-		}
+	if (port->pdt != DP_PEER_DEVICE_NONE) {
+		if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+			/*
+			 * If the new PDT would also have an i2c bus,
+			 * don't bother with reregistering it
+			 */
+			if (new_pdt != DP_PEER_DEVICE_NONE &&
+			    drm_dp_mst_is_dp_mst_end_device(new_pdt, new_mcs)) {
+				port->pdt = new_pdt;
+				port->mcs = new_mcs;
+				return 0;
+			}
 
-		/* remove i2c over sideband */
-		drm_dp_mst_unregister_i2c_bus(&port->aux);
-		break;
-	case DP_PEER_DEVICE_MST_BRANCHING:
-		mutex_lock(&mgr->lock);
-		drm_dp_mst_topology_put_mstb(port->mstb);
-		port->mstb = NULL;
-		mutex_unlock(&mgr->lock);
-		break;
+			/* remove i2c over sideband */
+			drm_dp_mst_unregister_i2c_bus(&port->aux);
+		} else {
+			mutex_lock(&mgr->lock);
+			drm_dp_mst_topology_put_mstb(port->mstb);
+			port->mstb = NULL;
+			mutex_unlock(&mgr->lock);
+		}
 	}
 
 	port->pdt = new_pdt;
-	switch (port->pdt) {
-	case DP_PEER_DEVICE_DP_LEGACY_CONV:
-	case DP_PEER_DEVICE_SST_SINK:
-		/* add i2c over sideband */
-		ret = drm_dp_mst_register_i2c_bus(&port->aux);
-		break;
+	port->mcs = new_mcs;
 
-	case DP_PEER_DEVICE_MST_BRANCHING:
-		lct = drm_dp_calculate_rad(port, rad);
-		mstb = drm_dp_add_mst_branch_device(lct, rad);
-		if (!mstb) {
-			ret = -ENOMEM;
-			DRM_ERROR("Failed to create MSTB for port %p", port);
-			goto out;
-		}
+	if (port->pdt != DP_PEER_DEVICE_NONE) {
+		if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+			/* add i2c over sideband */
+			ret = drm_dp_mst_register_i2c_bus(&port->aux);
+		} else {
+			lct = drm_dp_calculate_rad(port, rad);
+			mstb = drm_dp_add_mst_branch_device(lct, rad);
+			if (!mstb) {
+				ret = -ENOMEM;
+				DRM_ERROR("Failed to create MSTB for port %p",
+					  port);
+				goto out;
+			}
 
-		mutex_lock(&mgr->lock);
-		port->mstb = mstb;
-		mstb->mgr = port->mgr;
-		mstb->port_parent = port;
+			mutex_lock(&mgr->lock);
+			port->mstb = mstb;
+			mstb->mgr = port->mgr;
+			mstb->port_parent = port;
 
-		/*
-		 * Make sure this port's memory allocation stays
-		 * around until its child MSTB releases it
-		 */
-		drm_dp_mst_get_port_malloc(port);
-		mutex_unlock(&mgr->lock);
+			/*
+			 * Make sure this port's memory allocation stays
+			 * around until its child MSTB releases it
+			 */
+			drm_dp_mst_get_port_malloc(port);
+			mutex_unlock(&mgr->lock);
 
-		/* And make sure we send a link address for this */
-		ret = 1;
-		break;
+			/* And make sure we send a link address for this */
+			ret = 1;
+		}
 	}
 
 out:
@@ -2153,12 +2154,12 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
 		goto error;
 	}
 
-	if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
-	     port->pdt == DP_PEER_DEVICE_SST_SINK) &&
-	    port->port_num >= DP_MST_LOGICAL_PORT_0) {
-		port->cached_edid = drm_get_edid(port->connector,
-						 &port->aux.ddc);
-		drm_connector_set_tile_property(port->connector);
+	if (port->pdt != DP_PEER_DEVICE_NONE) {
+		if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+			port->cached_edid = drm_get_edid(port->connector,
+							 &port->aux.ddc);
+			drm_connector_set_tile_property(port->connector);
+		}
 	}
 
 	mgr->cbs->register_connector(port->connector);
@@ -2223,6 +2224,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
 	struct drm_dp_mst_port *port;
 	int old_ddps = 0, ret;
 	u8 new_pdt = DP_PEER_DEVICE_NONE;
+	bool new_mcs = 0;
 	bool created = false, send_link_addr = false, changed = false;
 
 	port = drm_dp_get_port(mstb, port_msg->port_number);
@@ -2267,7 +2269,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
 	port->input = port_msg->input_port;
 	if (!port->input)
 		new_pdt = port_msg->peer_device_type;
-	port->mcs = port_msg->mcs;
+	new_mcs = port_msg->mcs;
 	port->ddps = port_msg->ddps;
 	port->ldps = port_msg->legacy_device_plug_status;
 	port->dpcd_rev = port_msg->dpcd_revision;
@@ -2295,7 +2297,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
 		}
 	}
 
-	ret = drm_dp_port_set_pdt(port, new_pdt);
+	ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs);
 	if (ret == 1) {
 		send_link_addr = true;
 	} else if (ret < 0) {
@@ -2309,7 +2311,8 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
 	 * we're coming out of suspend. In this case, always resend the link
 	 * address if there's an MSTB on this port
 	 */
-	if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING)
+	if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING &&
+	    port->mcs)
 		send_link_addr = true;
 
 	if (port->connector)
@@ -2346,6 +2349,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
 	struct drm_dp_mst_port *port;
 	int old_ddps, old_input, ret, i;
 	u8 new_pdt;
+	bool new_mcs;
 	bool dowork = false, create_connector = false;
 
 	port = drm_dp_get_port(mstb, conn_stat->port_number);
@@ -2377,7 +2381,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
 	old_ddps = port->ddps;
 	old_input = port->input;
 	port->input = conn_stat->input_port;
-	port->mcs = conn_stat->message_capability_status;
 	port->ldps = conn_stat->legacy_device_plug_status;
 	port->ddps = conn_stat->displayport_device_plug_status;
 
@@ -2390,8 +2393,8 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
 	}
 
 	new_pdt = port->input ? DP_PEER_DEVICE_NONE : conn_stat->peer_device_type;
-
-	ret = drm_dp_port_set_pdt(port, new_pdt);
+	new_mcs = conn_stat->message_capability_status;
+	ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs);
 	if (ret == 1) {
 		dowork = true;
 	} else if (ret < 0) {
@@ -3958,6 +3961,8 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
 	switch (port->pdt) {
 	case DP_PEER_DEVICE_NONE:
 	case DP_PEER_DEVICE_MST_BRANCHING:
+		if (!port->mcs)
+			ret = connector_status_connected;
 		break;
 
 	case DP_PEER_DEVICE_SST_SINK:
@@ -4597,7 +4602,7 @@ drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port)
 	if (port->connector)
 		port->mgr->cbs->destroy_connector(port->mgr, port->connector);
 
-	drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE);
+	drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs);
 	drm_dp_mst_put_port_malloc(port);
 }
 
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-01-08  8:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08  8:44 [PATCH 0/2] Take SST-only branch device into account Wayne Lin
2020-01-08  8:44 ` [PATCH 1/2] drm/dp_mst: Add a function to determine the mst end device Wayne Lin
2020-01-14 21:15   ` Lyude Paul
2020-01-15  2:40     ` Lin, Wayne
2020-01-08  8:44 ` Wayne Lin [this message]
2020-01-14 21:18   ` [PATCH 2/2] drm/dp_mst: Handle SST-only branch device case Lyude Paul
2020-01-15  2:43     ` 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=20200108084416.6296-3-Wayne.Lin@amd.com \
    --to=wayne.lin@amd.com \
    --cc=Nicholas.Kazlauskas@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hwentlan@amd.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox