From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add width_mm/height_mm to qemu_edid_info, and use it if it is
set (non-zero) to generate the EDID.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/edid-generate.c | 21 +++++++++++++--------
 include/hw/display/edid.h  |  2 ++
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index e58472fde5..14cfb94447 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -205,12 +205,8 @@ static void edid_desc_dummy(uint8_t *desc)
 
 static void edid_desc_timing(uint8_t *desc,
                              uint32_t xres, uint32_t yres,
-                             uint32_t dpi)
+                             uint32_t xmm, uint32_t ymm)
 {
-    /* physical display size */
-    uint32_t xmm = xres * dpi / 254;
-    uint32_t ymm = yres * dpi / 254;
-
     /* pull some realistic looking timings out of thin air */
     uint32_t xfront = xres * 25 / 100;
     uint32_t xsync  = xres *  3 / 100;
@@ -296,6 +292,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     uint32_t desc = 54;
     uint8_t *xtra3 = NULL;
     uint8_t *dta = NULL;
+    uint32_t width_mm, height_mm;
 
     /* =============== set defaults  =============== */
 
@@ -314,6 +311,13 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     if (!info->prefy) {
         info->prefy = 768;
     }
+    if (info->width_mm && info->height_mm) {
+        width_mm = info->width_mm;
+        height_mm = info->height_mm;
+    } else {
+        width_mm = info->prefx * info->dpi / 254;
+        height_mm = info->prefy * info->dpi / 254;
+    }
 
     /* =============== extensions  =============== */
 
@@ -360,8 +364,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     edid[20] = 0xa5;
 
     /* screen size: undefined */
-    edid[21] = info->prefx * 254 / 100 / info->dpi;
-    edid[22] = info->prefy * 254 / 100 / info->dpi;
+    edid[21] = width_mm / 10;
+    edid[22] = height_mm / 10;
 
     /* display gamma: 2.2 */
     edid[23] = 220 - 100;
@@ -387,7 +391,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 
     /* =============== descriptor blocks =============== */
 
-    edid_desc_timing(edid + desc, info->prefx, info->prefy, info->dpi);
+    edid_desc_timing(edid + desc, info->prefx, info->prefy,
+                     width_mm, height_mm);
     desc += 18;
 
     edid_desc_ranges(edid + desc);
diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
index 5b1de57f24..f98b49a944 100644
--- a/include/hw/display/edid.h
+++ b/include/hw/display/edid.h
@@ -6,6 +6,8 @@ typedef struct qemu_edid_info {
     const char *name;
     const char *serial;
     uint32_t    dpi;
+    uint16_t    width_mm;
+    uint16_t    height_mm;
     uint32_t    prefx;
     uint32_t    prefy;
     uint32_t    maxx;
-- 
2.26.2

On Mon, Aug 17, 2020 at 04:00:53PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Add width_mm/height_mm to qemu_edid_info, and use it if it is
> set (non-zero) to generate the EDID.

Any specific reason why you switch from dpi to xmm/ymm?

take care,
  Gerd

Hi

On Mon, Aug 17, 2020 at 4:21 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Mon, Aug 17, 2020 at 04:00:53PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Add width_mm/height_mm to qemu_edid_info, and use it if it is
> > set (non-zero) to generate the EDID.
>
> Any specific reason why you switch from dpi to xmm/ymm?

Not really, but there is no DPI information from Gtk. I also find it
difficult to reason with DPI, dimensions are simpler to check about
correctness imho (I take the ruler from my desk for example ;). And
also DPI is a space density, without horizontal and vertical
distinction.

So by giving width/height in mm we actually have something more
correct and easier to debug imho. No?

On Mon, Aug 17, 2020 at 04:57:55PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Aug 17, 2020 at 4:21 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Mon, Aug 17, 2020 at 04:00:53PM +0400, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Add width_mm/height_mm to qemu_edid_info, and use it if it is
> > > set (non-zero) to generate the EDID.
> >
> > Any specific reason why you switch from dpi to xmm/ymm?
> 
> Not really, but there is no DPI information from Gtk. I also find it
> difficult to reason with DPI, dimensions are simpler to check about
> correctness imho (I take the ruler from my desk for example ;). And
> also DPI is a space density, without horizontal and vertical
> distinction.

Typically computer displays have square pixels, so that shouldn't be a
problem.  For manually configuration it is easier if you have to deal
with one value only not two.

> So by giving width/height in mm we actually have something more
> correct and easier to debug imho. No?

I dislike having both with/height and dpi in struct qemu_edid_info.

Suggestion:  Drop dpi struct member (should be easy, I think it isn't
wired anywhere yet).  Add two little qemu_edid_* helpers to convert
from/to dpi.  If only one of xmm/ymm is given go calculate the other
automatically (assuming square pixels).  If none is given use 100 dpi
(like the current code does).

take care,
  Gerd