Re: [PATCH V10 15/46] drm/vkms: Add kunit tests for linear and sRGB LUTs
Hi Alex, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.16-rc2] [cannot apply to drm-exynos/exynos-drm-next drm/drm-next drm-misc/drm-misc-next next-20250618] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Alex-Hung/drm-Add-helper-for-conversion-from-signed-magnitude/20250617-123027 base: linus/master patch link: https://lore.kernel.org/r/20250617041746.2884343-16-alex.hung%40amd.com patch subject: [PATCH V10 15/46] drm/vkms: Add kunit tests for linear and sRGB LUTs config: riscv-randconfig-001-20250618 (https://download.01.org/0day-ci/archive/20250619/202506190132.r51ujygm-...@intel.com/config) compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250619/202506190132.r51ujygm-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202506190132.r51ujygm-...@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "srgb_eotf" [drivers/gpu/drm/vkms/tests/vkms_color_test.ko] >> undefined! >> ERROR: modpost: "srgb_inv_eotf" >> [drivers/gpu/drm/vkms/tests/vkms_color_test.ko] undefined! >> ERROR: modpost: "linear_eotf" >> [drivers/gpu/drm/vkms/tests/vkms_color_test.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH V10 02/46] drm/vkms: Add kunit tests for VKMS LUT handling
Hi Alex,
On 17/06/25 01:16, Alex Hung wrote:
From: Harry Wentland
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
Signed-off-by: Alex Hung
Signed-off-by: Harry Wentland
Cc: Arthur Grillo
Reviewed-by: Daniel Stone
---
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 ..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 = 0x8000 - 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 = 0x8000;
+ 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 = 0x1;
+ 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 = 0x1 - 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 t
Re: [PATCH V10 19/46] drm/tests: Add a few tests around drm_fixed.h
Hi Alex,
On 17/06/25 01:17, Alex Hung wrote:
From: Harry Wentland
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
Signed-off-by: Alex Hung
Signed-off-by: Harry Wentland
Reviewed-by: Daniel Stone
---
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 ..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
+#include
+
+static void drm_test_sm2fixp(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, 0x7fffll, ((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`?
+ .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");
