[PATCH 04/19] common: edid: extract code for detailed timing search

[Jernej Škrabec]

Dne četrtek, 04. marec 2021 ob 02:41:45 CET je Andre Przywara napisal(a):
> On Tue, 23 Feb 2021 21:46:16 +0100
> 
> Jernej Skrabec <jernej.skrabec at siol.net> wrote:
> > Code which searches for valid detailed timing entry will be used in more
> > places. Extract it.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec at siol.net>
> > ---
> > 
> >  common/edid.c | 49 ++++++++++++++++++++++++++++---------------------
> >  1 file changed, 28 insertions(+), 21 deletions(-)
> > 
> > diff --git a/common/edid.c b/common/edid.c
> > index 1cb7177742e8..a6c875d9c8e8 100644
> > --- a/common/edid.c
> > +++ b/common/edid.c
> > @@ -169,6 +169,29 @@ static bool cea_is_hdmi_vsdb_present(struct
> > edid_cea861_info *info)> 
> >  	return false;
> >  
> >  }
> > 
> > +static bool edid_find_valid_timing(void *buf, int count,
> > +				   struct display_timing *timing,
> > +				   bool (*mode_valid)(void *priv,
> > +					const struct 
display_timing *timing),
> > +				   void *mode_valid_priv)
> > +{
> > +	struct edid_detailed_timing *t = buf;
> > +	bool found = false;
> > +	int i;
> > +
> > +	for (i = 0; i < count && !found; i++, t++)
> > +		if (EDID_DETAILED_TIMING_PIXEL_CLOCK(*t) != 0) {
> 
> I am slightly puzzled, edid_detailed_timing is a different structure
> from edid_monitor_descriptor, as used below. Effectively the code
> checks in both cases for the first halfword to be not 0, but if this
> wasn't the correct structure before, and this is fixing it on the
> way, can you mention it in the commit message?

The thing is that descriptors can be either detailed timing descriptors or 
display descriptor. So technically original code is ok - you can first cast 
pointer to display descriptor and then check if that's true or not.

I'll mention this change in commit message anyway.

Best regards,
Jernej

> 
> Other than that the code looks to be correctly refactored.
> 
> Cheers,
> Andre
> 
> > +			decode_timing((u8 *)t, timing);
> > +			if (mode_valid)
> > +				found = 
mode_valid(mode_valid_priv,
> > +						   timing);
> > +			else
> > +				found = true;
> > +		}
> > +
> > +	return found;
> > +}
> > +
> > 
> >  int edid_get_timing_validate(u8 *buf, int buf_size,
> >  
> >  			     struct display_timing *timing,
> >  			     int *panel_bits_per_colourp,
> > 
> > @@ -177,8 +200,7 @@ int edid_get_timing_validate(u8 *buf, int buf_size,
> > 
> >  			     void *mode_valid_priv)
> >  
> >  {
> >  
> >  	struct edid1_info *edid = (struct edid1_info *)buf;
> > 
> > -	bool timing_done;
> > -	int i;
> > +	bool found;
> > 
> >  	if (buf_size < sizeof(*edid) || edid_check_info(edid)) {
> >  	
> >  		debug("%s: Invalid buffer\n", __func__);
> > 
> > @@ -195,25 +217,10 @@ int edid_get_timing_validate(u8 *buf, int buf_size,
> > 
> >  		return -ENOENT;
> >  	
> >  	}
> > 
> > -	/* Look for detailed timing */
> > -	timing_done = false;
> > -	for (i = 0; i < 4; i++) {
> > -		struct edid_monitor_descriptor *desc;
> > -
> > -		desc = &edid->monitor_details.descriptor[i];
> > -		if (desc->zero_flag_1 != 0) {
> > -			decode_timing((u8 *)desc, timing);
> > -			if (mode_valid)
> > -				timing_done = 
mode_valid(mode_valid_priv,
> > -							 
timing);
> > -			else
> > -				timing_done = true;
> > -
> > -			if (timing_done)
> > -				break;
> > -		}
> > -	}
> > -	if (!timing_done)
> > +	/* Look for detailed timing in base EDID */
> > +	found = edid_find_valid_timing(edid->monitor_details.descriptor, 4,
> > +				       timing, mode_valid, 
mode_valid_priv);
> > +	if (!found)
> > 
> >  		return -EINVAL;
> >  	
> >  	if (edid->version != 1 || edid->revision < 4) {