tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Revert "drm/dp_mst: Remove single tx msg restriction."

This reverts commit 6bb0942e8f46863a745489cce27efe5be2a3885e.

Unfortunately it would appear that the rumors we've heard of sideband
message interleaving not being very well supported are true. On the
Lenovo ThinkPad Thunderbolt 3 dock that I have, interleaved messages
appear to just get dropped:

[drm:drm_dp_mst_wait_tx_reply [drm_kms_helper]] timedout msg send
00000000571ddfd0 2 1
[dp_mst] txmsg cur_offset=2 cur_len=2 seqno=1 state=SENT path_msg=1 dst=00
[dp_mst] type=ENUM_PATH_RESOURCES contents:
[dp_mst] port=2

DP descriptor for this hub:
OUI 90-cc-24 dev-ID SYNA3 HW-rev 1.0 SW-rev 3.12 quirks 0x0008

It would seem like as well that this is a somewhat well known issue in
the field. From section 5.4.2 of the DisplayPort 2.0 specification:

There are MST Sink/Branch devices in the field that do not handle
interleaved message transactions.

To facilitate message transaction handling by downstream devices, an
MST Source device shall generate message transactions in an atomic
manner (i.e., the MST Source device shall not concurrently interleave
multiple message transactions). Therefore, an MST Source device shall
clear the Message_Sequence_No value in the Sideband_MSG_Header to 0.

MST Source devices that support field policy updates by way of
software should update the policy to forego the generation of
interleaved message transactions.

This is a bit disappointing, as features like HDCP require that we send
a sideband request every ~2 seconds for each active stream. However,
there isn't really anything in the specification that allows us to
accurately probe for interleaved messages.

If it ends up being that we -really- need this in the future, we might
be able to whitelist hubs where interleaving is known to work-or maybe
try some sort of heuristics. But for now, let's just play it safe and
not use it.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 6bb0942e8f46 ("drm/dp_mst: Remove single tx msg restriction.")
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200423164225.680178-1-lyude@redhat.com
Reviewed-by: Sean Paul <sean@poorly.run>

Lyude Paul 973a5909 776d5882

+17 -2
+12 -2
drivers/gpu/drm/drm_dp_mst_topology.c
··· 1205 1205 txmsg->state == DRM_DP_SIDEBAND_TX_SENT) { 1206 1206 mstb->tx_slots[txmsg->seqno] = NULL; 1207 1207 } 1208 + mgr->is_waiting_for_dwn_reply = false; 1209 + 1208 1210 } 1209 1211 out: 1210 1212 if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) { ··· 1216 1214 } 1217 1215 mutex_unlock(&mgr->qlock); 1218 1216 1217 + drm_dp_mst_kick_tx(mgr); 1219 1218 return ret; 1220 1219 } 1221 1220 ··· 2792 2789 ret = process_single_tx_qlock(mgr, txmsg, false); 2793 2790 if (ret == 1) { 2794 2791 /* txmsg is sent it should be in the slots now */ 2792 + mgr->is_waiting_for_dwn_reply = true; 2795 2793 list_del(&txmsg->next); 2796 2794 } else if (ret) { 2797 2795 DRM_DEBUG_KMS("failed to send msg in q %d\n", ret); 2796 + mgr->is_waiting_for_dwn_reply = false; 2798 2797 list_del(&txmsg->next); 2799 2798 if (txmsg->seqno != -1) 2800 2799 txmsg->dst->tx_slots[txmsg->seqno] = NULL; ··· 2836 2831 drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); 2837 2832 } 2838 2833 2839 - if (list_is_singular(&mgr->tx_msg_downq)) 2834 + if (list_is_singular(&mgr->tx_msg_downq) && 2835 + !mgr->is_waiting_for_dwn_reply) 2840 2836 process_single_down_tx_qlock(mgr); 2841 2837 mutex_unlock(&mgr->qlock); 2842 2838 } ··· 3829 3823 mutex_lock(&mgr->qlock); 3830 3824 txmsg->state = DRM_DP_SIDEBAND_TX_RX; 3831 3825 mstb->tx_slots[seqno] = NULL; 3826 + mgr->is_waiting_for_dwn_reply = false; 3832 3827 mutex_unlock(&mgr->qlock); 3833 3828 3834 3829 wake_up_all(&mgr->tx_waitq); ··· 3837 3830 return 0; 3838 3831 3839 3832 out_clear_reply: 3833 + mutex_lock(&mgr->qlock); 3834 + mgr->is_waiting_for_dwn_reply = false; 3835 + mutex_unlock(&mgr->qlock); 3840 3836 if (msg) 3841 3837 memset(msg, 0, sizeof(struct drm_dp_sideband_msg_rx)); 3842 3838 out: ··· 4692 4682 struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, tx_work); 4693 4683 4694 4684 mutex_lock(&mgr->qlock); 4695 - if (!list_empty(&mgr->tx_msg_downq)) 4685 + if (!list_empty(&mgr->tx_msg_downq) && !mgr->is_waiting_for_dwn_reply) 4696 4686 process_single_down_tx_qlock(mgr); 4697 4687 mutex_unlock(&mgr->qlock); 4698 4688 }
+5
include/drm/drm_dp_mst_helper.h
··· 593 593 bool payload_id_table_cleared : 1; 594 594 595 595 /** 596 + * @is_waiting_for_dwn_reply: whether we're waiting for a down reply. 597 + */ 598 + bool is_waiting_for_dwn_reply : 1; 599 + 600 + /** 596 601 * @mst_primary: Pointer to the primary/first branch device. 597 602 */ 598 603 struct drm_dp_mst_branch *mst_primary;