[PATCH i-g-t v3 0/5] lib/igt_kms: Helpers for monitor edid managment

[PATCH i-g-t v3 0/5] lib/igt_kms: Helpers for monitor edid managment

[Louis Chauvet]

In preparation for the introduction of Chameleon v3, this series will add
a few helper functions that will be used:

- Helper to convert monitor edid to edid
- Helper to fetch monitor edid from its name
- Helper to display all monitor edids
- Few fixes and addition in edid list

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
Changes in v3:
- PATCH 3/5: Add function documentation
- PATCH 1/5: Replace igt_assert_f by igt_debug
- PATCH 1/5: Add const to the returned monitor_edid list
- PATCH 1/5: Add documentation
- Link to v2: https://lore.kernel.org/r/20241022-b4-cv3-02-monitor-edids-v2-0-7634786c21e6@bootlin.com

Changes in v2:
- Rebased
- Link to v1: https://lore.kernel.org/r/20240828-b4-cv3-02-monitor-edids-v1-0-29c846bcf251@bootlin.com

---
Louis Chauvet (5):
      lib/monitor_edids: Add helper functions for using monitor_edid objects
      lib/monitor_edids: Add helper to get an EDID by its name
      lib/monitor_edids: Add helper to print all available EDID names
      lib/monitor_edids: Fix missing names in some monitor EDID
      lib/monitor_edids: Add new EDID for HDMI 4k

 lib/monitor_edids/dp_edids.h             |   4 ++
 lib/monitor_edids/hdmi_edids.h           |  28 ++++++--
 lib/monitor_edids/monitor_edids_helper.c | 120 +++++++++++++++++++++++++++++++
 lib/monitor_edids/monitor_edids_helper.h |  10 ++-
 4 files changed, 157 insertions(+), 5 deletions(-)
---
base-commit: fb343db7fc59c760ef0a0c19303e7bcec177dbd9
change-id: 20240828-b4-cv3-02-monitor-edids-59a8f515c881

Best regards,
-- 
Louis Chauvet <louis.chauvet@bootlin.com>

[PATCH i-g-t v3 1/5] lib/monitor_edids: Add helper functions for using monitor_edid objects

[Louis Chauvet]

Introduce the functions edid_from_monitor_edid() and
get_edids_for_connector_type(). The former converts a monitor_edid object
to a struct edid, which can then be utilized by igt_kms helpers. The
latter returns a list of monitor_edid objects for a specific connector
with certain characteristics

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 lib/monitor_edids/dp_edids.h             |  4 ++
 lib/monitor_edids/hdmi_edids.h           |  4 ++
 lib/monitor_edids/monitor_edids_helper.c | 77 ++++++++++++++++++++++++++++++++
 lib/monitor_edids/monitor_edids_helper.h |  7 ++-
 4 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/lib/monitor_edids/dp_edids.h b/lib/monitor_edids/dp_edids.h
index 144907558be1faa306b646ce3b47b24c13050968..8192520c89e61d3d38c508ca97a61040cea4ba90 100644
--- a/lib/monitor_edids/dp_edids.h
+++ b/lib/monitor_edids/dp_edids.h
@@ -12,6 +12,7 @@
 #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
 #define TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
 
+#include "drmtest.h"
 #include "monitor_edids_helper.h"
 
 monitor_edid DP_EDIDS_4K[] = {
@@ -194,4 +195,7 @@ monitor_edid DP_EDIDS_NON_4K[] = {
 
  };
 
+#define DP_EDIDS_4K_COUNT = ARRAY_SIZE(DP_EDIDS_4K);
+#define DP_EDIDS_NON_4K_COUNT = ARRAY_SIZE(DP_EDIDS_NON_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 f6cfe82ff6e1c4419160bcdcc851a5e28eb08726..23ac04d94b6f672572e34b8059ff1b47d0730839 100644
--- a/lib/monitor_edids/hdmi_edids.h
+++ b/lib/monitor_edids/hdmi_edids.h
@@ -12,6 +12,7 @@
 #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
 #define TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
 
+#include "drmtest.h"
 #include "monitor_edids_helper.h"
 
 monitor_edid HDMI_EDIDS_4K[] = {
@@ -604,4 +605,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
 		  "1620582c2500baac4200009e0000006b" },
 };
 
+#define HDMI_EDIDS_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_4K);
+#define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
+
 #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 1cbf1c22f0bbc5fb4047ce42bde2042ec8170efb..a7a945659f751be99ecd9d55f9b7307df256d543 100644
--- a/lib/monitor_edids/monitor_edids_helper.c
+++ b/lib/monitor_edids/monitor_edids_helper.c
@@ -15,6 +15,9 @@
 #include <assert.h>
 
 #include "igt_core.h"
+#include "igt_edid.h"
+#include "dp_edids.h"
+#include "hdmi_edids.h"
 
 static uint8_t convert_hex_char_to_byte(char c)
 {
@@ -90,3 +93,77 @@ void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid)
 	free(edid);
 	edid = NULL;
 }
+
+/*
+ * edid_from_monitor_edid() - Get a struct edid from a monitor_edid
+ * @mon_edid: Monitor EDId to convert
+ *
+ * The caller must free the returned pointer
+ */
+struct edid *edid_from_monitor_edid(const monitor_edid *mon_edid)
+{
+	uint8_t *raw_edid;
+	size_t edid_size;
+	int i;
+
+	edid_size = strlen(mon_edid->edid) / 2; /* each ascii is a nibble. */
+	raw_edid = malloc(edid_size);
+	igt_assert(raw_edid);
+
+	for (i = 0; i < edid_size; i++) {
+		raw_edid[i] = convert_hex_char_to_byte(mon_edid->edid[i * 2]) << 4 |
+			      convert_hex_char_to_byte(mon_edid->edid[i * 2 + 1]);
+	}
+
+	if (edid_get_size((struct edid *)raw_edid) > edid_size) {
+		uint8_t *new_edid;
+
+		igt_debug("The edid size stored in the raw edid is shorter than the edid stored in the table.");
+		new_edid = realloc(raw_edid, edid_get_size((struct edid *)raw_edid));
+		igt_assert(new_edid);
+		raw_edid = new_edid;
+	}
+
+	return (struct edid *)raw_edid;
+}
+
+/*
+ * get_edids_for_connector_type() - Get the list of EDIDS for a
+ * specific connector type.
+ *
+ * @type: The connector type to get the EDIDs from
+ * @count: Used to store the number of EDIDs in the returned list
+ * @four_k: Use true to fetch 4k EDIDs, false to fetch non-4k EDIDs
+ */
+struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count, bool four_k)
+{
+	if (four_k) {
+		switch (type) {
+		case DRM_MODE_CONNECTOR_DisplayPort:
+			*count = DP_EDIDS_4K_COUNT;
+			return DP_EDIDS_4K;
+		case DRM_MODE_CONNECTOR_HDMIA:
+			*count = HDMI_EDIDS_4K_COUNT;
+			return HDMI_EDIDS_4K;
+		default:
+			*count = 0;
+			igt_debug("No 4k EDID for the connector %s\n",
+				  kmstest_connector_type_str(type));
+			return NULL;
+		}
+	} else {
+		switch (type) {
+		case DRM_MODE_CONNECTOR_DisplayPort:
+			*count = DP_EDIDS_NON_4K_COUNT;
+			return DP_EDIDS_NON_4K;
+		case DRM_MODE_CONNECTOR_HDMIA:
+			*count = HDMI_EDIDS_NON_4K_COUNT;
+			return HDMI_EDIDS_NON_4K;
+		default:
+			*count = 0;
+			igt_debug("No EDID for the connector %s\n",
+				  kmstest_connector_type_str(type));
+			return NULL;
+		}
+	}
+}
diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
index 05679f0897f3d0618d656cc071b565b48d31da28..e5069868683d97053d8e66666e83692f1b733db3 100644
--- a/lib/monitor_edids/monitor_edids_helper.h
+++ b/lib/monitor_edids/monitor_edids_helper.h
@@ -12,6 +12,8 @@
 #define TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_
 
 #include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
 
 #include "igt_chamelium.h"
 
@@ -30,4 +32,7 @@ get_chameleon_edid_from_monitor_edid(struct chamelium *chamelium,
 				     const monitor_edid *edid);
 void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid);
 
-#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
\ No newline at end of file
+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);
+
+#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */

-- 
2.47.0

Re: [PATCH i-g-t v3 1/5] lib/monitor_edids: Add helper functions for using monitor_edid objects

[Jani Nikula]

On Fri, 22 Nov 2024, Louis Chauvet <louis.chauvet@bootlin.com> wrote:
> Introduce the functions edid_from_monitor_edid() and
> get_edids_for_connector_type(). The former converts a monitor_edid object
> to a struct edid, which can then be utilized by igt_kms helpers. The
> latter returns a list of monitor_edid objects for a specific connector
> with certain characteristics
>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  lib/monitor_edids/dp_edids.h             |  4 ++
>  lib/monitor_edids/hdmi_edids.h           |  4 ++
>  lib/monitor_edids/monitor_edids_helper.c | 77 ++++++++++++++++++++++++++++++++
>  lib/monitor_edids/monitor_edids_helper.h |  7 ++-
>  4 files changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/lib/monitor_edids/dp_edids.h b/lib/monitor_edids/dp_edids.h
> index 144907558be1faa306b646ce3b47b24c13050968..8192520c89e61d3d38c508ca97a61040cea4ba90 100644
> --- a/lib/monitor_edids/dp_edids.h
> +++ b/lib/monitor_edids/dp_edids.h
> @@ -12,6 +12,7 @@
>  #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
>  #define TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
>  
> +#include "drmtest.h"
>  #include "monitor_edids_helper.h"
>  
>  monitor_edid DP_EDIDS_4K[] = {
> @@ -194,4 +195,7 @@ monitor_edid DP_EDIDS_NON_4K[] = {
>  
>   };
>  
> +#define DP_EDIDS_4K_COUNT = ARRAY_SIZE(DP_EDIDS_4K);
> +#define DP_EDIDS_NON_4K_COUNT = ARRAY_SIZE(DP_EDIDS_NON_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 f6cfe82ff6e1c4419160bcdcc851a5e28eb08726..23ac04d94b6f672572e34b8059ff1b47d0730839 100644
> --- a/lib/monitor_edids/hdmi_edids.h
> +++ b/lib/monitor_edids/hdmi_edids.h
> @@ -12,6 +12,7 @@
>  #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
>  #define TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
>  
> +#include "drmtest.h"
>  #include "monitor_edids_helper.h"
>  
>  monitor_edid HDMI_EDIDS_4K[] = {
> @@ -604,4 +605,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
>  		  "1620582c2500baac4200009e0000006b" },
>  };
>  
> +#define HDMI_EDIDS_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_4K);
> +#define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
> +
>  #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 1cbf1c22f0bbc5fb4047ce42bde2042ec8170efb..a7a945659f751be99ecd9d55f9b7307df256d543 100644
> --- a/lib/monitor_edids/monitor_edids_helper.c
> +++ b/lib/monitor_edids/monitor_edids_helper.c
> @@ -15,6 +15,9 @@
>  #include <assert.h>
>  
>  #include "igt_core.h"
> +#include "igt_edid.h"
> +#include "dp_edids.h"
> +#include "hdmi_edids.h"
>  
>  static uint8_t convert_hex_char_to_byte(char c)
>  {
> @@ -90,3 +93,77 @@ void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid)
>  	free(edid);
>  	edid = NULL;
>  }
> +
> +/*
> + * edid_from_monitor_edid() - Get a struct edid from a monitor_edid
> + * @mon_edid: Monitor EDId to convert
> + *
> + * The caller must free the returned pointer
> + */
> +struct edid *edid_from_monitor_edid(const monitor_edid *mon_edid)
> +{
> +	uint8_t *raw_edid;
> +	size_t edid_size;
> +	int i;
> +
> +	edid_size = strlen(mon_edid->edid) / 2; /* each ascii is a nibble. */
> +	raw_edid = malloc(edid_size);
> +	igt_assert(raw_edid);
> +
> +	for (i = 0; i < edid_size; i++) {
> +		raw_edid[i] = convert_hex_char_to_byte(mon_edid->edid[i * 2]) << 4 |
> +			      convert_hex_char_to_byte(mon_edid->edid[i * 2 + 1]);
> +	}
> +
> +	if (edid_get_size((struct edid *)raw_edid) > edid_size) {
> +		uint8_t *new_edid;
> +
> +		igt_debug("The edid size stored in the raw edid is shorter than the edid stored in the table.");
> +		new_edid = realloc(raw_edid, edid_get_size((struct edid *)raw_edid));
> +		igt_assert(new_edid);
> +		raw_edid = new_edid;
> +	}
> +
> +	return (struct edid *)raw_edid;
> +}

Only partially related to the patch at hand, IGT seems to be completely
unaware of the HF-EEODB extension which can indicate a bigger EDID size
than what edid_get_size() returns.

The above would reduce the allocation below the actual EDID size in such
cases. Luckily I don't think anyone in IGT interprets EDIDs according to
HF-EEODB, so there's no buffer overflow *yet*, but eventually that will
need to be covered, and basically every single EDID usage needs to be
amended.

BR,
Jani.



> +
> +/*
> + * get_edids_for_connector_type() - Get the list of EDIDS for a
> + * specific connector type.
> + *
> + * @type: The connector type to get the EDIDs from
> + * @count: Used to store the number of EDIDs in the returned list
> + * @four_k: Use true to fetch 4k EDIDs, false to fetch non-4k EDIDs
> + */
> +struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count, bool four_k)
> +{
> +	if (four_k) {
> +		switch (type) {
> +		case DRM_MODE_CONNECTOR_DisplayPort:
> +			*count = DP_EDIDS_4K_COUNT;
> +			return DP_EDIDS_4K;
> +		case DRM_MODE_CONNECTOR_HDMIA:
> +			*count = HDMI_EDIDS_4K_COUNT;
> +			return HDMI_EDIDS_4K;
> +		default:
> +			*count = 0;
> +			igt_debug("No 4k EDID for the connector %s\n",
> +				  kmstest_connector_type_str(type));
> +			return NULL;
> +		}
> +	} else {
> +		switch (type) {
> +		case DRM_MODE_CONNECTOR_DisplayPort:
> +			*count = DP_EDIDS_NON_4K_COUNT;
> +			return DP_EDIDS_NON_4K;
> +		case DRM_MODE_CONNECTOR_HDMIA:
> +			*count = HDMI_EDIDS_NON_4K_COUNT;
> +			return HDMI_EDIDS_NON_4K;
> +		default:
> +			*count = 0;
> +			igt_debug("No EDID for the connector %s\n",
> +				  kmstest_connector_type_str(type));
> +			return NULL;
> +		}
> +	}
> +}
> diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
> index 05679f0897f3d0618d656cc071b565b48d31da28..e5069868683d97053d8e66666e83692f1b733db3 100644
> --- a/lib/monitor_edids/monitor_edids_helper.h
> +++ b/lib/monitor_edids/monitor_edids_helper.h
> @@ -12,6 +12,8 @@
>  #define TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_
>  
>  #include <stdint.h>
> +#include <stddef.h>
> +#include <stdbool.h>
>  
>  #include "igt_chamelium.h"
>  
> @@ -30,4 +32,7 @@ get_chameleon_edid_from_monitor_edid(struct chamelium *chamelium,
>  				     const monitor_edid *edid);
>  void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid);
>  
> -#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> \ No newline at end of file
> +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);
> +
> +#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */

-- 
Jani Nikula, Intel

Re: [PATCH i-g-t v3 1/5] lib/monitor_edids: Add helper functions for using monitor_edid objects

[Kamil Konieczny]

Hi Jani,
On 2024-11-25 at 11:20:35 +0200, Jani Nikula wrote:
> On Fri, 22 Nov 2024, Louis Chauvet <louis.chauvet@bootlin.com> wrote:
> > Introduce the functions edid_from_monitor_edid() and
> > get_edids_for_connector_type(). The former converts a monitor_edid object
> > to a struct edid, which can then be utilized by igt_kms helpers. The
> > latter returns a list of monitor_edid objects for a specific connector
> > with certain characteristics
> >
> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> > ---
> >  lib/monitor_edids/dp_edids.h             |  4 ++
> >  lib/monitor_edids/hdmi_edids.h           |  4 ++
> >  lib/monitor_edids/monitor_edids_helper.c | 77 ++++++++++++++++++++++++++++++++
> >  lib/monitor_edids/monitor_edids_helper.h |  7 ++-
> >  4 files changed, 91 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/monitor_edids/dp_edids.h b/lib/monitor_edids/dp_edids.h
> > index 144907558be1faa306b646ce3b47b24c13050968..8192520c89e61d3d38c508ca97a61040cea4ba90 100644
> > --- a/lib/monitor_edids/dp_edids.h
> > +++ b/lib/monitor_edids/dp_edids.h
> > @@ -12,6 +12,7 @@
> >  #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
> >  #define TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
> >  
> > +#include "drmtest.h"
> >  #include "monitor_edids_helper.h"
> >  
> >  monitor_edid DP_EDIDS_4K[] = {
> > @@ -194,4 +195,7 @@ monitor_edid DP_EDIDS_NON_4K[] = {
> >  
> >   };
> >  
> > +#define DP_EDIDS_4K_COUNT = ARRAY_SIZE(DP_EDIDS_4K);
> > +#define DP_EDIDS_NON_4K_COUNT = ARRAY_SIZE(DP_EDIDS_NON_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 f6cfe82ff6e1c4419160bcdcc851a5e28eb08726..23ac04d94b6f672572e34b8059ff1b47d0730839 100644
> > --- a/lib/monitor_edids/hdmi_edids.h
> > +++ b/lib/monitor_edids/hdmi_edids.h
> > @@ -12,6 +12,7 @@
> >  #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
> >  #define TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
> >  
> > +#include "drmtest.h"
> >  #include "monitor_edids_helper.h"
> >  
> >  monitor_edid HDMI_EDIDS_4K[] = {
> > @@ -604,4 +605,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
> >  		  "1620582c2500baac4200009e0000006b" },
> >  };
> >  
> > +#define HDMI_EDIDS_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_4K);
> > +#define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
> > +
> >  #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 1cbf1c22f0bbc5fb4047ce42bde2042ec8170efb..a7a945659f751be99ecd9d55f9b7307df256d543 100644
> > --- a/lib/monitor_edids/monitor_edids_helper.c
> > +++ b/lib/monitor_edids/monitor_edids_helper.c
> > @@ -15,6 +15,9 @@
> >  #include <assert.h>
> >  
> >  #include "igt_core.h"
> > +#include "igt_edid.h"
> > +#include "dp_edids.h"
> > +#include "hdmi_edids.h"
> >  
> >  static uint8_t convert_hex_char_to_byte(char c)
> >  {
> > @@ -90,3 +93,77 @@ void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid)
> >  	free(edid);
> >  	edid = NULL;
> >  }
> > +
> > +/*
> > + * edid_from_monitor_edid() - Get a struct edid from a monitor_edid
> > + * @mon_edid: Monitor EDId to convert
> > + *
> > + * The caller must free the returned pointer
> > + */
> > +struct edid *edid_from_monitor_edid(const monitor_edid *mon_edid)
> > +{
> > +	uint8_t *raw_edid;
> > +	size_t edid_size;
> > +	int i;
> > +
> > +	edid_size = strlen(mon_edid->edid) / 2; /* each ascii is a nibble. */
> > +	raw_edid = malloc(edid_size);
> > +	igt_assert(raw_edid);
> > +
> > +	for (i = 0; i < edid_size; i++) {
> > +		raw_edid[i] = convert_hex_char_to_byte(mon_edid->edid[i * 2]) << 4 |
> > +			      convert_hex_char_to_byte(mon_edid->edid[i * 2 + 1]);
> > +	}
> > +
> > +	if (edid_get_size((struct edid *)raw_edid) > edid_size) {
> > +		uint8_t *new_edid;
> > +
> > +		igt_debug("The edid size stored in the raw edid is shorter than the edid stored in the table.");
> > +		new_edid = realloc(raw_edid, edid_get_size((struct edid *)raw_edid));
> > +		igt_assert(new_edid);
> > +		raw_edid = new_edid;
> > +	}
> > +
> > +	return (struct edid *)raw_edid;
> > +}
> 
> Only partially related to the patch at hand, IGT seems to be completely
> unaware of the HF-EEODB extension which can indicate a bigger EDID size
> than what edid_get_size() returns.
> 
> The above would reduce the allocation below the actual EDID size in such
> cases. Luckily I don't think anyone in IGT interprets EDIDs according to
> HF-EEODB, so there's no buffer overflow *yet*, but eventually that will
> need to be covered, and basically every single EDID usage needs to be
> amended.
> 
> BR,
> Jani.
> 
> 

Thank you for comments, imho this should be a TODO
in our library.

Regards,
Kamil

> 
> > +
> > +/*
> > + * get_edids_for_connector_type() - Get the list of EDIDS for a
> > + * specific connector type.
> > + *
> > + * @type: The connector type to get the EDIDs from
> > + * @count: Used to store the number of EDIDs in the returned list
> > + * @four_k: Use true to fetch 4k EDIDs, false to fetch non-4k EDIDs
> > + */
> > +struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count, bool four_k)
> > +{
> > +	if (four_k) {
> > +		switch (type) {
> > +		case DRM_MODE_CONNECTOR_DisplayPort:
> > +			*count = DP_EDIDS_4K_COUNT;
> > +			return DP_EDIDS_4K;
> > +		case DRM_MODE_CONNECTOR_HDMIA:
> > +			*count = HDMI_EDIDS_4K_COUNT;
> > +			return HDMI_EDIDS_4K;
> > +		default:
> > +			*count = 0;
> > +			igt_debug("No 4k EDID for the connector %s\n",
> > +				  kmstest_connector_type_str(type));
> > +			return NULL;
> > +		}
> > +	} else {
> > +		switch (type) {
> > +		case DRM_MODE_CONNECTOR_DisplayPort:
> > +			*count = DP_EDIDS_NON_4K_COUNT;
> > +			return DP_EDIDS_NON_4K;
> > +		case DRM_MODE_CONNECTOR_HDMIA:
> > +			*count = HDMI_EDIDS_NON_4K_COUNT;
> > +			return HDMI_EDIDS_NON_4K;
> > +		default:
> > +			*count = 0;
> > +			igt_debug("No EDID for the connector %s\n",
> > +				  kmstest_connector_type_str(type));
> > +			return NULL;
> > +		}
> > +	}
> > +}
> > diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
> > index 05679f0897f3d0618d656cc071b565b48d31da28..e5069868683d97053d8e66666e83692f1b733db3 100644
> > --- a/lib/monitor_edids/monitor_edids_helper.h
> > +++ b/lib/monitor_edids/monitor_edids_helper.h
> > @@ -12,6 +12,8 @@
> >  #define TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_
> >  
> >  #include <stdint.h>
> > +#include <stddef.h>
> > +#include <stdbool.h>
> >  
> >  #include "igt_chamelium.h"
> >  
> > @@ -30,4 +32,7 @@ get_chameleon_edid_from_monitor_edid(struct chamelium *chamelium,
> >  				     const monitor_edid *edid);
> >  void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid);
> >  
> > -#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> > \ No newline at end of file
> > +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);
> > +
> > +#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH i-g-t v3 1/5] lib/monitor_edids: Add helper functions for using monitor_edid objects

[Kamil Konieczny]

Hi Louis,
On 2024-11-22 at 16:51:34 +0100, Louis Chauvet wrote:
> Introduce the functions edid_from_monitor_edid() and
> get_edids_for_connector_type(). The former converts a monitor_edid object
> to a struct edid, which can then be utilized by igt_kms helpers. The
> latter returns a list of monitor_edid objects for a specific connector
> with certain characteristics
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  lib/monitor_edids/dp_edids.h             |  4 ++
>  lib/monitor_edids/hdmi_edids.h           |  4 ++
>  lib/monitor_edids/monitor_edids_helper.c | 77 ++++++++++++++++++++++++++++++++
>  lib/monitor_edids/monitor_edids_helper.h |  7 ++-
>  4 files changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/monitor_edids/dp_edids.h b/lib/monitor_edids/dp_edids.h
> index 144907558be1faa306b646ce3b47b24c13050968..8192520c89e61d3d38c508ca97a61040cea4ba90 100644
> --- a/lib/monitor_edids/dp_edids.h
> +++ b/lib/monitor_edids/dp_edids.h
> @@ -12,6 +12,7 @@
>  #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
>  #define TESTS_CHAMELIUM_MONITOR_EDIDS_DP_EDIDS_H_
>  
> +#include "drmtest.h"

Why do you need this include here? Imho proper place
is in .c helper

>  #include "monitor_edids_helper.h"
>  
>  monitor_edid DP_EDIDS_4K[] = {
> @@ -194,4 +195,7 @@ monitor_edid DP_EDIDS_NON_4K[] = {
>  
>   };
>  
> +#define DP_EDIDS_4K_COUNT = ARRAY_SIZE(DP_EDIDS_4K);
> +#define DP_EDIDS_NON_4K_COUNT = ARRAY_SIZE(DP_EDIDS_NON_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 f6cfe82ff6e1c4419160bcdcc851a5e28eb08726..23ac04d94b6f672572e34b8059ff1b47d0730839 100644
> --- a/lib/monitor_edids/hdmi_edids.h
> +++ b/lib/monitor_edids/hdmi_edids.h
> @@ -12,6 +12,7 @@
>  #ifndef TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
>  #define TESTS_CHAMELIUM_MONITOR_EDIDS_HDMI_EDIDS_H_
>  
> +#include "drmtest.h"

Same here.

>  #include "monitor_edids_helper.h"
>  
>  monitor_edid HDMI_EDIDS_4K[] = {
> @@ -604,4 +605,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
>  		  "1620582c2500baac4200009e0000006b" },
>  };
>  
> +#define HDMI_EDIDS_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_4K);
> +#define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
> +
>  #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 1cbf1c22f0bbc5fb4047ce42bde2042ec8170efb..a7a945659f751be99ecd9d55f9b7307df256d543 100644
> --- a/lib/monitor_edids/monitor_edids_helper.c
> +++ b/lib/monitor_edids/monitor_edids_helper.c
> @@ -15,6 +15,9 @@
>  #include <assert.h>
>  
>  #include "igt_core.h"
> +#include "igt_edid.h"
> +#include "dp_edids.h"
> +#include "hdmi_edids.h"
>  
>  static uint8_t convert_hex_char_to_byte(char c)
>  {
> @@ -90,3 +93,77 @@ void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid)
>  	free(edid);
>  	edid = NULL;
>  }
> +
> +/*

Start with '/**', also function names in description ends with ':'
without any '()' - please look into lib/drmtest.c for example

/**
 * edid_from_monitor_edid:
 * @mon_edid: Monitor EDID to convert
 *
 * Get a pointer to struct edid from a monitor_edid
 *
 * The caller must free the returned pointer
 */

Sorry I missed that in my few previous mails, please
reformat your descriptions like above.

> + * edid_from_monitor_edid() - Get a struct edid from a monitor_edid
> + * @mon_edid: Monitor EDId to convert
> + *
> + * The caller must free the returned pointer

Describe what you return.

> + */
> +struct edid *edid_from_monitor_edid(const monitor_edid *mon_edid)
> +{
> +	uint8_t *raw_edid;
> +	size_t edid_size;
> +	int i;
> +
> +	edid_size = strlen(mon_edid->edid) / 2; /* each ascii is a nibble. */
> +	raw_edid = malloc(edid_size);
> +	igt_assert(raw_edid);
> +
> +	for (i = 0; i < edid_size; i++) {
> +		raw_edid[i] = convert_hex_char_to_byte(mon_edid->edid[i * 2]) << 4 |
> +			      convert_hex_char_to_byte(mon_edid->edid[i * 2 + 1]);
> +	}
> +
> +	if (edid_get_size((struct edid *)raw_edid) > edid_size) {
> +		uint8_t *new_edid;
> +
> +		igt_debug("The edid size stored in the raw edid is shorter than the edid stored in the table.");

Isn't it opposite here? raw edid size is bigger then edid_size,
not shorter? Why not:

	if (edid_get_size((struct edid *)raw_edid) != edid_size) {

> +		new_edid = realloc(raw_edid, edid_get_size((struct edid *)raw_edid));

Isn't it a bug here or in that 'for' loop above?
Could you first decode data up to point where you could
call edid_get_size() and then decide if you have correct data
in mon_edid?

> +		igt_assert(new_edid);
> +		raw_edid = new_edid;
> +	}
> +
> +	return (struct edid *)raw_edid;
> +}
> +
> +/*

Same here.

> + * get_edids_for_connector_type() - Get the list of EDIDS for a

Same here.

Regards,
Kamil

> + * specific connector type.
> + *
> + * @type: The connector type to get the EDIDs from
> + * @count: Used to store the number of EDIDs in the returned list
> + * @four_k: Use true to fetch 4k EDIDs, false to fetch non-4k EDIDs
> + */
> +struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count, bool four_k)
> +{
> +	if (four_k) {
> +		switch (type) {
> +		case DRM_MODE_CONNECTOR_DisplayPort:
> +			*count = DP_EDIDS_4K_COUNT;
> +			return DP_EDIDS_4K;
> +		case DRM_MODE_CONNECTOR_HDMIA:
> +			*count = HDMI_EDIDS_4K_COUNT;
> +			return HDMI_EDIDS_4K;
> +		default:
> +			*count = 0;
> +			igt_debug("No 4k EDID for the connector %s\n",
> +				  kmstest_connector_type_str(type));
> +			return NULL;
> +		}
> +	} else {
> +		switch (type) {
> +		case DRM_MODE_CONNECTOR_DisplayPort:
> +			*count = DP_EDIDS_NON_4K_COUNT;
> +			return DP_EDIDS_NON_4K;
> +		case DRM_MODE_CONNECTOR_HDMIA:
> +			*count = HDMI_EDIDS_NON_4K_COUNT;
> +			return HDMI_EDIDS_NON_4K;
> +		default:
> +			*count = 0;
> +			igt_debug("No EDID for the connector %s\n",
> +				  kmstest_connector_type_str(type));
> +			return NULL;
> +		}
> +	}
> +}
> diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
> index 05679f0897f3d0618d656cc071b565b48d31da28..e5069868683d97053d8e66666e83692f1b733db3 100644
> --- a/lib/monitor_edids/monitor_edids_helper.h
> +++ b/lib/monitor_edids/monitor_edids_helper.h
> @@ -12,6 +12,8 @@
>  #define TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_
>  
>  #include <stdint.h>
> +#include <stddef.h>
> +#include <stdbool.h>
>  
>  #include "igt_chamelium.h"
>  
> @@ -30,4 +32,7 @@ get_chameleon_edid_from_monitor_edid(struct chamelium *chamelium,
>  				     const monitor_edid *edid);
>  void free_chamelium_edid_from_monitor_edid(struct chamelium_edid *edid);
>  
> -#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> \ No newline at end of file
> +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);
> +
> +#endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> 
> -- 
> 2.47.0
> 

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

[Louis Chauvet]

For testing specific EDID, it is useful to be able to retrieve an EDID by
a verbose name.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 lib/monitor_edids/monitor_edids_helper.c | 28 ++++++++++++++++++++++++++++
 lib/monitor_edids/monitor_edids_helper.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/lib/monitor_edids/monitor_edids_helper.c b/lib/monitor_edids/monitor_edids_helper.c
index a7a945659f751be99ecd9d55f9b7307df256d543..4d739937ede1e8f09eb6e8bf225f047d5c3dfd04 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_EDIDS_NON_4K_COUNT},
+	{DP_EDIDS_4K,		DP_EDIDS_4K_COUNT},
+	{HDMI_EDIDS_NON_4K,	HDMI_EDIDS_NON_4K_COUNT},
+	{HDMI_EDIDS_4K,		HDMI_EDIDS_4K_COUNT},
+};
+
 static uint8_t convert_hex_char_to_byte(char c)
 {
 	if (c >= '0' && c <= '9')
@@ -167,3 +177,21 @@ struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count,
 		}
 	}
 }
+
+/*
+ * get_edid_by_name() - Return the struct associated with a specific
+ *                      name
+ * @name: Name to search in available EDIDs
+ * Returns NULL if no edid are found
+ */
+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]);
+		}
+	}
+
+	return NULL;
+}
diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
index e5069868683d97053d8e66666e83692f1b733db3..a45dca870eb20f66c9472be6643019d23bc18b9a 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_ */

-- 
2.47.0

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

[Kamil Konieczny]

Hi Louis,
On 2024-11-22 at 16:51:35 +0100, Louis Chauvet wrote:
> For testing specific EDID, it is useful to be able to retrieve an EDID by
> a verbose name.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  lib/monitor_edids/monitor_edids_helper.c | 28 ++++++++++++++++++++++++++++
>  lib/monitor_edids/monitor_edids_helper.h |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/lib/monitor_edids/monitor_edids_helper.c b/lib/monitor_edids/monitor_edids_helper.c
> index a7a945659f751be99ecd9d55f9b7307df256d543..4d739937ede1e8f09eb6e8bf225f047d5c3dfd04 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 {

Consider adding here list name.

> +	struct monitor_edid *edid_list;
> +	int list_size;
> +} ALL_EDIDS[] = {
> +	{DP_EDIDS_NON_4K,	DP_EDIDS_NON_4K_COUNT},
> +	{DP_EDIDS_4K,		DP_EDIDS_4K_COUNT},
> +	{HDMI_EDIDS_NON_4K,	HDMI_EDIDS_NON_4K_COUNT},
> +	{HDMI_EDIDS_4K,		HDMI_EDIDS_4K_COUNT},
> +};
> +
>  static uint8_t convert_hex_char_to_byte(char c)
>  {
>  	if (c >= '0' && c <= '9')
> @@ -167,3 +177,21 @@ struct monitor_edid *get_edids_for_connector_type(uint32_t type, size_t *count,
>  		}
>  	}
>  }
> +
> +/*

Start with '/**', for example look into lib/drmtest.c

> + * get_edid_by_name() - Return the struct associated with a specific
> + *                      name
> + * @name: Name to search in available EDIDs

Add empty line ' *' here.

With above
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> + * Returns NULL if no edid are found
> + */
> +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]);
> +		}
> +	}
> +
> +	return NULL;
> +}
> diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
> index e5069868683d97053d8e66666e83692f1b733db3..a45dca870eb20f66c9472be6643019d23bc18b9a 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_ */
> 
> -- 
> 2.47.0
> 

[PATCH i-g-t v3 3/5] lib/monitor_edids: Add helper to print all available EDID names

[Louis Chauvet]

During the chamelium v3 configuration, it may be required to know the list
of supported EDID names, so add an helper to print them.

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 lib/monitor_edids/monitor_edids_helper.c | 15 +++++++++++++++
 lib/monitor_edids/monitor_edids_helper.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/lib/monitor_edids/monitor_edids_helper.c b/lib/monitor_edids/monitor_edids_helper.c
index 4d739937ede1e8f09eb6e8bf225f047d5c3dfd04..9de8aa06a45c164e928ae3821f841414b437eaac 100644
--- a/lib/monitor_edids/monitor_edids_helper.c
+++ b/lib/monitor_edids/monitor_edids_helper.c
@@ -195,3 +195,18 @@ struct edid *get_edid_by_name(char *name)
 
 	return NULL;
 }
+
+
+/*
+ * list_edid_names() - Print all the EDID available in igt
+ * @level: Log level to write the names on
+ */
+void list_edid_names(enum igt_log_level level)
+{
+	for (int i = 0; i < ARRAY_SIZE(ALL_EDIDS); i++) {
+		for (int j = 0; j < ALL_EDIDS[i].list_size; j++) {
+			igt_log(IGT_LOG_DOMAIN, level, " - \"%s\"\n",
+				ALL_EDIDS[i].edid_list[j].name);
+		}
+	}
+}
diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
index a45dca870eb20f66c9472be6643019d23bc18b9a..04a639f47b49ab176e914f6044bba6769dc84ccb 100644
--- a/lib/monitor_edids/monitor_edids_helper.h
+++ b/lib/monitor_edids/monitor_edids_helper.h
@@ -15,6 +15,7 @@
 #include <stddef.h>
 #include <stdbool.h>
 
+#include "igt_core.h"
 #include "igt_chamelium.h"
 
 /* Max Length can be increased as needed, when new EDIDs are added. */
@@ -35,5 +36,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);
+void list_edid_names(enum igt_log_level level);
 
 #endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */

-- 
2.47.0

Re: [PATCH i-g-t v3 3/5] lib/monitor_edids: Add helper to print all available EDID names

[Kamil Konieczny]

Hi Louis,
On 2024-11-22 at 16:51:36 +0100, Louis Chauvet wrote:
> During the chamelium v3 configuration, it may be required to know the list
> of supported EDID names, so add an helper to print them.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  lib/monitor_edids/monitor_edids_helper.c | 15 +++++++++++++++
>  lib/monitor_edids/monitor_edids_helper.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/lib/monitor_edids/monitor_edids_helper.c b/lib/monitor_edids/monitor_edids_helper.c
> index 4d739937ede1e8f09eb6e8bf225f047d5c3dfd04..9de8aa06a45c164e928ae3821f841414b437eaac 100644
> --- a/lib/monitor_edids/monitor_edids_helper.c
> +++ b/lib/monitor_edids/monitor_edids_helper.c
> @@ -195,3 +195,18 @@ struct edid *get_edid_by_name(char *name)
>  
>  	return NULL;
>  }
> +
> +
> +/*

Start with '/**'

With this
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> + * list_edid_names() - Print all the EDID available in igt
> + * @level: Log level to write the names on
> + */
> +void list_edid_names(enum igt_log_level level)
> +{
> +	for (int i = 0; i < ARRAY_SIZE(ALL_EDIDS); i++) {
> +		for (int j = 0; j < ALL_EDIDS[i].list_size; j++) {
> +			igt_log(IGT_LOG_DOMAIN, level, " - \"%s\"\n",
> +				ALL_EDIDS[i].edid_list[j].name);
> +		}
> +	}
> +}
> diff --git a/lib/monitor_edids/monitor_edids_helper.h b/lib/monitor_edids/monitor_edids_helper.h
> index a45dca870eb20f66c9472be6643019d23bc18b9a..04a639f47b49ab176e914f6044bba6769dc84ccb 100644
> --- a/lib/monitor_edids/monitor_edids_helper.h
> +++ b/lib/monitor_edids/monitor_edids_helper.h
> @@ -15,6 +15,7 @@
>  #include <stddef.h>
>  #include <stdbool.h>
>  
> +#include "igt_core.h"
>  #include "igt_chamelium.h"
>  
>  /* Max Length can be increased as needed, when new EDIDs are added. */
> @@ -35,5 +36,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);
> +void list_edid_names(enum igt_log_level level);
>  
>  #endif /* TESTS_CHAMELIUM_MONITOR_EDIDS_MONITOR_EDIDS_HELPER_H_ */
> 
> -- 
> 2.47.0
> 

[PATCH i-g-t v3 4/5] lib/monitor_edids: Fix missing names in some monitor EDID

[Louis Chauvet]

Some HDMI EDID did not have a good name identifier. Add one to avoid
confusion.

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 lib/monitor_edids/hdmi_edids.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/monitor_edids/hdmi_edids.h b/lib/monitor_edids/hdmi_edids.h
index 23ac04d94b6f672572e34b8059ff1b47d0730839..3ff331e9f7c2a48858359ca41f4aec1f7ec8204d 100644
--- a/lib/monitor_edids/hdmi_edids.h
+++ b/lib/monitor_edids/hdmi_edids.h
@@ -242,7 +242,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
 		  "33333633493056465320000000fc0044"
 		  "454c4c20323430354650570a000000fd"
 		  "00384c1e5111000a20202020202000e9" },
-	{ .name = "`",
+	{ .name = "DEL_DELL_2407WFP_HDMI",
 	  .edid = "00ffffffffffff0010ac17a053575930"
 		  "0511010380342178eeee91a3544c9926"
 		  "0f5054a54b008180a940714fb3000101"
@@ -431,7 +431,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
 		  "60350000003200001c011d8018711c16"
 		  "20582c2500c48e2100009e0000000000"
 		  "0000000000000000000000000000000a" },
-	{ .name = "",
+	{ .name = "GSM_50294_M3704C_HDMI",
 	  .edid = "00ffffffffffff001e6d76c401010101"
 		  "2514010380522e78eaac27a355499b25"
 		  "10474aa1080081807140614045403140"
@@ -483,7 +483,7 @@ monitor_edid HDMI_EDIDS_NON_4K[] = {
 		  "5011000a202020202020000000fc0048"
 		  "50205a5232343430770a2020000000ff"
 		  "00434e34333132303836580a2020007f" },
-	{ .name = "",
+	{ .name = "HWP_10582_HP_ZR2440w_HDMI",
 	  .edid = "00ffffffffffff0022f0562901010101"
 		  "0b170103803420782afc81a4554d9d25"
 		  "125054210800d1c081c0814081809500"

-- 
2.47.0

[PATCH i-g-t v3 5/5] lib/monitor_edids: Add new EDID for HDMI 4k

[Louis Chauvet]

Add EDID for the HDMI screen LG HDR 4K.

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 lib/monitor_edids/hdmi_edids.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/monitor_edids/hdmi_edids.h b/lib/monitor_edids/hdmi_edids.h
index 3ff331e9f7c2a48858359ca41f4aec1f7ec8204d..3f5c38982886297cc768cb99ce26d6e0f883520a 100644
--- a/lib/monitor_edids/hdmi_edids.h
+++ b/lib/monitor_edids/hdmi_edids.h
@@ -50,7 +50,23 @@ monitor_edid HDMI_EDIDS_4K[] = {
 		  "5E00A0A0A0295030203500C48F210000"
 		  "1EEF5100A0F070198030203500C48F21"
 		  "00001E000000000000000000000000A8" },
-
+	{ .name = "4K_GSM_LG_HDR_4K_HDMI",
+	  .edid = "00ffffffffffff001e6d4f77c75f1500"
+		  "0521010380462878ea7ba1ae4f44a926"
+		  "0c5054210800d1c06140454001010101"
+		  "010101010101a0cb0046f0703e802010"
+		  "3500b9882100001a000000fd00283c1e"
+		  "873c000a202020202020000000fc004c"
+		  "472048445220344b0a202020000000ff"
+		  "003330354d414e4a43335a37350a01e4"
+		  "0203427223090707830100004d010304"
+		  "10121f202261605f5e5d6d030c001000"
+		  "b83c20006001020367d85dc401788003"
+		  "e30f0003e2006ae305c000e606050159"
+		  "5952a36600a0f0701f8030203500b988"
+		  "2100001a565e00a0a0a0295030203500"
+		  "b9882100001a023a801871382d40582c"
+		  "4500b9882100001a00000000000000af" },
 };
 
 monitor_edid HDMI_EDIDS_NON_4K[] = {

-- 
2.47.0

✗ GitLab.Pipeline: warning for lib/igt_kms: Helpers for monitor edid managment (rev3)

[Patchwork]

== Series Details ==

Series: lib/igt_kms: Helpers for monitor edid managment (rev3)
URL   : https://patchwork.freedesktop.org/series/137925/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1315538 for the overview.

build:tests-debian-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/67059891):
      *count = DP_EDIDS_NON_4K_COUNT;
             ^
  In file included from ../lib/monitor_edids/monitor_edids_helper.c:20:
  ../lib/monitor_edids/hdmi_edids.h:625:33: error: expected expression before ‘=’ token
   #define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
                                   ^
  ../lib/monitor_edids/monitor_edids_helper.c:170:13: note: in expansion of macro ‘HDMI_EDIDS_NON_4K_COUNT’
      *count = HDMI_EDIDS_NON_4K_COUNT;
               ^~~~~~~~~~~~~~~~~~~~~~~
  ../lib/monitor_edids/monitor_edids_helper.c:170:11: error: assignment to ‘size_t’ {aka ‘long unsigned int’} from ‘struct <anonymous> *’ makes integer from pointer without a cast [-Werror=int-conversion]
      *count = HDMI_EDIDS_NON_4K_COUNT;
             ^
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1732300654:step_script
  section_start:1732300654:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1732300664:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/67059886):
    167 |    *count = DP_EDIDS_NON_4K_COUNT;
        |           ^
  In file included from ../lib/monitor_edids/monitor_edids_helper.c:20:
  ../lib/monitor_edids/hdmi_edids.h:625:33: error: expected expression before ‘=’ token
    625 | #define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
        |                                 ^
  ../lib/monitor_edids/monitor_edids_helper.c:170:13: note: in expansion of macro ‘HDMI_EDIDS_NON_4K_COUNT’
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |             ^~~~~~~~~~~~~~~~~~~~~~~
  ../lib/monitor_edids/monitor_edids_helper.c:170:11: error: assignment to ‘size_t’ {aka ‘long unsigned int’} from ‘struct <anonymous> *’ makes integer from pointer without a cast [-Werror=int-conversion]
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |           ^
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1732300664:step_script
  section_start:1732300664:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1732300665:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-clang has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/67059890):
  ../lib/monitor_edids/monitor_edids_helper.c:189:22: error: invalid application of 'sizeof' to an incomplete type 'struct (anonymous struct at ../lib/monitor_edids/monitor_edids_helper.c:22:1) []'
          for (int i = 0; i < ARRAY_SIZE(ALL_EDIDS); i++) {
                              ^~~~~~~~~~~~~~~~~~~~~
  ../lib/drmtest.h:85:32: note: expanded from macro 'ARRAY_SIZE'
  #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
                                 ^~~~~
  ../lib/monitor_edids/monitor_edids_helper.c:206:22: error: invalid application of 'sizeof' to an incomplete type 'struct (anonymous struct at ../lib/monitor_edids/monitor_edids_helper.c:22:1) []'
          for (int i = 0; i < ARRAY_SIZE(ALL_EDIDS); i++) {
                              ^~~~~~~~~~~~~~~~~~~~~
  ../lib/drmtest.h:85:32: note: expanded from macro 'ARRAY_SIZE'
  #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
                                 ^~~~~
  10 errors generated.
  ninja: build stopped: subcommand failed.
  section_end:1732300660:step_script
  section_start:1732300660:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1732300661:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libdrm-nouveau has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/67059889):
    167 |    *count = DP_EDIDS_NON_4K_COUNT;
        |           ^
  In file included from ../lib/monitor_edids/monitor_edids_helper.c:20:
  ../lib/monitor_edids/hdmi_edids.h:625:33: error: expected expression before ‘=’ token
    625 | #define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
        |                                 ^
  ../lib/monitor_edids/monitor_edids_helper.c:170:13: note: in expansion of macro ‘HDMI_EDIDS_NON_4K_COUNT’
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |             ^~~~~~~~~~~~~~~~~~~~~~~
  ../lib/monitor_edids/monitor_edids_helper.c:170:11: error: assignment to ‘size_t’ {aka ‘long unsigned int’} from ‘struct <anonymous> *’ makes integer from pointer without a cast [-Werror=int-conversion]
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |           ^
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1732300652:step_script
  section_start:1732300652:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1732300653:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libunwind has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/67059887):
    167 |    *count = DP_EDIDS_NON_4K_COUNT;
        |           ^
  In file included from ../lib/monitor_edids/monitor_edids_helper.c:20:
  ../lib/monitor_edids/hdmi_edids.h:625:33: error: expected expression before ‘=’ token
    625 | #define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
        |                                 ^
  ../lib/monitor_edids/monitor_edids_helper.c:170:13: note: in expansion of macro ‘HDMI_EDIDS_NON_4K_COUNT’
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |             ^~~~~~~~~~~~~~~~~~~~~~~
  ../lib/monitor_edids/monitor_edids_helper.c:170:11: error: assignment to ‘size_t’ {aka ‘long unsigned int’} from ‘struct <anonymous> *’ makes integer from pointer without a cast [-Werror=int-conversion]
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |           ^
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1732300658:step_script
  section_start:1732300658:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1732300658:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-oldest-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/67059888):
    167 |    *count = DP_EDIDS_NON_4K_COUNT;
        |           ^
  In file included from ../lib/monitor_edids/monitor_edids_helper.c:20:
  ../lib/monitor_edids/hdmi_edids.h:625:33: error: expected expression before ‘=’ token
    625 | #define HDMI_EDIDS_NON_4K_COUNT = ARRAY_SIZE(HDMI_EDIDS_NON_4K);
        |                                 ^
  ../lib/monitor_edids/monitor_edids_helper.c:170:13: note: in expansion of macro ‘HDMI_EDIDS_NON_4K_COUNT’
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |             ^~~~~~~~~~~~~~~~~~~~~~~
  ../lib/monitor_edids/monitor_edids_helper.c:170:11: error: assignment to ‘size_t’ {aka ‘long unsigned int’} from ‘struct <anonymous> *’ makes integer from pointer without a cast [-Werror=int-conversion]
    170 |    *count = HDMI_EDIDS_NON_4K_COUNT;
        |           ^
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1732300661:step_script
  section_start:1732300661:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1732300662:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1315538