Hi Alex, On 17/06/25 01:16, Alex Hung wrote:
From: Harry Wentland <harry.wentl...@amd.com>Debugging LUT math is much easier when we can unit test it. Add kunit functionality to VKMS and add tests for - get_lut_index - lerp_u16 Reviewed-by: Louis Chauvet <louis.chau...@bootlin.com> Signed-off-by: Alex Hung <alex.h...@amd.com> Signed-off-by: Harry Wentland <harry.wentl...@amd.com> Cc: Arthur Grillo <arthurgri...@riseup.net> Reviewed-by: Daniel Stone <dani...@collabora.com> --- v8: - Update config names (Louis Chauvet) v7: - Fix checkpatch warnings and errors (Louis Chauvet) - Change SPDX-License-Identifier: GPL-2.0+ from /* */ to // - Fix checkpatch errors and warnings (new line at EOF, redundant spaces, and long lines) - Add static to const struct vkms_color_lut test_linear_lut - Add "MODULE_DESCRIPTION" (Jeff Johnson) v6: - Eliminate need to include test as .c file (Louis Chauvet) v5: - Bring back static for lerp_u16 and get_lut_index (Arthur) v4: - Test the critical points of the lerp function (Pekka) v3: - Use include way of testing static functions (Arthur) drivers/gpu/drm/vkms/tests/Makefile | 2 +- drivers/gpu/drm/vkms/tests/vkms_color_test.c | 172 +++++++++++++++++++ drivers/gpu/drm/vkms/vkms_composer.c | 8 +- drivers/gpu/drm/vkms/vkms_composer.h | 13 ++ 4 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_test.c create mode 100644 drivers/gpu/drm/vkms/vkms_composer.h diff --git a/drivers/gpu/drm/vkms/tests/Makefile b/drivers/gpu/drm/vkms/tests/Makefile index 9ded37b67a46..e92f7143e540 100644 --- a/drivers/gpu/drm/vkms/tests/Makefile +++ b/drivers/gpu/drm/vkms/tests/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only-obj-$(CONFIG_DRM_VKMS_KUNIT_TEST) += vkms_config_test.o+obj-$(CONFIG_DRM_VKMS_KUNIT_TEST) += vkms_config_test.o vkms_color_test.o
I believe you might need to rebase this patch on top of drm-misc-next
due to 60ba94338047 ("drm/vkms: Compile all tests with
CONFIG_DRM_VKMS_KUNIT_TEST").
diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_test.c b/drivers/gpu/drm/vkms/tests/vkms_color_test.c new file mode 100644 index 000000000000..affca56cac7b --- /dev/null +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
[...]
+
+static void vkms_color_test_lerp(struct kunit *test)
+{
+ /*** half-way round down ***/
+ s64 t = 0x80000000 - 1;
+
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x8);
+
+ /* odd a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x8);
+
+ /* odd b */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x8);
+
+ /* b = a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
+
+ /* b = a + 1 */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
+
+ /*** half-way round up ***/
+ t = 0x80000000;
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x8);
+
+ /* odd a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x9);
+
+ /* odd b */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x8);
+
+ /* b = a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
+
+ /* b = a + 1 */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x11);
+
+ /*** t = 0.0 ***/
+ t = 0x0;
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x0);
+
+ /* odd a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x1);
+
+ /* odd b */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x1);
+
+ /* b = a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
+
+ /* b = a + 1 */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
+
+ /*** t = 1.0 ***/
+ t = 0x100000000;
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x10);
+
+ /* odd a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x10);
+
+ /* odd b */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0xf);
+
+ /* b = a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
+
+ /* b = a + 1 */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x11);
+
+ /*** t = 0.0 + 1 ***/
+ t = 0x0 + 1;
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x0);
+
+ /* odd a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x1);
+
+ /* odd b */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x1);
+
+ /* b = a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
+
+ /* b = a + 1 */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
+
+ /*** t = 1.0 - 1 ***/
+ t = 0x100000000 - 1;
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x10);
+
+ /* odd a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x10);
+
+ /* odd b */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0xf);
+
+ /* b = a */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
+
+ /* b = a + 1 */
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x11);
+
+ /*** t chosen to verify the flipping point of result a (or b) to a+1
(or b-1) ***/
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x1, 0x80000000 - 1), 0x0);
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x1, 0x80000000), 0x1);
+}
This test case seems to be a perfect use-case for parametized tests [1]. [1] https://docs.kernel.org/dev-tools/kunit/usage.html#parameterized-testing
+ +static struct kunit_case vkms_color_test_cases[] = { + KUNIT_CASE(vkms_color_test_get_lut_index), + KUNIT_CASE(vkms_color_test_lerp), + {} +}; + +static struct kunit_suite vkms_color_test_suite = { + .name = "vkms-color", + .test_cases = vkms_color_test_cases, +}; + +kunit_test_suite(vkms_color_test_suite); + +MODULE_DESCRIPTION("Kunit test for VKMS LUT handling"); +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c index fa269d279e25..b0dc95f971d8 100644 --- a/drivers/gpu/drm/vkms/vkms_composer.c +++ b/drivers/gpu/drm/vkms/vkms_composer.c @@ -12,6 +12,8 @@ #include <linux/minmax.h>#include "vkms_drv.h"+#include <kunit/visibility.h> +#include "vkms_composer.h"
Nit: IIRC, it's common to sort the includes entries. Best Regards, - Maíra
