[PATCH 11/13] drm/edid: introduce a helper that compares edid data from two drm_edid

[Melissa Wen]

On 14/04/2025 07:06, Jani Nikula wrote:
> On Fri, 11 Apr 2025, Melissa Wen <mwen at igalia.com> wrote:
>> AMD driver has a function used to compare if two edid are the same; this
>> is useful to some of the link detection algorithms implemented by
>> amdgpu. Since the amdgpu function can be helpful for other drivers, this
>> commit abstracts the AMD function to make it available at the DRM level
>> by wrapping existent drm_edid_eq().
>>
>> Co-developed-by: Rodrigo Siqueira <siqueira at igalia.com>
>> Signed-off-by: Rodrigo Siqueira <siqueira at igalia.com>
>> Signed-off-by: Melissa Wen <mwen at igalia.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
>>   include/drm/drm_edid.h     |  2 ++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 855beafb76ff..328a25d198e5 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -7502,3 +7502,21 @@ bool drm_edid_is_digital(const struct drm_edid *drm_edid)
>>   		drm_edid->edid->input & DRM_EDID_INPUT_DIGITAL;
>>   }
>>   EXPORT_SYMBOL(drm_edid_is_digital);
>> +
>> +/**
>> + * drm_edid_is_edid_eq - Check if it the EDID is equal
Hi Jani,
> I think drm_edid_eq() is the better name. Please rename the static one
> to make room. Maybe make it drm_edid_eq_buf() or something, because
> that's what it really does.
right
> "Check if the EDIDs are equal"
>
>> + *
>> + * @drm_edid_old: old drm_edid to compare edid
>> + * @drm_edid_new: new drm_edid to compare edid
> Old and new are meaningless here. It's supposed to be a generic
> function. Just a/b or edid1/edid2 or something.
ack
>> + *
>> + * Return true if the EDID is equal
> "Return true if the EDIDs are equal."
ack
>
>> + */
>> +bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_old,
>> +			 const struct drm_edid *drm_edid_new)
>> +{
>> +	const void *old_edid = drm_edid_old->edid;
>> +	size_t old_edid_size = drm_edid_old->size;
> The existing drm_edid_eq() function supports the use case of either or
> both EDIDs being NULL, and returning true for two NULL EDIDs. This one
> oopses if the "old" EDID is NULL.
>
> I'm not sure you can trivially replicate that behaviour by reusing the
> existing function, though.
Right.

I think I can replicate this behavior by doing:

         const void *old_edid = drm_edid ? drm_edid_old->edid : NULL;
         size_t old_edid_size = drm_edid ? drm_edid_old->size : 0;

         return drm_edid_eq(drm_edid_new, old_edid, old_edid_size);

WDTY?

Thanks for reviewing.

Melissa

>
>> +
>> +	return drm_edid_eq(drm_edid_new, old_edid, old_edid_size);
>> +}
>> +EXPORT_SYMBOL(drm_edid_is_edid_eq);
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index eaac5e665892..0e062761296c 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -469,6 +469,8 @@ int drm_edid_connector_update(struct drm_connector *connector,
>>   			      const struct drm_edid *edid);
>>   int drm_edid_connector_add_modes(struct drm_connector *connector);
>>   bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>> +bool drm_edid_is_edid_eq(const struct drm_edid *drm_edid_first,
>> +			 const struct drm_edid *drm_edid_second);
>>   void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>>   			     struct drm_edid_product_id *id);
>>   void drm_edid_print_product_id(struct drm_printer *p,