From a41c30338f8c632fa0e67128ad4fcac92216c29a Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Thu, 18 Feb 2021 11:27:06 +0900 Subject: [PATCH 1/2] backend/native: Calculate refresh rate in double-precision The old calculation was introduced to improve the precision with commit c16a5ec1cf14d67ee8100afd31a2e85e7003f9a8. Here, I call the calculation as "revision 2", and the calculation even older as "revision 1", and the new calculation introduced with this commit as "reivion 3". Revision 2 has two problems: 1. The calculation is mixed with fixed-point numbers and floating-point numbers. To overcome the precision loss of fixed-point numbers division, it first "calculates refresh rate in milliHz first for extra precision", but this requires converting the value back to Hz. An extra calculation has performance and precision costs. It is also hard to understand for programmers. 2. The calculation has a bias. In the process, it does: refresh += (drm_mode->vtotal / 2); It prevents the value from being rounded to a smaller value in a fixed-point integer arithmetics, but it only adds a small bias (0.0005) and consumes some fraction bits for floating point arithmetic. Revision 3, introduced with this commit always uses double-precision floating-point values for true precision and to ease understanding of this code. It also removes the bias. Another change is that it now has two internal values, numerator and denominator. Revision 1 also calculated those two values first, and later performed a division with them, which minimizes the precision loss caused by divisions. This method has risks of overflowing the two values and revision 1 caused problems due to that, but revision 3 won't thanks to double-precision. Therefore, revision 3 will theoretically have the result identical with the calculation with infinite-precision. Part-of: --- src/backends/native/meta-kms-utils.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/backends/native/meta-kms-utils.c b/src/backends/native/meta-kms-utils.c index 11df09be885..1c58db83d50 100644 --- a/src/backends/native/meta-kms-utils.c +++ b/src/backends/native/meta-kms-utils.c @@ -33,19 +33,18 @@ float meta_calculate_drm_mode_refresh_rate (const drmModeModeInfo *drm_mode) { - float refresh = 0.0; + double numerator; + double denominator; - if (drm_mode->htotal > 0 && drm_mode->vtotal > 0) - { - /* Calculate refresh rate in milliHz first for extra precision. */ - refresh = (drm_mode->clock * 1000000LL) / drm_mode->htotal; - refresh += (drm_mode->vtotal / 2); - refresh /= drm_mode->vtotal; - if (drm_mode->vscan > 1) - refresh /= drm_mode->vscan; - refresh /= 1000.0; - } - return refresh; + if (drm_mode->htotal <= 0 || drm_mode->vtotal <= 0) + return 0.0; + + numerator = drm_mode->clock * 1000.0; + denominator = (double) drm_mode->vtotal * drm_mode->htotal; + if (drm_mode->vscan > 1) + denominator *= drm_mode->vscan; + + return numerator / denominator; } /** -- GitLab From a6df6796ddc31a8e3093d327923a88e78222232b Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Fri, 26 Feb 2021 20:16:23 +0900 Subject: [PATCH 2/2] backend/native: Add tests for refresh rate calculation Part-of: --- src/backends/native/meta-kms-utils.h | 3 + src/tests/kms-utils-unit-tests.c | 145 +++++++++++++++++++++++++++ src/tests/kms-utils-unit-tests.h | 23 +++++ src/tests/meson.build | 2 + src/tests/unit-tests.c | 2 + 5 files changed, 175 insertions(+) create mode 100644 src/tests/kms-utils-unit-tests.c create mode 100644 src/tests/kms-utils-unit-tests.h diff --git a/src/backends/native/meta-kms-utils.h b/src/backends/native/meta-kms-utils.h index 7a2fdfd7900..c22ceaaa0ff 100644 --- a/src/backends/native/meta-kms-utils.h +++ b/src/backends/native/meta-kms-utils.h @@ -24,11 +24,14 @@ #include #include +#include "core/util-private.h" + typedef struct _MetaDrmFormatBuf { char s[5]; } MetaDrmFormatBuf; +META_EXPORT_TEST float meta_calculate_drm_mode_refresh_rate (const drmModeModeInfo *drm_mode); const char * meta_drm_format_to_string (MetaDrmFormatBuf *tmp, diff --git a/src/tests/kms-utils-unit-tests.c b/src/tests/kms-utils-unit-tests.c new file mode 100644 index 00000000000..743d46e0eb9 --- /dev/null +++ b/src/tests/kms-utils-unit-tests.c @@ -0,0 +1,145 @@ +/* + * Copyright (C) 2021 Akihiko Odaki + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + + +#include "config.h" + +#include "tests/kms-utils-unit-tests.h" + +#include "tests/test-utils.h" +#include "backends/native/meta-kms-utils.h" + +typedef struct { + drmModeModeInfo drm_mode; + float expected_refresh_rate; +} ModeInfoTestCase; + +static const ModeInfoTestCase test_cases[] = { + /* "cvt 640 480" */ + { + .drm_mode = { + .clock = 23975, + .hdisplay = 640, + .hsync_start = 664, + .hsync_end = 720, + .htotal = 800, + .vdisplay = 480, + .vsync_start = 483, + .vsync_end = 487, + .vtotal = 500, + .vscan = 0, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, + }, + .expected_refresh_rate = 59.9375, + }, + + /* "cvt 640 480" with htotal 0 */ + { + .drm_mode = { + .clock = 23975, + .hdisplay = 640, + .hsync_start = 664, + .hsync_end = 720, + .htotal = 0, + .vdisplay = 480, + .vsync_start = 483, + .vsync_end = 487, + .vtotal = 500, + .vscan = 0, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, + }, + .expected_refresh_rate = 0.0, + }, + + /* "cvt 640 480" with vtotal 0 */ + { + .drm_mode = { + .clock = 23975, + .hdisplay = 640, + .hsync_start = 664, + .hsync_end = 720, + .htotal = 800, + .vdisplay = 480, + .vsync_start = 483, + .vsync_end = 487, + .vtotal = 0, + .vscan = 0, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, + }, + .expected_refresh_rate = 0.0, + }, + + /* "cvt 320 240" with doubled clock and vscan 2 */ + { + .drm_mode = { + .clock = 12062, + .hdisplay = 320, + .hsync_start = 336, + .hsync_end = 360, + .htotal = 400, + .vdisplay = 240, + .vsync_start = 243, + .vsync_end = 247, + .vtotal = 252, + .vscan = 2, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, + }, + .expected_refresh_rate = 59.8313, + }, + + /* "cvt 15360 8640 180" */ + { + .drm_mode = { + .clock = 37793603, + .hdisplay = 15360, + .hsync_start = 16880, + .hsync_end = 18624, + .htotal = 21888, + .vdisplay = 8640, + .vsync_start = 8643, + .vsync_end = 8648, + .vtotal = 9593, + .vscan = 0, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, + }, + .expected_refresh_rate = 179.9939, + }, +}; + +static void +refresh_rate (void) +{ + size_t index; + + for (index = 0; index < G_N_ELEMENTS(test_cases); index++) + { + const ModeInfoTestCase *test_case = test_cases + index; + float refresh_rate; + + refresh_rate = + meta_calculate_drm_mode_refresh_rate (&test_case->drm_mode); + g_assert_cmpfloat_with_epsilon (refresh_rate, + test_case->expected_refresh_rate, + 0.0001); + } +} + +void +init_kms_utils_tests (void) +{ + g_test_add_func ("/kms-utils/refresh-rate", refresh_rate); +} diff --git a/src/tests/kms-utils-unit-tests.h b/src/tests/kms-utils-unit-tests.h new file mode 100644 index 00000000000..e85fd87fe01 --- /dev/null +++ b/src/tests/kms-utils-unit-tests.h @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2021 Akihiko Odaki + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + +#ifndef KMS_UTILS_UNIT_TESTS_H +#define KMS_UTILS_UNIT_TESTS_H + +void init_kms_utils_tests (void); + +#endif /* KMS_UTILS_UNIT_TESTS_H */ diff --git a/src/tests/meson.build b/src/tests/meson.build index f63e28952ce..05c6aaf701f 100644 --- a/src/tests/meson.build +++ b/src/tests/meson.build @@ -76,6 +76,8 @@ unit_tests = executable('mutter-test-unit-tests', 'unit-tests.c', 'boxes-tests.c', 'boxes-tests.h', + 'kms-utils-unit-tests.c', + 'kms-utils-unit-tests.h', 'meta-backend-test.c', 'meta-backend-test.h', 'meta-gpu-test.c', diff --git a/src/tests/unit-tests.c b/src/tests/unit-tests.c index 464fd769b3d..0ec709c1f4e 100644 --- a/src/tests/unit-tests.c +++ b/src/tests/unit-tests.c @@ -29,6 +29,7 @@ #include "core/boxes-private.h" #include "core/main-private.h" #include "tests/boxes-tests.h" +#include "tests/kms-utils-unit-tests.h" #include "tests/meta-backend-test.h" #include "tests/monitor-config-migration-unit-tests.h" #include "tests/monitor-unit-tests.h" @@ -250,6 +251,7 @@ init_tests (int argc, char **argv) g_test_add_func ("/core/boxes/adjacent-to", meta_test_adjacent_to); + init_kms_utils_tests (); init_monitor_store_tests (); init_monitor_config_migration_tests (); init_monitor_tests (); -- GitLab