[PATCH v2] drm/msm/dp: add missing drm_edid_connector_update() before add_modes on cached EDID

[PATCH v2] drm/msm/dp: add missing drm_edid_connector_update() before add_modes on cached EDID

[Jens Glathe via B4 Relay]

From: Jens Glathe <jens.glathe@oldschoolsolutions.biz>

After the refactor to struct drm_edid, the fast path in
msm_dp_panel_get_modes() that already held a cached EDID called
drm_edid_connector_add_modes() directly without first calling
drm_edid_connector_update().

The new API requires the update step to associate the EDID with the
connector. Add the missing call. This restores correct behaviour for
the cached-EDID path.

Fixes: 5bea90ad9743 ("drm/msm/dp: switch to struct drm_edid")

Signed-off-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
---
Hi there,

thank you for the feedback on V1. I dug a little deeper and heeded Janis'
and Dmitry's request to print the "garbage" EDID, and to my surprise it 
was the genuine valid EDID for the display. The code is documented in [1].

That changed the picture a bit - "right" EDID but not tied to the connector.
The retry code does the drm_edid_connector_update() before 
drm_edid_connector_add_modes(), so I only added that missing piece in 
msm_dp_panel_get_modes(). This proves to be sufficient to get a stable
link with the desired resolution. Therefore I dropped the entire re-read 
and retry logic, because it is of course an ugly hack.
I have tested this with my complete collection of adapters and hubs, and
all work well.

[1]: https://github.com/jglathe/linux_ms_dev_kit/tree/jg/flaky_edid_retry

with best regards,

Jens
---
Changes in v2:
- Dropped the retry + re-read logic
- Just add the missing drm_edid_connector_update() in the cached EDID path
- Link to v1: https://lore.kernel.org/r/20260531-drm_plug_flaky_edid-v1-1-2708c16dce97@oldschoolsolutions.biz
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 6bb021820d7c5..5b4954e7cb748 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -332,8 +332,10 @@ int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
 		return -EINVAL;
 	}
 
-	if (msm_dp_panel->drm_edid)
+	if (msm_dp_panel->drm_edid) {
+		drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
 		return drm_edid_connector_add_modes(connector);
+	}
 
 	return 0;
 }

---
base-commit: 7da7f07112610a520567421dd2ffcb51beaefbcc
change-id: 20260531-drm_plug_flaky_edid-cc7743f6f909

Best regards,
-- 
Jens Glathe <jens.glathe@oldschoolsolutions.biz>

Claude review: drm/msm/dp: add missing drm_edid_connector_update() before add_modes on cached EDID

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm/dp: add missing drm_edid_connector_update() before add_modes on cached EDID
Author: Jens Glathe via B4 Relay
 <devnull+jens.glathe.oldschoolsolutions.biz@kernel.org>
Patches: 1
Reviewed: 2026-06-04T12:21:12.498053

---

This is a single-patch fix for a real regression introduced by commit 5bea90ad9743 ("drm/msm/dp: switch to struct drm_edid"). The analysis is correct: the `struct drm_edid` API requires `drm_edid_connector_update()` to be called before `drm_edid_connector_add_modes()`, as explicitly documented in the kernel (`drm_edid.c:7152`: "Must be called before calling drm_edid_connector_add_modes()"). The initial EDID read path (`msm_dp_panel_read_sink_caps` at line 281) already calls `drm_edid_connector_update()`, but when `msm_dp_panel_get_modes()` is called with a cached EDID (i.e., the EDID was read on an earlier call), the connector's internal state may not have the EDID associated — for instance, after a connector reset or if `get_modes` is called on a different connector lifecycle. The fix is minimal, correct, and well-motivated by the v2 cover letter analysis.

**Verdict: Looks good. Recommend merge.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/dp: add missing drm_edid_connector_update() before add_modes on cached EDID

[Claude Code Review Bot]

Patch Review

**Correctness: Good.**

The change adds the required `drm_edid_connector_update()` call in the cached-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()` (line 281) where `drm_edid_connector_update()` is called right after `drm_edid_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 `update_display_info()`, `_drm_update_tile_info()`, and `_drm_edid_connector_property_update()` — all of which are idempotent for the same EDID data. If `get_modes` is called more than once during a single connection lifecycle, 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 is `.get_modes` which returns mode count. Other drivers (e.g., `drm_bridge_connector.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 introduced the `struct drm_edid` API usage and dropped the previously-implicit association.

**Scope is appropriate.** V1 had retry/re-read logic; v2 correctly identified the root cause and dropped the workaround. The fix is minimal — one added line plus the braces to wrap the block.

**No issues found.**

---
Generated by Claude Code Patch Reviewer