From: Fangzhi Zuo Jerry.Zuo@amd.com
[ Upstream commit 12cdfb61b32a7be581ec5932e0b6a482cb098204 ]
[Why] dm_mst_get_pbn_divider() returns value integer coming from the cast from fixed point, but the casted integer will then be used in dfixed_const to be multiplied by 4096. The cast from fixed point to integer causes the calculation error becomes bigger when multiplied by 4096.
That makes the calculated pbn_div value becomes smaller than it should be, which leads to the req_slot number becomes bigger.
Such error is getting reflected in 8k30 timing, where the correct and incorrect calculated req_slot 62.9 Vs 63.1. That makes the wrong calculation failed to light up 8k30 after a dock under HBR3 x 4.
[How] Restore the accuracy by keeping the fraction part calculated for the left shift operation.
Reviewed-by: Aurabindo Pillai aurabindo.pillai@amd.com Signed-off-by: Fangzhi Zuo Jerry.Zuo@amd.com Signed-off-by: Wayne Lin wayne.lin@amd.com Tested-by: Dan Wheeler daniel.wheeler@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- Fixes a real user-visible bug: The old code truncated the MST PBN-per- timeslot divider to an integer before converting to fixed20_12, enlarging the rounding error and causing over-allocation of VCPI slots. As the commit message notes, this leads to 8k30 failing to light after docking on HBR3 x4 due to slightly inflated slot requirements. The change corrects the math to preserve fractional precision and eliminates this failure.
- Small, contained change with clear intent: - In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c:857 the function is reworked to compute the divider with two decimal precision using 64-bit math, then return it in fixed20_12 form: - New signature: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c:857 - Precision-preserving computation: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c:862 - Convert to fixed20_12 while retaining fraction: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c:870 - This avoids the earlier integer truncation and preserves the fractional part used by MST slot calculations. - Header updated accordingly: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h:63 changes the prototype to return `uint32_t` (the fixed20_12 `.full` storage). - Call site updated to pass the fixed20_12 directly into the MST topology state instead of re-wrapping an integer with dfixed_const: - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8050 now assigns `mst_state->pbn_div.full = dm_mst_get_pbn_divider(...)`.
- Aligns with DRM MST core expectations: The MST core uses fixed20_12 for `pbn_div` and divides fixed-point PBN by this divider to compute timeslots: - req_slots uses fixed math in drm core: drivers/gpu/drm/display/drm_dp_mst_topology.c:4471 (`DIV_ROUND_UP(dfixed_const(pbn), topology_state->pbn_div.full)`). Feeding an accurate fixed20_12 divider here is exactly what the MST helpers expect. Previously, providing a fixed point made from a truncated integer degraded accuracy.
- Impacted calculations and symptom match: The report of 62.9 vs 63.1 “req_slot” pre-rounding reflects exactly the error introduced by integer-truncating the divider; with the fix, the preserved fractional component makes the “ceil(pbn / pbn_div)” calculation correct, avoiding off-by-one slot failures that can prevent 8k30 mode setup.
- Regression risk assessment: - Scope: Only the AMD DM MST divider computation and its immediate use are changed. No architectural changes, no new features. - API: The function now returns the fixed20_12 `.full` value as `uint32_t`, which is used directly to populate `mst_state->pbn_div.full` (drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8050). This is consistent and safe. - Other AMD call site: In the DSC fairness helper for non-DSC paths, `pbn_div` is still treated as an integer when computing a local `slot_num` (drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8121, 8133). That value is not used by the MST helpers for actual VCPI allocation, which relies on `drm_dp_atomic_find_time_slots()` and the state’s `pbn_div` (drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8065). Thus, this path does not gate the real allocation and does not introduce a regression; it could be cleaned up by truncating the fixed divider if needed, but it is not a blocker for the bugfix. - Math safety: Uses 64-bit intermediate (`div64_u64`) and bounds check for null link; no risk of overflow with realistic link bandwidth values.
- Stable criteria: - Important bugfix with user impact (8k30 MST failure). - Minimal, localized changes. - No architectural churn; aligns with existing fixed20_12 usage in DRM MST core. - Low regression risk; behavior is improved and consistent with core expectations.
Given the correctness and limited scope, and the clear real-world failure it fixes, this is a good candidate for backporting to stable trees.
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 13 ++++++++++--- .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index aca57cc815514..afe3a8279c3a9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7974,7 +7974,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, if (IS_ERR(mst_state)) return PTR_ERR(mst_state);
- mst_state->pbn_div.full = dfixed_const(dm_mst_get_pbn_divider(aconnector->mst_root->dc_link)); + mst_state->pbn_div.full = dm_mst_get_pbn_divider(aconnector->mst_root->dc_link);
if (!state->duplicated) { int max_bpc = conn_state->max_requested_bpc; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 77a9d2c7d3185..5412bf046062c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -822,13 +822,20 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, drm_connector_attach_dp_subconnector_property(&aconnector->base); }
-int dm_mst_get_pbn_divider(struct dc_link *link) +uint32_t dm_mst_get_pbn_divider(struct dc_link *link) { + uint32_t pbn_div_x100; + uint64_t dividend, divisor; + if (!link) return 0;
- return dc_link_bandwidth_kbps(link, - dc_link_get_link_cap(link)) / (8 * 1000 * 54); + dividend = (uint64_t)dc_link_bandwidth_kbps(link, dc_link_get_link_cap(link)) * 100; + divisor = 8 * 1000 * 54; + + pbn_div_x100 = div64_u64(dividend, divisor); + + return dfixed_const(pbn_div_x100) / 100; }
struct dsc_mst_fairness_params { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h index 600d6e2210111..179f622492dbf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h @@ -59,7 +59,7 @@ enum mst_msg_ready_type { struct amdgpu_display_manager; struct amdgpu_dm_connector;
-int dm_mst_get_pbn_divider(struct dc_link *link); +uint32_t dm_mst_get_pbn_divider(struct dc_link *link);
void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, struct amdgpu_dm_connector *aconnector,