[PATCH] drm/amd/display: clarify average bandwidth comment in dcn4_calcs

[PATCH] drm/amd/display: clarify average bandwidth comment in dcn4_calcs

[Adriano Vero]

Replace a stale FIXME_DCN4 comment in calculate_avg_bandwidth_required()
that questioned whether cursor_bw and tdlut bandwidth should be included
in the average bandwidth calculation.

cursor_bw is already correctly included in all four accumulation
sites below the comment. tdlut bandwidth is intentionally absent
because tdlut data is fetched only during prefetch and blanking
intervals, not during active display, and therefore does not
contribute to active average bandwidth.

Also clarify the phantom pipe handling: phantom pipes are excluded
from sys_active average BW but included in svp_prefetch average BW,
which matches the existing code structure.

Signed-off-by: Adriano Vero <adri.vero.dev@gmail.com>
---
 .../dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c  | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
index ca5ac3c0d..34a2a8326 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
@@ -2845,8 +2845,14 @@ static void calculate_avg_bandwidth_required(
 		dram_overhead_factor_p0 = dcc_dram_bw_nom_overhead_factor_p0[k] * mall_prefetch_dram_overhead_factor[k];
 		dram_overhead_factor_p1 = dcc_dram_bw_nom_overhead_factor_p1[k] * mall_prefetch_dram_overhead_factor[k];
 
-		// FIXME_DCN4, was missing cursor_bw in here, but do I actually need that and tdlut bw for average bandwidth calculation?
-		// active avg bw not include phantom, but svp_prefetch avg bw should include phantom pipes
+		/*
+		 * cursor_bw is included in the average bandwidth calculation below.
+		 * tdlut bandwidth is intentionally excluded: tdlut data is fetched
+		 * only during prefetch/blanking intervals and does not contribute
+		 * to active average bandwidth.
+		 * Phantom pipe contributions are excluded from sys_active but
+		 * included in svp_prefetch average bandwidth.
+		 */
 		if (!dml_is_phantom_pipe(&display_cfg->plane_descriptors[k])) {
 			avg_bandwidth_required[dml2_core_internal_soc_state_sys_active][dml2_core_internal_bw_sdp] += sdp_overhead_factor * (ReadBandwidthLuma[k] + ReadBandwidthChroma[k]) + cursor_bw[k];
 			avg_bandwidth_required[dml2_core_internal_soc_state_sys_active][dml2_core_internal_bw_dram] += dram_overhead_factor_p0 * ReadBandwidthLuma[k] + dram_overhead_factor_p1 * ReadBandwidthChroma[k] + cursor_bw[k];
-- 
2.53.0

Claude review: drm/amd/display: clarify average bandwidth comment in dcn4_calcs

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amd/display: clarify average bandwidth comment in dcn4_calcs
Author: Adriano Vero <adri.vero.dev@gmail.com>
Patches: 1
Reviewed: 2026-04-28T15:11:58.284280

---

This is a single comment-only patch that replaces a stale `FIXME_DCN4` with a proper block comment explaining the design rationale. There are no functional code changes.

The patch is **correct and well-motivated**. The old comment expressed uncertainty about whether `cursor_bw` and `tdlut` bandwidth should be included. Inspecting the four accumulation sites at lines 2867-2871 confirms that `cursor_bw[k]` is already included in all four (sys_active SDP, sys_active DRAM, svp_prefetch SDP, svp_prefetch DRAM), and `tdlut` bandwidth is absent from all of them. The new comment accurately documents this state and provides the rationale for tdlut's absence (fetched only during prefetch/blanking).

No concerns with this patch.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amd/display: clarify average bandwidth comment in dcn4_calcs

[Claude Code Review Bot]

Patch Review

**Accuracy of the replacement comment**: Verified against the code. The four accumulation sites at lines 2867-2871 all include `+ cursor_bw[k]`, confirming the "cursor_bw is included" claim. No `tdlut` term appears anywhere in this loop body, confirming the "tdlut bandwidth is intentionally excluded" claim. The phantom pipe handling is also correct: the `if (!dml_is_phantom_pipe(...))` guard on lines 2866-2869 excludes phantom pipes from sys_active, while lines 2870-2871 (outside the guard) include all pipes in svp_prefetch.

**Comment style**: The old comment used C++ style (`//`), the new one uses C block comment style (`/* ... */`). The kernel coding style prefers C-style block comments, so this is a minor style improvement as well.

**Minor nit**: The commit message says "cursor_bw is already correctly included in all four accumulation sites *below* the comment" — this is accurate. One could argue the new comment is slightly verbose for what it describes, but given that the original FIXME expressed genuine confusion about a non-obvious design choice, the added detail is justified.

**Verdict**: Looks good. The patch resolves a stale FIXME with an accurate explanation of the existing behavior. No functional changes, no risk.

Reviewed-by: appropriate.

---
Generated by Claude Code Patch Reviewer