From f77ad61c87dd19519993fce0e1fab2521e8bb6cf Mon Sep 17 00:00:00 2001 From: Cole Leavitt Date: Thu, 12 Feb 2026 22:43:35 -0700 Subject: [PATCH 1/3] displayport: skip unnecessary DSC for MST modes within link bandwidth The MST mode validation path in compoundQueryAttachMST() unconditionally tries DSC when the device supports it, even when the link has sufficient bandwidth for the uncompressed mode. This can cause instability through MST hubs and USB-C docks that don't handle DSC negotiation well, manifesting as spurious HPD short pulses and DPCD AUX channel failures. Add a PBN pre-check before entering the DSC path: if the uncompressed mode fits within the available local link PBN, skip DSC and proceed directly to compoundQueryAttachMSTGeneric() for full validation. This mirrors the SST behavior in compoundQueryAttachSST() which only enables DSC when willLinkSupportModeSST() fails. The pre-check preserves all existing behavior for forced DSC, DSC_DUAL mode requests, and bandwidth-insufficient cases. Signed-off-by: Cole Leavitt --- .../displayport/src/dp_connectorimpl.cpp | 56 ++++++++++++++++--- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/src/common/displayport/src/dp_connectorimpl.cpp b/src/common/displayport/src/dp_connectorimpl.cpp index f3267bc57..c9a0d765c 100644 --- a/src/common/displayport/src/dp_connectorimpl.cpp +++ b/src/common/displayport/src/dp_connectorimpl.cpp @@ -1395,6 +1395,49 @@ bool ConnectorImpl::compoundQueryAttachMST(Group * target, if (compoundQueryAttachMSTIsDscPossible(target, modesetParams, pDscParams)) { + // + // Before entering the DSC path, check if the mode can be supported + // without compression. The SST path (compoundQueryAttachSST) already + // does this correctly: it only enables DSC when willLinkSupportModeSST + // fails. For MST, perform an equivalent pre-check using PBN to avoid + // unnecessary DSC on links with sufficient bandwidth. + // + // Unnecessary DSC adds link training complexity and can cause + // instability through MST hubs and USB-C docks, manifesting as + // spurious HPD short pulses and DPCD AUX channel failures during + // DSC capability negotiation. + // + bool bForceDsc = pDscParams && + (pDscParams->forceDsc == DSC_FORCE_ENABLE); + bool bDscDualRequested = + (modesetParams.modesetInfo.mode == DSC_DUAL); + + if (!bForceDsc && !bDscDualRequested) + { + unsigned base_pbn, slots, slots_pbn; + localInfo.lc.pbnRequired(localInfo.localModesetInfo, + base_pbn, slots, slots_pbn); + + if (compoundQueryLocalLinkPBN + slots_pbn <= + localInfo.lc.pbnTotal()) + { + // + // The uncompressed mode fits within available local link PBN. + // Skip the DSC path and proceed directly to the full generic + // validation (watermark, per-device bandwidth). If the generic + // check fails for non-bandwidth reasons, the mode is not + // supportable regardless of DSC. + // + return compoundQueryAttachMSTGeneric(target, modesetParams, + &localInfo, pDscParams, + pErrorCode); + } + } + + // + // DSC is required: either forced by client, DSC_DUAL requested, + // or local link bandwidth is insufficient for uncompressed mode. + // unsigned int forceDscBitsPerPixelX16 = pDscParams->bitsPerPixelX16; result = compoundQueryAttachMSTDsc(target, modesetParams, &localInfo, pDscParams, pErrorCode); @@ -1406,13 +1449,12 @@ bool ConnectorImpl::compoundQueryAttachMST(Group * target, compoundQueryResult = compoundQueryAttachMSTGeneric(target, modesetParams, &localInfo, pDscParams, pErrorCode); // - // compoundQueryAttachMST Generic might fail due to the insufficient bandwidth , - // We only check whether bpp can be fit in the available bandwidth based on the tranied link config in compoundQueryAttachMSTDsc function. - // There might be cases where the default 10 bpp might fit in the available bandwidth based on the trained link config, - // however, the bandwidth might be insufficient at the actual bottleneck link between source and sink to drive the mode, causing CompoundQueryAttachMSTGeneric to fail. - // Incase of CompoundQueryAttachMSTGeneric failure, instead of returning false, check whether the mode can be supported with the max dsc compression bpp - // and return true if it can be supported. - + // compoundQueryAttachMSTGeneric might fail due to insufficient bandwidth + // at a bottleneck link between source and sink. The default 10 bpp might + // fit based on the trained link config, but the actual available bandwidth + // at intermediate MST branches may be lower. If so, retry with max DSC + // compression (8 bpp) to check if that can support the mode. + // if (!compoundQueryResult && forceDscBitsPerPixelX16 == 0U) { pDscParams->bitsPerPixelX16 = MAX_DSC_COMPRESSION_BPPX16; From e91a9fb402724af0def954e880def95a3a579c94 Mon Sep 17 00:00:00 2001 From: Cole Leavitt Date: Sun, 22 Feb 2026 13:23:33 -0700 Subject: [PATCH 2/3] displayport: fall through to DSC when MST generic check fails at intermediate hops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PBN pre-check only validates local (source) link bandwidth before skipping DSC. In MST daisy-chain or hub topologies, an intermediate branch link may have fewer timeslots than the local link. When compoundQueryAttachMSTGeneric fails at an intermediate branch, the mode is rejected without trying DSC — but DSC compression would reduce the PBN enough to fit through the bottleneck. Instead of returning the generic result directly, check if it succeeded: if so, return true (DSC unnecessary). If not, fall through to the DSC path which tries 10 bpp then 8 bpp max compression as fallback. Signed-off-by: Cole Leavitt --- .../displayport/src/dp_connectorimpl.cpp | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/common/displayport/src/dp_connectorimpl.cpp b/src/common/displayport/src/dp_connectorimpl.cpp index c9a0d765c..f1b55a73b 100644 --- a/src/common/displayport/src/dp_connectorimpl.cpp +++ b/src/common/displayport/src/dp_connectorimpl.cpp @@ -1417,20 +1417,27 @@ bool ConnectorImpl::compoundQueryAttachMST(Group * target, unsigned base_pbn, slots, slots_pbn; localInfo.lc.pbnRequired(localInfo.localModesetInfo, base_pbn, slots, slots_pbn); - if (compoundQueryLocalLinkPBN + slots_pbn <= localInfo.lc.pbnTotal()) { // // The uncompressed mode fits within available local link PBN. - // Skip the DSC path and proceed directly to the full generic - // validation (watermark, per-device bandwidth). If the generic - // check fails for non-bandwidth reasons, the mode is not - // supportable regardless of DSC. + // Try the full generic validation (watermark, per-device + // bandwidth at intermediate MST branches). If it succeeds, + // DSC is unnecessary. + // + if (compoundQueryAttachMSTGeneric(target, modesetParams, + &localInfo, pDscParams, + pErrorCode)) + { + return true; + } + // + // Generic validation failed — possibly due to insufficient + // bandwidth at an intermediate MST branch link. DSC can + // reduce PBN enough to fit through the bottleneck, so fall + // through to the DSC path below. // - return compoundQueryAttachMSTGeneric(target, modesetParams, - &localInfo, pDscParams, - pErrorCode); } } From 223a169517d25bde717a64ba22700ce79abf39d2 Mon Sep 17 00:00:00 2001 From: Cole Leavitt Date: Sun, 22 Feb 2026 13:38:47 -0700 Subject: [PATCH 3/3] displayport: save and restore compound query state around speculative MST pre-check The speculative uncompressed pre-check calls compoundQueryAttachMSTGeneric to determine if DSC can be skipped. However, this function mutates shared state that the subsequent DSC path depends on: 1. compoundQueryResult is set to false on failure but never reset, poisoning all subsequent compoundQueryAttachMSTGeneric calls which unconditionally return the (now-false) flag. 2. Per-device compound_query_state (timeslots_used_by_query and bandwidthAllocatedForIndex) is only partially rolled back on failure. Intermediate branch devices between the source and the bottleneck retain their uncompressed timeslot allocations. When the DSC path calls compoundQueryAttachMSTGeneric again, these devices are skipped via the bandwidthAllocatedForIndex bitmask, keeping incorrect (too-large) uncompressed allocations instead of the correct (smaller) compressed ones. Fix this by saving all relevant state before the speculative call and restoring it on failure, so the DSC path always starts from a clean slate. Also clear pErrorCode on rollback since the speculative failure is not a real error. --- .../displayport/src/dp_connectorimpl.cpp | 83 +++++++++++++++++-- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/src/common/displayport/src/dp_connectorimpl.cpp b/src/common/displayport/src/dp_connectorimpl.cpp index f1b55a73b..0ba86c2aa 100644 --- a/src/common/displayport/src/dp_connectorimpl.cpp +++ b/src/common/displayport/src/dp_connectorimpl.cpp @@ -1422,22 +1422,91 @@ bool ConnectorImpl::compoundQueryAttachMST(Group * target, { // // The uncompressed mode fits within available local link PBN. - // Try the full generic validation (watermark, per-device - // bandwidth at intermediate MST branches). If it succeeds, - // DSC is unnecessary. + // Speculatively try the full generic validation (watermark, + // per-device bandwidth at intermediate MST branches). If it + // succeeds, DSC is unnecessary. // + // compoundQueryAttachMSTGeneric mutates connector and + // per-device state (compoundQueryResult, compoundQueryLocalLinkPBN, + // and device compound_query_state). Since this is a speculative + // check, save the state before and restore it on failure so the + // DSC path starts from a clean slate. + // + bool savedCompoundQueryResult = compoundQueryResult; + + // + // Save per-device compound_query_state for all devices in this + // group's MST path. On failure, intermediate branch devices may + // retain stale timeslot allocations from the uncompressed attempt + // which would cause the DSC path to either skip them (via + // bandwidthAllocatedForIndex) or over-count their timeslots. + // + struct { + DeviceImpl *dev; + unsigned timeslots_used_by_query; + unsigned bandwidthAllocatedForIndex; + } savedDevState[63]; + unsigned savedDevCount = 0; + bool savedDevOverflow = false; + + for (Device * d = target->enumDevices(0); + d && !savedDevOverflow; + d = target->enumDevices(d)) + { + DeviceImpl * tail = (DeviceImpl *)d; + while (tail && tail->getParent()) + { + if (savedDevCount >= 63) + { + savedDevOverflow = true; + break; + } + savedDevState[savedDevCount].dev = tail; + savedDevState[savedDevCount].timeslots_used_by_query = + tail->bandwidth.compound_query_state.timeslots_used_by_query; + savedDevState[savedDevCount].bandwidthAllocatedForIndex = + tail->bandwidth.compound_query_state.bandwidthAllocatedForIndex; + savedDevCount++; + tail = (DeviceImpl*)tail->getParent(); + } + } + + // + // If we couldn't save all device state (extremely deep MST + // topology), skip the speculative pre-check to avoid partial + // state restoration on failure. + // + if (savedDevOverflow) + goto skipPreCheck; + if (compoundQueryAttachMSTGeneric(target, modesetParams, &localInfo, pDscParams, pErrorCode)) { return true; } + // - // Generic validation failed — possibly due to insufficient - // bandwidth at an intermediate MST branch link. DSC can - // reduce PBN enough to fit through the bottleneck, so fall - // through to the DSC path below. + // Generic validation failed — restore all state so the DSC + // path runs on a clean slate. // + // compoundQueryLocalLinkPBN is already rolled back by + // compoundQueryAttachMSTGeneric on failure. + // + compoundQueryResult = savedCompoundQueryResult; + for (unsigned idx = 0; idx < savedDevCount; idx++) + { + savedDevState[idx].dev->bandwidth.compound_query_state + .timeslots_used_by_query = + savedDevState[idx].timeslots_used_by_query; + savedDevState[idx].dev->bandwidth.compound_query_state + .bandwidthAllocatedForIndex = + savedDevState[idx].bandwidthAllocatedForIndex; + } + + if (pErrorCode) + *pErrorCode = DP_IMP_ERROR_NONE; + skipPreCheck: } }