Re: [RFC PATCH 2/7] drm/amd/display: start using drm_edid helpers to parse EDID caps

[Alex Hung <alex.hung@xxxxxxx>]

On 3/8/25 07:26, 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@xxxxxxxxxx>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 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 2cd35392e2da..7edc23267ee7 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
> @@ -108,6 +108,7 @@ 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;
> +	struct drm_edid *drm_edid;

This declaration generates the below build warning:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c: In 
function ‘dm_helpers_parse_edid_caps’:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:116:18: 
error: assignment discards ‘const’ qualifier from pointer target type 
[-Werror=discarded-qualifiers]
  116 |         drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * 
(edid_buf->extensions + 1));
      |                  ^
cc1: all warnings being treated as errors


It can be fixed as the following change

-       struct drm_edid *drm_edid;
+       const struct drm_edid *drm_edid;


>   	struct cea_sad *sads;
>   	int sad_count = -1;
>   	int sadb_count = -1;
> @@ -116,10 +117,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>   
>   	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));

drm_edid_alloc returns "const struct drm_edid *", so drm_edid needs to 
be const as well.


> +
> +	if (!drm_edid_valid(drm_edid))
>   		result = EDID_BAD_CHECKSUM;
>   
>   	edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] |
> @@ -139,8 +143,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;
> +	}
>   
>   	edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
>   	for (i = 0; i < edid_caps->audio_mode_count; ++i) {
> @@ -166,6 +172,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
>   
>   	kfree(sads);
>   	kfree(sadb);
> +	drm_edid_free(drm_edid);
>   
>   	return result;
>   }