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

[Andre Przywara <andre.przywara@arm.com>]

On Tue, 23 Feb 2021 21:46:16 +0100
Jernej Skrabec <jernej.skrabec@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@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?

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) {