Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 16.04.24 um 14:27 schrieb Jani Nikula:
> On Tue, 16 Apr 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> 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. :(

Feel free to add

Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

to the series.

Best regards
Thomas

> BR,
> Jani.
>
> > Best regards
> > Thomas
> >
> > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> > > ---
> > >    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);
> > >    		}
> > >    	}
> > >    

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)