[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] edid: add support for DisplayID extension (5k resolution)
From: |
Akihiko Odaki |
Subject: |
Re: [PATCH] edid: add support for DisplayID extension (5k resolution) |
Date: |
Sat, 13 Mar 2021 21:03:18 +0900 |
The logic looks good to me. I have a few style nitpicks.
2021年3月13日(土) 19:45 Konstantin Nazarov <mail@knazarov.com>:
> +struct timings {
> + uint32_t xfront;
> + uint32_t xsync;
> + uint32_t xblank;
> +
> + uint32_t yfront;
> + uint32_t ysync;
> + uint32_t yblank;
> +
> + uint64_t clock;
> +};
doc/devel/style.rst says:
> Typedefs are used to eliminate the redundant 'struct' keyword, since type
> names have a different style than other identifiers ("CamelCase" versus
> "snake_case"). Each named struct type should have a CamelCase name and a
> corresponding typedef.
> void qemu_edid_generate(uint8_t *edid, size_t size,
> qemu_edid_info *info)
> {
> uint32_t desc = 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;
> +
>
The added empty line is redundant.
> if (dta) {
> - edid_checksum(dta);
> + edid_checksum(dta, 127);
> }
> + if (did) {
> + edid_checksum(did, 127);
> + }
> +
> }
This function didn't have an empty line at its end. Please remove it.
Regards,
Akihiko Odaki