Skip to content

Commit fcf4638

Browse files
committed
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 cd82d82 ("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 <[email protected]> Fixes: cd82d82 ("drm/dp_mst: Add branch bandwidth validation to MST atomic check") Cc: Mikita Lipski <[email protected]> Cc: Hans de Goede <[email protected]> Cc: Sean Paul <[email protected]> Reviewed-by: Mikita Lipski <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Alex Deucher <[email protected]> Tested-by: Hans de Goede <[email protected]>
1 parent b2feb1d commit fcf4638

File tree

2 files changed

+9
-10
lines changed

2 files changed

+9
-10
lines changed

drivers/gpu/drm/drm_dp_mst_topology.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2309,7 +2309,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
23092309
port);
23102310
}
23112311
} else {
2312-
port->available_pbn = 0;
2312+
port->full_pbn = 0;
23132313
}
23142314
}
23152315

@@ -2404,7 +2404,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
24042404
if (port->ddps) {
24052405
dowork = true;
24062406
} else {
2407-
port->available_pbn = 0;
2407+
port->full_pbn = 0;
24082408
}
24092409
}
24102410

@@ -2556,7 +2556,7 @@ static int drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *mg
25562556
if (port->input || !port->ddps)
25572557
continue;
25582558

2559-
if (!port->available_pbn) {
2559+
if (!port->full_pbn) {
25602560
drm_modeset_lock(&mgr->base.lock, NULL);
25612561
drm_dp_send_enum_path_resources(mgr, mstb, port);
25622562
drm_modeset_unlock(&mgr->base.lock);
@@ -3002,8 +3002,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
30023002
path_res->port_number,
30033003
path_res->full_payload_bw_number,
30043004
path_res->avail_payload_bw_number);
3005-
port->available_pbn =
3006-
path_res->avail_payload_bw_number;
3005+
port->full_pbn = path_res->full_payload_bw_number;
30073006
port->fec_capable = path_res->fec_capable;
30083007
}
30093008
}
@@ -3598,7 +3597,7 @@ drm_dp_mst_topology_mgr_invalidate_mstb(struct drm_dp_mst_branch *mstb)
35983597

35993598
list_for_each_entry(port, &mstb->ports, next) {
36003599
/* The PBN for each port will also need to be re-probed */
3601-
port->available_pbn = 0;
3600+
port->full_pbn = 0;
36023601

36033602
if (port->mstb)
36043603
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,
48424841
if (drm_dp_mst_atomic_check_bw_limit(port->mstb, mst_state))
48434842
return -ENOSPC;
48444843

4845-
if (port->available_pbn > 0)
4846-
pbn_limit = port->available_pbn;
4844+
if (port->full_pbn > 0)
4845+
pbn_limit = port->full_pbn;
48474846
}
48484847
DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n",
48494848
branch, pbn_limit);

include/drm/drm_dp_mst_helper.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ struct drm_dp_vcpi {
8181
* &drm_dp_mst_topology_mgr.base.lock.
8282
* @num_sdp_stream_sinks: Number of stream sinks. Protected by
8383
* &drm_dp_mst_topology_mgr.base.lock.
84-
* @available_pbn: Available bandwidth for this port. Protected by
84+
* @full_pbn: Max possible bandwidth for this port. Protected by
8585
* &drm_dp_mst_topology_mgr.base.lock.
8686
* @next: link to next port on this branch device
8787
* @aux: i2c aux transport to talk to device connected to this port, protected
@@ -126,7 +126,7 @@ struct drm_dp_mst_port {
126126
u8 dpcd_rev;
127127
u8 num_sdp_streams;
128128
u8 num_sdp_stream_sinks;
129-
uint16_t available_pbn;
129+
uint16_t full_pbn;
130130
struct list_head next;
131131
/**
132132
* @mstb: the branch device connected to this port, if there is one.

0 commit comments

Comments
 (0)