Le 25/03/24 - 11:34, Maíra Canal a écrit : > On 3/13/24 14:45, Louis Chauvet wrote: > > From: Arthur Grillo <arthurgrillo@xxxxxxxxxx> > > > > 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 <arthurgrillo@xxxxxxxxxx> > > [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 <louis.chauvet@xxxxxxxxxxx> > > --- > > 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
