| Message ID | 20241003200129.1732122-4-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Color Pipeline API w/ VKMS | expand |
On 03/10/24 - 16:00, Harry Wentland wrote: > 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 > > Signed-off-by: Harry Wentland <harry.wentland@amd.com> > Cc: Arthur Grillo <arthurgrillo@riseup.net> > --- > v6: > - Eliminate need to include test as .c file (Louis Chauvet) Thanks for the modification! With the checkpatch warning fixed: Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com> > 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/Kconfig | 15 ++ > drivers/gpu/drm/vkms/Makefile | 1 + > drivers/gpu/drm/vkms/tests/.kunitconfig | 4 + > drivers/gpu/drm/vkms/tests/Makefile | 3 + > drivers/gpu/drm/vkms/tests/vkms_color_test.c | 168 +++++++++++++++++++ > drivers/gpu/drm/vkms/vkms_composer.c | 10 +- > drivers/gpu/drm/vkms/vkms_composer.h | 13 ++ > 7 files changed, 211 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig > create mode 100644 drivers/gpu/drm/vkms/tests/Makefile > 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/Kconfig b/drivers/gpu/drm/vkms/Kconfig > index b9ecdebecb0b..59c4a32adb9d 100644 > --- a/drivers/gpu/drm/vkms/Kconfig > +++ b/drivers/gpu/drm/vkms/Kconfig > @@ -13,3 +13,18 @@ config DRM_VKMS > a VKMS. > > If M is selected the module will be called vkms. > + > +config DRM_VKMS_KUNIT_TESTS > + tristate "KUnit tests for VKMS." if !KUNIT_ALL_TESTS > + depends on DRM_VKMS=y && KUNIT > + default KUNIT_ALL_TESTS > + help > + This builds unit tests for VKMS. This option is not useful for > + distributions or general kernels, but only for kernel > + developers working on VKMS. > + > + For more information on KUnit and unit tests in general, > + please refer to the KUnit documentation in > + Documentation/dev-tools/kunit/. > + > + If in doubt, say "N". > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > index 1b28a6a32948..8d3e46dde635 100644 > --- a/drivers/gpu/drm/vkms/Makefile > +++ b/drivers/gpu/drm/vkms/Makefile > @@ -9,3 +9,4 @@ vkms-y := \ > vkms_writeback.o > > obj-$(CONFIG_DRM_VKMS) += vkms.o > +obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += tests/ > diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig b/drivers/gpu/drm/vkms/tests/.kunitconfig > new file mode 100644 > index 000000000000..70e378228cbd > --- /dev/null > +++ b/drivers/gpu/drm/vkms/tests/.kunitconfig > @@ -0,0 +1,4 @@ > +CONFIG_KUNIT=y > +CONFIG_DRM=y > +CONFIG_DRM_VKMS=y > +CONFIG_DRM_VKMS_KUNIT_TESTS=y > diff --git a/drivers/gpu/drm/vkms/tests/Makefile b/drivers/gpu/drm/vkms/tests/Makefile > new file mode 100644 > index 000000000000..7876ca7a3c42 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/tests/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += vkms_color_test.o > \ No newline at end of file > 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..efe139978860 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c > @@ -0,0 +1,168 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ checkpatch ask for a // comment here for the SPDX license (If you know, why checkpatch ask for a // in .c but /* */ in .h?) > + > +#include <kunit/test.h> > + > +#include <drm/drm_fixed.h> > +#include <drm/drm_mode.h> > +#include "../vkms_drv.h" > +#include "../vkms_composer.h" > + > +#define TEST_LUT_SIZE 16 > + > +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING); > + > +static struct drm_color_lut test_linear_array[TEST_LUT_SIZE] = { > + { 0x0, 0x0, 0x0, 0 }, > + { 0x1111, 0x1111, 0x1111, 0 }, > + { 0x2222, 0x2222, 0x2222, 0 }, > + { 0x3333, 0x3333, 0x3333, 0 }, > + { 0x4444, 0x4444, 0x4444, 0 }, > + { 0x5555, 0x5555, 0x5555, 0 }, > + { 0x6666, 0x6666, 0x6666, 0 }, > + { 0x7777, 0x7777, 0x7777, 0 }, > + { 0x8888, 0x8888, 0x8888, 0 }, > + { 0x9999, 0x9999, 0x9999, 0 }, > + { 0xaaaa, 0xaaaa, 0xaaaa, 0 }, > + { 0xbbbb, 0xbbbb, 0xbbbb, 0 }, > + { 0xcccc, 0xcccc, 0xcccc, 0 }, > + { 0xdddd, 0xdddd, 0xdddd, 0 }, > + { 0xeeee, 0xeeee, 0xeeee, 0 }, > + { 0xffff, 0xffff, 0xffff, 0 }, > +}; > + > +const struct vkms_color_lut test_linear_lut = { > + .base = test_linear_array, > + .lut_length = TEST_LUT_SIZE, > + .channel_value2index_ratio = 0xf000fll > +}; > + > + checkpatch: Please don't use multiple blank lines > +static void vkms_color_test_get_lut_index(struct kunit *test) > +{ > + int i; > + > + KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&test_linear_lut, test_linear_array[0].red)), 0); checkpatch: line length of 106 exceeds 100 columns KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&test_linear_lut, test_linear_array[0].red)), 0); > + > + for (i = 0; i < TEST_LUT_SIZE; i++) > + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&test_linear_lut, test_linear_array[i].red)), i); checkpatch: line length of 119 exceeds 100 columns KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&test_linear_lut, test_linear_array[i].red)), i); > +} > + > +static void vkms_color_test_lerp(struct kunit *test) > +{ > + /*** half-way round down ***/ > + s64 t = 0x80000000 - 1; checkpatch: Missing a blank line after declarations > + 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); > + > + checkpatch: Please don't use multiple blank lines > + /*** 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); > + > + checkpatch: Please don't use multiple blank lines > + /*** 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); > + > + checkpatch: Please don't use multiple blank lines > + /*** 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); > +} > + > +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, > +}; checkpatch: Please use a blank line after function/struct/union/enum declarations > +kunit_test_suite(vkms_color_test_suite); > + > +MODULE_LICENSE("GPL"); > \ No newline at end of file checkpatch: adding a line without newline at end of file > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c > index 3d6785d081f2..93f4ed5ea52f 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" > > static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha) > { > @@ -91,7 +93,7 @@ static void fill_background(const struct pixel_argb_u16 *background_color, > } > > // lerp(a, b, t) = a + (b - a) * t > -static u16 lerp_u16(u16 a, u16 b, s64 t) > +VISIBLE_IF_KUNIT u16 lerp_u16(u16 a, u16 b, s64 t) > { > s64 a_fp = drm_int2fixp(a); > s64 b_fp = drm_int2fixp(b); > @@ -100,13 +102,15 @@ static u16 lerp_u16(u16 a, u16 b, s64 t) > > return drm_fixp2int_round(a_fp + delta); > } > +EXPORT_SYMBOL_IF_KUNIT(lerp_u16); > > -static s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value) > +VISIBLE_IF_KUNIT s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value) > { > s64 color_channel_fp = drm_int2fixp(channel_value); > > return drm_fixp_mul(color_channel_fp, lut->channel_value2index_ratio); > } > +EXPORT_SYMBOL_IF_KUNIT(get_lut_index); > > /* > * This enum is related to the positions of the variables inside > @@ -434,4 +438,4 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name) > vkms_set_composer(out, enabled); > > return ret; > -} > +} > \ No newline at end of file checkpatch: adding a line without newline at end of file > diff --git a/drivers/gpu/drm/vkms/vkms_composer.h b/drivers/gpu/drm/vkms/vkms_composer.h > new file mode 100644 > index 000000000000..9316a053e7d7 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/vkms_composer.h > @@ -0,0 +1,13 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#ifndef _VKMS_COMPOSER_H_ > +#define _VKMS_COMPOSER_H_ > + > +#include <kunit/visibility.h> > + > +#if IS_ENABLED(CONFIG_KUNIT) > +u16 lerp_u16(u16 a, u16 b, s64 t); > +s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value); > +#endif > + > +#endif /* _VKMS_COMPOSER_H_ */ > -- > 2.46.2 >
Hi Harry, kernel test robot noticed the following build warnings: [auto build test WARNING on drm/drm-next] [also build test WARNING on drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-misc/drm-misc-next drm-tip/drm-tip linus/master v6.12-rc1 next-20241004] [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/Harry-Wentland/drm-Add-helper-for-conversion-from-signed-magnitude/20241004-040629 base: git://anongit.freedesktop.org/drm/drm drm-next patch link: https://lore.kernel.org/r/20241003200129.1732122-4-harry.wentland%40amd.com patch subject: [PATCH v6 03/44] drm/vkms: Add kunit tests for VKMS LUT handling config: csky-randconfig-r112-20241005 (https://download.01.org/0day-ci/archive/20241005/202410051655.9wEF2CHW-lkp@intel.com/config) compiler: csky-linux-gcc (GCC) 14.1.0 reproduce: (https://download.01.org/0day-ci/archive/20241005/202410051655.9wEF2CHW-lkp@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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410051655.9wEF2CHW-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/vkms/tests/vkms_color_test.c:33:29: sparse: sparse: symbol 'test_linear_lut' was not declared. Should it be static? vim +/test_linear_lut +33 drivers/gpu/drm/vkms/tests/vkms_color_test.c 32 > 33 const struct vkms_color_lut test_linear_lut = { 34 .base = test_linear_array, 35 .lut_length = TEST_LUT_SIZE, 36 .channel_value2index_ratio = 0xf000fll 37 }; 38 39
On 10/3/24 13:00, Harry Wentland wrote: > Debugging LUT math is much easier when we can unit 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..efe139978860 > --- /dev/null > +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c > @@ -0,0 +1,168 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#include <kunit/test.h> ... > +kunit_test_suite(vkms_color_test_suite); > + > +MODULE_LICENSE("GPL"); Since commit 1fffe7a34c89 ("script: modpost: emit a warning when the description is missing"), a module without a MODULE_DESCRIPTION() will result in a warning when built with make W=1. Please add the missing MODULE_DESCRIPTION() /jeff
diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig index b9ecdebecb0b..59c4a32adb9d 100644 --- a/drivers/gpu/drm/vkms/Kconfig +++ b/drivers/gpu/drm/vkms/Kconfig @@ -13,3 +13,18 @@ config DRM_VKMS a VKMS. If M is selected the module will be called vkms. + +config DRM_VKMS_KUNIT_TESTS + tristate "KUnit tests for VKMS." if !KUNIT_ALL_TESTS + depends on DRM_VKMS=y && KUNIT + default KUNIT_ALL_TESTS + help + This builds unit tests for VKMS. This option is not useful for + distributions or general kernels, but only for kernel + developers working on VKMS. + + For more information on KUnit and unit tests in general, + please refer to the KUnit documentation in + Documentation/dev-tools/kunit/. + + If in doubt, say "N". diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile index 1b28a6a32948..8d3e46dde635 100644 --- a/drivers/gpu/drm/vkms/Makefile +++ b/drivers/gpu/drm/vkms/Makefile @@ -9,3 +9,4 @@ vkms-y := \ vkms_writeback.o obj-$(CONFIG_DRM_VKMS) += vkms.o +obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += tests/ diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig b/drivers/gpu/drm/vkms/tests/.kunitconfig new file mode 100644 index 000000000000..70e378228cbd --- /dev/null +++ b/drivers/gpu/drm/vkms/tests/.kunitconfig @@ -0,0 +1,4 @@ +CONFIG_KUNIT=y +CONFIG_DRM=y +CONFIG_DRM_VKMS=y +CONFIG_DRM_VKMS_KUNIT_TESTS=y diff --git a/drivers/gpu/drm/vkms/tests/Makefile b/drivers/gpu/drm/vkms/tests/Makefile new file mode 100644 index 000000000000..7876ca7a3c42 --- /dev/null +++ b/drivers/gpu/drm/vkms/tests/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only + +obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += vkms_color_test.o \ No newline at end of file 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..efe139978860 --- /dev/null +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c @@ -0,0 +1,168 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#include <kunit/test.h> + +#include <drm/drm_fixed.h> +#include <drm/drm_mode.h> +#include "../vkms_drv.h" +#include "../vkms_composer.h" + +#define TEST_LUT_SIZE 16 + +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING); + +static struct drm_color_lut test_linear_array[TEST_LUT_SIZE] = { + { 0x0, 0x0, 0x0, 0 }, + { 0x1111, 0x1111, 0x1111, 0 }, + { 0x2222, 0x2222, 0x2222, 0 }, + { 0x3333, 0x3333, 0x3333, 0 }, + { 0x4444, 0x4444, 0x4444, 0 }, + { 0x5555, 0x5555, 0x5555, 0 }, + { 0x6666, 0x6666, 0x6666, 0 }, + { 0x7777, 0x7777, 0x7777, 0 }, + { 0x8888, 0x8888, 0x8888, 0 }, + { 0x9999, 0x9999, 0x9999, 0 }, + { 0xaaaa, 0xaaaa, 0xaaaa, 0 }, + { 0xbbbb, 0xbbbb, 0xbbbb, 0 }, + { 0xcccc, 0xcccc, 0xcccc, 0 }, + { 0xdddd, 0xdddd, 0xdddd, 0 }, + { 0xeeee, 0xeeee, 0xeeee, 0 }, + { 0xffff, 0xffff, 0xffff, 0 }, +}; + +const struct vkms_color_lut test_linear_lut = { + .base = test_linear_array, + .lut_length = TEST_LUT_SIZE, + .channel_value2index_ratio = 0xf000fll +}; + + +static void vkms_color_test_get_lut_index(struct kunit *test) +{ + int i; + + KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&test_linear_lut, test_linear_array[0].red)), 0); + + for (i = 0; i < TEST_LUT_SIZE; i++) + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&test_linear_lut, test_linear_array[i].red)), i); +} + +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); +} + +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_LICENSE("GPL"); \ No newline at end of file diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c index 3d6785d081f2..93f4ed5ea52f 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" static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha) { @@ -91,7 +93,7 @@ static void fill_background(const struct pixel_argb_u16 *background_color, } // lerp(a, b, t) = a + (b - a) * t -static u16 lerp_u16(u16 a, u16 b, s64 t) +VISIBLE_IF_KUNIT u16 lerp_u16(u16 a, u16 b, s64 t) { s64 a_fp = drm_int2fixp(a); s64 b_fp = drm_int2fixp(b); @@ -100,13 +102,15 @@ static u16 lerp_u16(u16 a, u16 b, s64 t) return drm_fixp2int_round(a_fp + delta); } +EXPORT_SYMBOL_IF_KUNIT(lerp_u16); -static s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value) +VISIBLE_IF_KUNIT s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value) { s64 color_channel_fp = drm_int2fixp(channel_value); return drm_fixp_mul(color_channel_fp, lut->channel_value2index_ratio); } +EXPORT_SYMBOL_IF_KUNIT(get_lut_index); /* * This enum is related to the positions of the variables inside @@ -434,4 +438,4 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name) vkms_set_composer(out, enabled); return ret; -} +} \ No newline at end of file diff --git a/drivers/gpu/drm/vkms/vkms_composer.h b/drivers/gpu/drm/vkms/vkms_composer.h new file mode 100644 index 000000000000..9316a053e7d7 --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_composer.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#ifndef _VKMS_COMPOSER_H_ +#define _VKMS_COMPOSER_H_ + +#include <kunit/visibility.h> + +#if IS_ENABLED(CONFIG_KUNIT) +u16 lerp_u16(u16 a, u16 b, s64 t); +s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value); +#endif + +#endif /* _VKMS_COMPOSER_H_ */
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 Signed-off-by: Harry Wentland <harry.wentland@amd.com> Cc: Arthur Grillo <arthurgrillo@riseup.net> --- 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/Kconfig | 15 ++ drivers/gpu/drm/vkms/Makefile | 1 + drivers/gpu/drm/vkms/tests/.kunitconfig | 4 + drivers/gpu/drm/vkms/tests/Makefile | 3 + drivers/gpu/drm/vkms/tests/vkms_color_test.c | 168 +++++++++++++++++++ drivers/gpu/drm/vkms/vkms_composer.c | 10 +- drivers/gpu/drm/vkms/vkms_composer.h | 13 ++ 7 files changed, 211 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig create mode 100644 drivers/gpu/drm/vkms/tests/Makefile create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_test.c create mode 100644 drivers/gpu/drm/vkms/vkms_composer.h -- 2.46.2