linux-kernel - Re: [PATCH v12 13/15] drm/vkms: Create KUnit tests for YUV conversions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zx9ebb1sEehlM150@fedora>
Date: Mon, 28 Oct 2024 10:50:37 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Maíra Canal <mairacanal@...eup.net>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
	Melissa Wen <melissa.srw@...il.com>,
	Haneen Mohammed <hamohammed.sa@...il.com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Jonathan Corbet <corbet@....net>,
	Simona Vetter <simona@...ll.ch>, rdunlap@...radead.org,
	arthurgrillo@...eup.net, pekka.paalanen@...oniitty.fi,
	Simona Vetter <simona.vetter@...ll.ch>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, thomas.petazzoni@...tlin.com,
	jeremie.dautheribes@...tlin.com, miquel.raynal@...tlin.com,
	seanpaul@...gle.com, marcheu@...gle.com, nicolejadeyee@...gle.com,
	Pekka Paalanen <pekka.paalanen@...labora.com>
Subject: Re: [PATCH v12 13/15] drm/vkms: Create KUnit tests for YUV
 conversions

On 26/10/24 - 11:49, Maíra Canal wrote:
> Hi Louis,
> 
> On 07/10/24 13:10, Louis Chauvet wrote:
> > From: Arthur Grillo <arthurgrillo@...eup.net>
> > 
> > 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.
> > 
> > [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: Arthur Grillo <arthurgrillo@...eup.net>
> > Acked-by: Pekka Paalanen <pekka.paalanen@...labora.com>
> > Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > ---
> >   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 | 232 ++++++++++++++++++++++++++
> >   drivers/gpu/drm/vkms/vkms_formats.c           |   7 +-
> >   drivers/gpu/drm/vkms/vkms_formats.h           |   5 +
> >   7 files changed, 265 insertions(+), 2 deletions(-)
> > 
> 
> [...]
> 
> > +
> > +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 = &param->colors[i];
> > +		struct conversion_matrix matrix;
> > +
> > +		get_conversion_matrix_to_argb_u16
> > +			(DRM_FORMAT_NV12, param->encoding, param->range, &matrix);
> > +
> > +		argb = argb_u16_from_yuv888(color->yuv.y, color->yuv.u, color->yuv.v, &matrix);
> 
> This should be `argb_u16_from_yuv161616` as you fixed in [1].

(I suppose you talk about [2]?)

I understand that I change this function in a future series, but [2] is 
not Acked-By yet. I prefer to have the opportunity to merge this 
first series (with yuv888) quickly and to work on [2] later (I have less 
conflicts between [2] and the rest of my work on configFS).

If I get a Acked-by on [2], I can merge the two commits and directly use 
yuv161616 conversion functions.

Thanks,
Louis Chauvet

[2]:https://lore.kernel.org/all/20241007-b4-new-color-formats-v2-6-d47da50d4674@bootlin.com/
 
> [1] https://lore.kernel.org/all/20241007-b4-new-color-formats-v2-5-d47da50d4674@bootlin.com/
> 
> Best Regards,
> - Maíra
> 
> > +
> > +		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");
> > +MODULE_DESCRIPTION("Kunit test for vkms format conversion");
> > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> > index adb1228e5201..0b201185eae7 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"
> >   /**
> > @@ -247,8 +249,8 @@ static struct pixel_argb_u16 argb_u16_from_RGB565(const __le16 *pixel)
> >   	return out_pixel;
> >   }
> > -static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 channel_1, u8 channel_2,
> > -						  const struct conversion_matrix *matrix)
> > +VISIBLE_IF_KUNIT struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 channel_1, u8 channel_2,
> > +							    const struct conversion_matrix *matrix)
> >   {
> >   	u16 r, g, b;
> >   	s64 fp_y, fp_channel_1, fp_channel_2;
> > @@ -278,6 +280,7 @@ static struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 channel_1, u8 channel
> >   	return argb_u16_from_u16161616(0xffff, 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 d583855cb320..b4fe62ab9c65 100644
> > --- a/drivers/gpu/drm/vkms/vkms_formats.h
> > +++ b/drivers/gpu/drm/vkms/vkms_formats.h
> > @@ -13,4 +13,9 @@ void get_conversion_matrix_to_argb_u16(u32 format, enum drm_color_encoding encod
> >   				       enum drm_color_range range,
> >   				       struct conversion_matrix *matrix);
> > +#if IS_ENABLED(CONFIG_KUNIT)
> > +struct pixel_argb_u16 argb_u16_from_yuv888(u8 y, u8 channel_1, u8 channel_2,
> > +					   const struct conversion_matrix *matrix);
> > +#endif
> > +
> >   #endif /* _VKMS_FORMATS_H_ */
> >