On 2025-06-17 07:04, Maíra Canal wrote: > Hi Alex, > > On 17/06/25 01:17, Alex Hung wrote: >> From: Harry Wentland <harry.wentl...@amd.com> >> >> While working on the CTM implementation of VKMS I had to ascertain >> myself of a few assumptions. One of those is whether drm_fixed.h >> treats its numbers using signed-magnitude or twos-complement. It is >> twos-complement. >> >> In order to make someone else's day easier I am adding the >> drm_test_int2fixp test that validates the above assumption. >> >> I am also adding a test for the new sm2fixp function that converts >> from a signed-magnitude fixed point to the twos-complement fixed >> point. >> >> 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> >> Reviewed-by: Daniel Stone <dani...@collabora.com> >> --- >> v7: >> - Fix checkpatch warnings (Louis Chauvet) >> >> v6: >> - add missing MODULE_DESCRIPTION (Jeff Johnson) >> - fix buffer overflow >> >> >> drivers/gpu/drm/tests/Makefile | 3 +- >> drivers/gpu/drm/tests/drm_fixp_test.c | 71 +++++++++++++++++++++++++++ >> 2 files changed, 73 insertions(+), 1 deletion(-) >> create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c >> >> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/ >> Makefile >> index 3afd6587df08..91437cf34d92 100644 >> --- a/drivers/gpu/drm/tests/Makefile >> +++ b/drivers/gpu/drm/tests/Makefile >> @@ -23,6 +23,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \ >> drm_modes_test.o \ >> drm_plane_helper_test.o \ >> drm_probe_helper_test.o \ >> - drm_rect_test.o >> + drm_rect_test.o \ >> + drm_fixp_test.o >> CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN) >> diff --git a/drivers/gpu/drm/tests/drm_fixp_test.c b/drivers/gpu/drm/ >> tests/drm_fixp_test.c >> new file mode 100644 >> index 000000000000..de91177af213 >> --- /dev/null >> +++ b/drivers/gpu/drm/tests/drm_fixp_test.c >> @@ -0,0 +1,71 @@ >> +// SPDX-License-Identifier: MIT >> +/* >> + * Copyright 2022 Advanced Micro Devices, Inc. >> + */ >> + >> +#include <kunit/test.h> >> +#include <drm/drm_fixed.h> >> + >> +static void drm_test_sm2fixp(struct kunit *test) >> +{ >> + KUNIT_EXPECT_EQ(test, 0x7fffffffffffffffll, ((1ull << 63) - 1)); >> + >> + /* 1 */ >> + KUNIT_EXPECT_EQ(test, drm_int2fixp(1), drm_sm2fixp(1ull << >> DRM_FIXED_POINT)); >> + >> + /* -1 */ >> + KUNIT_EXPECT_EQ(test, drm_int2fixp(-1), >> + drm_sm2fixp((1ull << 63) | (1ull << DRM_FIXED_POINT))); >> + >> + /* 0.5 */ >> + KUNIT_EXPECT_EQ(test, drm_fixp_from_fraction(1, 2), >> + drm_sm2fixp(1ull << (DRM_FIXED_POINT - 1))); >> + >> + /* -0.5 */ >> + KUNIT_EXPECT_EQ(test, drm_fixp_from_fraction(-1, 2), >> + drm_sm2fixp((1ull << 63) | (1ull << (DRM_FIXED_POINT - >> 1)))); >> +} >> + >> +static void drm_test_int2fixp(struct kunit *test) >> +{ >> + /* 1 */ >> + KUNIT_EXPECT_EQ(test, 1ll << 32, drm_int2fixp(1)); >> + >> + /* -1 */ >> + KUNIT_EXPECT_EQ(test, -(1ll << 32), drm_int2fixp(-1)); >> + >> + /* 1 + (-1) = 0 */ >> + KUNIT_EXPECT_EQ(test, 0, drm_int2fixp(1) + drm_int2fixp(-1)); >> + >> + /* 1 / 2 */ >> + KUNIT_EXPECT_EQ(test, 1ll << 31, drm_fixp_from_fraction(1, 2)); >> + >> + /* -0.5 */ >> + KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(-1, 2)); >> + >> + /* (1 / 2) + (-1) = 0.5 */ >> + KUNIT_EXPECT_EQ(test, 1ll << 31, drm_fixp_from_fraction(-1, 2) + >> drm_int2fixp(1)); >> + >> + /* (1 / 2) - 1) = 0.5 */ >> + KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(1, 2) >> + drm_int2fixp(-1)); >> + >> + /* (1 / 2) - 1) = 0.5 */ >> + KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(1, 2) >> - drm_int2fixp(1)); >> +} >> + >> +static struct kunit_case drm_fixp_tests[] = { >> + KUNIT_CASE(drm_test_int2fixp), >> + KUNIT_CASE(drm_test_sm2fixp), >> + { } >> +}; >> + >> +static struct kunit_suite drm_rect_test_suite = { > > Instead of `drm_rect_test_suite`, how about `drm_fixp_test_suite`? >
Of course. I forgot to rename it when copy-pasting the example code. Thanks. Harry >> + .name = "drm_fixp", >> + .test_cases = drm_fixp_tests, >> +}; >> + >> +kunit_test_suite(drm_rect_test_suite); >> + >> +MODULE_AUTHOR("AMD"); >> +MODULE_LICENSE("GPL and additional rights"); > > From the kernel docs, "GPL and additional rights" shouldn't be used in > new code [1]. > > [1] https://docs.kernel.org/process/license-rules.html#id1 > > Best Regards, > - Maíra > >> +MODULE_DESCRIPTION("Unit tests for drm_fixed.h"); >