Re: [PATCH v7 02/15] drm/amd/display: start using drm_edid helpers to parse EDID caps

["Timur Kristóf" <timur.kristof@gmail.com>]

On Thu, 2025-11-06 at 13:49 -0300, Melissa Wen wrote:
> Groundwork that allocates a temporary drm_edid from raw edid to take
> advantage of DRM common-code helpers instead of driver-specific code.
> 
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c    | 12 +++++++++-
> --
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git
> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index eb2c587b0b9b..70014bec7099 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> @@ -110,18 +110,21 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>  	struct drm_connector *connector = &aconnector->base;
>  	struct drm_device *dev = connector->dev;
>  	struct edid *edid_buf = edid ? (struct edid *) edid-
> >raw_edid : NULL;
> +	const struct drm_edid *drm_edid;
>  	struct cea_sad *sads;
>  	int sad_count = -1;
>  	int sadb_count = -1;
>  	int i = 0;
>  	uint8_t *sadb = NULL;
> -
>  	enum dc_edid_status result = EDID_OK;
>  
> +
>  	if (!edid_caps || !edid)
>  		return EDID_BAD_INPUT;
>  
> -	if (!drm_edid_is_valid(edid_buf))
> +	drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * (edid_buf-
> >extensions + 1));
> +
> +	if (!drm_edid_valid(drm_edid))
>  		result = EDID_BAD_CHECKSUM;
>  
>  	edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0]
> |
> @@ -142,8 +145,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>  	apply_edid_quirks(dev, edid_buf, edid_caps);
>  
>  	sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid,
> &sads);
> -	if (sad_count <= 0)
> +	if (sad_count <= 0) {
> +		drm_edid_free(drm_edid);
>  		return result;
> +	}

For consistency with other parts of the codebase, I suggest adding a
label at the end of the function, above drm_edid_free. Then here you
could jump to that label.

>  
>  	edid_caps->audio_mode_count = min(sad_count,
> DC_MAX_AUDIO_DESC_COUNT);
>  	for (i = 0; i < edid_caps->audio_mode_count; ++i) {
> @@ -169,6 +174,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>  
>  	kfree(sads);
>  	kfree(sadb);
> +	drm_edid_free(drm_edid);
>  
>  	return result;
>  }