Re: [PATCH i-g-t v2 2/5] lib/monitor_edids: Add helper to get an EDID by its name

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 31/10/24 - 19:58, Kamil Konieczny wrote:
> Hi Louis,
> On 2024-10-22 at 14:53:11 +0200, Louis Chauvet wrote:
> 
> same here, in Cc you have:
> 
> 	20241022-b4-cv3-01-igt-kms-v2-0-8f654694b513@bootlin.com
> 
> > For testing specific EDID, it is useful to be able to retrieve an EDID by
> > a verbose name.
> > 
> 
> Add Mark here in Cc.
> 
> Cc: Mark Yacoub <markyacoub@chromium.org>
> 
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > ---
> >  lib/monitor_edids/dp_edids.h             |  3 +++
> >  lib/monitor_edids/hdmi_edids.h           |  3 +++
> >  lib/monitor_edids/monitor_edids_helper.c | 21 +++++++++++++++++++++
> >  lib/monitor_edids/monitor_edids_helper.h |  1 +
> >  4 files changed, 28 insertions(+)
> > 
> > diff --git a/lib/monitor_edids/dp_edids.h b/lib/monitor_edids/dp_edids.h
> > index 144907558be1..d11a81d167fc 100644
> > --- a/lib/monitor_edids/dp_edids.h
> > +++ b/lib/monitor_edids/dp_edids.h
> > @@ -194,4 +194,7 @@ monitor_edid DP_EDIDS_NON_4K[] = {
> >  
> >   };
> >  
> > +const int DP_EDID_NON_4K_COUNT = ARRAY_SIZE(DP_EDIDS_NON_4K);
> > +const int DP_EDID_4K_COUNT = ARRAY_SIZE(DP_EDIDS_4K);
> > +
> >  #endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_ */
> > diff --git a/lib/monitor_edids/hdmi_edids.h b/lib/monitor_edids/hdmi_edids.h
> > index 9d75cfa989b6..573e2149d5d3 100644
> > --- a/lib/monitor_edids/hdmi_edids.h
> > +++ b/lib/monitor_edids/hdmi_edids.h
> > @@ -604,4 +604,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
> >  		  "1620582c2500baac4200009e0000006b" },
> >  };
> >  
> > +const int HDMI_EDID_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_4K);
> > +const int HDMI_EDID_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
> > +
> 
> imho better make it a #define

Will do for v2.

What is the reason to use define over const int?

> >  #endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_ */
> > diff --git a/lib/monitor_edids/monitor_edids_helper.c b/lib/monitor_edids/monitor_edids_helper.c
> > index 0e0c2a9badcf..0f92ced64d06 100644
> > --- a/lib/monitor_edids/monitor_edids_helper.c
> > +++ b/lib/monitor_edids/monitor_edids_helper.c
> > @@ -19,6 +19,16 @@
> >  #include "dp_edids.h"
> >  #include "hdmi_edids.h"
> >  
> > +struct {
> > +	struct monitor_edid *edid_list;
> > +	int list_size;
> > +} ALL_EDIDS[] = {
> > +	{DP_EDIDS_NON_4K,	DP_EDID_NON_4K_COUNT},
> > +	{DP_EDIDS_4K,		DP_EDID_4K_COUNT},
> > +	{HDMI_EDIDS_NON_4K,	HDMI_EDID_NON_4K_COUNT},
> > +	{HDMI_EDIDS_4K,		HDMI_EDID_4K_COUNT},
> > +};
> > +
> >  static uint8_t convert_hex_char_to_byte(char c)
> >  {
> >  	if (c >= '0' && c <= '9')
> > @@ -149,3 +159,14 @@ struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count,
> >  		}
> >  	}
> >  }
> > +
> 
> Add description.
>
> > +struct edid *get_edid_by_name(char *name)
> > +{
> > +	for (int i = 0; i < ARRAY_SIZE(ALL_EDIDS); i++) {
> > +		for (int j = 0; j < ALL_EDIDS[i].list_size; j++) {
> > +			if (strcmp(ALL_EDIDS[i].edid_list[j].name, name) == 0)
> > +				return edid_from_monitor_edid(&ALL_EDIDS[i].edid_list[j]);
> > +		}
> > +	}
> 
> Add newline.
> 
> > +	return NULL;
> > +}
> > diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
> > index 2ec7aee5f13f..cd0e5a7b2645 100644
> > --- a/lib/monitor_edids/monitor_edids_helper.h
> > +++ b/lib/monitor_edids/monitor_edids_helper.h
> > @@ -34,5 +34,6 @@ void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid);
> >  
> >  struct edid *edid_from_monitor_edid(const monitor_edid *monitor_edid);
> >  struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count, bool four_k);
> > +struct edid *get_edid_by_name(char *name);
> >  
> >  #endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> > \ No newline at end of file
> 
> Add newline.
> 
> Regards,
> Kamil
> 
> > 
> > -- 
> > 2.46.2
> >