Skip to contentSkip to conversation

Backport patches for 5.13-stable tree

PATCH In review Wayne Lin 2 participants 5 messages Jul 16, 2021 – Jul 19, 2021

Subsystems (3)
lore
Share & tools
lore

Wayne Lin

author
#0 lore raw

cover Backport patches for 5.13-stable tree

Patches below in Linus's tree failed to apply to 5.13-stable tree. Adjust them so we can apply them to 5.13-stable tree.

original git commit id:
* 3769e4c0af5b82c8ea21d037013cb9564dfaa51f
[PATCH] drm/dp_mst: Avoid to mess up payload table by ports in stale topology

* 35d3e8cb35e75450f87f87e3d314e2d418b6954b [PATCH] drm/dp_mst: Do not set proposed vcpi directly

* 24ff3dc18b99c4b912ab1746e803ddb3be5ced4c
[PATCH] drm/dp_mst: Add missing drm parameters to recently added call to drm_dbg_kms()

José Roberto de Souza (1):
drm/dp_mst: Add missing drm parameters to recently added call to

    drm_dbg_kms()

Wayne Lin (2):
drm/dp_mst: Do not set proposed vcpi directly
drm/dp_mst: Avoid to mess up payload table by ports in stale topology

 drivers/gpu/drm/drm_dp_mst_topology.c | 68 +++++++++++++++++----------
 1 file changed, 42 insertions(+), 26 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 1/3 drm/dp_mst: Do not set proposed vcpi directly

[Why]
When we receive CSN message to notify one port is disconnected, we will implicitly set its corresponding num_slots to 0. Later on, we will eventually call drm_dp_update_payload_part1() to arrange down streams.

In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[] to do the update. Not specific to a target sink only. For example, if we light up 2 monitors, Monitor_A and Monitor_B, and then we unplug Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try to update payload for Monitor_A, we'll also implicitly clean payload for Monitor_B at the same time. And finally, when we try to call drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do nothing at this time since payload for Monitor_B has been cleaned up previously.

For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail and this polling will last for 3 seconds.

Therefore, guess the best way is we don't set the proposed_vcpi[] diretly. Let user of these herlper functions to set the proposed_vcpi directly.

[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
2. Tackle the issue in previous commit by skipping those trasient proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by user later on.

Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags

Fixes 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc Lyude Paul <[email protected]>
Cc Wayne Lin <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Maxime Ripard <[email protected]>
Cc Thomas Zimmermann <[email protected]>
Cc <[email protected]> # v5.5+
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 36 ++++++++-------------------
 1 file changed, 10 insertions(+), 26 deletions(-)
drivers/gpu/drm/drm_dp_mst_topology.c +10 -26
index 159014455fab..2495e2c014ac 100644
@@ -2497,7 +2497,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
 {
 	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
 	struct drm_dp_mst_port *port;
-	int old_ddps, old_input, ret, i;
+	int old_ddps, ret;
 	u8 new_pdt;
 	bool new_mcs;
 	bool dowork = false, create_connector = false;
@@ -2529,7 +2529,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->ldps = conn_stat->legacy_device_plug_status;
 	port->ddps = conn_stat->displayport_device_plug_status;
@@ -2552,28 +2551,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
 		dowork = false;
 	}
 
-	if (!old_input && old_ddps != port->ddps && !port->ddps) {
-		for (i = 0; i < mgr->max_payloads; i++) {
-			struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
-			struct drm_dp_mst_port *port_validated;
-
-			if (!vcpi)
-				continue;
-
-			port_validated =
-				container_of(vcpi, struct drm_dp_mst_port, vcpi);
-			port_validated =
-				drm_dp_mst_topology_get_port_validated(mgr, port_validated);
-			if (!port_validated) {
-				mutex_lock(&mgr->payload_lock);
-				vcpi->num_slots = 0;
-				mutex_unlock(&mgr->payload_lock);
-			} else {
-				drm_dp_mst_topology_put_port(port_validated);
-			}
-		}
-	}
-
 	if (port->connector)
 		drm_modeset_unlock(&mgr->base.lock);
 	else if (create_connector)
@@ -3404,8 +3381,15 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 				port = drm_dp_mst_topology_get_port_validated(
 				    mgr, port);
 				if (!port) {
-					mutex_unlock(&mgr->payload_lock);
-					return -EINVAL;
+					if (vcpi->num_slots == payload->num_slots) {
+						cur_slots += vcpi->num_slots;
+						payload->start_slot = req_payload.start_slot;
+						continue;
+					} else {
+						drm_dbg_kms("Fail:set payload to invalid sink");
+						mutex_unlock(&mgr->payload_lock);
+						return -EINVAL;
+					}
 				}
 				put_port = true;
 			}
Signature · 1 line
2.17.1
author
#2 lore raw

PATCH 2/3 drm/dp_mst: Avoid to mess up payload table by ports in stale topology

In reply to Wayne Lin

[Why]
After unplug/hotplug hub from the system, userspace might start to clear stale payloads gradually. If we call drm_dp_mst_deallocate_vcpi() to release stale VCPI of those ports which are not relating to current topology, we have chane to wrongly clear active payload table entry for current topology.

E.g.
We have allocated VCPI 1 in current payload table and we call drm_dp_mst_deallocate_vcpi() to clean VCPI 1 in stale topology. In drm_dp_mst_deallocate_vcpi(), it will call drm_dp_mst_put_payload_id() tp put VCPI 1 and which means ID 1 is available again. Thereafter, if we want to allocate a new payload stream, it will find ID 1 is available by drm_dp_mst_assign_payload_id(). However, ID 1 is being used

[How]
Check target sink is relating to current topology or not before doing any payload table update.
Searching upward to find the target sink's relevant root branch device. If the found root branch device is not the same root of current topology, don't update payload table.

Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Cc tag.

---
 drivers/gpu/drm/drm_dp_mst_topology.c | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/gpu/drm/drm_dp_mst_topology.c +29 -0
index 2495e2c014ac..bf6fa98b4ab7 100644
@@ -94,6 +94,9 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
 static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
 static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
 
+static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port,
+						 struct drm_dp_mst_branch *branch);
+
 #define DBG_PREFIX "[dp_mst]"
 
 #define DP_STR(x) [DP_ ## x] = #x
@@ -3360,6 +3363,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 	struct drm_dp_mst_port *port;
 	int i, j;
 	int cur_slots = 1;
+	bool skip;
 
 	mutex_lock(&mgr->payload_lock);
 	for (i = 0; i < mgr->max_payloads; i++) {
@@ -3374,6 +3378,14 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 			port = container_of(vcpi, struct drm_dp_mst_port,
 					    vcpi);
 
+			mutex_lock(&mgr->lock);
+			skip = !drm_dp_mst_port_downstream_of_branch(port, mgr->mst_primary);
+			mutex_unlock(&mgr->lock);
+
+			if (skip) {
+				drm_dbg_kms("Virtual channel %d is not in current topology\n", i);
+				continue;
+			}
 			/* Validated ports don't matter if we're releasing
 			 * VCPI
 			 */
@@ -3473,6 +3485,7 @@ int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr)
 	struct drm_dp_mst_port *port;
 	int i;
 	int ret = 0;
+	bool skip;
 
 	mutex_lock(&mgr->payload_lock);
 	for (i = 0; i < mgr->max_payloads; i++) {
@@ -3482,6 +3495,13 @@ int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr)
 
 		port = container_of(mgr->proposed_vcpis[i], struct drm_dp_mst_port, vcpi);
 
+		mutex_lock(&mgr->lock);
+		skip = !drm_dp_mst_port_downstream_of_branch(port, mgr->mst_primary);
+		mutex_unlock(&mgr->lock);
+
+		if (skip)
+			continue;
+
 		DRM_DEBUG_KMS("payload %d %d\n", i, mgr->payloads[i].payload_state);
 		if (mgr->payloads[i].payload_state == DP_PAYLOAD_LOCAL) {
 			ret = drm_dp_create_payload_step2(mgr, port, mgr->proposed_vcpis[i]->vcpi, &mgr->payloads[i]);
@@ -4561,9 +4581,18 @@ EXPORT_SYMBOL(drm_dp_mst_reset_vcpi_slots);
 void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 				struct drm_dp_mst_port *port)
 {
+	bool skip;
+
 	if (!port->vcpi.vcpi)
 		return;
 
+	mutex_lock(&mgr->lock);
+	skip = !drm_dp_mst_port_downstream_of_branch(port, mgr->mst_primary);
+	mutex_unlock(&mgr->lock);
+
+	if (skip)
+		return;
+
 	drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
 	port->vcpi.num_slots = 0;
 	port->vcpi.pbn = 0;
Signature · 1 line
2.17.1
author
#3 lore raw

PATCH 3/3 drm/dp_mst: Add missing drm parameters to recently added call to drm_dbg_kms()

In reply to Wayne Lin

From: José Roberto de Souza <[email protected]>

Commit 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by ports in stale topology") added to calls to drm_dbg_kms() but it missed the first parameter, the drm device breaking the build.

Fixes 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by ports in stale topology")
Cc Wayne Lin <[email protected]>
Cc Lyude Paul <[email protected]>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/gpu/drm/drm_dp_mst_topology.c +5 -2
index bf6fa98b4ab7..a68dc25a19c6 100644
@@ -3383,7 +3383,9 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 			mutex_unlock(&mgr->lock);
 
 			if (skip) {
-				drm_dbg_kms("Virtual channel %d is not in current topology\n", i);
+				drm_dbg_kms(mgr->dev,
+					    "Virtual channel %d is not in current topology\n",
+					    i);
 				continue;
 			}
 			/* Validated ports don't matter if we're releasing
@@ -3398,7 +3400,8 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 						payload->start_slot = req_payload.start_slot;
 						continue;
 					} else {
-						drm_dbg_kms("Fail:set payload to invalid sink");
+						drm_dbg_kms(mgr->dev,
+							    "Fail:set payload to invalid sink");
 						mutex_unlock(&mgr->payload_lock);
 						return -EINVAL;
 					}
Signature · 1 line
2.17.1
maintainer
#4 lore raw
In reply to Wayne Lin
Quoted 15 lines · Wayne Lin

Patches below in Linus's tree failed to apply to 5.13-stable tree. Adjust them so we can apply them to 5.13-stable tree.

original git commit id:
* 3769e4c0af5b82c8ea21d037013cb9564dfaa51f
[PATCH] drm/dp_mst: Avoid to mess up payload table by ports in stale topology

* 35d3e8cb35e75450f87f87e3d314e2d418b6954b [PATCH] drm/dp_mst: Do not set proposed vcpi directly

* 24ff3dc18b99c4b912ab1746e803ddb3be5ced4c
[PATCH] drm/dp_mst: Add missing drm parameters to recently added call to drm_dbg_kms()

José Roberto de Souza (1):
drm/dp_mst: Add missing drm parameters to recently added call to

Quoted 12 lines
    drm_dbg_kms()

Wayne Lin (2):
drm/dp_mst: Do not set proposed vcpi directly
drm/dp_mst: Avoid to mess up payload table by ports in stale topology

 drivers/gpu/drm/drm_dp_mst_topology.c | 68 +++++++++++++++++----------
 1 file changed, 42 insertions(+), 26 deletions(-)
Signature · 1 line
2.17.1

All now queued up, thanks.

greg k-h