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

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