All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs
@ 2019-04-15 12:38 Simon Ser
  2019-04-15 12:51 ` Petri Latvala
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Simon Ser @ 2019-04-15 12:38 UTC (permalink / raw)
  To: igt-dev

For the purposes of testing different EDID features, we need to generate more
and more complex EDID blobs (e.g. with audio support). However currently IGT
uses a macro-based system to generate EDIDs. This doesn't scale well and is
pretty inflexible.

This commit introduces a new little library to generate EDIDs. For now it can't
do more than the old macro. Future commits will extend the API.

The structures are mostly based on the Linux kernel code (drm_edid.h). Setters
have been added for convenience.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_edid.c          | 268 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_edid.h          | 196 +++++++++++++++++++++++++++++
 lib/igt_edid_template.h |  74 -----------
 lib/igt_kms.c           | 113 +++++++++--------
 lib/meson.build         |   3 +-
 5 files changed, 521 insertions(+), 133 deletions(-)
 create mode 100644 lib/igt_edid.c
 create mode 100644 lib/igt_edid.h
 delete mode 100644 lib/igt_edid_template.h

diff --git a/lib/igt_edid.c b/lib/igt_edid.c
new file mode 100644
index 00000000..155ebebe
--- /dev/null
+++ b/lib/igt_edid.c
@@ -0,0 +1,268 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors: Simon Ser <simon.ser@intel.com>
+ */
+
+#include "config.h"
+
+#include <assert.h>
+#include <string.h>
+#include <stdint.h>
+#include <time.h>
+#include <xf86drmMode.h>
+
+#include "igt_core.h"
+#include "igt_edid.h"
+
+static const char edid_header[] = {
+	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
+};
+
+/* vfreq is in Hz */
+static void std_timing_set(struct std_timing *st, int hsize, int vfreq,
+			   enum std_timing_aspect aspect)
+{
+	assert(hsize >= 256 && hsize <= 2288);
+	st->hsize = hsize / 8 - 31;
+	st->vfreq_aspect = aspect << 6 | (vfreq - 60);
+}
+
+static void std_timing_unset(struct std_timing *st)
+{
+	memset(st, 0x01, sizeof(struct std_timing));
+}
+
+/**
+ * detailed_timing_set_mode: fill a detailed timing based on a mode
+ */
+void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
+			      int width_mm, int height_mm)
+{
+	int hactive, hblank, vactive, vblank, hsync_offset, hsync_pulse_width,
+	    vsync_offset, vsync_pulse_width;
+	struct detailed_pixel_timing *pt = &dt->data.pixel_data;
+
+	hactive = mode->hdisplay;
+	hsync_offset = mode->hsync_start - mode->hdisplay;
+	hsync_pulse_width = mode->hsync_end - mode->hsync_start;
+	hblank = mode->htotal - mode->hdisplay;
+
+	vactive = mode->vdisplay;
+	vsync_offset = mode->vsync_start - mode->vdisplay;
+	vsync_pulse_width = mode->vsync_end - mode->vsync_start;
+	vblank = mode->vtotal - mode->vdisplay;
+
+	dt->pixel_clock[0] = (mode->clock / 10) & 0x00FF;
+	dt->pixel_clock[1] = ((mode->clock / 10) & 0xFF00) >> 8;
+
+	assert(hactive <= 0xFFF);
+	assert(hblank <= 0xFFF);
+	pt->hactive_lo = hactive & 0x0FF;
+	pt->hblank_lo = hblank & 0x0FF;
+	pt->hactive_hblank_hi = (hactive & 0xF00) >> 4 | (hblank & 0xF00) >> 8;
+
+	assert(vactive <= 0xFFF);
+	assert(vblank <= 0xFFF);
+	pt->vactive_lo = vactive & 0x0FF;
+	pt->vblank_lo = vblank & 0x0FF;
+	pt->vactive_vblank_hi = (vactive & 0xF00) >> 4 | (vblank & 0xF00) >> 8;
+
+	assert(hsync_offset <= 0x3FF);
+	assert(hsync_pulse_width <= 0x3FF);
+	assert(vsync_offset <= 0x3F);
+	assert(vsync_pulse_width <= 0x3F);
+	pt->hsync_offset_lo = hsync_offset & 0x0FF;
+	pt->hsync_pulse_width_lo = hsync_pulse_width & 0x0FF;
+	pt->vsync_offset_pulse_width_lo = (vsync_offset & 0xF) << 4
+					  | (vsync_pulse_width & 0xF);
+	pt->hsync_vsync_offset_pulse_width_hi = 
+		((hsync_offset & 0x300) >> 2) | ((hsync_pulse_width & 0x300) >> 4)
+		| ((vsync_offset & 0x30) >> 2) | ((vsync_pulse_width & 0x30) >> 4);
+
+	assert(width_mm <= 0xFFF);
+	assert(height_mm <= 0xFFF);
+	pt->width_mm_lo = width_mm & 0x0FF;
+	pt->height_mm_lo = height_mm & 0x0FF;
+	pt->width_height_mm_hi = (width_mm & 0xF00) >> 4
+				 | (height_mm & 0xF00) >> 8;
+	
+	pt->misc = 0;
+	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+		pt->misc |= EDID_PT_HSYNC_POSITIVE;
+	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+		pt->misc |= EDID_PT_VSYNC_POSITIVE;
+}
+
+/**
+ * detailed_timing_set_monitor_range_mode: set a detailed timing to be a
+ * monitor range based on a mode
+ */
+void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
+					    drmModeModeInfo *mode)
+{
+	struct detailed_non_pixel *np = &dt->data.other_data;
+	struct detailed_data_monitor_range *mr = &np->data.range;
+
+	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
+
+	np->type = EDID_DETAIL_MONITOR_RANGE;
+
+	mr->min_vfreq = mode->vrefresh - 1;
+	mr->max_vfreq = mode->vrefresh + 1;
+	mr->min_hfreq_khz = (mode->clock / mode->htotal) - 1;
+	mr->max_hfreq_khz = (mode->clock / mode->htotal) + 1;
+	mr->pixel_clock_mhz = (mode->clock / 10000) + 1;
+	mr->flags = 0;
+
+	static const char padding[] = {
+		0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
+	};
+	memcpy(mr->formula.pad, padding, sizeof(padding));
+}
+
+/**
+ * detailed_timing_set_string: set a detailed timing to be a string
+ */
+void detailed_timing_set_string(struct detailed_timing *dt,
+				enum detailed_non_pixel_type type,
+				const char *str)
+{
+	struct detailed_non_pixel *np = &dt->data.other_data;
+	struct detailed_data_string *ds = &np->data.string;
+	size_t len;
+
+	switch (type) {
+	case EDID_DETAIL_MONITOR_NAME:
+	case EDID_DETAIL_MONITOR_STRING:
+	case EDID_DETAIL_MONITOR_SERIAL:
+		break;
+	default:
+		assert(0); /* not a string type */
+	}
+
+	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
+
+	np->type = type;
+
+	strncpy(ds->str, str, sizeof(ds->str));
+	len = strlen(str);
+	if (len < sizeof(ds->str))
+		ds->str[len] = '\n';
+}
+
+static void edid_set_mfg(struct edid *edid, char mfg[static 3])
+{
+	edid->mfg_id[0] = (mfg[0] - '@') << 2 | (mfg[1] - '@') >> 3;
+	edid->mfg_id[1] = (mfg[1] - '@') << 5 | (mfg[2] - '@');
+}
+
+static void edid_set_gamma(struct edid *edid, float gamma)
+{
+	edid->gamma = (gamma * 100) - 100;
+}
+
+/**
+ * edid_init: initialize an EDID
+ *
+ * The EDID will be pre-filled with established and standard timings:
+ *
+ *  - 1920x1080 60Hz
+ *  - 1280x720 60Hz
+ *  - 1024x768 60Hz
+ *  - 800x600 60Hz
+ *  - 640x480 60Hz
+ */
+void edid_init(struct edid *edid)
+{
+	size_t i;
+	time_t t;
+	struct tm *tm;
+
+	memset(edid, 0, sizeof(struct edid));
+
+	memcpy(edid->header, edid_header, sizeof(edid_header));
+	edid_set_mfg(edid, "IGT");
+	edid->version = 1;
+	edid->revision = 3;
+	edid->input = 0x80;
+	edid->width_cm = 52;
+	edid->height_cm = 30;
+	edid_set_gamma(edid, 2.20);
+	edid->features = 0x02;
+
+	/* Year of manufacture */
+	t = time(NULL);
+	tm = localtime(&t);
+	edid->mfg_year = tm->tm_year - 90;
+
+	/* Established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
+	edid->established_timings.t1 = 0x21;
+	edid->established_timings.t2 = 0x08;
+
+	/* Standard timings */
+	/* 1920x1080 60Hz */
+	std_timing_set(&edid->standard_timings[0], 1920, 60, STD_TIMING_16_9);
+	/* 1280x720 60Hz */
+	std_timing_set(&edid->standard_timings[1], 1280, 60, STD_TIMING_16_9);
+	/* 1024x768 60Hz */
+	std_timing_set(&edid->standard_timings[2], 1024, 60, STD_TIMING_4_3);
+	/* 800x600 60Hz */
+	std_timing_set(&edid->standard_timings[3], 800, 60, STD_TIMING_4_3);
+	/* 640x480 60Hz */
+	std_timing_set(&edid->standard_timings[4], 640, 60, STD_TIMING_4_3);
+	for (i = 5; i < STD_TIMINGS_LEN; i++)
+		std_timing_unset(&edid->standard_timings[i]);
+}
+
+/**
+ * edid_init_with_mode: initialize an EDID and sets its preferred mode
+ */
+void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode)
+{
+	edid_init(edid);
+
+	/* Preferred timing */
+	detailed_timing_set_mode(&edid->detailed_timings[0], mode,
+				 edid->width_cm * 10, edid->height_cm * 10);
+	detailed_timing_set_monitor_range_mode(&edid->detailed_timings[1],
+					       mode);
+	detailed_timing_set_string(&edid->detailed_timings[2],
+				   EDID_DETAIL_MONITOR_NAME, "IGT");
+}
+
+/**
+ * edid_finalize: finalize an EDID by computing its checksum
+ */
+void edid_finalize(struct edid *edid)
+{
+	size_t i;
+	const uint8_t *buf = (const uint8_t *) edid;
+	uint8_t sum = 0;
+
+	/* calculate checksum */
+	for (i = 0; i < sizeof(struct edid) - 1; i++) {
+		sum = sum + buf[i];
+	}
+
+	edid->checksum = 256 - sum;
+}
diff --git a/lib/igt_edid.h b/lib/igt_edid.h
new file mode 100644
index 00000000..8c027c89
--- /dev/null
+++ b/lib/igt_edid.h
@@ -0,0 +1,196 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors: Simon Ser <simon.ser@intel.com>
+ */
+
+#ifndef IGT_CHAMELIUM_H
+#define IGT_CHAMELIUM_H
+
+#include "config.h"
+
+#include <stdint.h>
+
+struct est_timings {
+	uint8_t t1;
+	uint8_t t2;
+	uint8_t mfg_rsvd;
+} __attribute__((packed));
+
+#define STD_TIMINGS_LEN 8
+
+enum std_timing_aspect {
+	STD_TIMING_16_10 = 0b00,
+	STD_TIMING_4_3 = 0b01,
+	STD_TIMING_5_4 = 0b10,
+	STD_TIMING_16_9 = 0b11,
+};
+
+struct std_timing {
+	uint8_t hsize;
+	uint8_t vfreq_aspect;
+} __attribute__((packed));
+
+#define DETAILED_TIMINGS_LEN 4
+
+#define EDID_PT_HSYNC_POSITIVE (1 << 1)
+#define EDID_PT_VSYNC_POSITIVE (1 << 2)
+#define EDID_PT_SEPARATE_SYNC  (3 << 3)
+#define EDID_PT_STEREO         (1 << 5)
+#define EDID_PT_INTERLACED     (1 << 7)
+
+struct detailed_pixel_timing {
+	uint8_t hactive_lo;
+	uint8_t hblank_lo;
+	uint8_t hactive_hblank_hi;
+	uint8_t vactive_lo;
+	uint8_t vblank_lo;
+	uint8_t vactive_vblank_hi;
+	uint8_t hsync_offset_lo;
+	uint8_t hsync_pulse_width_lo;
+	uint8_t vsync_offset_pulse_width_lo;
+	uint8_t hsync_vsync_offset_pulse_width_hi;
+	uint8_t width_mm_lo;
+	uint8_t height_mm_lo;
+	uint8_t width_height_mm_hi;
+	uint8_t hborder;
+	uint8_t vborder;
+	uint8_t misc;
+} __attribute__((packed));
+
+struct detailed_data_string {
+	char str[13];
+} __attribute__((packed));
+
+struct detailed_data_monitor_range {
+	uint8_t min_vfreq;
+	uint8_t max_vfreq;
+	uint8_t min_hfreq_khz;
+	uint8_t max_hfreq_khz;
+	uint8_t pixel_clock_mhz; /* need to multiply by 10 */
+	uint8_t flags;
+	union {
+		char pad[7];
+		struct {
+			uint8_t reserved;
+			uint8_t hfreq_start_khz; /* need to multiply by 2 */
+			uint8_t c; /* need to divide by 2 */
+			uint8_t m[2];
+			uint8_t k;
+			uint8_t j; /* need to divide by 2 */
+		} __attribute__((packed)) gtf2;
+		struct {
+			uint8_t version;
+			uint8_t data1; /* high 6 bits: extra clock resolution */
+			uint8_t data2; /* plus low 2 of above: max hactive */
+			uint8_t supported_aspects;
+			uint8_t flags; /* preferred aspect and blanking support */
+			uint8_t supported_scalings;
+			uint8_t preferred_refresh;
+		} __attribute__((packed)) cvt;
+	} formula;
+} __attribute__((packed));
+
+enum detailed_non_pixel_type {
+	EDID_DETAIL_EST_TIMINGS = 0xf7,
+	EDID_DETAIL_CVT_3BYTE = 0xf8,
+	EDID_DETAIL_COLOR_MGMT_DATA = 0xf9,
+	EDID_DETAIL_STD_MODES = 0xfa,
+	EDID_DETAIL_MONITOR_CPDATA = 0xfb,
+	EDID_DETAIL_MONITOR_NAME = 0xfc,
+	EDID_DETAIL_MONITOR_RANGE = 0xfd,
+	EDID_DETAIL_MONITOR_STRING = 0xfe,
+	EDID_DETAIL_MONITOR_SERIAL = 0xff,
+};
+
+struct detailed_non_pixel {
+	uint8_t pad1;
+	uint8_t type; /* enum detailed_non_pixel_type */
+	uint8_t pad2;
+	union {
+		struct detailed_data_string str;
+		struct detailed_data_monitor_range range;
+		struct detailed_data_string string;
+		/* TODO: other types */
+	} data;
+} __attribute__((packed));
+
+struct detailed_timing {
+	uint8_t pixel_clock[2]; /* need to multiply by 10 KHz, zero if not a pixel timing */
+	union {
+		struct detailed_pixel_timing pixel_data;
+		struct detailed_non_pixel other_data;
+	} data;
+} __attribute__((packed));
+
+struct edid {
+	char header[8];
+	/* Vendor & product info */
+	uint8_t mfg_id[2];
+	uint8_t prod_code[2];
+	uint8_t serial[4];
+	uint8_t mfg_week;
+	uint8_t mfg_year;
+	/* EDID version */
+	uint8_t version;
+	uint8_t revision;
+	/* Display info: */
+	uint8_t input;
+	uint8_t width_cm;
+	uint8_t height_cm;
+	uint8_t gamma;
+	uint8_t features;
+	/* Color characteristics */
+	uint8_t red_green_lo;
+	uint8_t black_white_lo;
+	uint8_t red_x;
+	uint8_t red_y;
+	uint8_t green_x;
+	uint8_t green_y;
+	uint8_t blue_x;
+	uint8_t blue_y;
+	uint8_t white_x;
+	uint8_t white_y;
+	/* Est. timings and mfg rsvd timings*/
+	struct est_timings established_timings;
+	/* Standard timings 1-8*/
+	struct std_timing standard_timings[STD_TIMINGS_LEN];
+	/* Detailing timings 1-4 */
+	struct detailed_timing detailed_timings[DETAILED_TIMINGS_LEN];
+	/* Number of 128 byte ext. blocks */
+	uint8_t extensions;
+	/* Checksum */
+	uint8_t checksum;
+} __attribute__((packed));
+
+void edid_init(struct edid *edid);
+void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode);
+void edid_finalize(struct edid *edid);
+void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
+			      int width_mm, int height_mm);
+void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
+					    drmModeModeInfo *mode);
+void detailed_timing_set_string(struct detailed_timing *dt,
+				enum detailed_non_pixel_type type,
+				const char *str);
+
+#endif
diff --git a/lib/igt_edid_template.h b/lib/igt_edid_template.h
deleted file mode 100644
index 8dbd19ca..00000000
--- a/lib/igt_edid_template.h
+++ /dev/null
@@ -1,74 +0,0 @@
-#define GAMMA(x) (((x) * 100) - 100)
-
-#define MANUFACTURER_ID(a, b, c) (a - '@') << 2 | (b - '@') >> 3, \
-				 (b - '@') << 5 | (c - '@')
-
-
-#define ab(x, y) ((x) & 0xff), ((y) & 0xff), (((x) & 0xf00) >> 4) | (((y) & 0xf00) >> 8)
-#define op(ho, hp, vo, vp) ((ho) & 0xff), ((hp) & 0xff), \
-		(((vo) & 0xf) << 4) | ((vp) & 0xf), \
-		(((ho) & 0x300) >> 2) | (((hp) & 0x300) >> 4) \
-		| (((vo) & 0x30) >> 2) | (((vp) & 0x30) >> 4)
-
-static unsigned char EDID_NAME[EDID_LENGTH] = {
-	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, /* header */
-	MANUFACTURER_ID('I', 'G', 'T'),
-	/* product code, serial number, week and year of manufacture */
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x01, 0x03, /* edid version (1.3) */
-	/* basic display parameters */
-	/* digital display, maximum horizontal image size, maximum vertical
-	 * image size, gamma, features: RGB 4:4:4, native pixel format and
-	 * refresh rate in descriptor 1 */
-	0x80, HSIZE, VSIZE, GAMMA(2.20), 0x02,
-	/* chromaticity coordinates */
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	/* established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
-	0x21, 0x08, 0x00,
-	/* standard timings */
-	0xd1, 0xc0, /* 1920x1080 60Hz */
-	0x81, 0xc0, /* 1280x720 60Hz */
-	0x61, 0x40, /* 1024x768 60Hz */
-	0x45, 0x40, /* 800x600 60Hz */
-	0x31, 0x40, /* 640x480 60Hz */
-	0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
-	/* descriptor 1 (preferred timing) */
-	(CLOCK / 10) & 0x00ff, ((CLOCK / 10) & 0xff00) >> 8,
-	ab(HACTIVE, HBLANK), ab(VACTIVE, VBLANK),
-	op(HOFFSET, HPULSE, VOFFSET, VPULSE),
-	ab(HSIZE * 10, VSIZE * 10),
-	0x00, 0x00, 0x00,
-	/* descriptor 2 (monitor range limits) */
-	0x00, 0x00, 0x00, 0xfd, 0x00,
-	VFREQ - 1, VFREQ + 1, /* minimum, maximum vertical field rate */
-	(CLOCK / (HACTIVE + HBLANK)) - 1, /* minimum horizontal line rate */
-	(CLOCK / (HACTIVE + HBLANK)) + 1, /* maximum horizontal line rate */
-	(CLOCK / 10000) + 1, /* maximum pixel clock rate */
-	0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
-	/* descriptor 3 (name descriptor) */
-	0x00, 0x00, 0x00, 0xfc, 0x00,  'I',  'G',  'T', 0x0a,
-	0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
-	/* descriptor 4 */
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	/* extensions, checksum */
-	0x00, 0x00
-};
-
-#undef EDID_NAME
-#undef VFREQ
-#undef CLOCK
-#undef HACTIVE
-#undef HBLANK
-#undef VACTIVE
-#undef VBLANK
-#undef HOFFSET
-#undef HPULSE
-#undef VOFFSET
-#undef VPULSE
-#undef HSIZE
-#undef VSIZE
-#undef GAMMA
-#undef MANUFACTURER_ID
-#undef ab
-#undef op
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f8b0dab2..a2832f75 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -50,6 +50,7 @@
 #include "drmtest.h"
 #include "igt_kms.h"
 #include "igt_aux.h"
+#include "igt_edid.h"
 #include "intel_chipset.h"
 #include "igt_debugfs.h"
 #include "igt_device.h"
@@ -102,23 +103,6 @@ static void update_edid_csum(unsigned char *edid, int cea_pos)
 	edid[cea_pos + 127] = 256 - sum;
 }
 
-#define VFREQ 60
-#define CLOCK 148500
-#define HACTIVE 1920
-#define HBLANK 280
-#define VACTIVE 1080
-#define VBLANK 45
-#define HOFFSET 88
-#define HPULSE 44
-#define VOFFSET 4
-#define VPULSE 5
-
-#define HSIZE 52
-#define VSIZE 30
-
-#define EDID_NAME base_edid
-#include "igt_edid_template.h"
-
 /**
  * igt_kms_get_base_edid:
  *
@@ -135,29 +119,66 @@ static void update_edid_csum(unsigned char *edid, int cea_pos)
  *
  * Returns: a basic edid block
  */
-const unsigned char* igt_kms_get_base_edid(void)
+const unsigned char *igt_kms_get_base_edid(void)
 {
-	update_edid_csum(base_edid, 0);
+	static struct edid edid;
+	drmModeModeInfo mode = {};
 
-	return base_edid;
+	mode.clock = 148500;
+	mode.hdisplay = 1920;
+	mode.hsync_start = 2008;
+	mode.hsync_end = 2052;
+	mode.htotal = 2200;
+	mode.vdisplay = 1080;
+	mode.vsync_start = 1084;
+	mode.vsync_end = 1089;
+	mode.vtotal = 1125;
+	mode.vrefresh = 60;
+
+	edid_init_with_mode(&edid, &mode);
+	edid_finalize(&edid);
+
+	return (unsigned char *) &edid;
 }
 
-#define VFREQ 60
-#define CLOCK 101000
-#define HACTIVE 1400
-#define HBLANK 160
-#define VACTIVE 1050
-#define VBLANK 30
-#define HOFFSET 48
-#define HPULSE 32
-#define VOFFSET 3
-#define VPULSE 4
+/**
+ * igt_kms_get_alt_edid:
+ *
+ * Get an alternate edid block, which includes the following modes:
+ *
+ *  - 1400x1050 60Hz
+ *  - 1920x1080 60Hz
+ *  - 1280x720 60Hz
+ *  - 1024x768 60Hz
+ *  - 800x600 60Hz
+ *  - 640x480 60Hz
+ *
+ * This can be extended with further features using functions such as
+ * #kmstest_edid_add_3d.
+ *
+ * Returns: an alternate edid block
+ */
+const unsigned char* igt_kms_get_alt_edid(void)
+{
+	static struct edid edid;
+	drmModeModeInfo mode = {};
+
+	mode.clock = 101000;
+	mode.hdisplay = 1400;
+	mode.hsync_start = 1448;
+	mode.hsync_end = 1480;
+	mode.htotal = 1560;
+	mode.vdisplay = 1050;
+	mode.vsync_start = 1053;
+	mode.vsync_end = 1057;
+	mode.vtotal = 1080;
+	mode.vrefresh = 60;
 
-#define HSIZE 52
-#define VSIZE 30
+	edid_init_with_mode(&edid, &mode);
+	edid_finalize(&edid);
 
-#define EDID_NAME alt_edid
-#include "igt_edid_template.h"
+	return (unsigned char *) &edid;
+}
 
 const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	[IGT_PLANE_SRC_X] = "SRC_X",
@@ -301,30 +322,6 @@ igt_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
 	drmModeFreeObjectProperties(props);
 }
 
-/**
- * igt_kms_get_alt_edid:
- *
- * Get an alternate edid block, which includes the following modes:
- *
- *  - 1400x1050 60Hz
- *  - 1920x1080 60Hz
- *  - 1280x720 60Hz
- *  - 1024x768 60Hz
- *  - 800x600 60Hz
- *  - 640x480 60Hz
- *
- * This can be extended with further features using functions such as
- * #kmstest_edid_add_3d.
- *
- * Returns: an alternate edid block
- */
-const unsigned char* igt_kms_get_alt_edid(void)
-{
-	update_edid_csum(alt_edid, 0);
-
-	return alt_edid;
-}
-
 /**
  * kmstest_pipe_name:
  * @pipe: display pipe
diff --git a/lib/meson.build b/lib/meson.build
index a8462933..20c0e3e6 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -54,7 +54,8 @@ lib_sources = [
 	'igt_v3d.c',
 	'igt_vc4.c',
 	'igt_psr.c',
-	'igt_amd.c'
+	'igt_amd.c',
+	'igt_edid.c',
 ]
 
 lib_deps = [
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs
  2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
@ 2019-04-15 12:51 ` Petri Latvala
  2019-04-15 13:32 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Petri Latvala @ 2019-04-15 12:51 UTC (permalink / raw)
  To: Simon Ser; +Cc: igt-dev

On Mon, Apr 15, 2019 at 03:38:15PM +0300, Simon Ser wrote:
> diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> new file mode 100644
> index 00000000..8c027c89
> --- /dev/null
> +++ b/lib/igt_edid.h
> @@ -0,0 +1,196 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors: Simon Ser <simon.ser@intel.com>
> + */
> +
> +#ifndef IGT_CHAMELIUM_H
> +#define IGT_CHAMELIUM_H


Something is off here but not sure what...


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_edid: new library for generating EDIDs
  2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
  2019-04-15 12:51 ` Petri Latvala
@ 2019-04-15 13:32 ` Patchwork
  2019-04-15 14:20 ` [igt-dev] [PATCH i-g-t v2] " Simon Ser
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-15 13:32 UTC (permalink / raw)
  To: Simon Ser; +Cc: igt-dev

== Series Details ==

Series: lib/igt_edid: new library for generating EDIDs
URL   : https://patchwork.freedesktop.org/series/59496/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5934 -> IGTPW_2858
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59496/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2858 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +57

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - fi-glk-dsi:         PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191]

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       INCOMPLETE [fdo#108602] / [fdo#108744] -> PASS

  
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (50 -> 41)
------------------------------

  Additional (1): fi-bsw-n3050 
  Missing    (10): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-ilk-650 fi-ctg-p8600 fi-icl-y fi-bdw-samus fi-snb-2600 


Build changes
-------------

    * IGT: IGT_4946 -> IGTPW_2858

  CI_DRM_5934: cc5334c0e706ec423c5f1a139cf3da7bd3287db6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2858: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2858/
  IGT_4946: 56bdc68638cec64c6b02cd6b220b52b76059b51a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2858/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
  2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
  2019-04-15 12:51 ` Petri Latvala
  2019-04-15 13:32 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-04-15 14:20 ` Simon Ser
  2019-04-16  8:11   ` Arkadiusz Hiler
  2019-04-15 14:58 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Simon Ser @ 2019-04-15 14:20 UTC (permalink / raw)
  To: igt-dev

For the purposes of testing different EDID features, we need to generate more
and more complex EDID blobs (e.g. with audio support). However currently IGT
uses a macro-based system to generate EDIDs. This doesn't scale well and is
pretty inflexible.

This commit introduces a new little library to generate EDIDs. For now it can't
do more than the old macro. Future commits will extend the API.

The structures are mostly based on the Linux kernel code (drm_edid.h). Setters
have been added for convenience.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---

Changes in v2: fix include guard name

Adding Ville because he wrote the current EDID code, and Arek because he's
interested in this patch.

 lib/igt_edid.c          | 268 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_edid.h          | 196 +++++++++++++++++++++++++++++
 lib/igt_edid_template.h |  74 -----------
 lib/igt_kms.c           | 113 +++++++++--------
 lib/meson.build         |   3 +-
 5 files changed, 521 insertions(+), 133 deletions(-)
 create mode 100644 lib/igt_edid.c
 create mode 100644 lib/igt_edid.h
 delete mode 100644 lib/igt_edid_template.h

diff --git a/lib/igt_edid.c b/lib/igt_edid.c
new file mode 100644
index 00000000..155ebebe
--- /dev/null
+++ b/lib/igt_edid.c
@@ -0,0 +1,268 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors: Simon Ser <simon.ser@intel.com>
+ */
+
+#include "config.h"
+
+#include <assert.h>
+#include <string.h>
+#include <stdint.h>
+#include <time.h>
+#include <xf86drmMode.h>
+
+#include "igt_core.h"
+#include "igt_edid.h"
+
+static const char edid_header[] = {
+	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
+};
+
+/* vfreq is in Hz */
+static void std_timing_set(struct std_timing *st, int hsize, int vfreq,
+			   enum std_timing_aspect aspect)
+{
+	assert(hsize >= 256 && hsize <= 2288);
+	st->hsize = hsize / 8 - 31;
+	st->vfreq_aspect = aspect << 6 | (vfreq - 60);
+}
+
+static void std_timing_unset(struct std_timing *st)
+{
+	memset(st, 0x01, sizeof(struct std_timing));
+}
+
+/**
+ * detailed_timing_set_mode: fill a detailed timing based on a mode
+ */
+void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
+			      int width_mm, int height_mm)
+{
+	int hactive, hblank, vactive, vblank, hsync_offset, hsync_pulse_width,
+	    vsync_offset, vsync_pulse_width;
+	struct detailed_pixel_timing *pt = &dt->data.pixel_data;
+
+	hactive = mode->hdisplay;
+	hsync_offset = mode->hsync_start - mode->hdisplay;
+	hsync_pulse_width = mode->hsync_end - mode->hsync_start;
+	hblank = mode->htotal - mode->hdisplay;
+
+	vactive = mode->vdisplay;
+	vsync_offset = mode->vsync_start - mode->vdisplay;
+	vsync_pulse_width = mode->vsync_end - mode->vsync_start;
+	vblank = mode->vtotal - mode->vdisplay;
+
+	dt->pixel_clock[0] = (mode->clock / 10) & 0x00FF;
+	dt->pixel_clock[1] = ((mode->clock / 10) & 0xFF00) >> 8;
+
+	assert(hactive <= 0xFFF);
+	assert(hblank <= 0xFFF);
+	pt->hactive_lo = hactive & 0x0FF;
+	pt->hblank_lo = hblank & 0x0FF;
+	pt->hactive_hblank_hi = (hactive & 0xF00) >> 4 | (hblank & 0xF00) >> 8;
+
+	assert(vactive <= 0xFFF);
+	assert(vblank <= 0xFFF);
+	pt->vactive_lo = vactive & 0x0FF;
+	pt->vblank_lo = vblank & 0x0FF;
+	pt->vactive_vblank_hi = (vactive & 0xF00) >> 4 | (vblank & 0xF00) >> 8;
+
+	assert(hsync_offset <= 0x3FF);
+	assert(hsync_pulse_width <= 0x3FF);
+	assert(vsync_offset <= 0x3F);
+	assert(vsync_pulse_width <= 0x3F);
+	pt->hsync_offset_lo = hsync_offset & 0x0FF;
+	pt->hsync_pulse_width_lo = hsync_pulse_width & 0x0FF;
+	pt->vsync_offset_pulse_width_lo = (vsync_offset & 0xF) << 4
+					  | (vsync_pulse_width & 0xF);
+	pt->hsync_vsync_offset_pulse_width_hi = 
+		((hsync_offset & 0x300) >> 2) | ((hsync_pulse_width & 0x300) >> 4)
+		| ((vsync_offset & 0x30) >> 2) | ((vsync_pulse_width & 0x30) >> 4);
+
+	assert(width_mm <= 0xFFF);
+	assert(height_mm <= 0xFFF);
+	pt->width_mm_lo = width_mm & 0x0FF;
+	pt->height_mm_lo = height_mm & 0x0FF;
+	pt->width_height_mm_hi = (width_mm & 0xF00) >> 4
+				 | (height_mm & 0xF00) >> 8;
+	
+	pt->misc = 0;
+	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+		pt->misc |= EDID_PT_HSYNC_POSITIVE;
+	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+		pt->misc |= EDID_PT_VSYNC_POSITIVE;
+}
+
+/**
+ * detailed_timing_set_monitor_range_mode: set a detailed timing to be a
+ * monitor range based on a mode
+ */
+void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
+					    drmModeModeInfo *mode)
+{
+	struct detailed_non_pixel *np = &dt->data.other_data;
+	struct detailed_data_monitor_range *mr = &np->data.range;
+
+	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
+
+	np->type = EDID_DETAIL_MONITOR_RANGE;
+
+	mr->min_vfreq = mode->vrefresh - 1;
+	mr->max_vfreq = mode->vrefresh + 1;
+	mr->min_hfreq_khz = (mode->clock / mode->htotal) - 1;
+	mr->max_hfreq_khz = (mode->clock / mode->htotal) + 1;
+	mr->pixel_clock_mhz = (mode->clock / 10000) + 1;
+	mr->flags = 0;
+
+	static const char padding[] = {
+		0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
+	};
+	memcpy(mr->formula.pad, padding, sizeof(padding));
+}
+
+/**
+ * detailed_timing_set_string: set a detailed timing to be a string
+ */
+void detailed_timing_set_string(struct detailed_timing *dt,
+				enum detailed_non_pixel_type type,
+				const char *str)
+{
+	struct detailed_non_pixel *np = &dt->data.other_data;
+	struct detailed_data_string *ds = &np->data.string;
+	size_t len;
+
+	switch (type) {
+	case EDID_DETAIL_MONITOR_NAME:
+	case EDID_DETAIL_MONITOR_STRING:
+	case EDID_DETAIL_MONITOR_SERIAL:
+		break;
+	default:
+		assert(0); /* not a string type */
+	}
+
+	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
+
+	np->type = type;
+
+	strncpy(ds->str, str, sizeof(ds->str));
+	len = strlen(str);
+	if (len < sizeof(ds->str))
+		ds->str[len] = '\n';
+}
+
+static void edid_set_mfg(struct edid *edid, char mfg[static 3])
+{
+	edid->mfg_id[0] = (mfg[0] - '@') << 2 | (mfg[1] - '@') >> 3;
+	edid->mfg_id[1] = (mfg[1] - '@') << 5 | (mfg[2] - '@');
+}
+
+static void edid_set_gamma(struct edid *edid, float gamma)
+{
+	edid->gamma = (gamma * 100) - 100;
+}
+
+/**
+ * edid_init: initialize an EDID
+ *
+ * The EDID will be pre-filled with established and standard timings:
+ *
+ *  - 1920x1080 60Hz
+ *  - 1280x720 60Hz
+ *  - 1024x768 60Hz
+ *  - 800x600 60Hz
+ *  - 640x480 60Hz
+ */
+void edid_init(struct edid *edid)
+{
+	size_t i;
+	time_t t;
+	struct tm *tm;
+
+	memset(edid, 0, sizeof(struct edid));
+
+	memcpy(edid->header, edid_header, sizeof(edid_header));
+	edid_set_mfg(edid, "IGT");
+	edid->version = 1;
+	edid->revision = 3;
+	edid->input = 0x80;
+	edid->width_cm = 52;
+	edid->height_cm = 30;
+	edid_set_gamma(edid, 2.20);
+	edid->features = 0x02;
+
+	/* Year of manufacture */
+	t = time(NULL);
+	tm = localtime(&t);
+	edid->mfg_year = tm->tm_year - 90;
+
+	/* Established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
+	edid->established_timings.t1 = 0x21;
+	edid->established_timings.t2 = 0x08;
+
+	/* Standard timings */
+	/* 1920x1080 60Hz */
+	std_timing_set(&edid->standard_timings[0], 1920, 60, STD_TIMING_16_9);
+	/* 1280x720 60Hz */
+	std_timing_set(&edid->standard_timings[1], 1280, 60, STD_TIMING_16_9);
+	/* 1024x768 60Hz */
+	std_timing_set(&edid->standard_timings[2], 1024, 60, STD_TIMING_4_3);
+	/* 800x600 60Hz */
+	std_timing_set(&edid->standard_timings[3], 800, 60, STD_TIMING_4_3);
+	/* 640x480 60Hz */
+	std_timing_set(&edid->standard_timings[4], 640, 60, STD_TIMING_4_3);
+	for (i = 5; i < STD_TIMINGS_LEN; i++)
+		std_timing_unset(&edid->standard_timings[i]);
+}
+
+/**
+ * edid_init_with_mode: initialize an EDID and sets its preferred mode
+ */
+void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode)
+{
+	edid_init(edid);
+
+	/* Preferred timing */
+	detailed_timing_set_mode(&edid->detailed_timings[0], mode,
+				 edid->width_cm * 10, edid->height_cm * 10);
+	detailed_timing_set_monitor_range_mode(&edid->detailed_timings[1],
+					       mode);
+	detailed_timing_set_string(&edid->detailed_timings[2],
+				   EDID_DETAIL_MONITOR_NAME, "IGT");
+}
+
+/**
+ * edid_finalize: finalize an EDID by computing its checksum
+ */
+void edid_finalize(struct edid *edid)
+{
+	size_t i;
+	const uint8_t *buf = (const uint8_t *) edid;
+	uint8_t sum = 0;
+
+	/* calculate checksum */
+	for (i = 0; i < sizeof(struct edid) - 1; i++) {
+		sum = sum + buf[i];
+	}
+
+	edid->checksum = 256 - sum;
+}
diff --git a/lib/igt_edid.h b/lib/igt_edid.h
new file mode 100644
index 00000000..1754af22
--- /dev/null
+++ b/lib/igt_edid.h
@@ -0,0 +1,196 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors: Simon Ser <simon.ser@intel.com>
+ */
+
+#ifndef IGT_EDID_H
+#define IGT_EDID_H
+
+#include "config.h"
+
+#include <stdint.h>
+
+struct est_timings {
+	uint8_t t1;
+	uint8_t t2;
+	uint8_t mfg_rsvd;
+} __attribute__((packed));
+
+#define STD_TIMINGS_LEN 8
+
+enum std_timing_aspect {
+	STD_TIMING_16_10 = 0b00,
+	STD_TIMING_4_3 = 0b01,
+	STD_TIMING_5_4 = 0b10,
+	STD_TIMING_16_9 = 0b11,
+};
+
+struct std_timing {
+	uint8_t hsize;
+	uint8_t vfreq_aspect;
+} __attribute__((packed));
+
+#define DETAILED_TIMINGS_LEN 4
+
+#define EDID_PT_HSYNC_POSITIVE (1 << 1)
+#define EDID_PT_VSYNC_POSITIVE (1 << 2)
+#define EDID_PT_SEPARATE_SYNC  (3 << 3)
+#define EDID_PT_STEREO         (1 << 5)
+#define EDID_PT_INTERLACED     (1 << 7)
+
+struct detailed_pixel_timing {
+	uint8_t hactive_lo;
+	uint8_t hblank_lo;
+	uint8_t hactive_hblank_hi;
+	uint8_t vactive_lo;
+	uint8_t vblank_lo;
+	uint8_t vactive_vblank_hi;
+	uint8_t hsync_offset_lo;
+	uint8_t hsync_pulse_width_lo;
+	uint8_t vsync_offset_pulse_width_lo;
+	uint8_t hsync_vsync_offset_pulse_width_hi;
+	uint8_t width_mm_lo;
+	uint8_t height_mm_lo;
+	uint8_t width_height_mm_hi;
+	uint8_t hborder;
+	uint8_t vborder;
+	uint8_t misc;
+} __attribute__((packed));
+
+struct detailed_data_string {
+	char str[13];
+} __attribute__((packed));
+
+struct detailed_data_monitor_range {
+	uint8_t min_vfreq;
+	uint8_t max_vfreq;
+	uint8_t min_hfreq_khz;
+	uint8_t max_hfreq_khz;
+	uint8_t pixel_clock_mhz; /* need to multiply by 10 */
+	uint8_t flags;
+	union {
+		char pad[7];
+		struct {
+			uint8_t reserved;
+			uint8_t hfreq_start_khz; /* need to multiply by 2 */
+			uint8_t c; /* need to divide by 2 */
+			uint8_t m[2];
+			uint8_t k;
+			uint8_t j; /* need to divide by 2 */
+		} __attribute__((packed)) gtf2;
+		struct {
+			uint8_t version;
+			uint8_t data1; /* high 6 bits: extra clock resolution */
+			uint8_t data2; /* plus low 2 of above: max hactive */
+			uint8_t supported_aspects;
+			uint8_t flags; /* preferred aspect and blanking support */
+			uint8_t supported_scalings;
+			uint8_t preferred_refresh;
+		} __attribute__((packed)) cvt;
+	} formula;
+} __attribute__((packed));
+
+enum detailed_non_pixel_type {
+	EDID_DETAIL_EST_TIMINGS = 0xf7,
+	EDID_DETAIL_CVT_3BYTE = 0xf8,
+	EDID_DETAIL_COLOR_MGMT_DATA = 0xf9,
+	EDID_DETAIL_STD_MODES = 0xfa,
+	EDID_DETAIL_MONITOR_CPDATA = 0xfb,
+	EDID_DETAIL_MONITOR_NAME = 0xfc,
+	EDID_DETAIL_MONITOR_RANGE = 0xfd,
+	EDID_DETAIL_MONITOR_STRING = 0xfe,
+	EDID_DETAIL_MONITOR_SERIAL = 0xff,
+};
+
+struct detailed_non_pixel {
+	uint8_t pad1;
+	uint8_t type; /* enum detailed_non_pixel_type */
+	uint8_t pad2;
+	union {
+		struct detailed_data_string str;
+		struct detailed_data_monitor_range range;
+		struct detailed_data_string string;
+		/* TODO: other types */
+	} data;
+} __attribute__((packed));
+
+struct detailed_timing {
+	uint8_t pixel_clock[2]; /* need to multiply by 10 KHz, zero if not a pixel timing */
+	union {
+		struct detailed_pixel_timing pixel_data;
+		struct detailed_non_pixel other_data;
+	} data;
+} __attribute__((packed));
+
+struct edid {
+	char header[8];
+	/* Vendor & product info */
+	uint8_t mfg_id[2];
+	uint8_t prod_code[2];
+	uint8_t serial[4];
+	uint8_t mfg_week;
+	uint8_t mfg_year;
+	/* EDID version */
+	uint8_t version;
+	uint8_t revision;
+	/* Display info: */
+	uint8_t input;
+	uint8_t width_cm;
+	uint8_t height_cm;
+	uint8_t gamma;
+	uint8_t features;
+	/* Color characteristics */
+	uint8_t red_green_lo;
+	uint8_t black_white_lo;
+	uint8_t red_x;
+	uint8_t red_y;
+	uint8_t green_x;
+	uint8_t green_y;
+	uint8_t blue_x;
+	uint8_t blue_y;
+	uint8_t white_x;
+	uint8_t white_y;
+	/* Est. timings and mfg rsvd timings*/
+	struct est_timings established_timings;
+	/* Standard timings 1-8*/
+	struct std_timing standard_timings[STD_TIMINGS_LEN];
+	/* Detailing timings 1-4 */
+	struct detailed_timing detailed_timings[DETAILED_TIMINGS_LEN];
+	/* Number of 128 byte ext. blocks */
+	uint8_t extensions;
+	/* Checksum */
+	uint8_t checksum;
+} __attribute__((packed));
+
+void edid_init(struct edid *edid);
+void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode);
+void edid_finalize(struct edid *edid);
+void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
+			      int width_mm, int height_mm);
+void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
+					    drmModeModeInfo *mode);
+void detailed_timing_set_string(struct detailed_timing *dt,
+				enum detailed_non_pixel_type type,
+				const char *str);
+
+#endif
diff --git a/lib/igt_edid_template.h b/lib/igt_edid_template.h
deleted file mode 100644
index 8dbd19ca..00000000
--- a/lib/igt_edid_template.h
+++ /dev/null
@@ -1,74 +0,0 @@
-#define GAMMA(x) (((x) * 100) - 100)
-
-#define MANUFACTURER_ID(a, b, c) (a - '@') << 2 | (b - '@') >> 3, \
-				 (b - '@') << 5 | (c - '@')
-
-
-#define ab(x, y) ((x) & 0xff), ((y) & 0xff), (((x) & 0xf00) >> 4) | (((y) & 0xf00) >> 8)
-#define op(ho, hp, vo, vp) ((ho) & 0xff), ((hp) & 0xff), \
-		(((vo) & 0xf) << 4) | ((vp) & 0xf), \
-		(((ho) & 0x300) >> 2) | (((hp) & 0x300) >> 4) \
-		| (((vo) & 0x30) >> 2) | (((vp) & 0x30) >> 4)
-
-static unsigned char EDID_NAME[EDID_LENGTH] = {
-	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, /* header */
-	MANUFACTURER_ID('I', 'G', 'T'),
-	/* product code, serial number, week and year of manufacture */
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x01, 0x03, /* edid version (1.3) */
-	/* basic display parameters */
-	/* digital display, maximum horizontal image size, maximum vertical
-	 * image size, gamma, features: RGB 4:4:4, native pixel format and
-	 * refresh rate in descriptor 1 */
-	0x80, HSIZE, VSIZE, GAMMA(2.20), 0x02,
-	/* chromaticity coordinates */
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	/* established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
-	0x21, 0x08, 0x00,
-	/* standard timings */
-	0xd1, 0xc0, /* 1920x1080 60Hz */
-	0x81, 0xc0, /* 1280x720 60Hz */
-	0x61, 0x40, /* 1024x768 60Hz */
-	0x45, 0x40, /* 800x600 60Hz */
-	0x31, 0x40, /* 640x480 60Hz */
-	0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
-	/* descriptor 1 (preferred timing) */
-	(CLOCK / 10) & 0x00ff, ((CLOCK / 10) & 0xff00) >> 8,
-	ab(HACTIVE, HBLANK), ab(VACTIVE, VBLANK),
-	op(HOFFSET, HPULSE, VOFFSET, VPULSE),
-	ab(HSIZE * 10, VSIZE * 10),
-	0x00, 0x00, 0x00,
-	/* descriptor 2 (monitor range limits) */
-	0x00, 0x00, 0x00, 0xfd, 0x00,
-	VFREQ - 1, VFREQ + 1, /* minimum, maximum vertical field rate */
-	(CLOCK / (HACTIVE + HBLANK)) - 1, /* minimum horizontal line rate */
-	(CLOCK / (HACTIVE + HBLANK)) + 1, /* maximum horizontal line rate */
-	(CLOCK / 10000) + 1, /* maximum pixel clock rate */
-	0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
-	/* descriptor 3 (name descriptor) */
-	0x00, 0x00, 0x00, 0xfc, 0x00,  'I',  'G',  'T', 0x0a,
-	0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
-	/* descriptor 4 */
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	/* extensions, checksum */
-	0x00, 0x00
-};
-
-#undef EDID_NAME
-#undef VFREQ
-#undef CLOCK
-#undef HACTIVE
-#undef HBLANK
-#undef VACTIVE
-#undef VBLANK
-#undef HOFFSET
-#undef HPULSE
-#undef VOFFSET
-#undef VPULSE
-#undef HSIZE
-#undef VSIZE
-#undef GAMMA
-#undef MANUFACTURER_ID
-#undef ab
-#undef op
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f8b0dab2..a2832f75 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -50,6 +50,7 @@
 #include "drmtest.h"
 #include "igt_kms.h"
 #include "igt_aux.h"
+#include "igt_edid.h"
 #include "intel_chipset.h"
 #include "igt_debugfs.h"
 #include "igt_device.h"
@@ -102,23 +103,6 @@ static void update_edid_csum(unsigned char *edid, int cea_pos)
 	edid[cea_pos + 127] = 256 - sum;
 }
 
-#define VFREQ 60
-#define CLOCK 148500
-#define HACTIVE 1920
-#define HBLANK 280
-#define VACTIVE 1080
-#define VBLANK 45
-#define HOFFSET 88
-#define HPULSE 44
-#define VOFFSET 4
-#define VPULSE 5
-
-#define HSIZE 52
-#define VSIZE 30
-
-#define EDID_NAME base_edid
-#include "igt_edid_template.h"
-
 /**
  * igt_kms_get_base_edid:
  *
@@ -135,29 +119,66 @@ static void update_edid_csum(unsigned char *edid, int cea_pos)
  *
  * Returns: a basic edid block
  */
-const unsigned char* igt_kms_get_base_edid(void)
+const unsigned char *igt_kms_get_base_edid(void)
 {
-	update_edid_csum(base_edid, 0);
+	static struct edid edid;
+	drmModeModeInfo mode = {};
 
-	return base_edid;
+	mode.clock = 148500;
+	mode.hdisplay = 1920;
+	mode.hsync_start = 2008;
+	mode.hsync_end = 2052;
+	mode.htotal = 2200;
+	mode.vdisplay = 1080;
+	mode.vsync_start = 1084;
+	mode.vsync_end = 1089;
+	mode.vtotal = 1125;
+	mode.vrefresh = 60;
+
+	edid_init_with_mode(&edid, &mode);
+	edid_finalize(&edid);
+
+	return (unsigned char *) &edid;
 }
 
-#define VFREQ 60
-#define CLOCK 101000
-#define HACTIVE 1400
-#define HBLANK 160
-#define VACTIVE 1050
-#define VBLANK 30
-#define HOFFSET 48
-#define HPULSE 32
-#define VOFFSET 3
-#define VPULSE 4
+/**
+ * igt_kms_get_alt_edid:
+ *
+ * Get an alternate edid block, which includes the following modes:
+ *
+ *  - 1400x1050 60Hz
+ *  - 1920x1080 60Hz
+ *  - 1280x720 60Hz
+ *  - 1024x768 60Hz
+ *  - 800x600 60Hz
+ *  - 640x480 60Hz
+ *
+ * This can be extended with further features using functions such as
+ * #kmstest_edid_add_3d.
+ *
+ * Returns: an alternate edid block
+ */
+const unsigned char* igt_kms_get_alt_edid(void)
+{
+	static struct edid edid;
+	drmModeModeInfo mode = {};
+
+	mode.clock = 101000;
+	mode.hdisplay = 1400;
+	mode.hsync_start = 1448;
+	mode.hsync_end = 1480;
+	mode.htotal = 1560;
+	mode.vdisplay = 1050;
+	mode.vsync_start = 1053;
+	mode.vsync_end = 1057;
+	mode.vtotal = 1080;
+	mode.vrefresh = 60;
 
-#define HSIZE 52
-#define VSIZE 30
+	edid_init_with_mode(&edid, &mode);
+	edid_finalize(&edid);
 
-#define EDID_NAME alt_edid
-#include "igt_edid_template.h"
+	return (unsigned char *) &edid;
+}
 
 const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	[IGT_PLANE_SRC_X] = "SRC_X",
@@ -301,30 +322,6 @@ igt_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
 	drmModeFreeObjectProperties(props);
 }
 
-/**
- * igt_kms_get_alt_edid:
- *
- * Get an alternate edid block, which includes the following modes:
- *
- *  - 1400x1050 60Hz
- *  - 1920x1080 60Hz
- *  - 1280x720 60Hz
- *  - 1024x768 60Hz
- *  - 800x600 60Hz
- *  - 640x480 60Hz
- *
- * This can be extended with further features using functions such as
- * #kmstest_edid_add_3d.
- *
- * Returns: an alternate edid block
- */
-const unsigned char* igt_kms_get_alt_edid(void)
-{
-	update_edid_csum(alt_edid, 0);
-
-	return alt_edid;
-}
-
 /**
  * kmstest_pipe_name:
  * @pipe: display pipe
diff --git a/lib/meson.build b/lib/meson.build
index a8462933..20c0e3e6 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -54,7 +54,8 @@ lib_sources = [
 	'igt_v3d.c',
 	'igt_vc4.c',
 	'igt_psr.c',
-	'igt_amd.c'
+	'igt_amd.c',
+	'igt_edid.c',
 ]
 
 lib_deps = [
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_edid: new library for generating EDIDs (rev2)
  2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
                   ` (2 preceding siblings ...)
  2019-04-15 14:20 ` [igt-dev] [PATCH i-g-t v2] " Simon Ser
@ 2019-04-15 14:58 ` Patchwork
  2019-04-15 15:09 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs Patchwork
  2019-04-15 16:46 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-15 14:58 UTC (permalink / raw)
  To: Ser, Simon; +Cc: igt-dev

== Series Details ==

Series: lib/igt_edid: new library for generating EDIDs (rev2)
URL   : https://patchwork.freedesktop.org/series/59496/
State : success

== Summary ==

CI Bug Log - changes from IGT_4947 -> IGTPW_2859
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59496/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2859 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-kbl-guc:         NOTRUN -> SKIP [fdo#109271] +70

  * igt@amdgpu/amd_cs_nop@fork-compute0:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@basic-bsd2:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109289] +1

  * igt@kms_busy@basic-flip-b:
    - fi-kbl-guc:         NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_force_connector_basic@force-edid:
    - fi-glk-dsi:         NOTRUN -> SKIP [fdo#109271] +26

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-glk-dsi:         NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_psr@primary_mmap_gtt:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#110189] +3

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109294]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       INCOMPLETE [fdo#108602] / [fdo#108744] -> PASS

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (48 -> 43)
------------------------------

  Additional (2): fi-icl-y fi-kbl-guc 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4947 -> IGTPW_2859

  CI_DRM_5934: cc5334c0e706ec423c5f1a139cf3da7bd3287db6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2859: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2859/
  IGT_4947: ff28c28cd1e5a8ca219d9795e508c80221b9cf26 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2859/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs
  2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
                   ` (3 preceding siblings ...)
  2019-04-15 14:58 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork
@ 2019-04-15 15:09 ` Patchwork
  2019-04-15 16:46 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-15 15:09 UTC (permalink / raw)
  To: Ser, Simon; +Cc: igt-dev

== Series Details ==

Series: lib/igt_edid: new library for generating EDIDs
URL   : https://patchwork.freedesktop.org/series/59496/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5934_full -> IGTPW_2858_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59496/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2858_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parse@basic-allowed:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@gem_exec_schedule@independent-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +5

  * igt@gem_pipe_control_store_loop@reused-buffer:
    - shard-apl:          PASS -> DMESG-WARN [fdo#110376]

  * igt@gem_pread@stolen-uncached:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> FAIL [fdo#108686]

  * igt@gem_userptr_blits@coherency-unsync:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109290]

  * igt@i915_query@query-topology-known-pci-ids:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109303]

  * igt@kms_atomic_transition@2x-modeset-transitions-nonblocking:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +25

  * igt@kms_busy@extended-pageflip-hang-newfb-render-f:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +4

  * igt@kms_chamelium@hdmi-hpd-storm:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +1

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          NOTRUN -> FAIL [fdo#110321] / [fdo#110336]

  * igt@kms_cursor_crc@cursor-512x512-random:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109279]

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@2x-plain-flip-ts-check:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +44

  * igt@kms_flip@2x-plain-flip-ts-check-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_flip@flip-vs-suspend:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566] +2

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +5

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +13

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_psr@primary_render:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +2

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +2

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-kbl:          PASS -> FAIL [fdo#105010]

  * igt@v3d_get_bo_offset@create-get-offsets:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109315]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          DMESG-WARN [fdo#108566] -> PASS +1

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          FAIL [fdo#108686] -> PASS

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         FAIL [fdo#104097] -> PASS

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +1

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +5

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +20

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +2

  * igt@kms_psr@sprite_mmap_cpu:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
    - shard-iclb:         FAIL [fdo#109247] -> INCOMPLETE [fdo#106978]

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-apl:          DMESG-WARN [fdo#108566] -> FAIL [fdo#103375]

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [fdo#110376]: https://bugs.freedesktop.org/show_bug.cgi?id=110376
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

    * IGT: IGT_4946 -> IGTPW_2858
    * Piglit: piglit_4509 -> None

  CI_DRM_5934: cc5334c0e706ec423c5f1a139cf3da7bd3287db6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2858: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2858/
  IGT_4946: 56bdc68638cec64c6b02cd6b220b52b76059b51a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2858/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs (rev2)
  2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
                   ` (4 preceding siblings ...)
  2019-04-15 15:09 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs Patchwork
@ 2019-04-15 16:46 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-15 16:46 UTC (permalink / raw)
  To: Ser, Simon; +Cc: igt-dev

== Series Details ==

Series: lib/igt_edid: new library for generating EDIDs (rev2)
URL   : https://patchwork.freedesktop.org/series/59496/
State : success

== Summary ==

CI Bug Log - changes from IGT_4947_full -> IGTPW_2859_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59496/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2859_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276]

  * igt@gem_exec_schedule@out-order-vebox:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +28

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> FAIL [fdo#108686]

  * igt@gem_workarounds@suspend-resume:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#110376]

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +5

  * igt@kms_atomic_transition@5x-modeset-transitions-nonblocking-fencing:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278]

  * igt@kms_atomic_transition@6x-modeset-transitions-fencing:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-d:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_chamelium@hdmi-cmp-nv16:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-apl:          PASS -> DMESG-WARN [fdo#110376]

  * igt@kms_flip@2x-flip-vs-rmfb-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +6

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-rte:
    - shard-iclb:         PASS -> FAIL [fdo#103167] / [fdo#110378]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +12

  * igt@kms_psr@primary_blt:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +2

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566] +2

  * igt@perf_pmu@multi-client-vcs1:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +19

  * igt@prime_nv_pcopy@test2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  * igt@tools_test@tools_test:
    - shard-kbl:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-kbl:          DMESG-WARN [fdo#108566] -> PASS +1

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +5

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         DMESG-WARN [fdo#109982] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          FAIL [fdo#105767] -> PASS

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         DMESG-WARN [fdo#109593] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +5

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +20

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          SKIP [fdo#109271] -> PASS +1

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +5

  * igt@kms_psr@sprite_mmap_cpu:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +3

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_vblank@pipe-a-wait-busy-hang:
    - shard-kbl:          DMESG-WARN [fdo#110376] -> PASS

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110376]: https://bugs.freedesktop.org/show_bug.cgi?id=110376
  [fdo#110378]: https://bugs.freedesktop.org/show_bug.cgi?id=110378
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


Build changes
-------------

    * IGT: IGT_4947 -> IGTPW_2859

  CI_DRM_5934: cc5334c0e706ec423c5f1a139cf3da7bd3287db6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2859: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2859/
  IGT_4947: ff28c28cd1e5a8ca219d9795e508c80221b9cf26 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2859/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
  2019-04-15 14:20 ` [igt-dev] [PATCH i-g-t v2] " Simon Ser
@ 2019-04-16  8:11   ` Arkadiusz Hiler
  2019-04-16  8:46     ` Ser, Simon
  0 siblings, 1 reply; 12+ messages in thread
From: Arkadiusz Hiler @ 2019-04-16  8:11 UTC (permalink / raw)
  To: Simon Ser; +Cc: igt-dev

On Mon, Apr 15, 2019 at 05:20:43PM +0300, Simon Ser wrote:
> For the purposes of testing different EDID features, we need to generate more
> and more complex EDID blobs (e.g. with audio support). However currently IGT
> uses a macro-based system to generate EDIDs. This doesn't scale well and is
> pretty inflexible.
> 
> This commit introduces a new little library to generate EDIDs. For now it can't
> do more than the old macro. Future commits will extend the API.
> 
> The structures are mostly based on the Linux kernel code (drm_edid.h). Setters
> have been added for convenience.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
> 
> Changes in v2: fix include guard name
> 
> Adding Ville because he wrote the current EDID code, and Arek because he's
> interested in this patch.
> 
>  lib/igt_edid.c          | 268 ++++++++++++++++++++++++++++++++++++++++
>  lib/igt_edid.h          | 196 +++++++++++++++++++++++++++++
>  lib/igt_edid_template.h |  74 -----------
>  lib/igt_kms.c           | 113 +++++++++--------
>  lib/meson.build         |   3 +-
>  5 files changed, 521 insertions(+), 133 deletions(-)
>  create mode 100644 lib/igt_edid.c
>  create mode 100644 lib/igt_edid.h
>  delete mode 100644 lib/igt_edid_template.h
> 
> diff --git a/lib/igt_edid.c b/lib/igt_edid.c
> new file mode 100644
> index 00000000..155ebebe
> --- /dev/null
> +++ b/lib/igt_edid.c
> @@ -0,0 +1,268 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors: Simon Ser <simon.ser@intel.com>
> + */
> +
> +#include "config.h"
> +
> +#include <assert.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <time.h>
> +#include <xf86drmMode.h>
> +
> +#include "igt_core.h"
> +#include "igt_edid.h"
> +
> +static const char edid_header[] = {
> +	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
> +};
> +
> +/* vfreq is in Hz */
> +static void std_timing_set(struct std_timing *st, int hsize, int vfreq,
> +			   enum std_timing_aspect aspect)
> +{
> +	assert(hsize >= 256 && hsize <= 2288);
> +	st->hsize = hsize / 8 - 31;
> +	st->vfreq_aspect = aspect << 6 | (vfreq - 60);
> +}
> +
> +static void std_timing_unset(struct std_timing *st)
> +{
> +	memset(st, 0x01, sizeof(struct std_timing));
> +}
> +
> +/**
> + * detailed_timing_set_mode: fill a detailed timing based on a mode
> + */
> +void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
> +			      int width_mm, int height_mm)
> +{
> +	int hactive, hblank, vactive, vblank, hsync_offset, hsync_pulse_width,
> +	    vsync_offset, vsync_pulse_width;
> +	struct detailed_pixel_timing *pt = &dt->data.pixel_data;
> +
> +	hactive = mode->hdisplay;
> +	hsync_offset = mode->hsync_start - mode->hdisplay;
> +	hsync_pulse_width = mode->hsync_end - mode->hsync_start;
> +	hblank = mode->htotal - mode->hdisplay;
> +
> +	vactive = mode->vdisplay;
> +	vsync_offset = mode->vsync_start - mode->vdisplay;
> +	vsync_pulse_width = mode->vsync_end - mode->vsync_start;
> +	vblank = mode->vtotal - mode->vdisplay;
> +
> +	dt->pixel_clock[0] = (mode->clock / 10) & 0x00FF;
> +	dt->pixel_clock[1] = ((mode->clock / 10) & 0xFF00) >> 8;
> +
> +	assert(hactive <= 0xFFF);
> +	assert(hblank <= 0xFFF);
> +	pt->hactive_lo = hactive & 0x0FF;
> +	pt->hblank_lo = hblank & 0x0FF;
> +	pt->hactive_hblank_hi = (hactive & 0xF00) >> 4 | (hblank & 0xF00) >> 8;
> +
> +	assert(vactive <= 0xFFF);
> +	assert(vblank <= 0xFFF);
> +	pt->vactive_lo = vactive & 0x0FF;
> +	pt->vblank_lo = vblank & 0x0FF;
> +	pt->vactive_vblank_hi = (vactive & 0xF00) >> 4 | (vblank & 0xF00) >> 8;
> +
> +	assert(hsync_offset <= 0x3FF);
> +	assert(hsync_pulse_width <= 0x3FF);
> +	assert(vsync_offset <= 0x3F);
> +	assert(vsync_pulse_width <= 0x3F);
> +	pt->hsync_offset_lo = hsync_offset & 0x0FF;
> +	pt->hsync_pulse_width_lo = hsync_pulse_width & 0x0FF;
> +	pt->vsync_offset_pulse_width_lo = (vsync_offset & 0xF) << 4
> +					  | (vsync_pulse_width & 0xF);
> +	pt->hsync_vsync_offset_pulse_width_hi = 

                                               ^ tailing whitespace

> +		((hsync_offset & 0x300) >> 2) | ((hsync_pulse_width & 0x300) >> 4)
> +		| ((vsync_offset & 0x30) >> 2) | ((vsync_pulse_width & 0x30) >> 4);
> +
> +	assert(width_mm <= 0xFFF);
> +	assert(height_mm <= 0xFFF);
> +	pt->width_mm_lo = width_mm & 0x0FF;
> +	pt->height_mm_lo = height_mm & 0x0FF;
> +	pt->width_height_mm_hi = (width_mm & 0xF00) >> 4
> +				 | (height_mm & 0xF00) >> 8;
> +	

   ^^^^^ tab

autocmd BufWritePre * %s/\s\+$//e

may help you

> +	pt->misc = 0;
> +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +		pt->misc |= EDID_PT_HSYNC_POSITIVE;
> +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +		pt->misc |= EDID_PT_VSYNC_POSITIVE;
> +}
> +
> +/**
> + * detailed_timing_set_monitor_range_mode: set a detailed timing to be a
> + * monitor range based on a mode
> + */
> +void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
> +					    drmModeModeInfo *mode)
> +{
> +	struct detailed_non_pixel *np = &dt->data.other_data;
> +	struct detailed_data_monitor_range *mr = &np->data.range;
> +
> +	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
> +
> +	np->type = EDID_DETAIL_MONITOR_RANGE;
> +
> +	mr->min_vfreq = mode->vrefresh - 1;
> +	mr->max_vfreq = mode->vrefresh + 1;
> +	mr->min_hfreq_khz = (mode->clock / mode->htotal) - 1;
> +	mr->max_hfreq_khz = (mode->clock / mode->htotal) + 1;
> +	mr->pixel_clock_mhz = (mode->clock / 10000) + 1;
> +	mr->flags = 0;
> +
> +	static const char padding[] = {
> +		0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
> +	};
> +	memcpy(mr->formula.pad, padding, sizeof(padding));
> +}
> +
> +/**
> + * detailed_timing_set_string: set a detailed timing to be a string
> + */
> +void detailed_timing_set_string(struct detailed_timing *dt,
> +				enum detailed_non_pixel_type type,
> +				const char *str)
> +{
> +	struct detailed_non_pixel *np = &dt->data.other_data;
> +	struct detailed_data_string *ds = &np->data.string;
> +	size_t len;
> +
> +	switch (type) {
> +	case EDID_DETAIL_MONITOR_NAME:
> +	case EDID_DETAIL_MONITOR_STRING:
> +	case EDID_DETAIL_MONITOR_SERIAL:
> +		break;
> +	default:
> +		assert(0); /* not a string type */
> +	}
> +
> +	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
> +
> +	np->type = type;
> +
> +	strncpy(ds->str, str, sizeof(ds->str));
> +	len = strlen(str);
> +	if (len < sizeof(ds->str))
> +		ds->str[len] = '\n';
> +}
> +
> +static void edid_set_mfg(struct edid *edid, char mfg[static 3])
> +{
> +	edid->mfg_id[0] = (mfg[0] - '@') << 2 | (mfg[1] - '@') >> 3;
> +	edid->mfg_id[1] = (mfg[1] - '@') << 5 | (mfg[2] - '@');
> +}
> +
> +static void edid_set_gamma(struct edid *edid, float gamma)
> +{
> +	edid->gamma = (gamma * 100) - 100;
> +}
> +
> +/**
> + * edid_init: initialize an EDID
> + *
> + * The EDID will be pre-filled with established and standard timings:
> + *
> + *  - 1920x1080 60Hz
> + *  - 1280x720 60Hz
> + *  - 1024x768 60Hz
> + *  - 800x600 60Hz
> + *  - 640x480 60Hz
> + */
> +void edid_init(struct edid *edid)
> +{
> +	size_t i;
> +	time_t t;
> +	struct tm *tm;
> +
> +	memset(edid, 0, sizeof(struct edid));
> +
> +	memcpy(edid->header, edid_header, sizeof(edid_header));
> +	edid_set_mfg(edid, "IGT");
> +	edid->version = 1;
> +	edid->revision = 3;
> +	edid->input = 0x80;
> +	edid->width_cm = 52;
> +	edid->height_cm = 30;
> +	edid_set_gamma(edid, 2.20);
> +	edid->features = 0x02;
> +
> +	/* Year of manufacture */
> +	t = time(NULL);
> +	tm = localtime(&t);
> +	edid->mfg_year = tm->tm_year - 90;
> +
> +	/* Established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
> +	edid->established_timings.t1 = 0x21;
> +	edid->established_timings.t2 = 0x08;
> +
> +	/* Standard timings */
> +	/* 1920x1080 60Hz */
> +	std_timing_set(&edid->standard_timings[0], 1920, 60, STD_TIMING_16_9);
> +	/* 1280x720 60Hz */
> +	std_timing_set(&edid->standard_timings[1], 1280, 60, STD_TIMING_16_9);
> +	/* 1024x768 60Hz */
> +	std_timing_set(&edid->standard_timings[2], 1024, 60, STD_TIMING_4_3);
> +	/* 800x600 60Hz */
> +	std_timing_set(&edid->standard_timings[3], 800, 60, STD_TIMING_4_3);
> +	/* 640x480 60Hz */
> +	std_timing_set(&edid->standard_timings[4], 640, 60, STD_TIMING_4_3);
> +	for (i = 5; i < STD_TIMINGS_LEN; i++)
> +		std_timing_unset(&edid->standard_timings[i]);
> +}
> +
> +/**
> + * edid_init_with_mode: initialize an EDID and sets its preferred mode
> + */
> +void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode)
> +{
> +	edid_init(edid);
> +
> +	/* Preferred timing */
> +	detailed_timing_set_mode(&edid->detailed_timings[0], mode,
> +				 edid->width_cm * 10, edid->height_cm * 10);
> +	detailed_timing_set_monitor_range_mode(&edid->detailed_timings[1],
> +					       mode);
> +	detailed_timing_set_string(&edid->detailed_timings[2],
> +				   EDID_DETAIL_MONITOR_NAME, "IGT");
> +}
> +
> +/**
> + * edid_finalize: finalize an EDID by computing its checksum
> + */
> +void edid_finalize(struct edid *edid)
> +{
> +	size_t i;
> +	const uint8_t *buf = (const uint8_t *) edid;
> +	uint8_t sum = 0;
> +
> +	/* calculate checksum */
> +	for (i = 0; i < sizeof(struct edid) - 1; i++) {
> +		sum = sum + buf[i];
> +	}
> +
> +	edid->checksum = 256 - sum;
> +}

I get why using `finalize` here may seems tempting, but fini and
finalize tend to be used for destructors (or analogues in GCed
languages), so this may be a bit confusing. I think we should go with
edid_update_checksum() or similar.

Other than that, it's a solid improvement over what we had, which will
come handy the more we fiddle with EDIDs. Thanks!

Do you already have some thoughts on how we want to solve handling the
extensions blocks?

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
  2019-04-16  8:11   ` Arkadiusz Hiler
@ 2019-04-16  8:46     ` Ser, Simon
  2019-04-16 11:05       ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Ser, Simon @ 2019-04-16  8:46 UTC (permalink / raw)
  To: Hiler, Arkadiusz; +Cc: igt-dev

Thanks for your review!

On Tue, 2019-04-16 at 11:11 +0300, Arkadiusz Hiler wrote:
> On Mon, Apr 15, 2019 at 05:20:43PM +0300, Simon Ser wrote:
> > For the purposes of testing different EDID features, we need to generate more
> > and more complex EDID blobs (e.g. with audio support). However currently IGT
> > uses a macro-based system to generate EDIDs. This doesn't scale well and is
> > pretty inflexible.
> > 
> > This commit introduces a new little library to generate EDIDs. For now it can't
> > do more than the old macro. Future commits will extend the API.
> > 
> > The structures are mostly based on the Linux kernel code (drm_edid.h). Setters
> > have been added for convenience.
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> > 
> > Changes in v2: fix include guard name
> > 
> > Adding Ville because he wrote the current EDID code, and Arek because he's
> > interested in this patch.
> > 
> >  lib/igt_edid.c          | 268 ++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_edid.h          | 196 +++++++++++++++++++++++++++++
> >  lib/igt_edid_template.h |  74 -----------
> >  lib/igt_kms.c           | 113 +++++++++--------
> >  lib/meson.build         |   3 +-
> >  5 files changed, 521 insertions(+), 133 deletions(-)
> >  create mode 100644 lib/igt_edid.c
> >  create mode 100644 lib/igt_edid.h
> >  delete mode 100644 lib/igt_edid_template.h
> > 
> > diff --git a/lib/igt_edid.c b/lib/igt_edid.c
> > new file mode 100644
> > index 00000000..155ebebe
> > --- /dev/null
> > +++ b/lib/igt_edid.c
> > @@ -0,0 +1,268 @@
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + * Authors: Simon Ser <simon.ser@intel.com>
> > + */
> > +
> > +#include "config.h"
> > +
> > +#include <assert.h>
> > +#include <string.h>
> > +#include <stdint.h>
> > +#include <time.h>
> > +#include <xf86drmMode.h>
> > +
> > +#include "igt_core.h"
> > +#include "igt_edid.h"
> > +
> > +static const char edid_header[] = {
> > +	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
> > +};
> > +
> > +/* vfreq is in Hz */
> > +static void std_timing_set(struct std_timing *st, int hsize, int vfreq,
> > +			   enum std_timing_aspect aspect)
> > +{
> > +	assert(hsize >= 256 && hsize <= 2288);
> > +	st->hsize = hsize / 8 - 31;
> > +	st->vfreq_aspect = aspect << 6 | (vfreq - 60);
> > +}
> > +
> > +static void std_timing_unset(struct std_timing *st)
> > +{
> > +	memset(st, 0x01, sizeof(struct std_timing));
> > +}
> > +
> > +/**
> > + * detailed_timing_set_mode: fill a detailed timing based on a mode
> > + */
> > +void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
> > +			      int width_mm, int height_mm)
> > +{
> > +	int hactive, hblank, vactive, vblank, hsync_offset, hsync_pulse_width,
> > +	    vsync_offset, vsync_pulse_width;
> > +	struct detailed_pixel_timing *pt = &dt->data.pixel_data;
> > +
> > +	hactive = mode->hdisplay;
> > +	hsync_offset = mode->hsync_start - mode->hdisplay;
> > +	hsync_pulse_width = mode->hsync_end - mode->hsync_start;
> > +	hblank = mode->htotal - mode->hdisplay;
> > +
> > +	vactive = mode->vdisplay;
> > +	vsync_offset = mode->vsync_start - mode->vdisplay;
> > +	vsync_pulse_width = mode->vsync_end - mode->vsync_start;
> > +	vblank = mode->vtotal - mode->vdisplay;
> > +
> > +	dt->pixel_clock[0] = (mode->clock / 10) & 0x00FF;
> > +	dt->pixel_clock[1] = ((mode->clock / 10) & 0xFF00) >> 8;
> > +
> > +	assert(hactive <= 0xFFF);
> > +	assert(hblank <= 0xFFF);
> > +	pt->hactive_lo = hactive & 0x0FF;
> > +	pt->hblank_lo = hblank & 0x0FF;
> > +	pt->hactive_hblank_hi = (hactive & 0xF00) >> 4 | (hblank & 0xF00) >> 8;
> > +
> > +	assert(vactive <= 0xFFF);
> > +	assert(vblank <= 0xFFF);
> > +	pt->vactive_lo = vactive & 0x0FF;
> > +	pt->vblank_lo = vblank & 0x0FF;
> > +	pt->vactive_vblank_hi = (vactive & 0xF00) >> 4 | (vblank & 0xF00) >> 8;
> > +
> > +	assert(hsync_offset <= 0x3FF);
> > +	assert(hsync_pulse_width <= 0x3FF);
> > +	assert(vsync_offset <= 0x3F);
> > +	assert(vsync_pulse_width <= 0x3F);
> > +	pt->hsync_offset_lo = hsync_offset & 0x0FF;
> > +	pt->hsync_pulse_width_lo = hsync_pulse_width & 0x0FF;
> > +	pt->vsync_offset_pulse_width_lo = (vsync_offset & 0xF) << 4
> > +					  | (vsync_pulse_width & 0xF);
> > +	pt->hsync_vsync_offset_pulse_width_hi = 
> 
>                                                ^ tailing whitespace
> 
> > +		((hsync_offset & 0x300) >> 2) | ((hsync_pulse_width & 0x300) >> 4)
> > +		| ((vsync_offset & 0x30) >> 2) | ((vsync_pulse_width & 0x30) >> 4);
> > +
> > +	assert(width_mm <= 0xFFF);
> > +	assert(height_mm <= 0xFFF);
> > +	pt->width_mm_lo = width_mm & 0x0FF;
> > +	pt->height_mm_lo = height_mm & 0x0FF;
> > +	pt->width_height_mm_hi = (width_mm & 0xF00) >> 4
> > +				 | (height_mm & 0xF00) >> 8;
> > +	
> 
>    ^^^^^ tab
> 
> autocmd BufWritePre * %s/\s\+$//e
> 
> may help you

Ah, thanks for the tip!

Also pulled my vim dotfiles so that whitespace characters are visible.

> > +	pt->misc = 0;
> > +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > +		pt->misc |= EDID_PT_HSYNC_POSITIVE;
> > +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > +		pt->misc |= EDID_PT_VSYNC_POSITIVE;
> > +}
> > +
> > +/**
> > + * detailed_timing_set_monitor_range_mode: set a detailed timing to be a
> > + * monitor range based on a mode
> > + */
> > +void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
> > +					    drmModeModeInfo *mode)
> > +{
> > +	struct detailed_non_pixel *np = &dt->data.other_data;
> > +	struct detailed_data_monitor_range *mr = &np->data.range;
> > +
> > +	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
> > +
> > +	np->type = EDID_DETAIL_MONITOR_RANGE;
> > +
> > +	mr->min_vfreq = mode->vrefresh - 1;
> > +	mr->max_vfreq = mode->vrefresh + 1;
> > +	mr->min_hfreq_khz = (mode->clock / mode->htotal) - 1;
> > +	mr->max_hfreq_khz = (mode->clock / mode->htotal) + 1;
> > +	mr->pixel_clock_mhz = (mode->clock / 10000) + 1;
> > +	mr->flags = 0;
> > +
> > +	static const char padding[] = {
> > +		0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
> > +	};
> > +	memcpy(mr->formula.pad, padding, sizeof(padding));
> > +}
> > +
> > +/**
> > + * detailed_timing_set_string: set a detailed timing to be a string
> > + */
> > +void detailed_timing_set_string(struct detailed_timing *dt,
> > +				enum detailed_non_pixel_type type,
> > +				const char *str)
> > +{
> > +	struct detailed_non_pixel *np = &dt->data.other_data;
> > +	struct detailed_data_string *ds = &np->data.string;
> > +	size_t len;
> > +
> > +	switch (type) {
> > +	case EDID_DETAIL_MONITOR_NAME:
> > +	case EDID_DETAIL_MONITOR_STRING:
> > +	case EDID_DETAIL_MONITOR_SERIAL:
> > +		break;
> > +	default:
> > +		assert(0); /* not a string type */
> > +	}
> > +
> > +	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
> > +
> > +	np->type = type;
> > +
> > +	strncpy(ds->str, str, sizeof(ds->str));
> > +	len = strlen(str);
> > +	if (len < sizeof(ds->str))
> > +		ds->str[len] = '\n';
> > +}
> > +
> > +static void edid_set_mfg(struct edid *edid, char mfg[static 3])
> > +{
> > +	edid->mfg_id[0] = (mfg[0] - '@') << 2 | (mfg[1] - '@') >> 3;
> > +	edid->mfg_id[1] = (mfg[1] - '@') << 5 | (mfg[2] - '@');
> > +}
> > +
> > +static void edid_set_gamma(struct edid *edid, float gamma)
> > +{
> > +	edid->gamma = (gamma * 100) - 100;
> > +}
> > +
> > +/**
> > + * edid_init: initialize an EDID
> > + *
> > + * The EDID will be pre-filled with established and standard timings:
> > + *
> > + *  - 1920x1080 60Hz
> > + *  - 1280x720 60Hz
> > + *  - 1024x768 60Hz
> > + *  - 800x600 60Hz
> > + *  - 640x480 60Hz
> > + */
> > +void edid_init(struct edid *edid)
> > +{
> > +	size_t i;
> > +	time_t t;
> > +	struct tm *tm;
> > +
> > +	memset(edid, 0, sizeof(struct edid));
> > +
> > +	memcpy(edid->header, edid_header, sizeof(edid_header));
> > +	edid_set_mfg(edid, "IGT");
> > +	edid->version = 1;
> > +	edid->revision = 3;
> > +	edid->input = 0x80;
> > +	edid->width_cm = 52;
> > +	edid->height_cm = 30;
> > +	edid_set_gamma(edid, 2.20);
> > +	edid->features = 0x02;
> > +
> > +	/* Year of manufacture */
> > +	t = time(NULL);
> > +	tm = localtime(&t);
> > +	edid->mfg_year = tm->tm_year - 90;
> > +
> > +	/* Established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
> > +	edid->established_timings.t1 = 0x21;
> > +	edid->established_timings.t2 = 0x08;
> > +
> > +	/* Standard timings */
> > +	/* 1920x1080 60Hz */
> > +	std_timing_set(&edid->standard_timings[0], 1920, 60, STD_TIMING_16_9);
> > +	/* 1280x720 60Hz */
> > +	std_timing_set(&edid->standard_timings[1], 1280, 60, STD_TIMING_16_9);
> > +	/* 1024x768 60Hz */
> > +	std_timing_set(&edid->standard_timings[2], 1024, 60, STD_TIMING_4_3);
> > +	/* 800x600 60Hz */
> > +	std_timing_set(&edid->standard_timings[3], 800, 60, STD_TIMING_4_3);
> > +	/* 640x480 60Hz */
> > +	std_timing_set(&edid->standard_timings[4], 640, 60, STD_TIMING_4_3);
> > +	for (i = 5; i < STD_TIMINGS_LEN; i++)
> > +		std_timing_unset(&edid->standard_timings[i]);
> > +}
> > +
> > +/**
> > + * edid_init_with_mode: initialize an EDID and sets its preferred mode
> > + */
> > +void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode)
> > +{
> > +	edid_init(edid);
> > +
> > +	/* Preferred timing */
> > +	detailed_timing_set_mode(&edid->detailed_timings[0], mode,
> > +				 edid->width_cm * 10, edid->height_cm * 10);
> > +	detailed_timing_set_monitor_range_mode(&edid->detailed_timings[1],
> > +					       mode);
> > +	detailed_timing_set_string(&edid->detailed_timings[2],
> > +				   EDID_DETAIL_MONITOR_NAME, "IGT");
> > +}
> > +
> > +/**
> > + * edid_finalize: finalize an EDID by computing its checksum
> > + */
> > +void edid_finalize(struct edid *edid)
> > +{
> > +	size_t i;
> > +	const uint8_t *buf = (const uint8_t *) edid;
> > +	uint8_t sum = 0;
> > +
> > +	/* calculate checksum */
> > +	for (i = 0; i < sizeof(struct edid) - 1; i++) {
> > +		sum = sum + buf[i];
> > +	}
> > +
> > +	edid->checksum = 256 - sum;
> > +}
> 
> I get why using `finalize` here may seems tempting, but fini and
> finalize tend to be used for destructors (or analogues in GCed
> languages), so this may be a bit confusing. I think we should go with
> edid_update_checksum() or similar.

Agreed, a more explicit name is pobably not a bad idea.

> Other than that, it's a solid improvement over what we had, which will
> come handy the more we fiddle with EDIDs. Thanks!

:)

Will send a v3 with your comments fixed.

> Do you already have some thoughts on how we want to solve handling the
> extensions blocks?

I've already started to work on this. I think callers should allocate a
large enough buffer, e.g.:

  char buf[sizeof(struct edid) + 2*sizeof(struct edid_ext)];

(Extension blocks have a fixed size)

And then should map a struct edid to it:

  struct edid *edid = (struct edid *) buf;

The annoying thing is that the number of extensions at the end of the
EDID block isn't fixed. We could add a new field at the end of struct
edid:

  struct edid {
    …
    struct edid_ext extensions[];
  };

So that callers can do this:

  struct edid_ext *ext1 = edid->extensions[0];
  struct edid_ext *ext2 = edid->extensions[1];

Another annoying thing is that extensions can themselves contain
multiple sub-blocks of different sizes… For now I've opted for a char
buffer that can be casted into sub-blocks. This isn't the best solution
ever, but I haven't found a better design yet. Let's discuss about this
in the next patch. :)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
  2019-04-16  8:46     ` Ser, Simon
@ 2019-04-16 11:05       ` Jani Nikula
  2019-04-16 12:11         ` Ser, Simon
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2019-04-16 11:05 UTC (permalink / raw)
  To: Ser, Simon, Hiler, Arkadiusz; +Cc: igt-dev

On Tue, 16 Apr 2019, "Ser, Simon" <simon.ser@intel.com> wrote:
> Thanks for your review!
>
> On Tue, 2019-04-16 at 11:11 +0300, Arkadiusz Hiler wrote:
>> On Mon, Apr 15, 2019 at 05:20:43PM +0300, Simon Ser wrote:
>> > For the purposes of testing different EDID features, we need to generate more
>> > and more complex EDID blobs (e.g. with audio support). However currently IGT
>> > uses a macro-based system to generate EDIDs. This doesn't scale well and is
>> > pretty inflexible.
>> > 
>> > This commit introduces a new little library to generate EDIDs. For now it can't
>> > do more than the old macro. Future commits will extend the API.
>> > 
>> > The structures are mostly based on the Linux kernel code (drm_edid.h). Setters
>> > have been added for convenience.
>> > 
>> > Signed-off-by: Simon Ser <simon.ser@intel.com>
>> > ---
>> > 
>> > Changes in v2: fix include guard name
>> > 
>> > Adding Ville because he wrote the current EDID code, and Arek because he's
>> > interested in this patch.
>> > 
>> >  lib/igt_edid.c          | 268 ++++++++++++++++++++++++++++++++++++++++
>> >  lib/igt_edid.h          | 196 +++++++++++++++++++++++++++++
>> >  lib/igt_edid_template.h |  74 -----------
>> >  lib/igt_kms.c           | 113 +++++++++--------
>> >  lib/meson.build         |   3 +-
>> >  5 files changed, 521 insertions(+), 133 deletions(-)
>> >  create mode 100644 lib/igt_edid.c
>> >  create mode 100644 lib/igt_edid.h
>> >  delete mode 100644 lib/igt_edid_template.h
>> > 
>> > diff --git a/lib/igt_edid.c b/lib/igt_edid.c
>> > new file mode 100644
>> > index 00000000..155ebebe
>> > --- /dev/null
>> > +++ b/lib/igt_edid.c
>> > @@ -0,0 +1,268 @@
>> > +/*
>> > + * Copyright © 2019 Intel Corporation
>> > + *
>> > + * Permission is hereby granted, free of charge, to any person obtaining a
>> > + * copy of this software and associated documentation files (the "Software"),
>> > + * to deal in the Software without restriction, including without limitation
>> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> > + * and/or sell copies of the Software, and to permit persons to whom the
>> > + * Software is furnished to do so, subject to the following conditions:
>> > + *
>> > + * The above copyright notice and this permission notice (including the next
>> > + * paragraph) shall be included in all copies or substantial portions of the
>> > + * Software.
>> > + *
>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> > + * IN THE SOFTWARE.
>> > + *
>> > + * Authors: Simon Ser <simon.ser@intel.com>
>> > + */
>> > +
>> > +#include "config.h"
>> > +
>> > +#include <assert.h>
>> > +#include <string.h>
>> > +#include <stdint.h>
>> > +#include <time.h>
>> > +#include <xf86drmMode.h>
>> > +
>> > +#include "igt_core.h"
>> > +#include "igt_edid.h"
>> > +
>> > +static const char edid_header[] = {
>> > +	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
>> > +};
>> > +
>> > +/* vfreq is in Hz */
>> > +static void std_timing_set(struct std_timing *st, int hsize, int vfreq,
>> > +			   enum std_timing_aspect aspect)
>> > +{
>> > +	assert(hsize >= 256 && hsize <= 2288);
>> > +	st->hsize = hsize / 8 - 31;
>> > +	st->vfreq_aspect = aspect << 6 | (vfreq - 60);
>> > +}
>> > +
>> > +static void std_timing_unset(struct std_timing *st)
>> > +{
>> > +	memset(st, 0x01, sizeof(struct std_timing));
>> > +}
>> > +
>> > +/**
>> > + * detailed_timing_set_mode: fill a detailed timing based on a mode
>> > + */
>> > +void detailed_timing_set_mode(struct detailed_timing *dt, drmModeModeInfo *mode,
>> > +			      int width_mm, int height_mm)
>> > +{
>> > +	int hactive, hblank, vactive, vblank, hsync_offset, hsync_pulse_width,
>> > +	    vsync_offset, vsync_pulse_width;
>> > +	struct detailed_pixel_timing *pt = &dt->data.pixel_data;
>> > +
>> > +	hactive = mode->hdisplay;
>> > +	hsync_offset = mode->hsync_start - mode->hdisplay;
>> > +	hsync_pulse_width = mode->hsync_end - mode->hsync_start;
>> > +	hblank = mode->htotal - mode->hdisplay;
>> > +
>> > +	vactive = mode->vdisplay;
>> > +	vsync_offset = mode->vsync_start - mode->vdisplay;
>> > +	vsync_pulse_width = mode->vsync_end - mode->vsync_start;
>> > +	vblank = mode->vtotal - mode->vdisplay;
>> > +
>> > +	dt->pixel_clock[0] = (mode->clock / 10) & 0x00FF;
>> > +	dt->pixel_clock[1] = ((mode->clock / 10) & 0xFF00) >> 8;
>> > +
>> > +	assert(hactive <= 0xFFF);
>> > +	assert(hblank <= 0xFFF);
>> > +	pt->hactive_lo = hactive & 0x0FF;
>> > +	pt->hblank_lo = hblank & 0x0FF;
>> > +	pt->hactive_hblank_hi = (hactive & 0xF00) >> 4 | (hblank & 0xF00) >> 8;
>> > +
>> > +	assert(vactive <= 0xFFF);
>> > +	assert(vblank <= 0xFFF);
>> > +	pt->vactive_lo = vactive & 0x0FF;
>> > +	pt->vblank_lo = vblank & 0x0FF;
>> > +	pt->vactive_vblank_hi = (vactive & 0xF00) >> 4 | (vblank & 0xF00) >> 8;
>> > +
>> > +	assert(hsync_offset <= 0x3FF);
>> > +	assert(hsync_pulse_width <= 0x3FF);
>> > +	assert(vsync_offset <= 0x3F);
>> > +	assert(vsync_pulse_width <= 0x3F);
>> > +	pt->hsync_offset_lo = hsync_offset & 0x0FF;
>> > +	pt->hsync_pulse_width_lo = hsync_pulse_width & 0x0FF;
>> > +	pt->vsync_offset_pulse_width_lo = (vsync_offset & 0xF) << 4
>> > +					  | (vsync_pulse_width & 0xF);
>> > +	pt->hsync_vsync_offset_pulse_width_hi = 
>> 
>>                                                ^ tailing whitespace
>> 
>> > +		((hsync_offset & 0x300) >> 2) | ((hsync_pulse_width & 0x300) >> 4)
>> > +		| ((vsync_offset & 0x30) >> 2) | ((vsync_pulse_width & 0x30) >> 4);
>> > +
>> > +	assert(width_mm <= 0xFFF);
>> > +	assert(height_mm <= 0xFFF);
>> > +	pt->width_mm_lo = width_mm & 0x0FF;
>> > +	pt->height_mm_lo = height_mm & 0x0FF;
>> > +	pt->width_height_mm_hi = (width_mm & 0xF00) >> 4
>> > +				 | (height_mm & 0xF00) >> 8;
>> > +	
>> 
>>    ^^^^^ tab
>> 
>> autocmd BufWritePre * %s/\s\+$//e
>> 
>> may help you
>
> Ah, thanks for the tip!
>
> Also pulled my vim dotfiles so that whitespace characters are visible.
>
>> > +	pt->misc = 0;
>> > +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>> > +		pt->misc |= EDID_PT_HSYNC_POSITIVE;
>> > +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>> > +		pt->misc |= EDID_PT_VSYNC_POSITIVE;
>> > +}
>> > +
>> > +/**
>> > + * detailed_timing_set_monitor_range_mode: set a detailed timing to be a
>> > + * monitor range based on a mode
>> > + */
>> > +void detailed_timing_set_monitor_range_mode(struct detailed_timing *dt,
>> > +					    drmModeModeInfo *mode)
>> > +{
>> > +	struct detailed_non_pixel *np = &dt->data.other_data;
>> > +	struct detailed_data_monitor_range *mr = &np->data.range;
>> > +
>> > +	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
>> > +
>> > +	np->type = EDID_DETAIL_MONITOR_RANGE;
>> > +
>> > +	mr->min_vfreq = mode->vrefresh - 1;
>> > +	mr->max_vfreq = mode->vrefresh + 1;
>> > +	mr->min_hfreq_khz = (mode->clock / mode->htotal) - 1;
>> > +	mr->max_hfreq_khz = (mode->clock / mode->htotal) + 1;
>> > +	mr->pixel_clock_mhz = (mode->clock / 10000) + 1;
>> > +	mr->flags = 0;
>> > +
>> > +	static const char padding[] = {
>> > +		0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20
>> > +	};
>> > +	memcpy(mr->formula.pad, padding, sizeof(padding));
>> > +}
>> > +
>> > +/**
>> > + * detailed_timing_set_string: set a detailed timing to be a string
>> > + */
>> > +void detailed_timing_set_string(struct detailed_timing *dt,
>> > +				enum detailed_non_pixel_type type,
>> > +				const char *str)
>> > +{
>> > +	struct detailed_non_pixel *np = &dt->data.other_data;
>> > +	struct detailed_data_string *ds = &np->data.string;
>> > +	size_t len;
>> > +
>> > +	switch (type) {
>> > +	case EDID_DETAIL_MONITOR_NAME:
>> > +	case EDID_DETAIL_MONITOR_STRING:
>> > +	case EDID_DETAIL_MONITOR_SERIAL:
>> > +		break;
>> > +	default:
>> > +		assert(0); /* not a string type */
>> > +	}
>> > +
>> > +	dt->pixel_clock[0] = dt->pixel_clock[1] = 0;
>> > +
>> > +	np->type = type;
>> > +
>> > +	strncpy(ds->str, str, sizeof(ds->str));
>> > +	len = strlen(str);
>> > +	if (len < sizeof(ds->str))
>> > +		ds->str[len] = '\n';
>> > +}
>> > +
>> > +static void edid_set_mfg(struct edid *edid, char mfg[static 3])
>> > +{
>> > +	edid->mfg_id[0] = (mfg[0] - '@') << 2 | (mfg[1] - '@') >> 3;
>> > +	edid->mfg_id[1] = (mfg[1] - '@') << 5 | (mfg[2] - '@');
>> > +}
>> > +
>> > +static void edid_set_gamma(struct edid *edid, float gamma)
>> > +{
>> > +	edid->gamma = (gamma * 100) - 100;
>> > +}
>> > +
>> > +/**
>> > + * edid_init: initialize an EDID
>> > + *
>> > + * The EDID will be pre-filled with established and standard timings:
>> > + *
>> > + *  - 1920x1080 60Hz
>> > + *  - 1280x720 60Hz
>> > + *  - 1024x768 60Hz
>> > + *  - 800x600 60Hz
>> > + *  - 640x480 60Hz
>> > + */
>> > +void edid_init(struct edid *edid)
>> > +{
>> > +	size_t i;
>> > +	time_t t;
>> > +	struct tm *tm;
>> > +
>> > +	memset(edid, 0, sizeof(struct edid));
>> > +
>> > +	memcpy(edid->header, edid_header, sizeof(edid_header));
>> > +	edid_set_mfg(edid, "IGT");
>> > +	edid->version = 1;
>> > +	edid->revision = 3;
>> > +	edid->input = 0x80;
>> > +	edid->width_cm = 52;
>> > +	edid->height_cm = 30;
>> > +	edid_set_gamma(edid, 2.20);
>> > +	edid->features = 0x02;
>> > +
>> > +	/* Year of manufacture */
>> > +	t = time(NULL);
>> > +	tm = localtime(&t);
>> > +	edid->mfg_year = tm->tm_year - 90;
>> > +
>> > +	/* Established timings: 640x480 60Hz, 800x600 60Hz, 1024x768 60Hz */
>> > +	edid->established_timings.t1 = 0x21;
>> > +	edid->established_timings.t2 = 0x08;
>> > +
>> > +	/* Standard timings */
>> > +	/* 1920x1080 60Hz */
>> > +	std_timing_set(&edid->standard_timings[0], 1920, 60, STD_TIMING_16_9);
>> > +	/* 1280x720 60Hz */
>> > +	std_timing_set(&edid->standard_timings[1], 1280, 60, STD_TIMING_16_9);
>> > +	/* 1024x768 60Hz */
>> > +	std_timing_set(&edid->standard_timings[2], 1024, 60, STD_TIMING_4_3);
>> > +	/* 800x600 60Hz */
>> > +	std_timing_set(&edid->standard_timings[3], 800, 60, STD_TIMING_4_3);
>> > +	/* 640x480 60Hz */
>> > +	std_timing_set(&edid->standard_timings[4], 640, 60, STD_TIMING_4_3);
>> > +	for (i = 5; i < STD_TIMINGS_LEN; i++)
>> > +		std_timing_unset(&edid->standard_timings[i]);
>> > +}
>> > +
>> > +/**
>> > + * edid_init_with_mode: initialize an EDID and sets its preferred mode
>> > + */
>> > +void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode)
>> > +{
>> > +	edid_init(edid);
>> > +
>> > +	/* Preferred timing */
>> > +	detailed_timing_set_mode(&edid->detailed_timings[0], mode,
>> > +				 edid->width_cm * 10, edid->height_cm * 10);
>> > +	detailed_timing_set_monitor_range_mode(&edid->detailed_timings[1],
>> > +					       mode);
>> > +	detailed_timing_set_string(&edid->detailed_timings[2],
>> > +				   EDID_DETAIL_MONITOR_NAME, "IGT");
>> > +}
>> > +
>> > +/**
>> > + * edid_finalize: finalize an EDID by computing its checksum
>> > + */
>> > +void edid_finalize(struct edid *edid)
>> > +{
>> > +	size_t i;
>> > +	const uint8_t *buf = (const uint8_t *) edid;
>> > +	uint8_t sum = 0;
>> > +
>> > +	/* calculate checksum */
>> > +	for (i = 0; i < sizeof(struct edid) - 1; i++) {
>> > +		sum = sum + buf[i];
>> > +	}
>> > +
>> > +	edid->checksum = 256 - sum;
>> > +}
>> 
>> I get why using `finalize` here may seems tempting, but fini and
>> finalize tend to be used for destructors (or analogues in GCed
>> languages), so this may be a bit confusing. I think we should go with
>> edid_update_checksum() or similar.
>
> Agreed, a more explicit name is pobably not a bad idea.
>
>> Other than that, it's a solid improvement over what we had, which will
>> come handy the more we fiddle with EDIDs. Thanks!
>
> :)
>
> Will send a v3 with your comments fixed.
>
>> Do you already have some thoughts on how we want to solve handling the
>> extensions blocks?
>
> I've already started to work on this. I think callers should allocate a
> large enough buffer, e.g.:
>
>   char buf[sizeof(struct edid) + 2*sizeof(struct edid_ext)];
>
> (Extension blocks have a fixed size)
>
> And then should map a struct edid to it:
>
>   struct edid *edid = (struct edid *) buf;
>
> The annoying thing is that the number of extensions at the end of the
> EDID block isn't fixed. We could add a new field at the end of struct
> edid:
>
>   struct edid {
>     …
>     struct edid_ext extensions[];
>   };
>
> So that callers can do this:
>
>   struct edid_ext *ext1 = edid->extensions[0];
>   struct edid_ext *ext2 = edid->extensions[1];
>
> Another annoying thing is that extensions can themselves contain
> multiple sub-blocks of different sizes… For now I've opted for a char
> buffer that can be casted into sub-blocks. This isn't the best solution
> ever, but I haven't found a better design yet. Let's discuss about this
> in the next patch. :)

While it's easy to write something that's better than what we currently
have, as you see this gets complicated more quickly than you'd like,
especially working in C. We're not the only ones needing EDID
generation, so I'd suggest looking at what others are doing before
embarking on writing a full blown generator. See e.g. [1].

BR,
Jani.


[1] http://mid.mail-archive.com/20181121133624.i4vxgt7atnv3ads6@sirius.home.kraxel.org


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
  2019-04-16 11:05       ` Jani Nikula
@ 2019-04-16 12:11         ` Ser, Simon
  2019-04-16 13:23           ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Ser, Simon @ 2019-04-16 12:11 UTC (permalink / raw)
  To: Hiler, Arkadiusz, jani.nikula; +Cc: igt-dev

On Tue, 2019-04-16 at 14:05 +0300, Jani Nikula wrote:
> > > Do you already have some thoughts on how we want to solve handling the
> > > extensions blocks?
> > 
> > I've already started to work on this. I think callers should allocate a
> > large enough buffer, e.g.:
> > 
> >   char buf[sizeof(struct edid) + 2*sizeof(struct edid_ext)];
> > 
> > (Extension blocks have a fixed size)
> > 
> > And then should map a struct edid to it:
> > 
> >   struct edid *edid = (struct edid *) buf;
> > 
> > The annoying thing is that the number of extensions at the end of the
> > EDID block isn't fixed. We could add a new field at the end of struct
> > edid:
> > 
> >   struct edid {
> >     …
> >     struct edid_ext extensions[];
> >   };
> > 
> > So that callers can do this:
> > 
> >   struct edid_ext *ext1 = edid->extensions[0];
> >   struct edid_ext *ext2 = edid->extensions[1];
> > 
> > Another annoying thing is that extensions can themselves contain
> > multiple sub-blocks of different sizes… For now I've opted for a char
> > buffer that can be casted into sub-blocks. This isn't the best solution
> > ever, but I haven't found a better design yet. Let's discuss about this
> > in the next patch. :)
> 
> While it's easy to write something that's better than what we currently
> have, as you see this gets complicated more quickly than you'd like,
> especially working in C. We're not the only ones needing EDID
> generation, so I'd suggest looking at what others are doing before
> embarking on writing a full blown generator. See e.g. [1].
> 
> [1] http://mid.mail-archive.com/20181121133624.i4vxgt7atnv3ads6@sirius.home.kraxel.org

Thanks for the link! I can't find other EDID generators written in C,
it seems most people focus on parsing (and most people = a few…).

Here is the code for reference: [1].

It's really too simple for us. It takes the output identifier, the DPI
and the resolution and generates an EDID from that. It also operates on
a raw `char *` buffer which makes getting the indexes right tricky. All
in all, not a lot we can learn from.

FWIW, I've been thnking about a possible "libedid" for a while. Many
projects need to parse/format EDIDs and it's a little unfortunate that
we need to duplicate code (especially when parsing also involves
applying quirks). See [2] for a discussion about this.

[1]: https://github.com/qemu/qemu/blob/master/hw/display/edid-generate.c
[2]: https://marc.info/?t=154555671400001&r=1&w=2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] lib/igt_edid: new library for generating EDIDs
  2019-04-16 12:11         ` Ser, Simon
@ 2019-04-16 13:23           ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2019-04-16 13:23 UTC (permalink / raw)
  To: Ser, Simon, Hiler, Arkadiusz; +Cc: igt-dev

On Tue, 16 Apr 2019, "Ser, Simon" <simon.ser@intel.com> wrote:
> On Tue, 2019-04-16 at 14:05 +0300, Jani Nikula wrote:
>> > > Do you already have some thoughts on how we want to solve handling the
>> > > extensions blocks?
>> > 
>> > I've already started to work on this. I think callers should allocate a
>> > large enough buffer, e.g.:
>> > 
>> >   char buf[sizeof(struct edid) + 2*sizeof(struct edid_ext)];
>> > 
>> > (Extension blocks have a fixed size)
>> > 
>> > And then should map a struct edid to it:
>> > 
>> >   struct edid *edid = (struct edid *) buf;
>> > 
>> > The annoying thing is that the number of extensions at the end of the
>> > EDID block isn't fixed. We could add a new field at the end of struct
>> > edid:
>> > 
>> >   struct edid {
>> >     …
>> >     struct edid_ext extensions[];
>> >   };
>> > 
>> > So that callers can do this:
>> > 
>> >   struct edid_ext *ext1 = edid->extensions[0];
>> >   struct edid_ext *ext2 = edid->extensions[1];
>> > 
>> > Another annoying thing is that extensions can themselves contain
>> > multiple sub-blocks of different sizes… For now I've opted for a char
>> > buffer that can be casted into sub-blocks. This isn't the best solution
>> > ever, but I haven't found a better design yet. Let's discuss about this
>> > in the next patch. :)
>> 
>> While it's easy to write something that's better than what we currently
>> have, as you see this gets complicated more quickly than you'd like,
>> especially working in C. We're not the only ones needing EDID
>> generation, so I'd suggest looking at what others are doing before
>> embarking on writing a full blown generator. See e.g. [1].
>> 
>> [1] http://mid.mail-archive.com/20181121133624.i4vxgt7atnv3ads6@sirius.home.kraxel.org
>
> Thanks for the link! I can't find other EDID generators written in C,
> it seems most people focus on parsing (and most people = a few…).
>
> Here is the code for reference: [1].
>
> It's really too simple for us. It takes the output identifier, the DPI
> and the resolution and generates an EDID from that. It also operates on
> a raw `char *` buffer which makes getting the indexes right tricky. All
> in all, not a lot we can learn from.

Fair enough, thanks for looking into it.

> FWIW, I've been thnking about a possible "libedid" for a while. Many
> projects need to parse/format EDIDs and it's a little unfortunate that
> we need to duplicate code (especially when parsing also involves
> applying quirks). See [2] for a discussion about this.

For starters I think a libedid could start within igt, as long as you
make sure it remains independent of the rampant igt-isms.

BR,
Jani.


>
> [1]: https://github.com/qemu/qemu/blob/master/hw/display/edid-generate.c
> [2]: https://marc.info/?t=154555671400001&r=1&w=2

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-04-16 13:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15 12:38 [igt-dev] [PATCH i-g-t] lib/igt_edid: new library for generating EDIDs Simon Ser
2019-04-15 12:51 ` Petri Latvala
2019-04-15 13:32 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-04-15 14:20 ` [igt-dev] [PATCH i-g-t v2] " Simon Ser
2019-04-16  8:11   ` Arkadiusz Hiler
2019-04-16  8:46     ` Ser, Simon
2019-04-16 11:05       ` Jani Nikula
2019-04-16 12:11         ` Ser, Simon
2019-04-16 13:23           ` Jani Nikula
2019-04-15 14:58 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork
2019-04-15 15:09 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs Patchwork
2019-04-15 16:46 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/igt_edid: new library for generating EDIDs (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.