[PATCH v2 0/8] edid: windows fixes

[PATCH v2 0/8] 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.

v2:
 - rebase the latest master.
 - drop one bugfix patch which was added to 6.0.

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

Gerd Hoffmann (4):
  qemu-edid: use qemu_edid_size()
  edid: edid_desc_next
  edid: move xtra3 descriptor
  edid: use dta extension block descriptors

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 | 232 +++++++++++++++++++++++++++----------
 hw/display/vga-pci.c       |   2 +-
 qemu-edid.c                |   6 +-
 4 files changed, 183 insertions(+), 69 deletions(-)

-- 
2.30.2

[PATCH v2 1/8] 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 v2 1/8] qemu-edid: use qemu_edid_size()

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

On Tue, Apr 27, 2021 at 7:15 PM Gerd Hoffmann <kraxel@redhat.com> 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>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1944 bytes --]

[PATCH v2 2/8] 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 a1bea9a3aa35..ae34999f9eb5 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -144,6 +144,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;
@@ -300,7 +311,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;
@@ -401,32 +412,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

Re: [PATCH v2 2/8] edid: edid_desc_next

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]

On Tue, Apr 27, 2021 at 7:09 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> 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>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@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 a1bea9a3aa35..ae34999f9eb5 100644
> --- a/hw/display/edid-generate.c
> +++ b/hw/display/edid-generate.c
> @@ -144,6 +144,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;
> @@ -300,7 +311,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;
> @@ -401,32 +412,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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4049 bytes --]

[PATCH v2 3/8] 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 ae34999f9eb5..25f790c7bd85 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -416,25 +416,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);
@@ -442,7 +445,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

Re: [PATCH v2 3/8] edid: move xtra3 descriptor

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]

On Tue, Apr 27, 2021 at 7:20 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> 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>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@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 ae34999f9eb5..25f790c7bd85 100644
> --- a/hw/display/edid-generate.c
> +++ b/hw/display/edid-generate.c
> @@ -416,25 +416,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);
> @@ -442,7 +445,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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3277 bytes --]

[PATCH v2 4/8] 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 25f790c7bd85..42a130f0ff5c 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -152,6 +152,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

Re: [PATCH v2 4/8] edid: use dta extension block descriptors

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]

On Tue, Apr 27, 2021 at 7:23 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> 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>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@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 25f790c7bd85..42a130f0ff5c 100644
> --- a/hw/display/edid-generate.c
> +++ b/hw/display/edid-generate.c
> @@ -152,6 +152,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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1981 bytes --]

[PATCH v2 5/8] 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 v2 6/8] 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 v2 7/8] 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 v2 8/8] 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 v2 0/8] edid: windows fixes

[no-reply]

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



Hi,

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

Type: series
Message-id: 20210427150824.638359-1-kraxel@redhat.com
Subject: [PATCH v2 0/8] 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
 * [new tag]         patchew/20210427150824.638359-1-kraxel@redhat.com -> patchew/20210427150824.638359-1-kraxel@redhat.com
Switched to a new branch 'test'
b31a1b5 edid: add support for DisplayID extension (5k resolution)
e399779 edid: allow arbitrary-length checksums
9491d39 edid: move timing generation into a separate function
ae580e9 edid: Make refresh rate configurable
241f41b edid: use dta extension block descriptors
befe944 edid: move xtra3 descriptor
e031225 edid: edid_desc_next
fd14046 qemu-edid: use qemu_edid_size()

=== OUTPUT BEGIN ===
1/8 Checking commit fd14046ca07b (qemu-edid: use qemu_edid_size())
2/8 Checking commit e031225d558f (edid: edid_desc_next)
3/8 Checking commit befe94451d11 (edid: move xtra3 descriptor)
4/8 Checking commit 241f41bfe772 (edid: use dta extension block descriptors)
5/8 Checking commit ae580e9b2d33 (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 5/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/8 Checking commit 9491d3944b98 (edid: move timing generation into a separate function)
7/8 Checking commit e39977915fbb (edid: allow arbitrary-length checksums)
8/8 Checking commit b31a1b5618ad (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/20210427150824.638359-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