displayport: fix multiple dplib bugs found during exhaustive audit#2
Merged
coleleavitt merged 3 commits intomainfrom Feb 23, 2026
Merged
displayport: fix multiple dplib bugs found during exhaustive audit#2coleleavitt merged 3 commits intomainfrom
coleleavitt merged 3 commits intomainfrom
Conversation
The interlane alignment status was checked twice in isLinkLost() -- once before the per-lane status checks and once after. Both reads come from the same SW cache (refreshed by hal->refreshLinkStatus() before the first check), so the second check at the end of the function always returns the same result as the first. Remove the duplicate to reduce unnecessary code and eliminate any timing window for spurious false-positives on systems where the HAL cache could be externally invalidated between the two reads. Signed-off-by: Cole Leavitt <cole.leavitt@proton.me>
In allocateDpTunnelBw(), the result of divide_ceil(bandwidth * granularityMultiplier, 1e9) was directly cast to NvU8 without bounds checking. For high-bandwidth links (e.g. >64 Gbps with 0.25 Gbps granularity), the computed DPCD value can exceed 255, causing silent truncation and an incorrect bandwidth request. Compute the result in NvU64 first and clamp to NV_U8_MAX before the cast.
In trainLinkOptimized(), the MST path trains to highestAssessedLC and retries up to DP_LT_MAX_FOR_MST_MAX_RETRIES times if the active config does not match. However, when main->train() internally falls back to a lower lane count or link rate (detected by train() comparing lConfig vs activeLinkConfig), the retries may never restore the desired config. After retries exhaust, the code proceeds with bLinkTrainingSuccessful still set to true from initialization, even though activeLinkConfig is lower than what was used to compute DSC PPS parameters during compoundQuery. This can cause the caller to commit a mode with stale DSC configuration computed for a higher bandwidth link. Add a check after the retry loop: if activeLinkConfig is valid but does not match highestAssessedLC, mark link training as failed. This ensures the caller sees the mismatch and can handle it appropriately (e.g. zombie mode) rather than silently programming incorrect DSC parameters.
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughModified three functions in the DisplayPort connector implementation to improve correctness: guarded bandwidth calculation against overflow, removed a code path that bypassed alignment checks, and added detection logic for link training fallbacks when RM forces lower configurations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three verified bug fixes in the DisplayPort library (
dplib), found during an exhaustive audit of the codebase. Each commit is independent and self-contained.Commits
1.
displayport: remove redundant interlane alignment check in isLinkLost()isLinkLost()readsgetInterlaneAlignDone()twice from the same cached DPCD register — once at the top of the function and again partway through. The second check always returns the same result as the first (same cache), making it dead code. Removes the redundant check and its unreachable early-return path.File:
dp_connectorimpl.cpp(isLinkLost)2.
displayport: clamp tunnel bandwidth request to prevent NvU8 truncationIn
allocateDpTunnelBw(), the computed DPCD bandwidth value was directly cast toNvU8without bounds checking. For high-bandwidth tunnel configurations (e.g., >64 Gbps with 0.25 Gbps granularity), the value can exceed 255, causing silent truncation and an incorrect bandwidth allocation request. Now computes inNvU64first and clamps toNV_U8_MAX.File:
dp_connectorimpl.cpp(allocateDpTunnelBw)3.
displayport: detect RM link training fallback in MST optimized pathIn the MST path of
trainLinkOptimized(), whenmain->train()internally falls back to a lower link configuration (fewer lanes or lower rate), the function detects this via thelConfig != activeLinkConfigcheck intrain()and returns false. The retry loop then attempts to restorehighestAssessedLCup toDP_LT_MAX_FOR_MST_MAX_RETRIEStimes.However, if retries exhaust without restoring the desired config,
bLinkTrainingSuccessfulremainstrue(from initialization) because the force-train path (which sets it tofalse) is only entered whenactiveLinkConfigis completely invalid (lanes=0). When RM falls back to a valid but lower config, the function returns success, and the caller proceeds to program DSC with PPS parameters computed for the higherhighestAssessedLCbandwidth — a mismatch that can cause visual corruption or link instability on MST topologies.File:
dp_connectorimpl.cpp(trainLinkOptimized, MST path)Testing
These are code-level fixes based on source analysis. Hardware testing on MST/DSC configurations would validate the behavioral improvements.
Context
Found during the same exhaustive dplib audit that produced PR NVIDIA#1025 (MST DSC unnecessary compression fix).
Summary by cubic
Fixes three DisplayPort bugs affecting bandwidth requests, link-loss checks, and MST link training. Improves stability and prevents DSC mismatches on MST.
Written for commit 27567d7. Summary will update on new commits.
Summary by CodeRabbit