drm/dp_mst: Use full_pbn instead of available_pbn for bandwidth checks

DisplayPort specifications are fun. For a while, it's been really
unclear to us what available_pbn actually does. There's a somewhat vague
explanation in the DisplayPort spec (starting from 1.2) that partially
explains it:

  The minimum payload bandwidth number supported by the path. Each node
  updates this number with its available payload bandwidth number if its
  payload bandwidth number is less than that in the Message Transaction
  reply.

So, it sounds like available_pbn represents the smallest link rate in
use between the source and the branch device. Cool, so full_pbn is just
the highest possible PBN that the branch device supports right?

Well, we assumed that for quite a while until Sean Paul noticed that on
some MST hubs, available_pbn will actually get set to 0 whenever there's
any active payloads on the respective branch device. This caused quite a
bit of confusion since clearing the payload ID table would end up fixing
the available_pbn value.

So, we just went with that until commit 1b05a0166e9b ("drm/dp_mst: Add
branch bandwidth validation to MST atomic check") started breaking
people's setups due to us getting erroneous available_pbn values. So, we
did some more digging and got confused until we finally looked at the
definition for full_pbn:

  The bandwidth of the link at the trained link rate and lane count
  between the DP Source device and the DP Sink device with no time slots
  allocated to VC Payloads, represented as a Payload Bandwidth Number. As
  with the Available_Payload_Bandwidth_Number, this number is determined
  by the link with the lowest lane count and link rate.

That's what we get for not reading specs closely enough, hehe. So, since
full_pbn is definitely what we want for doing bandwidth restriction
checks - let's start using that instead and ignore available_pbn
entirely.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 1b05a0166e9b ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Reviewed-by: Mikita Lipski <mikita.lipski@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200306234623.547525-3-lyude@redhat.com
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9188c53f5c965f1ec6f8d3e6f598e7c85b5a3894..7df7676b45c4350795e360e481a2696586a8f2f0 100644 (file)
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2309,7 +2309,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
                                                                port);
                        }
                } else {
-                       port->available_pbn = 0;
+                       port->full_pbn = 0;
                }
        }
 
@@ -2404,7 +2404,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
                if (port->ddps) {
                        dowork = true;
                } else {
-                       port->available_pbn = 0;
+                       port->full_pbn = 0;
                }
        }
 
@@ -2556,7 +2556,7 @@ static int drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *mg
                if (port->input || !port->ddps)
                        continue;
 
-               if (!port->available_pbn) {
+               if (!port->full_pbn) {
                        drm_modeset_lock(&mgr->base.lock, NULL);
                        drm_dp_send_enum_path_resources(mgr, mstb, port);
                        drm_modeset_unlock(&mgr->base.lock);
@@ -3002,8 +3002,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
                                      path_res->port_number,
                                      path_res->full_payload_bw_number,
                                      path_res->avail_payload_bw_number);
-                       port->available_pbn =
-                               path_res->avail_payload_bw_number;
+                       port->full_pbn = path_res->full_payload_bw_number;
                        port->fec_capable = path_res->fec_capable;
                }
        }
@@ -3598,7 +3597,7 @@ drm_dp_mst_topology_mgr_invalidate_mstb(struct drm_dp_mst_branch *mstb)
 
        list_for_each_entry(port, &mstb->ports, next) {
                /* The PBN for each port will also need to be re-probed */
-               port->available_pbn = 0;
+               port->full_pbn = 0;
 
                if (port->mstb)
                        drm_dp_mst_topology_mgr_invalidate_mstb(port->mstb);
@@ -4842,8 +4841,8 @@ int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch,
                        if (drm_dp_mst_atomic_check_bw_limit(port->mstb, mst_state))
                                return -ENOSPC;
 
-               if (port->available_pbn > 0)
-                       pbn_limit = port->available_pbn;
+               if (port->full_pbn > 0)
+                       pbn_limit = port->full_pbn;
        }
        DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n",
                         branch, pbn_limit);

diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index bcb39da9adb4ac4bd6e2a4455c23033599c88561..41725d88d27e7ff7011bdd95a3886d77c4f73ef1 100644 (file)
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -81,7 +81,7 @@ struct drm_dp_vcpi {
  * &drm_dp_mst_topology_mgr.base.lock.
  * @num_sdp_stream_sinks: Number of stream sinks. Protected by
  * &drm_dp_mst_topology_mgr.base.lock.
- * @available_pbn: Available bandwidth for this port. Protected by
+ * @full_pbn: Max possible bandwidth for this port. Protected by
  * &drm_dp_mst_topology_mgr.base.lock.
  * @next: link to next port on this branch device
  * @aux: i2c aux transport to talk to device connected to this port, protected
@@ -126,7 +126,7 @@ struct drm_dp_mst_port {
        u8 dpcd_rev;
        u8 num_sdp_streams;
        u8 num_sdp_stream_sinks;
-       uint16_t available_pbn;
+       uint16_t full_pbn;
        struct list_head next;
        /**
         * @mstb: the branch device connected to this port, if there is one.