| Message ID | 20210223204631.1609597-5-jernej.skrabec@siol.net |
|---|---|
| State | Superseded |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | video: sunxi: Rework DE2 driver | expand |
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) {
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@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? 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) {
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) { + 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) {
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(-)