* [PATCH RESEND 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid
@ 2026-03-03 21:18 Joshua Peisach
2026-03-03 21:18 ` [PATCH RESEND 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Joshua Peisach @ 2026-03-03 21:18 UTC (permalink / raw)
To: amd-gfx, dri-devel; +Cc: Alex Deucher, Christian König, Joshua Peisach
These patches replace uses of struct edid with struct drm_edid, and
updates function calls appropriately.
Additionally, amdgpu_connector_free_edid is no longer needed, and can be
removed.
Joshua Peisach (2):
drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct
edid
drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 54 ++++++++-----------
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 +-
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 4 +-
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4 +-
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4 +-
5 files changed, 30 insertions(+), 38 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH RESEND 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid 2026-03-03 21:18 [PATCH RESEND 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid Joshua Peisach @ 2026-03-03 21:18 ` Joshua Peisach 2026-03-03 22:35 ` Claude review: " Claude Code Review Bot 2026-03-03 21:18 ` [PATCH RESEND 2/2] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid Joshua Peisach 2026-03-03 22:35 ` Claude review: drm/amdgpu/amdgpu_connectors: Use struct drm_edid Claude Code Review Bot 2 siblings, 1 reply; 7+ messages in thread From: Joshua Peisach @ 2026-03-03 21:18 UTC (permalink / raw) To: amd-gfx, dri-devel; +Cc: Alex Deucher, Christian König, Joshua Peisach Some amdgpu code is still using deprecated edid functions. Switch to the newer functions and update the amdgpu_connector struct's edid type to the drm_edid type. At the same time, use the raw EDID when we need to for speaker allocations and for determining if the input is digital. Signed-off-by: Joshua Peisach <jpeisach@ubuntu.com> --- .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 32 +++++++++---------- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 +- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 4 +-- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4 +-- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4 +-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index d1bf2e150..6336cadad 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -246,10 +246,10 @@ amdgpu_connector_find_encoder(struct drm_connector *connector, return NULL; } -static struct edid * +static const struct drm_edid * amdgpu_connector_get_hardcoded_edid(struct amdgpu_device *adev) { - return drm_edid_duplicate(drm_edid_raw(adev->mode_info.bios_hardcoded_edid)); + return drm_edid_dup(adev->mode_info.bios_hardcoded_edid); } static void amdgpu_connector_get_edid(struct drm_connector *connector) @@ -268,8 +268,8 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) if ((amdgpu_connector_encoder_get_dp_bridge_encoder_id(connector) != ENCODER_OBJECT_ID_NONE) && amdgpu_connector->ddc_bus->has_aux) { - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->aux.ddc); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->aux.ddc); } else if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) || (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) { struct amdgpu_connector_atom_dig *dig = amdgpu_connector->con_priv; @@ -277,14 +277,14 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT || dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) && amdgpu_connector->ddc_bus->has_aux) - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->aux.ddc); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->aux.ddc); else if (amdgpu_connector->ddc_bus) - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->adapter); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->adapter); } else if (amdgpu_connector->ddc_bus) { - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->adapter); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->adapter); } if (!amdgpu_connector->edid) { @@ -292,7 +292,7 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) || (connector->connector_type == DRM_MODE_CONNECTOR_eDP))) { amdgpu_connector->edid = amdgpu_connector_get_hardcoded_edid(adev); - drm_connector_update_edid_property(connector, amdgpu_connector->edid); + drm_edid_connector_update(connector, amdgpu_connector->edid); } } } @@ -311,11 +311,11 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector) int ret; if (amdgpu_connector->edid) { - drm_connector_update_edid_property(connector, amdgpu_connector->edid); - ret = drm_add_edid_modes(connector, amdgpu_connector->edid); + drm_edid_connector_update(connector, amdgpu_connector->edid); + ret = drm_edid_connector_add_modes(connector); return ret; } - drm_connector_update_edid_property(connector, NULL); + drm_edid_connector_update(connector, NULL); return 0; } @@ -883,7 +883,7 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force) ret = connector_status_connected; } else { amdgpu_connector->use_digital = - !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL); + drm_edid_is_digital(amdgpu_connector->edid); /* some oems have boards with separate digital and analog connectors * with a shared ddc line (often vga + hdmi) @@ -1063,7 +1063,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) broken_edid = true; /* defer use_digital to later */ } else { amdgpu_connector->use_digital = - !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL); + drm_edid_is_digital(amdgpu_connector->edid); /* some oems have boards with separate digital and analog connectors * with a shared ddc line (often vga + hdmi) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index dc8d2f52c..c4e025581 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -624,7 +624,7 @@ struct amdgpu_connector { bool use_digital; /* we need to mind the EDID between detect and get modes due to analog/digital/tvencoder */ - struct edid *edid; + const struct drm_edid *edid; void *con_priv; bool dac_load_detect; bool detected_by_load; /* if the connection status was determined by load */ diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index a7ffe10ee..4aaee1bf9 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1298,7 +1298,7 @@ static void dce_v10_0_audio_write_speaker_allocation(struct drm_encoder *encoder return; } - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); if (sad_count < 0) { DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count); sad_count = 0; @@ -1368,7 +1368,7 @@ static void dce_v10_0_audio_write_sad_regs(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads); + sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->edid), &sads); if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index a72e20db5..7cb207c7f 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -1265,7 +1265,7 @@ static void dce_v6_0_audio_write_speaker_allocation(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); if (sad_count < 0) { DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count); sad_count = 0; @@ -1346,7 +1346,7 @@ static void dce_v6_0_audio_write_sad_regs(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads); + sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->edid), &sads); if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index 4221c7b7c..0eff41e94 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -1271,7 +1271,7 @@ static void dce_v8_0_audio_write_speaker_allocation(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); if (sad_count < 0) { DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count); sad_count = 0; @@ -1339,7 +1339,7 @@ static void dce_v8_0_audio_write_sad_regs(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads); + sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->edid), &sads); if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) -- 2.51.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Claude review: drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid 2026-03-03 21:18 ` [PATCH RESEND 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach @ 2026-03-03 22:35 ` Claude Code Review Bot 0 siblings, 0 replies; 7+ messages in thread From: Claude Code Review Bot @ 2026-03-03 22:35 UTC (permalink / raw) To: dri-devel-reviews Patch Review This patch is largely correct. The API migrations are: - `struct edid *edid` → `const struct drm_edid *edid` in `amdgpu_connector` — **correct** - `drm_get_edid()` → `drm_edid_read_ddc()` — **correct** - `drm_edid_duplicate(drm_edid_raw(...))` → `drm_edid_dup()` — **correct**, cleaner - `drm_connector_update_edid_property()` → `drm_edid_connector_update()` — **correct** - `drm_add_edid_modes()` → `drm_edid_connector_add_modes()` — **correct** - `!!(edid->input & DRM_EDID_INPUT_DIGITAL)` → `drm_edid_is_digital()` — **correct** - `drm_edid_to_speaker_allocation(edid, ...)` → `drm_edid_to_speaker_allocation(drm_edid_raw(edid), ...)` — **correct**, these functions still require `struct edid *` - `drm_edid_to_sad(edid, ...)` → `drm_edid_to_sad(drm_edid_raw(edid), ...)` — **correct** One minor observation: the `drm_edid_to_sad()` and `drm_edid_to_speaker_allocation()` calls in dce_v6_0.c, dce_v8_0.c, and dce_v10_0.c still require the raw `struct edid *`, so wrapping with `drm_edid_raw()` is the right approach. Ideally these would eventually get their own `struct drm_edid` versions, but that's out of scope for this series. **Verdict: Patch 1 looks correct.** --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RESEND 2/2] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid 2026-03-03 21:18 [PATCH RESEND 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid Joshua Peisach 2026-03-03 21:18 ` [PATCH RESEND 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach @ 2026-03-03 21:18 ` Joshua Peisach 2026-03-03 22:35 ` Claude review: " Claude Code Review Bot 2026-03-03 22:35 ` Claude review: drm/amdgpu/amdgpu_connectors: Use struct drm_edid Claude Code Review Bot 2 siblings, 1 reply; 7+ messages in thread From: Joshua Peisach @ 2026-03-03 21:18 UTC (permalink / raw) To: amd-gfx, dri-devel; +Cc: Alex Deucher, Christian König, Joshua Peisach Now that we are using struct drm_edid, we can just call drm_edid_free directly. Remove the function and update calls to drm_edid_free. Signed-off-by: Joshua Peisach <jpeisach@ubuntu.com> --- .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index 6336cadad..aabe9d58c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -297,14 +297,6 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) } } -static void amdgpu_connector_free_edid(struct drm_connector *connector) -{ - struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); - - kfree(amdgpu_connector->edid); - amdgpu_connector->edid = NULL; -} - static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector) { struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); @@ -754,7 +746,7 @@ static void amdgpu_connector_destroy(struct drm_connector *connector) { struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); kfree(amdgpu_connector->con_priv); drm_connector_unregister(connector); drm_connector_cleanup(connector); @@ -873,7 +865,7 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force) dret = amdgpu_display_ddc_probe(amdgpu_connector, false); if (dret) { amdgpu_connector->detected_by_load = false; - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); amdgpu_connector_get_edid(connector); if (!amdgpu_connector->edid) { @@ -889,7 +881,7 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force) * with a shared ddc line (often vga + hdmi) */ if (amdgpu_connector->use_digital && amdgpu_connector->shared_ddc) { - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); ret = connector_status_disconnected; } else { ret = connector_status_connected; @@ -984,7 +976,7 @@ static void amdgpu_connector_shared_ddc(enum drm_connector_status *status, /* hpd is our only option in this case */ if (!amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd)) { - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); *status = connector_status_disconnected; } } @@ -1053,7 +1045,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) } if (dret) { amdgpu_connector->detected_by_load = false; - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); amdgpu_connector_get_edid(connector); if (!amdgpu_connector->edid) { @@ -1069,7 +1061,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) * with a shared ddc line (often vga + hdmi) */ if ((!amdgpu_connector->use_digital) && amdgpu_connector->shared_ddc) { - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); ret = connector_status_disconnected; } else { ret = connector_status_connected; @@ -1417,7 +1409,7 @@ amdgpu_connector_dp_detect(struct drm_connector *connector, bool force) goto out; } - amdgpu_connector_free_edid(connector); + drm_edid_free(amdgpu_connector->edid); if ((connector->connector_type == DRM_MODE_CONNECTOR_eDP) || (connector->connector_type == DRM_MODE_CONNECTOR_LVDS)) { -- 2.51.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Claude review: drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid 2026-03-03 21:18 ` [PATCH RESEND 2/2] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid Joshua Peisach @ 2026-03-03 22:35 ` Claude Code Review Bot 0 siblings, 0 replies; 7+ messages in thread From: Claude Code Review Bot @ 2026-03-03 22:35 UTC (permalink / raw) To: dri-devel-reviews Patch Review **This patch has a critical bug.** The removed function was: ```c static void amdgpu_connector_free_edid(struct drm_connector *connector) { struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); kfree(amdgpu_connector->edid); amdgpu_connector->edid = NULL; // <-- THIS IS IMPORTANT } ``` `drm_edid_free()` only frees the memory — it **does not** and **cannot** set the caller's pointer to NULL. Every replacement site leaves `amdgpu_connector->edid` as a dangling pointer. **Specific use-after-free sites:** 1. **`amdgpu_connector_vga_detect()` line 868:** ```c drm_edid_free(amdgpu_connector->edid); amdgpu_connector_get_edid(connector); ``` `amdgpu_connector_get_edid()` starts with `if (amdgpu_connector->edid) return;` — the dangling non-NULL pointer causes it to return immediately without reading a new EDID. The subsequent `if (!amdgpu_connector->edid)` check at line 871 then sees a non-NULL freed pointer and proceeds to call `drm_edid_is_digital()` on freed memory. 2. **`amdgpu_connector_vga_detect()` line 884** (shared DDC disconnect path): `drm_edid_free()` without NULL leaves a dangling pointer that persists in the struct for future accesses. 3. **`amdgpu_connector_shared_ddc()` line 979:** Same dangling pointer issue. 4. **`amdgpu_connector_dvi_detect()` line 1048:** Same bug as site 1 — `amdgpu_connector_get_edid()` will see the dangling pointer and skip the EDID read. 5. **`amdgpu_connector_dvi_detect()` line 1064** (shared DDC disconnect): Same dangling pointer. 6. **`amdgpu_connector_dp_detect()` line 1412:** Dangling pointer left after free. 7. **`amdgpu_connector_destroy()` line 749:** This is the **only** safe call site — the connector struct is `kfree()`d at line 753, so the dangling pointer is irrelevant. **Fix:** Every `drm_edid_free(amdgpu_connector->edid)` call (except in `_destroy`) must be followed by `amdgpu_connector->edid = NULL`. Alternatively, keep a helper function: ```c static void amdgpu_connector_free_edid(struct drm_connector *connector) { struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector); drm_edid_free(amdgpu_connector->edid); amdgpu_connector->edid = NULL; } ``` This would preserve the original behavior while still removing the old `kfree()` call. The commit message claim that "we can just call drm_edid_free directly" is incorrect because the semantics differ — the wrapper provided NULL-clearing that `drm_edid_free()` does not. **Verdict: Patch 2 must be reworked. It introduces use-after-free bugs at 6 call sites.** --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 7+ messages in thread
* Claude review: drm/amdgpu/amdgpu_connectors: Use struct drm_edid 2026-03-03 21:18 [PATCH RESEND 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid Joshua Peisach 2026-03-03 21:18 ` [PATCH RESEND 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach 2026-03-03 21:18 ` [PATCH RESEND 2/2] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid Joshua Peisach @ 2026-03-03 22:35 ` Claude Code Review Bot 2 siblings, 0 replies; 7+ messages in thread From: Claude Code Review Bot @ 2026-03-03 22:35 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: drm/amdgpu/amdgpu_connectors: Use struct drm_edid Author: Joshua Peisach <jpeisach@ubuntu.com> Patches: 3 Reviewed: 2026-03-04T08:35:53.790529 --- This 2-patch series converts the amdgpu connector code from the deprecated `struct edid` / `drm_get_edid()` API to the modern opaque `struct drm_edid` / `drm_edid_read_ddc()` API. The intent is correct and the direction is right — this is a well-understood modernization pattern across DRM drivers. **However, patch 2 introduces use-after-free bugs at every call site.** The old `amdgpu_connector_free_edid()` both freed the EDID and set `amdgpu_connector->edid = NULL`. The replacement `drm_edid_free()` only frees the memory — it cannot NULL out the caller's pointer. The patch fails to add `amdgpu_connector->edid = NULL` after any of the `drm_edid_free()` calls, leaving dangling pointers that are subsequently dereferenced. **This series should not be applied as-is.** --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid
@ 2026-02-12 22:20 Joshua Peisach
2026-02-12 22:20 ` [PATCH 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach
0 siblings, 1 reply; 7+ messages in thread
From: Joshua Peisach @ 2026-02-12 22:20 UTC (permalink / raw)
To: amd-gfx, dri-devel; +Cc: alexander.deucher, christian.koenig, Joshua Peisach
These patches replace uses of struct edid with struct drm_edid, and
updates function calls appropriately.
Additionally, amdgpu_connector_free_edid is no longer needed, and can be
removed.
Joshua Peisach (2):
drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct
edid
drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 54 ++++++++-----------
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 +-
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 4 +-
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4 +-
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4 +-
5 files changed, 30 insertions(+), 38 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid 2026-02-12 22:20 [PATCH 0/2] " Joshua Peisach @ 2026-02-12 22:20 ` Joshua Peisach 2026-02-13 6:24 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 7+ messages in thread From: Joshua Peisach @ 2026-02-12 22:20 UTC (permalink / raw) To: amd-gfx, dri-devel; +Cc: alexander.deucher, christian.koenig, Joshua Peisach Some amdgpu code is still using deprecated edid functions. Switch to the newer functions and update the amdgpu_connector struct's edid type to the drm_edid type. At the same time, use the raw EDID when we need to for speaker allocations and for determining if the input is digital. Signed-off-by: Joshua Peisach <jpeisach@ubuntu.com> --- .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 32 +++++++++---------- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 +- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 4 +-- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4 +-- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4 +-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index d3e312bda..ab83b3a87 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -246,10 +246,10 @@ amdgpu_connector_find_encoder(struct drm_connector *connector, return NULL; } -static struct edid * +static const struct drm_edid * amdgpu_connector_get_hardcoded_edid(struct amdgpu_device *adev) { - return drm_edid_duplicate(drm_edid_raw(adev->mode_info.bios_hardcoded_edid)); + return drm_edid_dup(adev->mode_info.bios_hardcoded_edid); } static void amdgpu_connector_get_edid(struct drm_connector *connector) @@ -268,8 +268,8 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) if ((amdgpu_connector_encoder_get_dp_bridge_encoder_id(connector) != ENCODER_OBJECT_ID_NONE) && amdgpu_connector->ddc_bus->has_aux) { - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->aux.ddc); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->aux.ddc); } else if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) || (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) { struct amdgpu_connector_atom_dig *dig = amdgpu_connector->con_priv; @@ -277,14 +277,14 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT || dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) && amdgpu_connector->ddc_bus->has_aux) - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->aux.ddc); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->aux.ddc); else if (amdgpu_connector->ddc_bus) - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->adapter); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->adapter); } else if (amdgpu_connector->ddc_bus) { - amdgpu_connector->edid = drm_get_edid(connector, - &amdgpu_connector->ddc_bus->adapter); + amdgpu_connector->edid = drm_edid_read_ddc(connector, + &amdgpu_connector->ddc_bus->adapter); } if (!amdgpu_connector->edid) { @@ -292,7 +292,7 @@ static void amdgpu_connector_get_edid(struct drm_connector *connector) if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) || (connector->connector_type == DRM_MODE_CONNECTOR_eDP))) { amdgpu_connector->edid = amdgpu_connector_get_hardcoded_edid(adev); - drm_connector_update_edid_property(connector, amdgpu_connector->edid); + drm_edid_connector_update(connector, amdgpu_connector->edid); } } } @@ -311,11 +311,11 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector) int ret; if (amdgpu_connector->edid) { - drm_connector_update_edid_property(connector, amdgpu_connector->edid); - ret = drm_add_edid_modes(connector, amdgpu_connector->edid); + drm_edid_connector_update(connector, amdgpu_connector->edid); + ret = drm_edid_connector_add_modes(connector); return ret; } - drm_connector_update_edid_property(connector, NULL); + drm_edid_connector_update(connector, NULL); return 0; } @@ -883,7 +883,7 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force) ret = connector_status_connected; } else { amdgpu_connector->use_digital = - !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL); + drm_edid_is_digital(amdgpu_connector->edid); /* some oems have boards with separate digital and analog connectors * with a shared ddc line (often vga + hdmi) @@ -1063,7 +1063,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) broken_edid = true; /* defer use_digital to later */ } else { amdgpu_connector->use_digital = - !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL); + drm_edid_is_digital(amdgpu_connector->edid); /* some oems have boards with separate digital and analog connectors * with a shared ddc line (often vga + hdmi) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index dc8d2f52c..c4e025581 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -624,7 +624,7 @@ struct amdgpu_connector { bool use_digital; /* we need to mind the EDID between detect and get modes due to analog/digital/tvencoder */ - struct edid *edid; + const struct drm_edid *edid; void *con_priv; bool dac_load_detect; bool detected_by_load; /* if the connection status was determined by load */ diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index 72ca6538b..b456a6ec2 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1298,7 +1298,7 @@ static void dce_v10_0_audio_write_speaker_allocation(struct drm_encoder *encoder return; } - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); if (sad_count < 0) { DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count); sad_count = 0; @@ -1368,7 +1368,7 @@ static void dce_v10_0_audio_write_sad_regs(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads); + sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->edid), &sads); if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index acc887a58..13496ba38 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -1265,7 +1265,7 @@ static void dce_v6_0_audio_write_speaker_allocation(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); if (sad_count < 0) { DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count); sad_count = 0; @@ -1346,7 +1346,7 @@ static void dce_v6_0_audio_write_sad_regs(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads); + sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->edid), &sads); if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index 2ccd6aad8..63687c40f 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -1271,7 +1271,7 @@ static void dce_v8_0_audio_write_speaker_allocation(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); if (sad_count < 0) { DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n", sad_count); sad_count = 0; @@ -1339,7 +1339,7 @@ static void dce_v8_0_audio_write_sad_regs(struct drm_encoder *encoder) return; } - sad_count = drm_edid_to_sad(amdgpu_connector->edid, &sads); + sad_count = drm_edid_to_sad(drm_edid_raw(amdgpu_connector->edid), &sads); if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) -- 2.51.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Claude review: drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid 2026-02-12 22:20 ` [PATCH 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach @ 2026-02-13 6:24 ` Claude Code Review Bot 0 siblings, 0 replies; 7+ messages in thread From: Claude Code Review Bot @ 2026-02-13 6:24 UTC (permalink / raw) To: dri-devel-reviews Patch Review This patch performs the core migration from `struct edid` to `const struct drm_edid`, updating the field type in `struct amdgpu_connector` and converting all associated function calls to use the new API. > @@ -246,10 +246,10 @@ amdgpu_connector_find_encoder(struct drm_connector *connector, > return NULL; > } > > -static struct edid * > +static const struct drm_edid * > amdgpu_connector_get_hardcoded_edid(struct amdgpu_device *adev) > { > - return drm_edid_duplicate(drm_edid_raw(adev->mode_info.bios_hardcoded_edid)); > + return drm_edid_dup(adev->mode_info.bios_hardcoded_edid); > } The conversion from `drm_edid_duplicate(drm_edid_raw(...))` to `drm_edid_dup()` is correct and cleaner. > - amdgpu_connector->edid = drm_get_edid(connector, > - &amdgpu_connector->ddc_bus->aux.ddc); > + amdgpu_connector->edid = drm_edid_read_ddc(connector, > + &amdgpu_connector->ddc_bus->aux.ddc); The conversion from `drm_get_edid()` to `drm_edid_read_ddc()` is correct throughout. > @@ -311,11 +311,11 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector) > int ret; > > if (amdgpu_connector->edid) { > - drm_connector_update_edid_property(connector, amdgpu_connector->edid); > - ret = drm_add_edid_modes(connector, amdgpu_connector->edid); > + drm_edid_connector_update(connector, amdgpu_connector->edid); > + ret = drm_edid_connector_add_modes(connector); > return ret; > } The change to `drm_edid_connector_add_modes(connector)` is correct. The new API retrieves the EDID from the connector's internal state that was set by the earlier `drm_edid_connector_update()` call. > @@ -883,7 +883,7 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force) > ret = connector_status_connected; > } else { > amdgpu_connector->use_digital = > - !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL); > + drm_edid_is_digital(amdgpu_connector->edid); Good use of the helper function instead of raw field access. > @@ -1298,7 +1298,7 @@ static void dce_v10_0_audio_write_speaker_allocation(struct drm_encoder *encoder > return; > } > > - sad_count = drm_edid_to_speaker_allocation(amdgpu_connector->edid, &sadb); > + sad_count = drm_edid_to_speaker_allocation(drm_edid_raw(amdgpu_connector->edid), &sadb); Correctly unwraps to the raw EDID for functions that haven't been updated to accept `struct drm_edid`. Overall, this patch is well-executed with no issues found. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-03 22:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-03 21:18 [PATCH RESEND 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid Joshua Peisach 2026-03-03 21:18 ` [PATCH RESEND 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach 2026-03-03 22:35 ` Claude review: " Claude Code Review Bot 2026-03-03 21:18 ` [PATCH RESEND 2/2] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid Joshua Peisach 2026-03-03 22:35 ` Claude review: " Claude Code Review Bot 2026-03-03 22:35 ` Claude review: drm/amdgpu/amdgpu_connectors: Use struct drm_edid Claude Code Review Bot -- strict thread matches above, loose matches on Subject: below -- 2026-02-12 22:20 [PATCH 0/2] " Joshua Peisach 2026-02-12 22:20 ` [PATCH 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid Joshua Peisach 2026-02-13 6:24 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox