On Tue, 16 Apr 2024, Thomas Zimmermann <tzimmerm...@suse.de> wrote:
> Hi
>
> Am 16.04.24 um 11:20 schrieb Jani Nikula:
>> Repurpose drm_edid_are_equal() to be more helpful for its single user,
>> and rename drm_edid_eq(). Functionally deduce the length from the blob
>> size, not the blob data, making it more robust against any errors.
>
> Could be squashed into patch 6.

Ack.

Thanks for the review. I'll hold of on resending these until there are
some R-b's... I've send them a few times already with no comments. :(

BR,
Jani.

>
> Best regards
> Thomas
>
>>
>> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 41 ++++++++++++++------------------------
>>   1 file changed, 15 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 463fbad85d90..513590931cc5 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1820,30 +1820,20 @@ static bool edid_block_is_zero(const void *edid)
>>      return !memchr_inv(edid, 0, EDID_LENGTH);
>>   }
>>   
>> -/**
>> - * drm_edid_are_equal - compare two edid blobs.
>> - * @edid1: pointer to first blob
>> - * @edid2: pointer to second blob
>> - * This helper can be used during probing to determine if
>> - * edid had changed.
>> - */
>> -static bool drm_edid_are_equal(const struct edid *edid1, const struct edid 
>> *edid2)
>> +static bool drm_edid_eq(const struct drm_edid *drm_edid,
>> +                    const void *raw_edid, size_t raw_edid_size)
>>   {
>> -    int edid1_len, edid2_len;
>> -    bool edid1_present = edid1 != NULL;
>> -    bool edid2_present = edid2 != NULL;
>> +    bool edid1_present = drm_edid && drm_edid->edid && drm_edid->size;
>> +    bool edid2_present = raw_edid && raw_edid_size;
>>   
>>      if (edid1_present != edid2_present)
>>              return false;
>>   
>> -    if (edid1) {
>> -            edid1_len = edid_size(edid1);
>> -            edid2_len = edid_size(edid2);
>> -
>> -            if (edid1_len != edid2_len)
>> +    if (edid1_present) {
>> +            if (drm_edid->size != raw_edid_size)
>>                      return false;
>>   
>> -            if (memcmp(edid1, edid2, edid1_len))
>> +            if (memcmp(drm_edid->edid, raw_edid, drm_edid->size))
>>                      return false;
>>      }
>>   
>> @@ -6936,15 +6926,14 @@ static int 
>> _drm_edid_connector_property_update(struct drm_connector *connector,
>>      int ret;
>>   
>>      if (connector->edid_blob_ptr) {
>> -            const struct edid *old_edid = connector->edid_blob_ptr->data;
>> -
>> -            if (old_edid) {
>> -                    if (!drm_edid_are_equal(drm_edid ? drm_edid->edid : 
>> NULL, old_edid)) {
>> -                            connector->epoch_counter++;
>> -                            drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID 
>> changed, epoch counter %llu\n",
>> -                                        connector->base.id, connector->name,
>> -                                        connector->epoch_counter);
>> -                    }
>> +            const void *old_edid = connector->edid_blob_ptr->data;
>> +            size_t old_edid_size = connector->edid_blob_ptr->length;
>> +
>> +            if (old_edid && !drm_edid_eq(drm_edid, old_edid, 
>> old_edid_size)) {
>> +                    connector->epoch_counter++;
>> +                    drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch 
>> counter %llu\n",
>> +                                connector->base.id, connector->name,
>> +                                connector->epoch_counter);
>>              }
>>      }
>>   

-- 
Jani Nikula, Intel

Reply via email to