[PATCH V10 02/46] drm/vkms: Add kunit tests for VKMS LUT handling
Maíra Canal
mcanal at igalia.com
Tue Jun 17 10:54:36 UTC 2025
Hi Alex,
On 17/06/25 01:16, Alex Hung wrote:
> From: Harry Wentland <harry.wentland at 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.chauvet at bootlin.com>
> Signed-off-by: Alex Hung <alex.hung at amd.com>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> Cc: Arthur Grillo <arthurgrillo at riseup.net>
> Reviewed-by: Daniel Stone <daniels at 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
More information about the amd-gfx
mailing list