From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Re: [PATCH 3/6] drm/msm/dp: Limit voltage swing level to 2 for RA620 bridge Date: Mon, 25 May 2026 19:16:36 +1000 Message-ID: In-Reply-To: <7e435e8c-ba0e-453a-b95c-450446dfc0ff@oss.qualcomm.com> References: <20260522060645.4399-1-graham.oconnor@gmail.com> <7e435e8c-ba0e-453a-b95c-450446dfc0ff@oss.qualcomm.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 patch has a serious problem: it changes a global constant.** ```c -#define DP_TRAIN_LEVEL_MAX 3 +#define DP_TRAIN_LEVEL_MAX 2 ``` `DP_TRAIN_LEVEL_MAX` is used throughout the MSM DP driver for *all* DP conn= ections: - `dp_ctrl.c`: Determines when `DP_TRAIN_MAX_SWING_REACHED` is signaled to = the sink - `dp_link.c`: Clamps voltage swing and pre-emphasis levels; calculates `ma= x_p_level =3D DP_TRAIN_LEVEL_MAX - v_level` The DP specification defines voltage swing levels 0-3. Level 3 (1200 mV) is= a valid, in-spec operating point. Some monitors and bridges legitimately r= equire level 3 to establish a link. Capping the global maximum at 2 will br= eak those configurations on all Qualcomm DP platforms =E2=80=94 not just th= e Radxa Dragon Q6A. **Recommendation**: This needs to be a per-bridge or per-platform quirk, no= t a global constant change. Options include: 1. A device tree property on the DP node specifying the max voltage swing l= evel 2. A quirk based on the connected bridge's identity (though the RA620 is pa= ssive and has no identity) 3. A platform-specific override in the board DTS that the DP driver reads The commit message correctly identifies the problem (RA620 doesn't complete= link training at level 3) but the fix has too broad a blast radius. --- --- Generated by Claude Code Patch Reviewer