From: Arthur Grillo <[email protected]>
Create KUnit tests to test the conversion between YUV and RGB. Test each
conversion and range combination with some common colors.
The code used to compute the expected result can be found in comment.
Signed-off-by: Arthur Grillo <[email protected]>
[Louis Chauvet:
- fix minor formating issues (whitespace, double line)
- change expected alpha from 0x0000 to 0xffff
- adapt to the new get_conversion_matrix usage
- apply the changes from Arthur
- move struct pixel_yuv_u8 to the test itself]
Signed-off-by: Louis Chauvet <[email protected]>
---
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_format_test.c | 230 ++++++++++++++++++++++++++
drivers/gpu/drm/vkms/vkms_formats.c | 7 +-
drivers/gpu/drm/vkms/vkms_formats.h | 4 +
7 files changed, 262 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
index b9ecdebecb0b..9b0e1940c14f 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 "Tests for VKMS" if !KUNIT_ALL_TESTS
+ depends on DRM_VKMS && 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".
\ No newline at end of file
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..2d1df668569e
--- /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_format_test.o
diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
new file mode 100644
index 000000000000..0954d606e44a
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <kunit/test.h>
+
+#include <drm/drm_fixed.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_print.h>
+
+#include "../../drm_crtc_internal.h"
+
+#include "../vkms_drv.h"
+#include "../vkms_formats.h"
+
+#define TEST_BUFF_SIZE 50
+
+struct pixel_yuv_u8 {
+ u8 y, u, v;
+};
+
+struct yuv_u8_to_argb_u16_case {
+ enum drm_color_encoding encoding;
+ enum drm_color_range range;
+ size_t n_colors;
+ struct format_pair {
+ char *name;
+ struct pixel_yuv_u8 yuv;
+ struct pixel_argb_u16 argb;
+ } colors[TEST_BUFF_SIZE];
+};
+
+/*
+ * The YUV color representation were acquired via the colour python framework.
+ * Below are the function calls used for generating each case.
+ *
+ * for more information got to the docs:
+ * https://colour.readthedocs.io/en/master/generated/colour.RGB_to_YCbCr.html
+ */
+static struct yuv_u8_to_argb_u16_case yuv_u8_to_argb_u16_cases[] = {
+ /*
+ * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
+ * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
+ * in_bits = 16,
+ * in_legal = False,
+ * in_int = True,
+ * out_bits = 8,
+ * out_legal = False,
+ * out_int = True)
+ */
+ {
+ .encoding = DRM_COLOR_YCBCR_BT601,
+ .range = DRM_COLOR_YCBCR_FULL_RANGE,
+ .n_colors = 6,
+ .colors = {
+ { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
+ { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
+ { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
+ { "red", { 0x4c, 0x55, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
+ { "green", { 0x96, 0x2c, 0x15 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
+ { "blue", { 0x1d, 0xff, 0x6b }, { 0xffff, 0x0000, 0x0000, 0xffff }},
+ },
+ },
+ /*
+ * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
+ * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
+ * in_bits = 16,
+ * in_legal = False,
+ * in_int = True,
+ * out_bits = 8,
+ * out_legal = True,
+ * out_int = True)
+ */
+ {
+ .encoding = DRM_COLOR_YCBCR_BT601,
+ .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ .n_colors = 6,
+ .colors = {
+ { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
+ { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
+ { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
+ { "red", { 0x51, 0x5a, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
+ { "green", { 0x91, 0x36, 0x22 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
+ { "blue", { 0x29, 0xf0, 0x6e }, { 0xffff, 0x0000, 0x0000, 0xffff }},
+ },
+ },
+ /*
+ * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
+ * K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
+ * in_bits = 16,
+ * in_legal = False,
+ * in_int = True,
+ * out_bits = 8,
+ * out_legal = False,
+ * out_int = True)
+ */
+ {
+ .encoding = DRM_COLOR_YCBCR_BT709,
+ .range = DRM_COLOR_YCBCR_FULL_RANGE,
+ .n_colors = 4,
+ .colors = {
+ { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
+ { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
+ { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
+ { "red", { 0x36, 0x63, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
+ { "green", { 0xb6, 0x1e, 0x0c }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
+ { "blue", { 0x12, 0xff, 0x74 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
+ },
+ },
+ /*
+ * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
+ * K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
+ * in_bits = 16,
+ * int_legal = False,
+ * in_int = True,
+ * out_bits = 8,
+ * out_legal = True,
+ * out_int = True)
+ */
+ {
+ .encoding = DRM_COLOR_YCBCR_BT709,
+ .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ .n_colors = 4,
+ .colors = {
+ { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
+ { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
+ { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
+ { "red", { 0x3f, 0x66, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
+ { "green", { 0xad, 0x2a, 0x1a }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
+ { "blue", { 0x20, 0xf0, 0x76 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
+ },
+ },
+ /*
+ * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
+ * K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
+ * in_bits = 16,
+ * in_legal = False,
+ * in_int = True,
+ * out_bits = 8,
+ * out_legal = False,
+ * out_int = True)
+ */
+ {
+ .encoding = DRM_COLOR_YCBCR_BT2020,
+ .range = DRM_COLOR_YCBCR_FULL_RANGE,
+ .n_colors = 4,
+ .colors = {
+ { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
+ { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
+ { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
+ { "red", { 0x43, 0x5c, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
+ { "green", { 0xad, 0x24, 0x0b }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
+ { "blue", { 0x0f, 0xff, 0x76 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
+ },
+ },
+ /*
+ * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
+ * K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
+ * in_bits = 16,
+ * in_legal = False,
+ * in_int = True,
+ * out_bits = 8,
+ * out_legal = True,
+ * out_int = True)
+ */
+ {
+ .encoding = DRM_COLOR_YCBCR_BT2020,
+ .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ .n_colors = 4,
+ .colors = {
+ { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
+ { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
+ { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
+ { "red", { 0x4a, 0x61, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
+ { "green", { 0xa4, 0x2f, 0x19 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
+ { "blue", { 0x1d, 0xf0, 0x77 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
+ },
+ },
+};
+
+static void vkms_format_test_yuv_u8_to_argb_u16(struct kunit *test)
+{
+ const struct yuv_u8_to_argb_u16_case *param = test->param_value;
+ struct pixel_argb_u16 argb;
+
+ for (size_t i = 0; i < param->n_colors; i++) {
+ const struct format_pair *color = ¶m->colors[i];
+
+ struct conversion_matrix *matrix = get_conversion_matrix_to_argb_u16
+ (DRM_FORMAT_NV12, param->encoding, param->range);
+
+ argb = argb_u16_from_yuv888(color->yuv.y, color->yuv.u, color->yuv.v, matrix);
+
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.a, color->argb.a), 257,
+ "On the A channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.a, argb.a);
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.r, color->argb.r), 257,
+ "On the R channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.r, argb.r);
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.g, color->argb.g), 257,
+ "On the G channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.g, argb.g);
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.b, color->argb.b), 257,
+ "On the B channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.b, argb.b);
+ }
+}
+
+static void vkms_format_test_yuv_u8_to_argb_u16_case_desc(struct yuv_u8_to_argb_u16_case *t,
+ char *desc)
+{
+ snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s - %s",
+ drm_get_color_encoding_name(t->encoding), drm_get_color_range_name(t->range));
+}
+
+KUNIT_ARRAY_PARAM(yuv_u8_to_argb_u16, yuv_u8_to_argb_u16_cases,
+ vkms_format_test_yuv_u8_to_argb_u16_case_desc
+);
+
+static struct kunit_case vkms_format_test_cases[] = {
+ KUNIT_CASE_PARAM(vkms_format_test_yuv_u8_to_argb_u16, yuv_u8_to_argb_u16_gen_params),
+ {}
+};
+
+static struct kunit_suite vkms_format_test_suite = {
+ .name = "vkms-format",
+ .test_cases = vkms_format_test_cases,
+};
+
+kunit_test_suite(vkms_format_test_suite);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index edbf4b321b91..863fc91d6d48 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -7,6 +7,8 @@
#include <drm/drm_rect.h>
#include <drm/drm_fixed.h>
+#include <kunit/visibility.h>
+
#include "vkms_formats.h"
/**
@@ -199,8 +201,8 @@ static struct pixel_argb_u16 argb_u16_from_RGB565(const u16 *pixel)
return out_pixel;
}
-static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
- struct conversion_matrix *matrix)
+VISIBLE_IF_KUNIT struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
+ struct conversion_matrix *matrix)
{
u8 r, g, b;
s64 fp_y, fp_cb, fp_cr;
@@ -234,6 +236,7 @@ static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
return argb_u16_from_u8888(255, r, g, b);
}
+EXPORT_SYMBOL_IF_KUNIT(argb_u16_from_yuv888);
/*
* The following functions are read_line function for each pixel format supported by VKMS.
diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
index e1d324764b17..21e66a0cac16 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.h
+++ b/drivers/gpu/drm/vkms/vkms_formats.h
@@ -13,4 +13,8 @@ struct conversion_matrix *
get_conversion_matrix_to_argb_u16(u32 format, enum drm_color_encoding encoding,
enum drm_color_range range);
+#if IS_ENABLED(CONFIG_KUNIT)
+struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr, struct conversion_matrix *matrix);
+#endif
+
#endif /* _VKMS_FORMATS_H_ */
--
2.43.0
On 3/13/24 14:45, Louis Chauvet wrote:
> From: Arthur Grillo <[email protected]>
>
> Create KUnit tests to test the conversion between YUV and RGB. Test each
> conversion and range combination with some common colors.
>
> The code used to compute the expected result can be found in comment.
>
> Signed-off-by: Arthur Grillo <[email protected]>
> [Louis Chauvet:
> - fix minor formating issues (whitespace, double line)
> - change expected alpha from 0x0000 to 0xffff
> - adapt to the new get_conversion_matrix usage
> - apply the changes from Arthur
> - move struct pixel_yuv_u8 to the test itself]
Again, a Co-developed-by tag might be more proper.
> Signed-off-by: Louis Chauvet <[email protected]>
> ---
> 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_format_test.c | 230 ++++++++++++++++++++++++++
> drivers/gpu/drm/vkms/vkms_formats.c | 7 +-
> drivers/gpu/drm/vkms/vkms_formats.h | 4 +
> 7 files changed, 262 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
> index b9ecdebecb0b..9b0e1940c14f 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 "Tests for VKMS" if !KUNIT_ALL_TESTS
"KUnit tests for VKMS"
> + depends on DRM_VKMS && 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".
> \ No newline at end of file
> 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..2d1df668569e
> --- /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_format_test.o
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> new file mode 100644
> index 000000000000..0954d606e44a
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> @@ -0,0 +1,230 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <kunit/test.h>
> +
> +#include <drm/drm_fixed.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_print.h>
> +
> +#include "../../drm_crtc_internal.h"
> +
> +#include "../vkms_drv.h"
> +#include "../vkms_formats.h"
> +
> +#define TEST_BUFF_SIZE 50
> +
> +struct pixel_yuv_u8 {
> + u8 y, u, v;
> +};
> +
> +struct yuv_u8_to_argb_u16_case {
> + enum drm_color_encoding encoding;
> + enum drm_color_range range;
> + size_t n_colors;
> + struct format_pair {
> + char *name;
> + struct pixel_yuv_u8 yuv;
> + struct pixel_argb_u16 argb;
> + } colors[TEST_BUFF_SIZE];
> +};
> +
> +/*
> + * The YUV color representation were acquired via the colour python framework.
> + * Below are the function calls used for generating each case.
> + *
> + * for more information got to the docs:
s/for/For
> + * https://colour.readthedocs.io/en/master/generated/colour.RGB_to_YCbCr.html
> + */
> +static struct yuv_u8_to_argb_u16_case yuv_u8_to_argb_u16_cases[] = {
> + /*
> + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> + * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> + * in_bits = 16,
> + * in_legal = False,
> + * in_int = True,
> + * out_bits = 8,
> + * out_legal = False,
> + * out_int = True)
> + */
I feel that this Python code is kind of poluting the test cases.
> + {
> + .encoding = DRM_COLOR_YCBCR_BT601,
> + .range = DRM_COLOR_YCBCR_FULL_RANGE,
> + .n_colors = 6,
> + .colors = {
> + { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> + { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> + { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> + { "red", { 0x4c, 0x55, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> + { "green", { 0x96, 0x2c, 0x15 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> + { "blue", { 0x1d, 0xff, 0x6b }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> + },
> + },
> + /*
> + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> + * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> + * in_bits = 16,
> + * in_legal = False,
> + * in_int = True,
> + * out_bits = 8,
> + * out_legal = True,
> + * out_int = True)
> + */
> + {
> + .encoding = DRM_COLOR_YCBCR_BT601,
> + .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + .n_colors = 6,
> + .colors = {
> + { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> + { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> + { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> + { "red", { 0x51, 0x5a, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> + { "green", { 0x91, 0x36, 0x22 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> + { "blue", { 0x29, 0xf0, 0x6e }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> + },
> + },
> + /*
> + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> + * K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
> + * in_bits = 16,
> + * in_legal = False,
> + * in_int = True,
> + * out_bits = 8,
> + * out_legal = False,
> + * out_int = True)
> + */
> + {
> + .encoding = DRM_COLOR_YCBCR_BT709,
> + .range = DRM_COLOR_YCBCR_FULL_RANGE,
> + .n_colors = 4,
> + .colors = {
> + { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> + { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> + { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> + { "red", { 0x36, 0x63, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> + { "green", { 0xb6, 0x1e, 0x0c }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> + { "blue", { 0x12, 0xff, 0x74 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> + },
> + },
> + /*
> + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> + * K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
> + * in_bits = 16,
> + * int_legal = False,
> + * in_int = True,
> + * out_bits = 8,
> + * out_legal = True,
> + * out_int = True)
> + */
> + {
> + .encoding = DRM_COLOR_YCBCR_BT709,
> + .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + .n_colors = 4,
> + .colors = {
> + { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> + { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> + { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> + { "red", { 0x3f, 0x66, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> + { "green", { 0xad, 0x2a, 0x1a }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> + { "blue", { 0x20, 0xf0, 0x76 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> + },
> + },
> + /*
> + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> + * K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
> + * in_bits = 16,
> + * in_legal = False,
> + * in_int = True,
> + * out_bits = 8,
> + * out_legal = False,
> + * out_int = True)
> + */
> + {
> + .encoding = DRM_COLOR_YCBCR_BT2020,
> + .range = DRM_COLOR_YCBCR_FULL_RANGE,
> + .n_colors = 4,
> + .colors = {
> + { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> + { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> + { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> + { "red", { 0x43, 0x5c, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> + { "green", { 0xad, 0x24, 0x0b }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> + { "blue", { 0x0f, 0xff, 0x76 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> + },
> + },
> + /*
> + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> + * K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
> + * in_bits = 16,
> + * in_legal = False,
> + * in_int = True,
> + * out_bits = 8,
> + * out_legal = True,
> + * out_int = True)
> + */
> + {
> + .encoding = DRM_COLOR_YCBCR_BT2020,
> + .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + .n_colors = 4,
> + .colors = {
> + { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> + { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> + { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> + { "red", { 0x4a, 0x61, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> + { "green", { 0xa4, 0x2f, 0x19 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> + { "blue", { 0x1d, 0xf0, 0x77 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> + },
> + },
> +};
> +
> +static void vkms_format_test_yuv_u8_to_argb_u16(struct kunit *test)
> +{
> + const struct yuv_u8_to_argb_u16_case *param = test->param_value;
> + struct pixel_argb_u16 argb;
> +
> + for (size_t i = 0; i < param->n_colors; i++) {
> + const struct format_pair *color = ¶m->colors[i];
> +
> + struct conversion_matrix *matrix = get_conversion_matrix_to_argb_u16
> + (DRM_FORMAT_NV12, param->encoding, param->range);
> +
> + argb = argb_u16_from_yuv888(color->yuv.y, color->yuv.u, color->yuv.v, matrix);
> +
> + KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.a, color->argb.a), 257,
> + "On the A channel of the color %s expected 0x%04x, got 0x%04x",
> + color->name, color->argb.a, argb.a);
> + KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.r, color->argb.r), 257,
> + "On the R channel of the color %s expected 0x%04x, got 0x%04x",
> + color->name, color->argb.r, argb.r);
> + KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.g, color->argb.g), 257,
> + "On the G channel of the color %s expected 0x%04x, got 0x%04x",
> + color->name, color->argb.g, argb.g);
> + KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.b, color->argb.b), 257,
> + "On the B channel of the color %s expected 0x%04x, got 0x%04x",
> + color->name, color->argb.b, argb.b);
> + }
> +}
> +
> +static void vkms_format_test_yuv_u8_to_argb_u16_case_desc(struct yuv_u8_to_argb_u16_case *t,
> + char *desc)
> +{
> + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s - %s",
> + drm_get_color_encoding_name(t->encoding), drm_get_color_range_name(t->range));
> +}
> +
> +KUNIT_ARRAY_PARAM(yuv_u8_to_argb_u16, yuv_u8_to_argb_u16_cases,
> + vkms_format_test_yuv_u8_to_argb_u16_case_desc
> +);
> +
> +static struct kunit_case vkms_format_test_cases[] = {
> + KUNIT_CASE_PARAM(vkms_format_test_yuv_u8_to_argb_u16, yuv_u8_to_argb_u16_gen_params),
> + {}
> +};
> +
> +static struct kunit_suite vkms_format_test_suite = {
> + .name = "vkms-format",
> + .test_cases = vkms_format_test_cases,
> +};
> +
> +kunit_test_suite(vkms_format_test_suite);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index edbf4b321b91..863fc91d6d48 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -7,6 +7,8 @@
> #include <drm/drm_rect.h>
> #include <drm/drm_fixed.h>
>
> +#include <kunit/visibility.h>
> +
> #include "vkms_formats.h"
>
> /**
> @@ -199,8 +201,8 @@ static struct pixel_argb_u16 argb_u16_from_RGB565(const u16 *pixel)
> return out_pixel;
> }
>
> -static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
> - struct conversion_matrix *matrix)
> +VISIBLE_IF_KUNIT struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
> + struct conversion_matrix *matrix)
> {
> u8 r, g, b;
> s64 fp_y, fp_cb, fp_cr;
> @@ -234,6 +236,7 @@ static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
>
> return argb_u16_from_u8888(255, r, g, b);
> }
> +EXPORT_SYMBOL_IF_KUNIT(argb_u16_from_yuv888);
>
> /*
> * The following functions are read_line function for each pixel format supported by VKMS.
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
> index e1d324764b17..21e66a0cac16 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.h
> +++ b/drivers/gpu/drm/vkms/vkms_formats.h
> @@ -13,4 +13,8 @@ struct conversion_matrix *
> get_conversion_matrix_to_argb_u16(u32 format, enum drm_color_encoding encoding,
> enum drm_color_range range);
>
> +#if IS_ENABLED(CONFIG_KUNIT)
What about the CONFIG_DRM_EXPORT_FOR_TESTS?
Best Regards,
- Maíra
> +struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr, struct conversion_matrix *matrix);
> +#endif
> +
> #endif /* _VKMS_FORMATS_H_ */
>
Le 25/03/24 - 11:34, Ma?ra Canal a ?crit :
> On 3/13/24 14:45, Louis Chauvet wrote:
> > From: Arthur Grillo <[email protected]>
> >
> > Create KUnit tests to test the conversion between YUV and RGB. Test each
> > conversion and range combination with some common colors.
> >
> > The code used to compute the expected result can be found in comment.
> >
> > Signed-off-by: Arthur Grillo <[email protected]>
> > [Louis Chauvet:
> > - fix minor formating issues (whitespace, double line)
> > - change expected alpha from 0x0000 to 0xffff
> > - adapt to the new get_conversion_matrix usage
> > - apply the changes from Arthur
> > - move struct pixel_yuv_u8 to the test itself]
>
> Again, a Co-developed-by tag might be more proper.
For this patch, my contribution was very minimal (I only add a call to
get_conversion_matrix_to_argb_u16), so I will not add the Co-developed-by.
> > Signed-off-by: Louis Chauvet <[email protected]>
> > ---
> > 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_format_test.c | 230 ++++++++++++++++++++++++++
> > drivers/gpu/drm/vkms/vkms_formats.c | 7 +-
> > drivers/gpu/drm/vkms/vkms_formats.h | 4 +
> > 7 files changed, 262 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
> > index b9ecdebecb0b..9b0e1940c14f 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 "Tests for VKMS" if !KUNIT_ALL_TESTS
>
> "KUnit tests for VKMS"
Fixed in v6.
> > + depends on DRM_VKMS && 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".
> > \ No newline at end of file
> > 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..2d1df668569e
> > --- /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_format_test.o
> > diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> > new file mode 100644
> > index 000000000000..0954d606e44a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
[...]
> > +/*
> > + * The YUV color representation were acquired via the colour python framework.
> > + * Below are the function calls used for generating each case.
> > + *
> > + * for more information got to the docs:
>
> s/for/For
Fixed in v6.
> > + * https://colour.readthedocs.io/en/master/generated/colour.RGB_to_YCbCr.html
> > + */
> > +static struct yuv_u8_to_argb_u16_case yuv_u8_to_argb_u16_cases[] = {
> > + /*
> > + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> > + * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> > + * in_bits = 16,
> > + * in_legal = False,
> > + * in_int = True,
> > + * out_bits = 8,
> > + * out_legal = False,
> > + * out_int = True)
> > + */
>
> I feel that this Python code is kind of poluting the test cases.
This python code is needed to understand where the values come from. I
think we should keep it for future reference (add more cases, test yuv 16
bits...)
Maybe we can change the array comment to
/*
* The yuv color representation were acquired via the colour python framework:
*
* colour.RGB_to_YCbCr(<rgb color in 16 bits form>,
* K=color.WEIGHTS_YCBCR["<format>"],
* [...],
* out_legal = <limited or full range>)
*
* The exact function call arguments are given for each element of this list.
*
* [...]
*/
And above each test case:
/*
* format = "ITU-R BT.601"
* out_legal = False
*/
@Arthur, do you agree with those modifications?
> > + {
> > + .encoding = DRM_COLOR_YCBCR_BT601,
> > + .range = DRM_COLOR_YCBCR_FULL_RANGE,
> > + .n_colors = 6,
> > + .colors = {
> > + { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> > + { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> > + { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> > + { "red", { 0x4c, 0x55, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> > + { "green", { 0x96, 0x2c, 0x15 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> > + { "blue", { 0x1d, 0xff, 0x6b }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> > + },
> > + },
[...]
> > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> > index edbf4b321b91..863fc91d6d48 100644
> > --- a/drivers/gpu/drm/vkms/vkms_formats.c
> > +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> > @@ -7,6 +7,8 @@
> > #include <drm/drm_rect.h>
> > #include <drm/drm_fixed.h>
> >
> > +#include <kunit/visibility.h>
> > +
> > #include "vkms_formats.h"
> >
> > /**
> > @@ -199,8 +201,8 @@ static struct pixel_argb_u16 argb_u16_from_RGB565(const u16 *pixel)
> > return out_pixel;
> > }
> >
> > -static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
> > - struct conversion_matrix *matrix)
> > +VISIBLE_IF_KUNIT struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
> > + struct conversion_matrix *matrix)
> > {
> > u8 r, g, b;
> > s64 fp_y, fp_cb, fp_cr;
> > @@ -234,6 +236,7 @@ static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 cb, u8 cr,
> >
> > return argb_u16_from_u8888(255, r, g, b);
> > }
> > +EXPORT_SYMBOL_IF_KUNIT(argb_u16_from_yuv888);
> >
> > /*
> > * The following functions are read_line function for each pixel format supported by VKMS.
> > diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
> > index e1d324764b17..21e66a0cac16 100644
> > --- a/drivers/gpu/drm/vkms/vkms_formats.h
> > +++ b/drivers/gpu/drm/vkms/vkms_formats.h
> > @@ -13,4 +13,8 @@ struct conversion_matrix *
> > get_conversion_matrix_to_argb_u16(u32 format, enum drm_color_encoding encoding,
> > enum drm_color_range range);
> >
> > +#if IS_ENABLED(CONFIG_KUNIT)
>
> What about the CONFIG_DRM_EXPORT_FOR_TESTS?
As the documentation for CONFIG_DRM_EXPORT_FOR_TESTS don't exists, I don't
know what to use. Maybe Arthur knows what to do here? If needed I can
apply the modifications for the next iteration.
Thanks for all your reviews,
Louis Chauvet
[...]
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Mon, 25 Mar 2024 11:34:17 -0300
Maíra Canal <[email protected]> wrote:
> On 3/13/24 14:45, Louis Chauvet wrote:
> > From: Arthur Grillo <[email protected]>
> >
> > Create KUnit tests to test the conversion between YUV and RGB. Test each
> > conversion and range combination with some common colors.
> >
> > The code used to compute the expected result can be found in comment.
> >
> > Signed-off-by: Arthur Grillo <[email protected]>
> > [Louis Chauvet:
> > - fix minor formating issues (whitespace, double line)
> > - change expected alpha from 0x0000 to 0xffff
> > - adapt to the new get_conversion_matrix usage
> > - apply the changes from Arthur
> > - move struct pixel_yuv_u8 to the test itself]
>
> Again, a Co-developed-by tag might be more proper.
>
> > Signed-off-by: Louis Chauvet <[email protected]>
> > ---
> > 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_format_test.c | 230 ++++++++++++++++++++++++++
> > drivers/gpu/drm/vkms/vkms_formats.c | 7 +-
> > drivers/gpu/drm/vkms/vkms_formats.h | 4 +
> > 7 files changed, 262 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
> > index b9ecdebecb0b..9b0e1940c14f 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 "Tests for VKMS" if !KUNIT_ALL_TESTS
>
> "KUnit tests for VKMS"
>
> > + depends on DRM_VKMS && 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".
> > \ No newline at end of file
> > 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..2d1df668569e
> > --- /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_format_test.o
> > diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> > new file mode 100644
> > index 000000000000..0954d606e44a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> > @@ -0,0 +1,230 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <kunit/test.h>
> > +
> > +#include <drm/drm_fixed.h>
> > +#include <drm/drm_fourcc.h>
> > +#include <drm/drm_print.h>
> > +
> > +#include "../../drm_crtc_internal.h"
> > +
> > +#include "../vkms_drv.h"
> > +#include "../vkms_formats.h"
> > +
> > +#define TEST_BUFF_SIZE 50
> > +
> > +struct pixel_yuv_u8 {
> > + u8 y, u, v;
> > +};
> > +
> > +struct yuv_u8_to_argb_u16_case {
> > + enum drm_color_encoding encoding;
> > + enum drm_color_range range;
> > + size_t n_colors;
> > + struct format_pair {
> > + char *name;
> > + struct pixel_yuv_u8 yuv;
> > + struct pixel_argb_u16 argb;
> > + } colors[TEST_BUFF_SIZE];
> > +};
> > +
> > +/*
> > + * The YUV color representation were acquired via the colour python framework.
> > + * Below are the function calls used for generating each case.
> > + *
> > + * for more information got to the docs:
>
> s/for/For
>
> > + * https://colour.readthedocs.io/en/master/generated/colour.RGB_to_YCbCr.html
> > + */
> > +static struct yuv_u8_to_argb_u16_case yuv_u8_to_argb_u16_cases[] = {
> > + /*
> > + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> > + * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> > + * in_bits = 16,
> > + * in_legal = False,
> > + * in_int = True,
> > + * out_bits = 8,
> > + * out_legal = False,
> > + * out_int = True)
> > + */
>
> I feel that this Python code is kind of poluting the test cases.
I asked for the python code.
How would you verify that the expected values are actually correct
without these comments?
You cannot trust that the test values are good if the tests pass. Both
the test values and the tested code could be wrong simultaneously and
agree on wrong results.
I love these comments that explicitly give the python snippets used to
generate the test values. Well done!
Louis' suggestion of collecting the common python bits together is
fine, too. As long as the comments clearly explain what python commands
produced the test values, I'm happy.
Anyway,
Acked-by: Pekka Paalanen <[email protected]>
Thanks,
pq
> > + {
> > + .encoding = DRM_COLOR_YCBCR_BT601,
> > + .range = DRM_COLOR_YCBCR_FULL_RANGE,
> > + .n_colors = 6,
> > + .colors = {
> > + { "white", { 0xff, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> > + { "gray", { 0x80, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> > + { "black", { 0x00, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> > + { "red", { 0x4c, 0x55, 0xff }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> > + { "green", { 0x96, 0x2c, 0x15 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> > + { "blue", { 0x1d, 0xff, 0x6b }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> > + },
> > + },
> > + /*
> > + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> > + * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> > + * in_bits = 16,
> > + * in_legal = False,
> > + * in_int = True,
> > + * out_bits = 8,
> > + * out_legal = True,
> > + * out_int = True)
> > + */
> > + {
> > + .encoding = DRM_COLOR_YCBCR_BT601,
> > + .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> > + .n_colors = 6,
> > + .colors = {
> > + { "white", { 0xeb, 0x80, 0x80 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> > + { "gray", { 0x7e, 0x80, 0x80 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> > + { "black", { 0x10, 0x80, 0x80 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> > + { "red", { 0x51, 0x5a, 0xf0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> > + { "green", { 0x91, 0x36, 0x22 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> > + { "blue", { 0x29, 0xf0, 0x6e }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> > + },
> > + },
> > + /*
> > + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> > + * K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
> > + * in_bits = 16,
> > + * in_legal = False,
> > + * in_int = True,
> > + * out_bits = 8,
> > + * out_legal = False,
> > + * out_int = True)
> > + */
> > + {