Re: [Qemu-devel] [PATCH 1/3] display/edid: add edid generator to qemu.

[Eric Blake]

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 <address@hidden>
> ---

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