[PATCH 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid

[PATCH 0/2] drm/amdgpu/amdgpu_connectors: Use struct drm_edid

[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

[PATCH 1/2] drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid

[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

[PATCH 2/2] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid

[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 ab83b3a87..04842e3f1 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

Claude review: drm/amdgpu/amdgpu_connectors: Use struct drm_edid

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amdgpu/amdgpu_connectors: Use struct drm_edid
Author: Joshua Peisach <jpeisach@ubuntu.com>
Patches: 3
Reviewed: 2026-02-13T16:18:05.671505

---

This two-patch series converts amdgpu connector code from the deprecated `struct edid` type to the newer `struct drm_edid` type, updating all related function calls to use the modern DRM EDID APIs. The conversion is mostly straightforward and follows the established pattern used elsewhere in the DRM subsystem.

The series changes the `amdgpu_connector->edid` field type from `struct edid *` to `const struct drm_edid *`, replaces `drm_get_edid()` calls with `drm_edid_read_ddc()`, replaces `drm_connector_update_edid_property()` with `drm_edid_connector_update()`, and updates audio SAD/speaker allocation code to use `drm_edid_raw()` to access the underlying raw EDID structure. The second patch removes the now-unnecessary `amdgpu_connector_free_edid()` wrapper and calls `drm_edid_free()` directly.

The main concern with this series is a potential use-after-free bug introduced in patch 2. When removing the `amdgpu_connector_free_edid()` wrapper, the patches lose the critical `amdgpu_connector->edid = NULL` assignment that was part of the wrapper. While `drm_edid_free()` safely handles NULL pointers, it does not set the passed pointer to NULL. In several code paths, the EDID is freed but the pointer is left dangling, and the code later checks `if (!amdgpu_connector->edid)` or attempts to use the pointer. This creates a classic use-after-free vulnerability.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amdgpu/amdgpu_connectors: use struct drm_edid instead of struct edid

[Claude Code Review Bot]

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

Claude review: drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid

[Claude Code Review Bot]

Patch Review

This patch removes the wrapper function and calls `drm_edid_free()` directly at all call sites. While this seems like a straightforward cleanup, it introduces a critical resource management bug.

> @@ -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;
> -}

The removed function performs two critical operations: freeing the EDID and setting the pointer to NULL. The replacement code below only does the first operation.

> @@ -873,7 +865,7 @@ amdgpu_connector_vga_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);

This is the first problematic location. After freeing `amdgpu_connector->edid`, the pointer is not set to NULL. Then `amdgpu_connector_get_edid()` is called. Looking at line 261 of that function:

```c
if (amdgpu_connector->edid)
    return;
```

This early-return check now sees a dangling pointer instead of NULL, so the function returns immediately without fetching new EDID data. This breaks the intended behavior of refreshing the EDID after freeing the old one. The same issue appears at lines 1048-1049 in the DVI detect function.

> @@ -883,7 +881,7 @@ amdgpu_connector_vga_detect(struct drm_connector *connector, bool force)
>  		 */
>  		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;

After freeing the EDID here, if the code path later reaches `amdgpu_connector_destroy()` (line 749) or returns through another path that accesses `amdgpu_connector->edid`, you'll have use-after-free or double-free bugs. The same issue occurs at lines 1064, 979, and 1412.

The fix is straightforward: after each `drm_edid_free(amdgpu_connector->edid)` call, add `amdgpu_connector->edid = NULL;`. Alternatively, create a helper function that does both operations atomically, or modify the locations to use a pattern like:

```c
const struct drm_edid *old_edid = amdgpu_connector->edid;
amdgpu_connector->edid = NULL;
drm_edid_free(old_edid);
```

The only location where setting NULL is not needed is in `amdgpu_connector_destroy()` (line 749) since the entire connector structure is being freed immediately after.

---
Generated by Claude Code Patch Reviewer