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/amd: treat max_tmds_clock==0 as unconstrained Date: Mon, 16 Mar 2026 11:48:52 +1000 Message-ID: In-Reply-To: <20260315223307.45807-4-johanneswueller@gmail.com> References: <20260315223307.45807-1-johanneswueller@gmail.com> <20260315223307.45807-4-johanneswueller@gmail.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 most straightforward and correct patch in the series.** When = an EDID lacks an HDMI VSDB, `max_tmds_clock` will be 0. Treating 0 as "unli= mited" rather than "no clock allowed" is the right fix, and matches how oth= er drivers handle this (e.g. the DRM core's `drm_hdmi_compute_mode_clock()`= treats 0 as unconstrained). ```c if (info->max_tmds_clock =3D=3D 0 || normalized_clk <=3D info->max_tmds_clock) { ``` This is clean and correct. However, should this also be gated on `edid_over= ridden`? Without the override flag check, this changes behavior for any dis= play that genuinely reports `max_tmds_clock =3D=3D 0` through its real EDID= , which could potentially allow modes that exceed the display's capabilitie= s. That said, the HDMI spec says a conformant EDID should always include a = VSDB with a valid max_tmds_clock if it supports HDMI, so 0 generally means = "not specified" rather than "zero limit" =E2=80=94 making this change safe = regardless of override status. --- Generated by Claude Code Patch Reviewer