All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu.
@ 2018-09-12 12:36 Gerd Hoffmann
  2018-09-12 12:36 ` [Qemu-devel] [PATCH 2/3] display/edid: add region helper Gerd Hoffmann
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2018-09-12 12:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

EDID is a metadata format to describe monitors.  On physical hardware
the monitor has an eeprom with that data block which can be read over
i2c bus.

On a linux system you can usually find the EDID data block in
/sys/class/drm/$card/$connector/edid.  xorg ships a edid-decode utility
which you can use to turn the blob into readable form.

I think it would be a good idea to use EDID for virtual displays too.
Needs changes in both qemu and guest kms drivers.  This patch is the
first step, it adds an generator for EDID blobs to qemu.  Comes with a
qemu-edid test tool included.

With EDID we can pass more information to the guest.  Names and serial
numbers, so the guests display configuration has no boring "Unknown
Monitor".  List of video modes.  Display resolution, pretty important
in case we want add HiDPI support some day.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 configure                  |   2 +-
 Makefile                   |   2 +
 include/hw/display/edid.h  |  18 +++
 hw/display/edid-generate.c | 328 +++++++++++++++++++++++++++++++++++++++++++++
 qemu-edid.c                |  86 ++++++++++++
 hw/display/Makefile.objs   |   2 +
 6 files changed, 437 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/display/edid.h
 create mode 100644 hw/display/edid-generate.c
 create mode 100644 qemu-edid.c

diff --git a/configure b/configure
index 58862d2ae8..053facfb68 100755
--- a/configure
+++ b/configure
@@ -5715,7 +5715,7 @@ fi
 
 tools=""
 if test "$want_tools" = "yes" ; then
-  tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
+  tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) qemu-edid\$(EXESUF) $tools"
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
     tools="qemu-nbd\$(EXESUF) $tools"
   fi
diff --git a/Makefile b/Makefile
index fe623e4634..7bb6675f4a 100644
--- a/Makefile
+++ b/Makefile
@@ -543,6 +543,8 @@ qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
 
 qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS)
 
+qemu-edid$(EXESUF): qemu-edid.o hw/display/edid-generate.o $(COMMON_LDADDS)
+
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
new file mode 100644
index 0000000000..63b60015c3
--- /dev/null
+++ b/include/hw/display/edid.h
@@ -0,0 +1,18 @@
+#ifndef EDID_H
+#define EDID_H
+
+typedef struct qemu_edid_info {
+    const char *vendor;
+    const char *name;
+    const char *serial;
+    uint32_t    dpi;
+    uint32_t    prefx;
+    uint32_t    prefy;
+    uint32_t    maxx;
+    uint32_t    maxy;
+} qemu_edid_info;
+
+void qemu_edid_generate(uint8_t *edid, size_t size,
+                        qemu_edid_info *info);
+
+#endif /* EDID_H */
diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
new file mode 100644
index 0000000000..a73acf6bc7
--- /dev/null
+++ b/hw/display/edid-generate.c
@@ -0,0 +1,328 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "hw/display/edid.h"
+
+static struct edid_mode {
+    uint32_t xres;
+    uint32_t yres;
+    uint32_t byte;
+    uint32_t xtra3;
+    uint32_t bit;
+} modes[] = {
+    /* other */
+    { .xres = 2048,   .yres = 1152 },
+    { .xres = 1920,   .yres = 1440 },
+    { .xres = 1920,   .yres = 1080 },
+
+    /* additional standard timings 3 (all @75Hz) */
+    { .xres = 1920,   .yres = 1200,   .xtra3 = 11,   .bit = 7 },
+    { .xres = 1856,   .yres = 1392,   .xtra3 = 10,   .bit = 2 },
+    { .xres = 1792,   .yres = 1344,   .xtra3 = 10,   .bit = 4 },
+    { .xres = 1600,   .yres = 1200,   .xtra3 = 10,   .bit = 7 },
+    { .xres = 1680,   .yres = 1050,   .xtra3 =  9,   .bit = 4 },
+    { .xres = 1440,   .yres = 1050,   .xtra3 =  8,   .bit = 0 },
+    { .xres = 1440,   .yres =  900,   .xtra3 =  8,   .bit = 4 },
+    { .xres = 1280,   .yres =  768,   .xtra3 =  7,   .bit = 5 },
+
+    /* established timings (all @75Hz) */
+    { .xres = 1280,   .yres = 1024,   .byte  = 36,   .bit = 0 },
+    { .xres = 1024,   .yres =  768,   .byte  = 36,   .bit = 1 },
+    { .xres =  800,   .yres =  600,   .byte  = 36,   .bit = 6 },
+    { .xres =  640,   .yres =  480,   .byte  = 35,   .bit = 2 },
+};
+
+static int edid_std_mode(uint8_t *mode, uint32_t xres, uint32_t yres)
+{
+    uint32_t aspect;
+
+    if (xres == 0 || yres == 0) {
+        mode[0] = 0x01;
+        mode[1] = 0x01;
+        return 0;
+
+    } else if (xres * 10 == yres * 16) {
+        aspect = 0;
+    } else if (xres * 3 == yres * 4) {
+        aspect = 1;
+    } else if (xres * 4 == yres * 5) {
+        aspect = 2;
+    } else if (xres * 9 == yres * 16) {
+        aspect = 3;
+    } else {
+        return -1;
+    }
+
+    mode[0] = (xres / 8) - 31;
+    mode[1] = ((aspect << 6) | (75 - 60));
+    return 0;
+}
+
+static void edid_fill_modes(uint8_t *edid, uint8_t *xtra3,
+                            uint32_t maxx, uint32_t maxy)
+{
+    struct edid_mode *mode;
+    int std = 38;
+    int rc, i;
+
+    for (i = 0; i < ARRAY_SIZE(modes); i++) {
+        mode = modes + i;
+
+        if ((maxx && mode->xres > maxx) ||
+            (maxy && mode->yres > maxy)) {
+            continue;
+        }
+
+        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;
+            }
+        }
+    }
+
+    while (std < 54) {
+        edid_std_mode(edid + std, 0, 0);
+        std += 2;
+    }
+}
+
+static void edid_checksum(uint8_t *edid)
+{
+    uint32_t sum = 0;
+    int i;
+
+    for (i = 0; i < 127; i++) {
+        sum += edid[i];
+    }
+    sum &= 0xff;
+    if (sum) {
+        edid[127] = 0x100 - sum;
+    }
+}
+
+static void edid_desc_type(uint8_t *desc, uint8_t type)
+{
+    desc[0] = 0;
+    desc[1] = 0;
+    desc[2] = 0;
+    desc[3] = type;
+    desc[4] = 0;
+}
+
+static void edid_desc_text(uint8_t *desc, uint8_t type,
+                           const char *text)
+{
+    size_t len;
+
+    edid_desc_type(desc, type);
+    memset(desc + 5, ' ', 13);
+
+    len = strlen(text);
+    if (len > 12)
+        len = 12;
+    strncpy((char*)(desc + 5), text, len);
+    desc[ 5 + len ] = '\n';
+}
+
+static void edid_desc_ranges(uint8_t *desc)
+{
+    edid_desc_type(desc, 0xfd);
+
+    /* vertical */
+    desc[ 5] =  50;
+    desc[ 6] = 100;
+
+    /* horizontal */
+    desc[ 7] =  30;
+    desc[ 8] = 120;
+
+    /* dot clock */
+    desc[ 9] = 250 / 10;
+
+    /* padding */
+    desc[11] = '\n';
+    memset(desc + 12, ' ', 6);
+}
+
+/* additional standard timings 3 */
+static void edid_desc_xtra3_std(uint8_t *desc)
+{
+    edid_desc_type(desc, 0xf7);
+    desc[5] = 10;
+}
+
+static void edid_desc_dummy(uint8_t *desc)
+{
+    edid_desc_type(desc, 0x10);
+}
+
+static void edid_desc_timing(uint8_t *desc,
+                             uint32_t xres, uint32_t yres,
+                             uint32_t dpi)
+{
+    /* 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;
+    uint32_t xblank = xres * 35 / 100;
+
+    uint32_t yfront = yres *  5 / 1000;
+    uint32_t ysync  = yres *  5 / 1000;
+    uint32_t yblank = yres * 35 / 1000;
+
+    uint32_t clock  = 75 * (xres + xblank) * (yres + yblank);
+
+    *(uint32_t*)(desc) = cpu_to_le32(clock / 10000);
+
+    desc[2] = xres   & 0xff;
+    desc[3] = xblank & 0xff;
+    desc[4] = ((( xres   & 0xf00 ) >> 4) |
+               (( xblank & 0xf00 ) >> 8));
+
+    desc[5] = yres   & 0xff;
+    desc[6] = yblank & 0xff;
+    desc[7] = ((( yres   & 0xf00 ) >> 4) |
+               (( yblank & 0xf00 ) >> 8));
+
+    desc[8] = xfront & 0xff;
+    desc[9] = 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[12] = xmm & 0xff;
+    desc[13] = ymm & 0xff;
+    desc[14] = ((( xmm & 0xf00 ) >> 4) |
+                (( ymm & 0xf00 ) >> 8));
+
+    desc[17] = 0x18;
+}
+
+void qemu_edid_generate(uint8_t *edid, size_t size,
+                        qemu_edid_info *info)
+{
+    uint32_t desc = 54;
+    uint8_t *xtra3 = NULL;
+
+    /* =============== set defaults  =============== */
+
+    if (!info->vendor || strlen(info->vendor) != 3) {
+        info->vendor = "EMU";
+    }
+    if (!info->name) {
+        info->name = "QEMU Monitor";
+    }
+    if (!info->dpi) {
+        info->dpi = 100;
+    }
+    if (!info->prefx) {
+        info->prefx = 1024;
+    }
+    if (!info->prefy) {
+        info->prefy = 768;
+    }
+
+    /* =============== header information =============== */
+
+    /* fixed */
+    edid[0] = 0x00;
+    edid[1] = 0xff;
+    edid[2] = 0xff;
+    edid[3] = 0xff;
+    edid[4] = 0xff;
+    edid[5] = 0xff;
+    edid[6] = 0xff;
+    edid[7] = 0x00;
+
+    /* manufacturer id, product code, serial number */
+    uint16_t vendor_id = ((((info->vendor[0] - '@') & 0x1f) << 10) |
+                          (((info->vendor[1] - '@') & 0x1f) <<  5) |
+                          (((info->vendor[2] - '@') & 0x1f) <<  0));
+    uint16_t model_nr = 0x1234;
+    uint32_t serial_nr = info->serial ? atoi(info->serial) : 0;
+    *(uint16_t*)(edid +  8) = cpu_to_be16(vendor_id);
+    *(uint16_t*)(edid + 10) = cpu_to_le16(model_nr);
+    *(uint32_t*)(edid + 12) = cpu_to_le32(serial_nr);
+
+    /* manufacture week and year */
+    edid[16] = 42;
+    edid[17] = 2014 - 1990;
+
+    /* edid version */
+    edid[18] = 1;
+    edid[19] = 4;
+
+
+    /* =============== basic display parameters =============== */
+
+    /* video input: digital, 8bpc, displayport */
+    edid[20] = 0xa5;
+
+    /* screen size: undefined */
+    edid[21] = info->prefx * info->dpi / 2540;
+    edid[22] = info->prefy * info->dpi / 2540;
+
+    /* display gamma: 1.0 */
+    edid[23] = 0x00;
+
+    /* supported features bitmap: prefered timing */
+    edid[24] = 0x02;
+
+
+    /* =============== chromaticity coordinates =============== */
+
+    /* TODO (bytes 25 -> 34) */
+
+
+    /* =============== established timing bitmap =============== */
+    /* =============== standard timing information =============== */
+
+    /* both filled by edid_fill_modes() */
+
+
+    /* =============== descriptor blocks =============== */
+
+    edid_desc_timing(edid + desc, info->prefx, info->prefy, info->dpi);
+    desc += 18;
+
+    edid_desc_ranges(edid + desc);
+    desc += 18;
+
+    if (info->name) {
+        edid_desc_text(edid + desc, 0xfc, info->name);
+        desc += 18;
+    }
+
+    if (info->serial) {
+        edid_desc_text(edid + desc, 0xff, info->serial);
+        desc += 18;
+    }
+
+    if (desc < 126) {
+        xtra3 = edid + desc;
+        edid_desc_xtra3_std(xtra3);
+        desc += 18;
+    }
+
+    while (desc < 126) {
+        edid_desc_dummy(edid + desc);
+        desc += 18;
+    }
+
+    /* =============== finish up =============== */
+
+    edid_fill_modes(edid, xtra3, info->maxx, info->maxy);
+    edid_checksum(edid);
+}
diff --git a/qemu-edid.c b/qemu-edid.c
new file mode 100644
index 0000000000..8d6f1bbca3
--- /dev/null
+++ b/qemu-edid.c
@@ -0,0 +1,86 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "hw/display/edid.h"
+
+static qemu_edid_info info;
+
+static void usage(FILE *out)
+{
+    fprintf(out,
+            "\n"
+            "This is a test tool for the qemu edid generator.\n"
+            "\n"
+            "Typically you'll pipe the output into edid-decode\n"
+            "to check if the generator works correctly.\n"
+            "\n"
+            "usage: qemu-edid <options>\n"
+            "options:\n"
+            "    -h             print this text\n"
+            "    -o <file>      set output file (stdout by default)\n"
+            "    -v <vendor>    set monitor vendor (three letters)\n"
+            "    -n <name>      set monitor name\n"
+            "    -s <serial>    set monitor serial\n"
+            "    -d <dpi>       set display resolution\n"
+            "    -x <prefx>     set preferred width\n"
+            "    -y <prefy>     set preferred height\n"
+            "\n");
+}
+
+int main(int argc, char *argv[])
+{
+    FILE *outfile = NULL;
+    uint8_t blob[128];
+    int rc;
+
+    for (;;) {
+        rc = getopt(argc, argv, "ho:x:y:d:v:n:s:");
+        if (rc == -1) {
+            break;
+        }
+        switch (rc) {
+        case 'o':
+            outfile = fopen(optarg, "w");
+            if (outfile == NULL) {
+                fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
+                exit(1);
+            }
+            break;
+        case 'x':
+            info.prefx = atoi(optarg);
+            break;
+        case 'y':
+            info.maxy = atoi(optarg);
+            break;
+        case 'd':
+            info.dpi = atoi(optarg);
+            break;
+        case 'v':
+            info.vendor = optarg;
+            break;
+        case 'n':
+            info.name = optarg;
+            break;
+        case 's':
+            info.serial = optarg;
+            break;
+        case 'h':
+            usage(stdout);
+            exit(0);
+        default:
+            usage(stderr);
+            exit(1);
+        }
+    }
+
+    if (outfile == NULL) {
+        outfile = stdout;
+    }
+
+    memset(blob, 0, sizeof(blob));
+    qemu_edid_generate(blob, sizeof(blob), &info);
+    fwrite(blob, sizeof(blob), 1, outfile);
+    fflush(outfile);
+
+    exit(0);
+}
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index a606fb7404..492404c09e 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -1,3 +1,5 @@
+common-obj-y += edid-generate.o
+
 common-obj-$(CONFIG_FW_CFG_DMA) += ramfb.o
 common-obj-$(CONFIG_FW_CFG_DMA) += ramfb-standalone.o
 
-- 
2.9.3

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

* [Qemu-devel] [PATCH 2/3] display/edid: add region helper.
  2018-09-12 12:36 [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Gerd Hoffmann
@ 2018-09-12 12:36 ` Gerd Hoffmann
  2018-09-13 21:05   ` Philippe Mathieu-Daudé
  2018-09-12 12:37 ` [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support Gerd Hoffmann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2018-09-12 12:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Create a io region for an EDID data block.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/display/edid.h |  4 ++++
 hw/display/edid-region.c  | 33 +++++++++++++++++++++++++++++++++
 hw/display/Makefile.objs  |  1 +
 3 files changed, 38 insertions(+)
 create mode 100644 hw/display/edid-region.c

diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
index 63b60015c3..c39d5cd77b 100644
--- a/include/hw/display/edid.h
+++ b/include/hw/display/edid.h
@@ -1,6 +1,8 @@
 #ifndef EDID_H
 #define EDID_H
 
+#include "hw/hw.h"
+
 typedef struct qemu_edid_info {
     const char *vendor;
     const char *name;
@@ -14,5 +16,7 @@ typedef struct qemu_edid_info {
 
 void qemu_edid_generate(uint8_t *edid, size_t size,
                         qemu_edid_info *info);
+void qemu_edid_region_io(MemoryRegion *region, Object *owner,
+                         uint8_t *edid, size_t size);
 
 #endif /* EDID_H */
diff --git a/hw/display/edid-region.c b/hw/display/edid-region.c
new file mode 100644
index 0000000000..9a15734d3a
--- /dev/null
+++ b/hw/display/edid-region.c
@@ -0,0 +1,33 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "hw/display/edid.h"
+
+static uint64_t edid_region_read(void *ptr, hwaddr addr, unsigned size)
+{
+    uint8_t *edid = ptr;
+
+    return edid[addr];
+}
+
+static void edid_region_write(void *ptr, hwaddr addr,
+                             uint64_t val, unsigned size)
+{
+    /* read only */
+}
+
+static const MemoryRegionOps edid_region_ops = {
+    .read = edid_region_read,
+    .write = edid_region_write,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 4,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 1,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+void qemu_edid_region_io(MemoryRegion *region, Object *owner,
+                         uint8_t *edid, size_t size)
+{
+    memory_region_init_io(region, owner, &edid_region_ops,
+                          edid, "edid", size);
+}
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 492404c09e..780a76b9f0 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -15,6 +15,7 @@ common-obj-$(CONFIG_XEN) += xenfb.o
 
 common-obj-$(CONFIG_VGA_PCI) += vga-pci.o
 common-obj-$(CONFIG_VGA_PCI) += bochs-display.o
+common-obj-$(CONFIG_VGA_PCI) += edid-region.o
 common-obj-$(CONFIG_VGA_ISA) += vga-isa.o
 common-obj-$(CONFIG_VGA_ISA_MM) += vga-isa-mm.o
 common-obj-$(CONFIG_VMWARE_VGA) += vmware_vga.o
-- 
2.9.3

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

* [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support.
  2018-09-12 12:36 [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Gerd Hoffmann
  2018-09-12 12:36 ` [Qemu-devel] [PATCH 2/3] display/edid: add region helper Gerd Hoffmann
@ 2018-09-12 12:37 ` Gerd Hoffmann
  2018-09-12 14:58   ` Eric Blake
  2018-09-12 14:55 ` [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Eric Blake
  2018-09-12 18:01 ` Richard Henderson
  3 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2018-09-12 12:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin

This patch adds edid support to the qemu stdvga.  It is turned off by
default and can be enabled with the new edid property.  The patch also
adds xres and yres properties to specify the video mode you want the
guest use.  Works only with edid enabled and updated guest driver.

The mmio bar of the stdvga has some unused address space at the start.
It was reserved just in case it'll be needed for virtio, but it turned
out to not be needed for that.  So lets use that region to place the
EDID data block there.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/specs/standard-vga.txt |  2 +-
 hw/display/vga_int.h        |  2 +-
 hw/display/vga-pci.c        | 42 ++++++++++++++++++++++++++++++++++++++----
 hw/display/virtio-vga.c     |  2 +-
 4 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/docs/specs/standard-vga.txt b/docs/specs/standard-vga.txt
index 19d2a74509..18f75f1b30 100644
--- a/docs/specs/standard-vga.txt
+++ b/docs/specs/standard-vga.txt
@@ -61,7 +61,7 @@ MMIO area spec
 
 Likewise applies to the pci variant only for obvious reasons.
 
-0000 - 03ff : reserved, for possible virtio extension.
+0000 - 03ff : edid data blob.
 0400 - 041f : vga ioports (0x3c0 -> 0x3df), remapped 1:1.
               word access is supported, bytes are written
               in little endia order (aka index port first),
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 339661bc01..6e4fa48a79 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -197,6 +197,6 @@ void pci_std_vga_mmio_region_init(VGACommonState *s,
                                   Object *owner,
                                   MemoryRegion *parent,
                                   MemoryRegion *subs,
-                                  bool qext);
+                                  bool qext, bool edid);
 
 #endif
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index e9e62eac70..5e692baf78 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -30,18 +30,23 @@
 #include "ui/pixel_ops.h"
 #include "qemu/timer.h"
 #include "hw/loader.h"
+#include "hw/display/edid.h"
 
 enum vga_pci_flags {
     PCI_VGA_FLAG_ENABLE_MMIO = 1,
     PCI_VGA_FLAG_ENABLE_QEXT = 2,
+    PCI_VGA_FLAG_ENABLE_EDID = 3,
 };
 
 typedef struct PCIVGAState {
     PCIDevice dev;
     VGACommonState vga;
     uint32_t flags;
+    uint32_t xres;
+    uint32_t yres;
     MemoryRegion mmio;
-    MemoryRegion mrs[3];
+    MemoryRegion mrs[4];
+    uint8_t edid[128];
 } PCIVGAState;
 
 #define TYPE_PCI_VGA "pci-vga"
@@ -195,8 +200,10 @@ void pci_std_vga_mmio_region_init(VGACommonState *s,
                                   Object *owner,
                                   MemoryRegion *parent,
                                   MemoryRegion *subs,
-                                  bool qext)
+                                  bool qext, bool edid)
 {
+    PCIVGAState *d = container_of(s, PCIVGAState, vga);
+
     memory_region_init_io(&subs[0], owner, &pci_vga_ioport_ops, s,
                           "vga ioports remapped", PCI_VGA_IOPORT_SIZE);
     memory_region_add_subregion(parent, PCI_VGA_IOPORT_OFFSET,
@@ -213,6 +220,16 @@ void pci_std_vga_mmio_region_init(VGACommonState *s,
         memory_region_add_subregion(parent, PCI_VGA_QEXT_OFFSET,
                                     &subs[2]);
     }
+
+    if (edid) {
+        qemu_edid_info info = {
+            .prefx = d->xres,
+            .prefy = d->yres,
+        };
+        qemu_edid_generate(d->edid, sizeof(d->edid), &info);
+        qemu_edid_region_io(&subs[3], owner, d->edid, sizeof(d->edid));
+        memory_region_add_subregion(parent, 0, &subs[3]);
+    }
 }
 
 static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
@@ -220,6 +237,7 @@ static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
     PCIVGAState *d = PCI_VGA(dev);
     VGACommonState *s = &d->vga;
     bool qext = false;
+    bool edid = false;
 
     /* vga + console init */
     vga_common_init(s, OBJECT(dev));
@@ -240,7 +258,11 @@ static void pci_std_vga_realize(PCIDevice *dev, Error **errp)
             qext = true;
             pci_set_byte(&d->dev.config[PCI_REVISION_ID], 2);
         }
-        pci_std_vga_mmio_region_init(s, OBJECT(dev), &d->mmio, d->mrs, qext);
+        if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_EDID)) {
+            edid = true;
+        }
+        pci_std_vga_mmio_region_init(s, OBJECT(dev), &d->mmio, d->mrs,
+                                     qext, edid);
 
         pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
     }
@@ -263,6 +285,7 @@ static void pci_secondary_vga_realize(PCIDevice *dev, Error **errp)
     PCIVGAState *d = PCI_VGA(dev);
     VGACommonState *s = &d->vga;
     bool qext = false;
+    bool edid = false;
 
     /* vga + console init */
     vga_common_init(s, OBJECT(dev));
@@ -276,7 +299,10 @@ static void pci_secondary_vga_realize(PCIDevice *dev, Error **errp)
         qext = true;
         pci_set_byte(&d->dev.config[PCI_REVISION_ID], 2);
     }
-    pci_std_vga_mmio_region_init(s, OBJECT(dev), &d->mmio, d->mrs, qext);
+    if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_EDID)) {
+        edid = true;
+    }
+    pci_std_vga_mmio_region_init(s, OBJECT(dev), &d->mmio, d->mrs, qext, edid);
 
     pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram);
     pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
@@ -308,6 +334,10 @@ static Property vga_pci_properties[] = {
     DEFINE_PROP_BIT("mmio", PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_MMIO, true),
     DEFINE_PROP_BIT("qemu-extended-regs",
                     PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_QEXT, true),
+    DEFINE_PROP_BIT("edid",
+                    PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_EDID, false),
+    DEFINE_PROP_UINT32("xres", PCIVGAState, xres, 0),
+    DEFINE_PROP_UINT32("yres", PCIVGAState, yres, 0),
     DEFINE_PROP_BOOL("global-vmstate", PCIVGAState, vga.global_vmstate, false),
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -316,6 +346,10 @@ static Property secondary_pci_properties[] = {
     DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
     DEFINE_PROP_BIT("qemu-extended-regs",
                     PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_QEXT, true),
+    DEFINE_PROP_BIT("edid",
+                    PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_EDID, false),
+    DEFINE_PROP_UINT32("xres", PCIVGAState, xres, 0),
+    DEFINE_PROP_UINT32("yres", PCIVGAState, yres, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 701d980872..3bea0693f6 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -163,7 +163,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 
     /* add stdvga mmio regions */
     pci_std_vga_mmio_region_init(vga, OBJECT(vvga), &vpci_dev->modern_bar,
-                                 vvga->vga_mrs, true);
+                                 vvga->vga_mrs, true, false);
 
     vga->con = g->scanout[0].con;
     g->disable_scanout = virtio_vga_disable_scanout;
-- 
2.9.3

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

* Re: [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu.
  2018-09-12 12:36 [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Gerd Hoffmann
  2018-09-12 12:36 ` [Qemu-devel] [PATCH 2/3] display/edid: add region helper Gerd Hoffmann
  2018-09-12 12:37 ` [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support Gerd Hoffmann
@ 2018-09-12 14:55 ` Eric Blake
  2018-09-12 18:01 ` Richard Henderson
  3 siblings, 0 replies; 8+ messages in thread
From: Eric Blake @ 2018-09-12 14:55 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

On 9/12/18 7:36 AM, Gerd Hoffmann wrote:
> EDID is a metadata format to describe monitors.  On physical hardware
> the monitor has an eeprom with that data block which can be read over
> i2c bus.
> 
> On a linux system you can usually find the EDID data block in
> /sys/class/drm/$card/$connector/edid.  xorg ships a edid-decode utility
> which you can use to turn the blob into readable form.
> 
> I think it would be a good idea to use EDID for virtual displays too.
> Needs changes in both qemu and guest kms drivers.  This patch is the
> first step, it adds an generator for EDID blobs to qemu.  Comes with a
> qemu-edid test tool included.
> 
> With EDID we can pass more information to the guest.  Names and serial
> numbers, so the guests display configuration has no boring "Unknown
> Monitor".  List of video modes.  Display resolution, pretty important
> in case we want add HiDPI support some day.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

> --- /dev/null
> +++ b/include/hw/display/edid.h
> @@ -0,0 +1,18 @@
> +#ifndef EDID_H
> +#define EDID_H

> --- /dev/null
> +++ b/hw/display/edid-generate.c
> @@ -0,0 +1,328 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/bswap.h"
> +#include "hw/display/edid.h"
> +

No copyright blurbs?

> +    /* =============== basic display parameters =============== */
> +
> +    /* video input: digital, 8bpc, displayport */
> +    edid[20] = 0xa5;
> +
> +    /* screen size: undefined */
> +    edid[21] = info->prefx * info->dpi / 2540;
> +    edid[22] = info->prefy * info->dpi / 2540;
> +
> +    /* display gamma: 1.0 */
> +    edid[23] = 0x00;
> +
> +    /* supported features bitmap: prefered timing */

preferred

> --- /dev/null
> +++ b/qemu-edid.c
> @@ -0,0 +1,86 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/bswap.h"
> +#include "hw/display/edid.h"

and again


> +int main(int argc, char *argv[])
> +{
> +    FILE *outfile = NULL;
> +    uint8_t blob[128];
> +    int rc;
> +
> +    for (;;) {
> +        rc = getopt(argc, argv, "ho:x:y:d:v:n:s:");
> +        if (rc == -1) {
> +            break;
> +        }
> +        switch (rc) {
> +        case 'o':
> +            outfile = fopen(optarg, "w");
> +            if (outfile == NULL) {
> +                fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
> +                exit(1);

Leaks if -o is passed on the command line more than once.

> +            }
> +            break;
> +        case 'x':
> +            info.prefx = atoi(optarg);

atoi() can't flag errors like '-x 1garbage'. Better is to use qemu_strtoi().

> +    memset(blob, 0, sizeof(blob));
> +    qemu_edid_generate(blob, sizeof(blob), &info);
> +    fwrite(blob, sizeof(blob), 1, outfile);
> +    fflush(outfile);
> +
> +    exit(0);

You could just 'return 0;' instead.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

* Re: [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support.
  2018-09-12 12:37 ` [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support Gerd Hoffmann
@ 2018-09-12 14:58   ` Eric Blake
  2018-09-13  7:19     ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2018-09-12 14:58 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel; +Cc: Michael S. Tsirkin

On 9/12/18 7:37 AM, Gerd Hoffmann wrote:
> This patch adds edid support to the qemu stdvga.  It is turned off by
> default and can be enabled with the new edid property.  The patch also
> adds xres and yres properties to specify the video mode you want the
> guest use.  Works only with edid enabled and updated guest driver.
> 
> The mmio bar of the stdvga has some unused address space at the start.
> It was reserved just in case it'll be needed for virtio, but it turned
> out to not be needed for that.  So lets use that region to place the

s/lets/let's/

> EDID data block there.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   docs/specs/standard-vga.txt |  2 +-
>   hw/display/vga_int.h        |  2 +-
>   hw/display/vga-pci.c        | 42 ++++++++++++++++++++++++++++++++++++++----
>   hw/display/virtio-vga.c     |  2 +-
>   4 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/specs/standard-vga.txt b/docs/specs/standard-vga.txt
> index 19d2a74509..18f75f1b30 100644
> --- a/docs/specs/standard-vga.txt
> +++ b/docs/specs/standard-vga.txt
> @@ -61,7 +61,7 @@ MMIO area spec
>   
>   Likewise applies to the pci variant only for obvious reasons.
>   
> -0000 - 03ff : reserved, for possible virtio extension.
> +0000 - 03ff : edid data blob.


> +    if (edid) {
> +        qemu_edid_info info = {
> +            .prefx = d->xres,
> +            .prefy = d->yres,
> +        };
> +        qemu_edid_generate(d->edid, sizeof(d->edid), &info);
> +        qemu_edid_region_io(&subs[3], owner, d->edid, sizeof(d->edid));
> +        memory_region_add_subregion(parent, 0, &subs[3]);

Is there ever a risk that this could overflow?  Or, if the blob is 
always smaller than 0x3ff, should we still leave reserved bytes?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

* Re: [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu.
  2018-09-12 12:36 [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2018-09-12 14:55 ` [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Eric Blake
@ 2018-09-12 18:01 ` Richard Henderson
  3 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2018-09-12 18:01 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

On 09/12/2018 05:36 AM, Gerd Hoffmann wrote:
> +static struct edid_mode {
> +    uint32_t xres;
> +    uint32_t yres;
> +    uint32_t byte;
> +    uint32_t xtra3;
> +    uint32_t bit;
> +} modes[] = {

static const?


r~

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

* Re: [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support.
  2018-09-12 14:58   ` Eric Blake
@ 2018-09-13  7:19     ` Gerd Hoffmann
  0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2018-09-13  7:19 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel, Michael S. Tsirkin

> > --- a/docs/specs/standard-vga.txt
> > +++ b/docs/specs/standard-vga.txt
> > @@ -61,7 +61,7 @@ MMIO area spec
> >   Likewise applies to the pci variant only for obvious reasons.
> > -0000 - 03ff : reserved, for possible virtio extension.
> > +0000 - 03ff : edid data blob.
> 
> 
> > +    if (edid) {
> > +        qemu_edid_info info = {
> > +            .prefx = d->xres,
> > +            .prefy = d->yres,
> > +        };
> > +        qemu_edid_generate(d->edid, sizeof(d->edid), &info);
> > +        qemu_edid_region_io(&subs[3], owner, d->edid, sizeof(d->edid));
> > +        memory_region_add_subregion(parent, 0, &subs[3]);
> 
> Is there ever a risk that this could overflow?  Or, if the blob is always
> smaller than 0x3ff, should we still leave reserved bytes?

It's 128 bytes long.  There can be (multiple) extensions, each 128 bytes
long too.  Current generator doesn't create any extensions.  So, only a
fraction of the space is actually used right now.

I'd leave the room reserved for edid nevertheless.  There is still
plenty of unused address space should we need something at some point in
the future.  The mmio bar is one page (4k), with more than 2k at the end
being not used (from info mtree):

  00000000febf0000-00000000febf0fff (prio 1, i/o): vga.mmio
    00000000febf0000-00000000febf007f (prio 0, i/o): edid
    00000000febf0400-00000000febf041f (prio 0, i/o): vga ioports remapped
    00000000febf0500-00000000febf0515 (prio 0, i/o): bochs dispi interface
    00000000febf0600-00000000febf0607 (prio 0, i/o): qemu extended regs

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH 2/3] display/edid: add region helper.
  2018-09-12 12:36 ` [Qemu-devel] [PATCH 2/3] display/edid: add region helper Gerd Hoffmann
@ 2018-09-13 21:05   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-09-13 21:05 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel

On 9/12/18 2:36 PM, Gerd Hoffmann wrote:
> Create a io region for an EDID data block.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/hw/display/edid.h |  4 ++++
>  hw/display/edid-region.c  | 33 +++++++++++++++++++++++++++++++++
>  hw/display/Makefile.objs  |  1 +
>  3 files changed, 38 insertions(+)
>  create mode 100644 hw/display/edid-region.c
> 
> diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
> index 63b60015c3..c39d5cd77b 100644
> --- a/include/hw/display/edid.h
> +++ b/include/hw/display/edid.h
> @@ -1,6 +1,8 @@
>  #ifndef EDID_H
>  #define EDID_H
>  
> +#include "hw/hw.h"
> +
>  typedef struct qemu_edid_info {
>      const char *vendor;
>      const char *name;
> @@ -14,5 +16,7 @@ typedef struct qemu_edid_info {
>  
>  void qemu_edid_generate(uint8_t *edid, size_t size,
>                          qemu_edid_info *info);
> +void qemu_edid_region_io(MemoryRegion *region, Object *owner,
> +                         uint8_t *edid, size_t size);
>  
>  #endif /* EDID_H */
> diff --git a/hw/display/edid-region.c b/hw/display/edid-region.c
> new file mode 100644
> index 0000000000..9a15734d3a
> --- /dev/null
> +++ b/hw/display/edid-region.c
> @@ -0,0 +1,33 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "hw/display/edid.h"
> +
> +static uint64_t edid_region_read(void *ptr, hwaddr addr, unsigned size)
> +{
> +    uint8_t *edid = ptr;
> +
> +    return edid[addr];
> +}
> +
> +static void edid_region_write(void *ptr, hwaddr addr,
> +                             uint64_t val, unsigned size)
> +{
> +    /* read only */
> +}
> +
> +static const MemoryRegionOps edid_region_ops = {
> +    .read = edid_region_read,
> +    .write = edid_region_write,
> +    .valid.min_access_size = 1,
> +    .valid.max_access_size = 4,
> +    .impl.min_access_size = 1,
> +    .impl.max_access_size = 1,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +void qemu_edid_region_io(MemoryRegion *region, Object *owner,
> +                         uint8_t *edid, size_t size)
> +{
> +    memory_region_init_io(region, owner, &edid_region_ops,
> +                          edid, "edid", size);
> +}
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index 492404c09e..780a76b9f0 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -15,6 +15,7 @@ common-obj-$(CONFIG_XEN) += xenfb.o
>  
>  common-obj-$(CONFIG_VGA_PCI) += vga-pci.o
>  common-obj-$(CONFIG_VGA_PCI) += bochs-display.o
> +common-obj-$(CONFIG_VGA_PCI) += edid-region.o
>  common-obj-$(CONFIG_VGA_ISA) += vga-isa.o
>  common-obj-$(CONFIG_VGA_ISA_MM) += vga-isa-mm.o
>  common-obj-$(CONFIG_VMWARE_VGA) += vmware_vga.o
> 

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

end of thread, other threads:[~2018-09-13 21:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 12:36 [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Gerd Hoffmann
2018-09-12 12:36 ` [Qemu-devel] [PATCH 2/3] display/edid: add region helper Gerd Hoffmann
2018-09-13 21:05   ` Philippe Mathieu-Daudé
2018-09-12 12:37 ` [Qemu-devel] [PATCH 3/3] display/stdvga: add edid support Gerd Hoffmann
2018-09-12 14:58   ` Eric Blake
2018-09-13  7:19     ` Gerd Hoffmann
2018-09-12 14:55 ` [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu Eric Blake
2018-09-12 18:01 ` Richard Henderson

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.