Arithmetic error in EDID generation fixed

[PATCH v2] Arithmetic error in EDID generation fixed

Anton V. Boyarshinov posted 1 patch 5 years, 8 months ago

Download mbox

Test docker-quick@centos7 failed

Test FreeBSD failed

Test docker-mingw@fedora failed

Test checkpatch failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200226122054.366b9cda@table.localdomain

Maintainers: Gerd Hoffmann <kraxel@redhat.com>

hw/display/edid-generate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Expand all Fold all

[PATCH v2] Arithmetic error in EDID generation fixed

Posted by Anton V. Boyarshinov 5 years, 8 months ago

To calculate screen size in centimeters we should calculate:
pixels/dpi*2.54
but not
pixels*dpi/2540

Using wrong formula we actually get 65 DPI and very small fonts.

Signed-off-by: Anton V. Boyarshinov <boyarsh@altlinux.org>
---
Changes from v1: get rid of casts

 hw/display/edid-generate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 75c945a948..e58472fde5 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -360,8 +360,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
     edid[20] = 0xa5;
 
     /* screen size: undefined */
-    edid[21] = info->prefx * info->dpi / 2540;
-    edid[22] = info->prefy * info->dpi / 2540;
+    edid[21] = info->prefx * 254 / 100 / info->dpi;
+    edid[22] = info->prefy * 254 / 100 / info->dpi;
 
     /* display gamma: 2.2 */
     edid[23] = 220 - 100;
-- 
2.21.0

Re: [PATCH v2] Arithmetic error in EDID generation fixed

Posted by Gerd Hoffmann 5 years, 8 months ago

On Wed, Feb 26, 2020 at 03:20:54PM +0300, Anton V. Boyarshinov wrote:
> To calculate screen size in centimeters we should calculate:
> pixels/dpi*2.54
> but not
> pixels*dpi/2540
> 
> Using wrong formula we actually get 65 DPI and very small fonts.

Added to vga patch queu.

thanks,
  Gerd

Re: [PATCH v2] Arithmetic error in EDID generation fixed

Posted by Stefan Weil 5 years, 8 months ago

Am 26.02.20 um 13:20 schrieb Anton V. Boyarshinov:
> To calculate screen size in centimeters we should calculate:
> pixels/dpi*2.54
> but not
> pixels*dpi/2540
>
> Using wrong formula we actually get 65 DPI and very small fonts.
>
> Signed-off-by: Anton V. Boyarshinov <boyarsh@altlinux.org>
> ---
> Changes from v1: get rid of casts
>
>  hw/display/edid-generate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
> index 75c945a948..e58472fde5 100644
> --- a/hw/display/edid-generate.c
> +++ b/hw/display/edid-generate.c
> @@ -360,8 +360,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
>      edid[20] = 0xa5;
>  
>      /* screen size: undefined */
Gerd, is this comment still correct?
> -    edid[21] = info->prefx * info->dpi / 2540;
> -    edid[22] = info->prefy * info->dpi / 2540;
> +    edid[21] = info->prefx * 254 / 100 / info->dpi;
> +    edid[22] = info->prefy * 254 / 100 / info->dpi;


The fix is good, but can be improved:

According to the standard, the values are "rounded to the nearest
centimeter". Currently they are not rounded, but truncated. So this
would be a better approximation:

edid[21] = (info->prefx * 254 / info->dpi + 50) / 100;

...


Regards,

Stefan