[PATCH] drm: dp_mst_topology: Ensure the proposed pbn does not exceed available bandwidth

[Sean Paul]

From: Sean Paul <seanpaul at chromium.org>

The PBN is calculated by the DP helpers during atomic check using the
adjusted mode. In some cases, the EDID may contain modes which are not
possible given the available PBN. One such example of this is if you
downgrade the DP version of a 4k display from 1.2 to 1.1. The EDID would
still contain the 4k mode, but it is not possible without HBR2. Another
case might be if the branch device and sink have to downgrade their link
speed during training.

This patch checks that the proposed pbn does not exceed a port's
available pbn before allocating vcpi slots.

Signed-off-by: Sean Paul <seanpaul at chromium.org>
---

This is my first dip into MST, so it's possible (probable?) that I'm
doing something wrong. However this seems to fix the issue I'm
experiencing, so at least we have a base to work with.

I'm more than a bit confused when available_pbn is 0, and I've tried
retrying enum_path_resources and even doing a phy powerup before epr,
but it still insists available_pbn=0. I've been looking at the DP 1.3
spec and it isn't too clear on why this might be. If there are better
resources, I'm interested :)

 drivers/gpu/drm/drm_dp_mst_topology.c | 31 ++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 82add736e17d..16a88230091a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2182,7 +2182,26 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
 				DRM_ERROR("got incorrect port in response\n");
 			DRM_DEBUG_KMS("enum path resources %d: %d %d\n", txmsg->reply.u.path_resources.port_number, txmsg->reply.u.path_resources.full_payload_bw_number,
 			       txmsg->reply.u.path_resources.avail_payload_bw_number);
-			port->available_pbn = txmsg->reply.u.path_resources.avail_payload_bw_number;
+
+			/*
+			 * Some monitors (Samsung U28D590 at least) return 0 for
+			 * available pbn while in low power mode.
+			 *
+			 * If we were to trust this value, we'd end up failing
+			 * all vcpi allocations, since the requested bandwidth
+			 * will be compared to 0. So use the full pbn as
+			 * available. Doing this will cap the vcpi allocations
+			 * at the trained link rate and will at least prevent
+			 * us from trying to allocate payloads larger than our
+			 * full pbn.
+			 *
+			 * If there is actually no bandwidth left, we'll fail
+			 * on ALLOCATE_PAYLOAD anyways.
+			 */
+			if (txmsg->reply.u.path_resources.avail_payload_bw_number)
+				port->available_pbn = txmsg->reply.u.path_resources.avail_payload_bw_number;
+			else
+				port->available_pbn = txmsg->reply.u.path_resources.full_payload_bw_number;
 		}
 	}
 
@@ -3239,6 +3258,16 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
 	struct drm_dp_vcpi_allocation *pos, *vcpi = NULL;
 	int prev_slots, req_slots, ret;
 
+	/*
+	 * Sanity check that the pbn proposed doesn't exceed the maximum
+	 * available pbn for the port. This could happen if the EDID is
+	 * advertising a mode which needs a faster link rate than has been
+	 * trained between the sink and the branch device (easy to repro by
+	 * using "compatibility" mode on a 4k display and downgrading to DP 1.1)
+	 */
+	if (pbn > port->available_pbn)
+		return -EINVAL;
+
 	topology_state = drm_atomic_get_mst_topology_state(state, mgr);
 	if (IS_ERR(topology_state))
 		return PTR_ERR(topology_state);
-- 
Sean Paul, Software Engineer, Google / Chromium OS