From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/i915/display: Panel Replay BW optimization for DP2.0 tunneling
Date: Tue, 31 Mar 2026 17:12:02 +1000
Message-ID: <review-patch3-20260330133620.3750559-4-animesh.manna@intel.com>
In-Reply-To: <20260330133620.3750559-4-animesh.manna@intel.com>
References: <20260330133620.3750559-1-animesh.manna@intel.com> <20260330133620.3750559-4-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

This is the main functional patch. The `intel_psr_allow_pr_bw_optimization(=
)` helper is clean and correctly checks: not eDP, BW alloc enabled, and PR =
optimization supported.

**Functional concern =E2=80=94 deactivation path does not clear `TRANS_DP2_=
PR_TUNNELING_ENABLE`:**

The activation in `dg2_activate_panel_replay()` sets or clears `TRANS_DP2_P=
R_TUNNELING_ENABLE` correctly:
```c
	if (intel_psr_allow_pr_bw_optimization(intel_dp))
		dp2_ctl_set |=3D TRANS_DP2_PR_TUNNELING_ENABLE;
	else
		dp2_ctl_clear =3D TRANS_DP2_PR_TUNNELING_ENABLE;
```

But the deactivation at `intel_psr.c:2281` only clears `TRANS_DP2_PANEL_REP=
LAY_ENABLE`:
```c
	intel_de_rmw(display, TRANS_DP2_CTL(intel_dp->psr.transcoder),
		     TRANS_DP2_PANEL_REPLAY_ENABLE, 0);
```

This leaves `TRANS_DP2_PR_TUNNELING_ENABLE` set after Panel Replay is deact=
ivated. This could cause the tunneling hardware to continue attempting BW o=
ptimization when PR is no longer active, which seems wrong. The deactivatio=
n should also clear `TRANS_DP2_PR_TUNNELING_ENABLE`:
```c
	intel_de_rmw(display, TRANS_DP2_CTL(intel_dp->psr.transcoder),
		     TRANS_DP2_PANEL_REPLAY_ENABLE | TRANS_DP2_PR_TUNNELING_ENABLE, 0);
```

Similarly, the `intel_psr_get_config()` readback at line 4290 only checks `=
TRANS_DP2_PANEL_REPLAY_ENABLE` =E2=80=94 it may be worth reading back the t=
unneling bit as well for debug/state verification, though this is less crit=
ical.

**The register bit definition** (`REG_BIT(26)`) and placement is consistent=
 with the existing definitions in `intel_display_regs.h`.

**Overall**, this is a well-structured series. The deactivation path issue =
should be fixed before merging.

---
Generated by Claude Code Patch Reviewer