From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/i915/display: Panel Replay BW optimization for DP2.0 tunneling Date: Fri, 13 Mar 2026 14:30:27 +1000 Message-ID: In-Reply-To: <20260312050035.3493690-3-animesh.manna@intel.com> References: <20260312050035.3493690-1-animesh.manna@intel.com> <20260312050035.3493690-3-animesh.manna@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Reviewed-by already present from Arun R Murthy.**=20 **Register bit definition looks correct:** `TRANS_DP2_PR_TUNNELING_ENABLE` = at `REG_BIT(26)` =E2=80=94 fits between existing bit 30 (`PANEL_REPLAY_ENAB= LE`) and bit 23 (`DEBUG_ENABLE`), so the ordering in the header is correct. **The `intel_psr_allow_pr_bw_optimization` check is clean:** It gates on DI= SPLAY_VER >=3D 35, bw_alloc being enabled, and PR optimization being suppor= ted. The DISPLAY_VER check is appropriate since this is a new hardware feat= ure. **Redundant `!intel_dp_is_edp()` check in `dg2_activate_panel_replay`:** ```c if (!intel_dp_is_edp(intel_dp) && intel_psr_allow_pr_bw_optimization(intel_= dp)) dp2_ctl_val |=3D TRANS_DP2_PR_TUNNELING_ENABLE; ``` The `intel_psr_allow_pr_bw_optimization()` function already calls `intel_dp= _tunnel_bw_alloc_is_enabled()` and `intel_dp_tunnel_pr_optimization_support= ed()`, which would both return false for eDP (no tunnel). The explicit `!in= tel_dp_is_edp()` check is technically redundant but serves as defensive doc= umentation =E2=80=94 acceptable. **Missing deactivation/disable path:** When Panel Replay is deactivated, `T= RANS_DP2_PR_TUNNELING_ENABLE` (bit 26) needs to be cleared. Looking at the = existing code, `intel_psr_disable_locked()` calls `intel_de_rmw()` to clear= `TRANS_DP2_PANEL_REPLAY_ENABLE` on the `TRANS_DP2_CTL` register. Does it a= lso clear bit 26? I don't see the deactivation path being updated in this p= atch. If the existing disable code does `intel_de_rmw(display, TRANS_DP2_CT= L(...), TRANS_DP2_PANEL_REPLAY_ENABLE, 0)`, then bit 26 would be left set a= fter disable. This is a potential bug =E2=80=94 the tunneling enable bit sh= ould be cleared when Panel Replay is disabled. Please verify the disable pa= th clears `TRANS_DP2_PR_TUNNELING_ENABLE` as well. **Copy-paste in wrapper doc comment (patch 1, but affects understanding):** ```c * Returns %true if the BW allocation mode is supported on @intel_dp. ``` This comment in `intel_dp_tunnel_pr_optimization_supported()` says "BW allo= cation mode" but should say "PR BW optimization". This is a copy-paste erro= r from `intel_dp_tunnel_bw_alloc_is_enabled`. --- Generated by Claude Code Patch Reviewer