[PATCH 0/9] edid: windows fixes

[PATCH 0/9] edid: windows fixes

[Gerd Hoffmann]

Windows guests are not that happy with the edid blob we provide.
Tweak things a bit to make windows pick up more resolutions.

Also rebased all pending edid patches on top of that, so this
is the latest "all in one" patch series.

Akihiko Odaki (1):
  edid: Make refresh rate configurable

Gerd Hoffmann (5):
  qemu-edid: use qemu_edid_size()
  edid: edid_desc_next
  edid: move xtra3 descriptor
  edid: use dta extension block descriptors
  edid: prefer standard timings

Konstantin Nazarov (3):
  edid: move timing generation into a separate function
  edid: allow arbitrary-length checksums
  edid: add support for DisplayID extension (5k resolution)

 include/hw/display/edid.h  |  12 +-
 hw/display/edid-generate.c | 247 +++++++++++++++++++++++++++----------
 hw/display/vga-pci.c       |   2 +-
 qemu-edid.c                |   6 +-
 4 files changed, 191 insertions(+), 76 deletions(-)

-- 
2.30.2

[PATCH 1/9] qemu-edid: use qemu_edid_size()

[Gerd Hoffmann]

So we only write out that part of the edid blob
which has been filled with data.
Also use a larger buffer for the blob.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-edid.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qemu-edid.c b/qemu-edid.c
index 1cd6a9517238..c3a9fba10dc4 100644
--- a/qemu-edid.c
+++ b/qemu-edid.c
@@ -41,7 +41,8 @@ static void usage(FILE *out)
 int main(int argc, char *argv[])
 {
     FILE *outfile = NULL;
-    uint8_t blob[256];
+    uint8_t blob[512];
+    size_t size;
     uint32_t dpi = 100;
     int rc;
 
@@ -119,7 +120,8 @@ int main(int argc, char *argv[])
 
     memset(blob, 0, sizeof(blob));
     qemu_edid_generate(blob, sizeof(blob), &info);
-    fwrite(blob, sizeof(blob), 1, outfile);
+    size = qemu_edid_size(blob);
+    fwrite(blob, size, 1, outfile);
     fflush(outfile);
 
     exit(0);
-- 
2.30.2

Re: [PATCH 1/9] qemu-edid: use qemu_edid_size()

[Philippe Mathieu-Daudé]

On 3/16/21 3:38 PM, Gerd Hoffmann wrote:
> So we only write out that part of the edid blob
> which has been filled with data.
> Also use a larger buffer for the blob.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-edid.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 2/9] edid: edid_desc_next

[Gerd Hoffmann]

Add helper function to find the next free desc block.
Needed when we start to use the dta descriptor entries.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 41 ++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 1665b7cbb29a..ef7a70fc9654 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -143,6 +143,17 @@ static void edid_checksum(uint8_t *edid)
     }
 }
 
+static uint8_t *edid_desc_next(uint8_t *edid, uint8_t *dta, uint8_t *desc)
+{
+    if (desc == NULL) {
+        return NULL;
+    }
+    if (desc + 18 + 18 < edid + 127) {
+        return desc + 18;
+    }
+    return NULL;
+}
+
 static void edid_desc_type(uint8_t *desc, uint8_t type)
 {
     desc[0] = 0;
@@ -299,7 +310,7 @@ uint32_t qemu_edid_dpi_to_mm(uint32_t dpi, uint32_t res)
 void qemu_edid_generate(uint8_t *edid, size_t size,
                         qemu_edid_info *info)
 {
-    uint32_t desc = 54;
+    uint8_t *desc = edid + 54;
     uint8_t *xtra3 = NULL;
     uint8_t *dta = NULL;
     uint32_t width_mm, height_mm;
@@ -400,32 +411,32 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 
     /* =============== descriptor blocks =============== */
 
-    edid_desc_timing(edid + desc, info->prefx, info->prefy,
+    edid_desc_timing(desc, info->prefx, info->prefy,
                      width_mm, height_mm);
-    desc += 18;
+    desc = edid_desc_next(edid, dta, desc);
 
-    edid_desc_ranges(edid + desc);
-    desc += 18;
+    edid_desc_ranges(desc);
+    desc = edid_desc_next(edid, dta, desc);
 
     if (info->name) {
-        edid_desc_text(edid + desc, 0xfc, info->name);
-        desc += 18;
+        edid_desc_text(desc, 0xfc, info->name);
+        desc = edid_desc_next(edid, dta, desc);
     }
 
     if (info->serial) {
-        edid_desc_text(edid + desc, 0xff, info->serial);
-        desc += 18;
+        edid_desc_text(desc, 0xff, info->serial);
+        desc = edid_desc_next(edid, dta, desc);
     }
 
-    if (desc < 126) {
-        xtra3 = edid + desc;
+    if (desc) {
+        xtra3 = desc;
         edid_desc_xtra3_std(xtra3);
-        desc += 18;
+        desc = edid_desc_next(edid, dta, desc);
     }
 
-    while (desc < 126) {
-        edid_desc_dummy(edid + desc);
-        desc += 18;
+    while (desc) {
+        edid_desc_dummy(desc);
+        desc = edid_desc_next(edid, dta, desc);
     }
 
     /* =============== finish up =============== */
-- 
2.30.2

[PATCH 3/9] edid: move xtra3 descriptor

[Gerd Hoffmann]

Initialize the "Established timings III" block earlier.  Also move up
edid_fill_modes().  That'll make sure the offset for the additional
descriptors in the dta block don't move any more, which in turn makes it
easier to actually use them.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index ef7a70fc9654..259ef7315217 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -415,25 +415,28 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
                      width_mm, height_mm);
     desc = edid_desc_next(edid, dta, desc);
 
+    xtra3 = desc;
+    edid_desc_xtra3_std(xtra3);
+    desc = edid_desc_next(edid, dta, desc);
+    edid_fill_modes(edid, xtra3, dta, info->maxx, info->maxy);
+    /*
+     * dta video data block is finished at thus point,
+     * so dta descriptor offsets don't move any more.
+     */
+
     edid_desc_ranges(desc);
     desc = edid_desc_next(edid, dta, desc);
 
-    if (info->name) {
+    if (desc && info->name) {
         edid_desc_text(desc, 0xfc, info->name);
         desc = edid_desc_next(edid, dta, desc);
     }
 
-    if (info->serial) {
+    if (desc && info->serial) {
         edid_desc_text(desc, 0xff, info->serial);
         desc = edid_desc_next(edid, dta, desc);
     }
 
-    if (desc) {
-        xtra3 = desc;
-        edid_desc_xtra3_std(xtra3);
-        desc = edid_desc_next(edid, dta, desc);
-    }
-
     while (desc) {
         edid_desc_dummy(desc);
         desc = edid_desc_next(edid, dta, desc);
@@ -441,7 +444,6 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 
     /* =============== finish up =============== */
 
-    edid_fill_modes(edid, xtra3, dta, info->maxx, info->maxy);
     edid_checksum(edid);
     if (dta) {
         edid_checksum(dta);
-- 
2.30.2

[PATCH 4/9] edid: use dta extension block descriptors

[Gerd Hoffmann]

When the 4 descriptors in the base edid block are filled, jump to the
dta extension block.  This allows for more than four descriptors.
Happens for example when generating an edid blob with a serial number
(qemu-edid -s $serial).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 259ef7315217..489532c3bc5f 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -151,6 +151,14 @@ static uint8_t *edid_desc_next(uint8_t *edid, uint8_t *dta, uint8_t *desc)
     if (desc + 18 + 18 < edid + 127) {
         return desc + 18;
     }
+    if (dta) {
+        if (desc < edid + 127) {
+            return dta + dta[2];
+        }
+        if (desc + 18 + 18 < dta + 127) {
+            return desc + 18;
+        }
+    }
     return NULL;
 }
 
-- 
2.30.2

[PATCH 5/9] edid: prefer standard timings

[Gerd Hoffmann]

Windows guests using the "Basic Display Adapter" don't parse the
"Established timings III" block.  They also don't parse any edid
extension.

So prefer the "Standard Timings" block to store the display resolutions
in edid_fill_modes().  Also reorder the mode list, so more exotic
resolutions (specifically the ones which are not supported by vgabios)
are moved down and the remaining ones have a better chance to get one of
the eight slots in the "Standard Timings" block.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 489532c3bc5f..42a130f0ff5c 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -25,19 +25,20 @@ static const struct edid_mode {
     { .xres = 1920,   .yres = 1080,   .dta =  31 },
 
     /* additional standard timings 3 (all @ 60Hz) */
-    { .xres = 1920,   .yres = 1440,   .xtra3 = 11,   .bit = 5 },
     { .xres = 1920,   .yres = 1200,   .xtra3 = 10,   .bit = 0 },
-    { .xres = 1856,   .yres = 1392,   .xtra3 = 10,   .bit = 3 },
-    { .xres = 1792,   .yres = 1344,   .xtra3 = 10,   .bit = 5 },
     { .xres = 1600,   .yres = 1200,   .xtra3 =  9,   .bit = 2 },
     { .xres = 1680,   .yres = 1050,   .xtra3 =  9,   .bit = 5 },
-    { .xres = 1440,   .yres = 1050,   .xtra3 =  8,   .bit = 1 },
     { .xres = 1440,   .yres =  900,   .xtra3 =  8,   .bit = 5 },
-    { .xres = 1360,   .yres =  768,   .xtra3 =  8,   .bit = 7 },
     { .xres = 1280,   .yres = 1024,   .xtra3 =  7,   .bit = 1 },
     { .xres = 1280,   .yres =  960,   .xtra3 =  7,   .bit = 3 },
     { .xres = 1280,   .yres =  768,   .xtra3 =  7,   .bit = 6 },
 
+    { .xres = 1920,   .yres = 1440,   .xtra3 = 11,   .bit = 5 },
+    { .xres = 1856,   .yres = 1392,   .xtra3 = 10,   .bit = 3 },
+    { .xres = 1792,   .yres = 1344,   .xtra3 = 10,   .bit = 5 },
+    { .xres = 1440,   .yres = 1050,   .xtra3 =  8,   .bit = 1 },
+    { .xres = 1360,   .yres =  768,   .xtra3 =  8,   .bit = 7 },
+
     /* established timings (all @ 60Hz) */
     { .xres = 1024,   .yres =  768,   .byte  = 36,   .bit = 3 },
     { .xres =  800,   .yres =  600,   .byte  = 35,   .bit = 0 },
@@ -109,13 +110,13 @@ static void edid_fill_modes(uint8_t *edid, uint8_t *xtra3, uint8_t *dta,
 
         if (mode->byte) {
             edid[mode->byte] |= (1 << mode->bit);
-        } else if (mode->xtra3 && xtra3) {
-            xtra3[mode->xtra3] |= (1 << mode->bit);
         } else if (std < 54) {
             rc = edid_std_mode(edid + std, mode->xres, mode->yres);
             if (rc == 0) {
                 std += 2;
             }
+        } else if (mode->xtra3 && xtra3) {
+            xtra3[mode->xtra3] |= (1 << mode->bit);
         }
 
         if (dta && mode->dta) {
-- 
2.30.2

[PATCH 6/9] edid: Make refresh rate configurable

[Gerd Hoffmann]

From: Akihiko Odaki <akihiko.odaki@gmail.com>

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/display/edid.h  | 12 +++++++-----
 hw/display/edid-generate.c |  9 +++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
index 1f8fc9b37500..520f8ec20279 100644
--- a/include/hw/display/edid.h
+++ b/include/hw/display/edid.h
@@ -11,6 +11,7 @@ typedef struct qemu_edid_info {
     uint32_t    prefy;
     uint32_t    maxx;
     uint32_t    maxy;
+    uint32_t    refresh_rate;
 } qemu_edid_info;
 
 void qemu_edid_generate(uint8_t *edid, size_t size,
@@ -21,10 +22,11 @@ void qemu_edid_region_io(MemoryRegion *region, Object *owner,
 
 uint32_t qemu_edid_dpi_to_mm(uint32_t dpi, uint32_t res);
 
-#define DEFINE_EDID_PROPERTIES(_state, _edid_info)              \
-    DEFINE_PROP_UINT32("xres", _state, _edid_info.prefx, 0),    \
-    DEFINE_PROP_UINT32("yres", _state, _edid_info.prefy, 0),    \
-    DEFINE_PROP_UINT32("xmax", _state, _edid_info.maxx, 0),     \
-    DEFINE_PROP_UINT32("ymax", _state, _edid_info.maxy, 0)
+#define DEFINE_EDID_PROPERTIES(_state, _edid_info)                         \
+    DEFINE_PROP_UINT32("xres", _state, _edid_info.prefx, 0),               \
+    DEFINE_PROP_UINT32("yres", _state, _edid_info.prefy, 0),               \
+    DEFINE_PROP_UINT32("xmax", _state, _edid_info.maxx, 0),                \
+    DEFINE_PROP_UINT32("ymax", _state, _edid_info.maxy, 0),                \
+    DEFINE_PROP_UINT32("refresh_rate", _state, _edid_info.refresh_rate, 0)
 
 #endif /* EDID_H */
diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 42a130f0ff5c..8662218822f6 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -223,7 +223,7 @@ static void edid_desc_dummy(uint8_t *desc)
     edid_desc_type(desc, 0x10);
 }
 
-static void edid_desc_timing(uint8_t *desc,
+static void edid_desc_timing(uint8_t *desc, uint32_t refresh_rate,
                              uint32_t xres, uint32_t yres,
                              uint32_t xmm, uint32_t ymm)
 {
@@ -236,9 +236,9 @@ static void edid_desc_timing(uint8_t *desc,
     uint32_t ysync  = yres *  5 / 1000;
     uint32_t yblank = yres * 35 / 1000;
 
-    uint32_t clock  = 75 * (xres + xblank) * (yres + yblank);
+    uint64_t clock  = (uint64_t)refresh_rate * (xres + xblank) * (yres + yblank);
 
-    stl_le_p(desc, clock / 10000);
+    stl_le_p(desc, clock / 10000000);
 
     desc[2] = xres   & 0xff;
     desc[3] = xblank & 0xff;
@@ -323,6 +323,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     uint8_t *xtra3 = NULL;
     uint8_t *dta = NULL;
     uint32_t width_mm, height_mm;
+    uint32_t refresh_rate = info->refresh_rate ? info->refresh_rate : 75000;
     uint32_t dpi = 100; /* if no width_mm/height_mm */
 
     /* =============== set defaults  =============== */
@@ -420,7 +421,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 
     /* =============== descriptor blocks =============== */
 
-    edid_desc_timing(desc, info->prefx, info->prefy,
+    edid_desc_timing(desc, refresh_rate, info->prefx, info->prefy,
                      width_mm, height_mm);
     desc = edid_desc_next(edid, dta, desc);
 
-- 
2.30.2

[PATCH 7/9] edid: move timing generation into a separate function

[Gerd Hoffmann]

From: Konstantin Nazarov <mail@knazarov.com>

The timing generation is currently performed inside the function that
fills in the DTD. The DisplayID generation needs it as well, so moving
it out to a separate function.

Based-on: <20210303152948.59943-2-akihiko.odaki@gmail.com>
Signed-off-by: Konstantin Nazarov <mail@knazarov.com>
Message-Id: <20210315114639.91953-1-mail@knazarov.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 68 ++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 8662218822f6..b70ab1557e50 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -45,6 +45,35 @@ static const struct edid_mode {
     { .xres =  640,   .yres =  480,   .byte  = 35,   .bit = 5 },
 };
 
+typedef struct Timings {
+    uint32_t xfront;
+    uint32_t xsync;
+    uint32_t xblank;
+
+    uint32_t yfront;
+    uint32_t ysync;
+    uint32_t yblank;
+
+    uint64_t clock;
+} Timings;
+
+static void generate_timings(Timings *timings, uint32_t refresh_rate,
+                             uint32_t xres, uint32_t yres)
+{
+    /* pull some realistic looking timings out of thin air */
+    timings->xfront = xres * 25 / 100;
+    timings->xsync  = xres *  3 / 100;
+    timings->xblank = xres * 35 / 100;
+
+    timings->yfront = yres *  5 / 1000;
+    timings->ysync  = yres *  5 / 1000;
+    timings->yblank = yres * 35 / 1000;
+
+    timings->clock  = ((uint64_t)refresh_rate *
+                       (xres + timings->xblank) *
+                       (yres + timings->yblank)) / 10000000;
+}
+
 static void edid_ext_dta(uint8_t *dta)
 {
     dta[0] = 0x02;
@@ -227,38 +256,29 @@ static void edid_desc_timing(uint8_t *desc, uint32_t refresh_rate,
                              uint32_t xres, uint32_t yres,
                              uint32_t xmm, uint32_t ymm)
 {
-    /* pull some realistic looking timings out of thin air */
-    uint32_t xfront = xres * 25 / 100;
-    uint32_t xsync  = xres *  3 / 100;
-    uint32_t xblank = xres * 35 / 100;
-
-    uint32_t yfront = yres *  5 / 1000;
-    uint32_t ysync  = yres *  5 / 1000;
-    uint32_t yblank = yres * 35 / 1000;
-
-    uint64_t clock  = (uint64_t)refresh_rate * (xres + xblank) * (yres + yblank);
-
-    stl_le_p(desc, clock / 10000000);
+    Timings timings;
+    generate_timings(&timings, refresh_rate, xres, yres);
+    stl_le_p(desc, timings.clock);
 
     desc[2] = xres   & 0xff;
-    desc[3] = xblank & 0xff;
+    desc[3] = timings.xblank & 0xff;
     desc[4] = (((xres   & 0xf00) >> 4) |
-               ((xblank & 0xf00) >> 8));
+               ((timings.xblank & 0xf00) >> 8));
 
     desc[5] = yres   & 0xff;
-    desc[6] = yblank & 0xff;
+    desc[6] = timings.yblank & 0xff;
     desc[7] = (((yres   & 0xf00) >> 4) |
-               ((yblank & 0xf00) >> 8));
+               ((timings.yblank & 0xf00) >> 8));
 
-    desc[8] = xfront & 0xff;
-    desc[9] = xsync  & 0xff;
+    desc[8] = timings.xfront & 0xff;
+    desc[9] = timings.xsync  & 0xff;
 
-    desc[10] = (((yfront & 0x00f) << 4) |
-                ((ysync  & 0x00f) << 0));
-    desc[11] = (((xfront & 0x300) >> 2) |
-                ((xsync  & 0x300) >> 4) |
-                ((yfront & 0x030) >> 2) |
-                ((ysync  & 0x030) >> 4));
+    desc[10] = (((timings.yfront & 0x00f) << 4) |
+                ((timings.ysync  & 0x00f) << 0));
+    desc[11] = (((timings.xfront & 0x300) >> 2) |
+                ((timings.xsync  & 0x300) >> 4) |
+                ((timings.yfront & 0x030) >> 2) |
+                ((timings.ysync  & 0x030) >> 4));
 
     desc[12] = xmm & 0xff;
     desc[13] = ymm & 0xff;
-- 
2.30.2

[PATCH 8/9] edid: allow arbitrary-length checksums

[Gerd Hoffmann]

From: Konstantin Nazarov <mail@knazarov.com>

Some of the EDID extensions like DisplayID do checksums of their
subsections. Currently checksums can be only applied to the whole
extension blocks which are 128 bytes.

This patch allows to checksum arbitrary parts of EDID, and not only
whole extension blocks.

Based-on: <20210303152948.59943-2-akihiko.odaki@gmail.com>
Signed-off-by: Konstantin Nazarov <mail@knazarov.com>
Message-Id: <20210315114639.91953-2-mail@knazarov.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index b70ab1557e50..bdd01571fc9b 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -159,17 +159,17 @@ static void edid_fill_modes(uint8_t *edid, uint8_t *xtra3, uint8_t *dta,
     }
 }
 
-static void edid_checksum(uint8_t *edid)
+static void edid_checksum(uint8_t *edid, size_t len)
 {
     uint32_t sum = 0;
     int i;
 
-    for (i = 0; i < 127; i++) {
+    for (i = 0; i < len; i++) {
         sum += edid[i];
     }
     sum &= 0xff;
     if (sum) {
-        edid[127] = 0x100 - sum;
+        edid[len] = 0x100 - sum;
     }
 }
 
@@ -474,9 +474,9 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 
     /* =============== finish up =============== */
 
-    edid_checksum(edid);
+    edid_checksum(edid, 127);
     if (dta) {
-        edid_checksum(dta);
+        edid_checksum(dta, 127);
     }
 }
 
-- 
2.30.2

[PATCH 9/9] edid: add support for DisplayID extension (5k resolution)

[Gerd Hoffmann]

From: Konstantin Nazarov <mail@knazarov.com>

The Detailed Timing Descriptor has only 12 bits to store the
resolution. This limits the guest to 4095 pixels.

This patch adds support for the DisplayID extension, that has 2 full
bytes for that purpose, thus allowing 5k resolutions and above.

Based-on: <20210303152948.59943-2-akihiko.odaki@gmail.com>
Signed-off-by: Konstantin Nazarov <mail@knazarov.com>
Message-Id: <20210315114639.91953-3-mail@knazarov.com>

[ kraxel: minor workflow tweaks ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/edid-generate.c | 78 +++++++++++++++++++++++++++++++++++---
 hw/display/vga-pci.c       |  2 +-
 2 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index bdd01571fc9b..f2b874d5e358 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -229,8 +229,8 @@ static void edid_desc_ranges(uint8_t *desc)
     desc[7] =  30;
     desc[8] = 160;
 
-    /* max dot clock (1200 MHz) */
-    desc[9] = 1200 / 10;
+    /* max dot clock (2550 MHz) */
+    desc[9] = 2550 / 10;
 
     /* no extended timing information */
     desc[10] = 0x01;
@@ -336,15 +336,61 @@ uint32_t qemu_edid_dpi_to_mm(uint32_t dpi, uint32_t res)
     return res * 254 / 10 / dpi;
 }
 
+static void init_displayid(uint8_t *did)
+{
+    did[0] = 0x70; /* display id extension */
+    did[1] = 0x13; /* version 1.3 */
+    did[2] = 4;    /* length */
+    did[3] = 0x03; /* product type (0x03 == standalone display device) */
+    edid_checksum(did + 1, did[2] + 4);
+}
+
+static void qemu_displayid_generate(uint8_t *did, uint32_t refresh_rate,
+                                    uint32_t xres, uint32_t yres,
+                                    uint32_t xmm, uint32_t ymm)
+{
+    Timings timings;
+    generate_timings(&timings, refresh_rate, xres, yres);
+
+    did[0] = 0x70; /* display id extension */
+    did[1] = 0x13; /* version 1.3 */
+    did[2] = 23;   /* length */
+    did[3] = 0x03; /* product type (0x03 == standalone display device) */
+
+    did[5] = 0x03; /* Detailed Timings Data Block */
+    did[6] = 0x00; /* revision */
+    did[7] = 0x14; /* block length */
+
+    did[8]  = timings.clock  & 0xff;
+    did[9]  = (timings.clock & 0xff00) >> 8;
+    did[10] = (timings.clock & 0xff0000) >> 16;
+
+    did[11] = 0x88; /* leave aspect ratio undefined */
+
+    stw_le_p(did + 12, 0xffff & (xres - 1));
+    stw_le_p(did + 14, 0xffff & (timings.xblank - 1));
+    stw_le_p(did + 16, 0xffff & (timings.xfront - 1));
+    stw_le_p(did + 18, 0xffff & (timings.xsync - 1));
+
+    stw_le_p(did + 20, 0xffff & (yres - 1));
+    stw_le_p(did + 22, 0xffff & (timings.yblank - 1));
+    stw_le_p(did + 24, 0xffff & (timings.yfront - 1));
+    stw_le_p(did + 26, 0xffff & (timings.ysync - 1));
+
+    edid_checksum(did + 1, did[2] + 4);
+}
+
 void qemu_edid_generate(uint8_t *edid, size_t size,
                         qemu_edid_info *info)
 {
     uint8_t *desc = edid + 54;
     uint8_t *xtra3 = NULL;
     uint8_t *dta = NULL;
+    uint8_t *did = NULL;
     uint32_t width_mm, height_mm;
     uint32_t refresh_rate = info->refresh_rate ? info->refresh_rate : 75000;
     uint32_t dpi = 100; /* if no width_mm/height_mm */
+    uint32_t large_screen = 0;
 
     /* =============== set defaults  =============== */
 
@@ -360,6 +406,9 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     if (!info->prefy) {
         info->prefy = 768;
     }
+    if (info->prefx >= 4096 || info->prefy >= 4096) {
+        large_screen = 1;
+    }
     if (info->width_mm && info->height_mm) {
         width_mm = info->width_mm;
         height_mm = info->height_mm;
@@ -377,6 +426,12 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
         edid_ext_dta(dta);
     }
 
+    if (size >= 384 && large_screen) {
+        did = edid + 256;
+        edid[126]++;
+        init_displayid(did);
+    }
+
     /* =============== header information =============== */
 
     /* fixed */
@@ -441,9 +496,12 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 
     /* =============== descriptor blocks =============== */
 
-    edid_desc_timing(desc, refresh_rate, info->prefx, info->prefy,
-                     width_mm, height_mm);
-    desc = edid_desc_next(edid, dta, desc);
+    if (!large_screen) {
+        /* The DTD section has only 12 bits to store the resolution */
+        edid_desc_timing(desc, refresh_rate, info->prefx, info->prefy,
+                         width_mm, height_mm);
+        desc = edid_desc_next(edid, dta, desc);
+    }
 
     xtra3 = desc;
     edid_desc_xtra3_std(xtra3);
@@ -472,12 +530,22 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
         desc = edid_desc_next(edid, dta, desc);
     }
 
+    /* =============== display id extensions =============== */
+
+    if (did && large_screen) {
+        qemu_displayid_generate(did, refresh_rate, info->prefx, info->prefy,
+                                width_mm, height_mm);
+    }
+
     /* =============== finish up =============== */
 
     edid_checksum(edid, 127);
     if (dta) {
         edid_checksum(dta, 127);
     }
+    if (did) {
+        edid_checksum(did, 127);
+    }
 }
 
 size_t qemu_edid_size(uint8_t *edid)
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index 48d29630ab77..62fb5c38c1fd 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -49,7 +49,7 @@ struct PCIVGAState {
     qemu_edid_info edid_info;
     MemoryRegion mmio;
     MemoryRegion mrs[4];
-    uint8_t edid[256];
+    uint8_t edid[384];
 };
 
 #define TYPE_PCI_VGA "pci-vga"
-- 
2.30.2

Re: [PATCH 9/9] edid: add support for DisplayID extension (5k resolution)

[Philippe Mathieu-Daudé]

On 3/16/21 3:38 PM, Gerd Hoffmann wrote:
> From: Konstantin Nazarov <mail@knazarov.com>
> 
> The Detailed Timing Descriptor has only 12 bits to store the
> resolution. This limits the guest to 4095 pixels.
> 
> This patch adds support for the DisplayID extension, that has 2 full
> bytes for that purpose, thus allowing 5k resolutions and above.
> 
> Based-on: <20210303152948.59943-2-akihiko.odaki@gmail.com>
> Signed-off-by: Konstantin Nazarov <mail@knazarov.com>
> Message-Id: <20210315114639.91953-3-mail@knazarov.com>
> 
> [ kraxel: minor workflow tweaks ]
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/edid-generate.c | 78 +++++++++++++++++++++++++++++++++++---
>  hw/display/vga-pci.c       |  2 +-
>  2 files changed, 74 insertions(+), 6 deletions(-)

> +static void qemu_displayid_generate(uint8_t *did, uint32_t refresh_rate,
> +                                    uint32_t xres, uint32_t yres,
> +                                    uint32_t xmm, uint32_t ymm)
> +{
> +    Timings timings;
> +    generate_timings(&timings, refresh_rate, xres, yres);
> +
> +    did[0] = 0x70; /* display id extension */
> +    did[1] = 0x13; /* version 1.3 */
> +    did[2] = 23;   /* length */
> +    did[3] = 0x03; /* product type (0x03 == standalone display device) */
> +
> +    did[5] = 0x03; /* Detailed Timings Data Block */
> +    did[6] = 0x00; /* revision */
> +    did[7] = 0x14; /* block length */
> +
> +    did[8]  = timings.clock  & 0xff;
> +    did[9]  = (timings.clock & 0xff00) >> 8;
> +    did[10] = (timings.clock & 0xff0000) >> 16;
> +
> +    did[11] = 0x88; /* leave aspect ratio undefined */
> +
> +    stw_le_p(did + 12, 0xffff & (xres - 1));
> +    stw_le_p(did + 14, 0xffff & (timings.xblank - 1));
> +    stw_le_p(did + 16, 0xffff & (timings.xfront - 1));
> +    stw_le_p(did + 18, 0xffff & (timings.xsync - 1));
> +
> +    stw_le_p(did + 20, 0xffff & (yres - 1));
> +    stw_le_p(did + 22, 0xffff & (timings.yblank - 1));
> +    stw_le_p(did + 24, 0xffff & (timings.yfront - 1));
> +    stw_le_p(did + 26, 0xffff & (timings.ysync - 1));

Pointless 0xffff mask.

> +
> +    edid_checksum(did + 1, did[2] + 4);
> +}

Re: [PATCH 0/9] edid: windows fixes

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210316143812.2363588-1-kraxel@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210316143812.2363588-1-kraxel@redhat.com
Subject: [PATCH 0/9] edid: windows fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20210212150256.885-1-zhiwei_liu@c-sky.com -> patchew/20210212150256.885-1-zhiwei_liu@c-sky.com
 - [tag update]      patchew/20210316135907.3646901-1-armbru@redhat.com -> patchew/20210316135907.3646901-1-armbru@redhat.com
 * [new tag]         patchew/20210316143812.2363588-1-kraxel@redhat.com -> patchew/20210316143812.2363588-1-kraxel@redhat.com
Switched to a new branch 'test'
abfdb3e edid: add support for DisplayID extension (5k resolution)
d330534 edid: allow arbitrary-length checksums
7543da1 edid: move timing generation into a separate function
0c42894 edid: Make refresh rate configurable
d12afa5 edid: prefer standard timings
5f683a4 edid: use dta extension block descriptors
c6dda2c edid: move xtra3 descriptor
572e698 edid: edid_desc_next
29d53b8 qemu-edid: use qemu_edid_size()

=== OUTPUT BEGIN ===
1/9 Checking commit 29d53b84d897 (qemu-edid: use qemu_edid_size())
2/9 Checking commit 572e698aa678 (edid: edid_desc_next)
3/9 Checking commit c6dda2cef0ef (edid: move xtra3 descriptor)
4/9 Checking commit 5f683a4f31e1 (edid: use dta extension block descriptors)
5/9 Checking commit d12afa5e5dd1 (edid: prefer standard timings)
6/9 Checking commit 0c42894ad0e4 (edid: Make refresh rate configurable)
WARNING: line over 80 characters
#33: FILE: hw/display/edid-generate.c:239:
+    uint64_t clock  = (uint64_t)refresh_rate * (xres + xblank) * (yres + yblank);

ERROR: Macros with complex values should be enclosed in parenthesis
#78: FILE: include/hw/display/edid.h:25:
+#define DEFINE_EDID_PROPERTIES(_state, _edid_info)                         \
+    DEFINE_PROP_UINT32("xres", _state, _edid_info.prefx, 0),               \
+    DEFINE_PROP_UINT32("yres", _state, _edid_info.prefy, 0),               \
+    DEFINE_PROP_UINT32("xmax", _state, _edid_info.maxx, 0),                \
+    DEFINE_PROP_UINT32("ymax", _state, _edid_info.maxy, 0),                \
+    DEFINE_PROP_UINT32("refresh_rate", _state, _edid_info.refresh_rate, 0)

total: 1 errors, 1 warnings, 57 lines checked

Patch 6/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/9 Checking commit 7543da15b755 (edid: move timing generation into a separate function)
8/9 Checking commit d330534fc64b (edid: allow arbitrary-length checksums)
9/9 Checking commit abfdb3e0f116 (edid: add support for DisplayID extension (5k resolution))
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210316143812.2363588-1-kraxel@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com