drm/dp_mst: Increase ACT retry timeout to 3s

[ Upstream commit 873a95e0d59ac06901ae261dda0b7165ffd002b8 ]

Currently we only poll for an ACT up to 30 times, with a busy-wait delay
of 100µs between each attempt - giving us a timeout of 2900µs. While
this might seem sensible, it would appear that in certain scenarios it
can take dramatically longer then that for us to receive an ACT. On one
of the EVGA MST hubs that I have available, I observed said hub
sometimes taking longer then a second before signalling the ACT. These
delays mostly seem to occur when previous sideband messages we've sent
are NAKd by the hub, however it wouldn't be particularly surprising if
it's possible to reproduce times like this simply by introducing branch
devices with large LCTs since payload allocations have to take effect on
every downstream device up to the payload's target.

So, instead of just retrying 30 times we poll for the ACT for up to 3ms,
and additionally use usleep_range() to avoid a very long and rude
busy-wait. Note that the previous retry count of 30 appears to have been
arbitrarily chosen, as I can't find any mention of a recommended timeout
or retry count for ACTs in the DisplayPort 2.0 specification. This also
goes for the range we were previously using for udelay(), although I
suspect that was just copied from the recommended delay for link
training on SST devices.

Changes since v1:
* Use readx_poll_timeout() instead of open-coding timeout loop - Sean
  Paul
Changes since v2:
* Increase poll interval to 200us - Sean Paul
* Print status in hex when we timeout waiting for ACT - Sean Paul

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: Sean Paul <sean@poorly.run>
Cc: <stable@vger.kernel.org> # v3.17+
Reviewed-by: Sean Paul <sean@poorly.run>
Link: https://patchwork.freedesktop.org/patch/msgid/20200406221253.1307209-4-lyude@redhat.com
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 528d6575b01e3fc3a3b772e78318b6ac66529459..bb70c5272fe8e04ca97dc85b0090d3daee50a70d 100644 (file)
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -29,6 +29,7 @@
 #include <linux/i2c.h>
 #include <drm/drm_dp_mst_helper.h>
 #include <drm/drmP.h>
+#include <linux/iopoll.h>
 
 #include <drm/drm_fixed.h>
 
@@ -2673,6 +2674,17 @@ fail:
        return ret;
 }
 
+static int do_get_act_status(struct drm_dp_aux *aux)
+{
+       int ret;
+       u8 status;
+
+       ret = drm_dp_dpcd_readb(aux, DP_PAYLOAD_TABLE_UPDATE_STATUS, &status);
+       if (ret < 0)
+               return ret;
+
+       return status;
+}
 
 /**
  * drm_dp_check_act_status() - Check ACT handled status.
@@ -2682,30 +2694,28 @@ fail:
  */
 int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
 {
-       int count = 0, ret;
-       u8 status;
-
-       do {
-               ret = drm_dp_dpcd_readb(mgr->aux,
-                                       DP_PAYLOAD_TABLE_UPDATE_STATUS,
-                                       &status);
-               if (ret < 0) {
-                       DRM_DEBUG_KMS("failed to read payload table status %d\n",
-                                     ret);
-                       return ret;
-               }
-
-               if (status & DP_PAYLOAD_ACT_HANDLED)
-                       break;
-               count++;
-               udelay(100);
-       } while (count < 30);
-
-       if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
-               DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
-                             status, count);
+       /*
+        * There doesn't seem to be any recommended retry count or timeout in
+        * the MST specification. Since some hubs have been observed to take
+        * over 1 second to update their payload allocations under certain
+        * conditions, we use a rather large timeout value.
+        */
+       const int timeout_ms = 3000;
+       int ret, status;
+
+       ret = readx_poll_timeout(do_get_act_status, mgr->aux, status,
+                                status & DP_PAYLOAD_ACT_HANDLED || status < 0,
+                                200, timeout_ms * USEC_PER_MSEC);
+       if (ret < 0 && status >= 0) {
+               DRM_DEBUG_KMS("Failed to get ACT after %dms, last status: %02x\n",
+                             timeout_ms, status);
                return -EINVAL;
+       } else if (status < 0) {
+               DRM_DEBUG_KMS("Failed to read payload table status: %d\n",
+                             status);
+               return status;
        }
+
        return 0;
 }
 EXPORT_SYMBOL(drm_dp_check_act_status);