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/dp: Make provision for AS SDP version 1 Date: Thu, 12 Mar 2026 06:55:31 +1000 Message-ID: In-Reply-To: <20260311113611.3393194-19-ankit.k.nautiyal@intel.com> References: <20260311113611.3393194-1-ankit.k.nautiyal@intel.com> <20260311113611.3393194-19-ankit.k.nautiyal@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 **BUG:** `intel_dp_get_as_sdp_revision()` is declared as returning `bool` b= ut returns integer values 1 or 2: ```c static bool intel_dp_get_as_sdp_revision(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_stat= e) { if (crtc_state->has_panel_replay && !intel_alpm_is_alpm_aux_less(intel_dp, crtc_state)) return 1; if (intel_dp->as_sdp_v2_supported) return 2; return 1; } ``` With `bool` return type, both 1 and 2 would be truncated to `true` (1), so = `as_sdp->revision` would always be 1. The return type **must** be `int` or = `u8`. Also, the commit message is slightly confusing: "If AS SDP V1 is not suppor= ted, we always send V1, without the payload." =E2=80=94 should this say "If= AS SDP V2 is not supported"? **Concern:** In the pack function, version 1 skips the payload: ```c if (as_sdp->revision =3D=3D 0x1) return length; ``` But `length` is computed as `sizeof(struct dp_sdp)` which includes headers = and all DB bytes. For V1 with no payload, should this return just the heade= r size? The current code returns the full struct size but doesn't fill DB b= ytes =E2=80=94 the caller may still transmit garbage in the payload area de= pending on how `length` is used. --- Generated by Claude Code Patch Reviewer