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/msm/dp: add missing drm_edid_connector_update() before add_modes on cached EDID Date: Thu, 04 Jun 2026 12:21:12 +1000 Message-ID: In-Reply-To: <20260602-drm_plug_flaky_edid-v2-1-1269fb942299@oldschoolsolutions.biz> References: <20260602-drm_plug_flaky_edid-v2-1-1269fb942299@oldschoolsolutions.biz> <20260602-drm_plug_flaky_edid-v2-1-1269fb942299@oldschoolsolutions.biz> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness: Good.** The change adds the required `drm_edid_connector_update()` call in the cach= ed-EDID fast path of `msm_dp_panel_get_modes()`: ```c if (msm_dp_panel->drm_edid) { drm_edid_connector_update(connector, msm_dp_panel->drm_edid); return drm_edid_connector_add_modes(connector); } ``` This mirrors the pattern already used in `msm_dp_panel_read_sink_caps()` (l= ine 281) where `drm_edid_connector_update()` is called right after `drm_edi= d_read_ddc()`. The documentation in `drm_edid.c` is explicit that `drm_edid= _connector_update()` "must be called before calling `drm_edid_connector_add= _modes()`", so this is the correct fix. **Calling `drm_edid_connector_update()` redundantly is safe.** It calls `up= date_display_info()`, `_drm_update_tile_info()`, and `_drm_edid_connector_p= roperty_update()` =E2=80=94 all of which are idempotent for the same EDID d= ata. If `get_modes` is called more than once during a single connection lif= ecycle, the second call to `drm_edid_connector_update()` simply re-sets the= same values. No concern here. **Return value of `drm_edid_connector_update()` is intentionally ignored.**= The function returns an error code from property updates, but the caller i= s `.get_modes` which returns mode count. Other drivers (e.g., `drm_bridge_c= onnector.c`) also ignore this return value in the `get_modes` path, so this= is consistent with existing patterns. **Fixes tag is correct.** Commit 5bea90ad9743 is the refactor that introduc= ed the `struct drm_edid` API usage and dropped the previously-implicit asso= ciation. **Scope is appropriate.** V1 had retry/re-read logic; v2 correctly identifi= ed the root cause and dropped the workaround. The fix is minimal =E2=80=94 = one added line plus the braces to wrap the block. **No issues found.** --- Generated by Claude Code Patch Reviewer