Re: [PATCH v5 14/16] drm/vkms: Create KUnit tests for YUV conversions
On Mon, 25 Mar 2024 11:34:17 -0300
Maíra Canal wrote:
> On 3/13/24 14:45, Louis Chauvet wrote:
> > From: Arthur Grillo
> >
> > 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
> > [Louis Chauvet:
> > - fix minor formating issues (whitespace, double line)
> > - change expected alpha from 0x to 0x
> > - 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
> > ---
> > 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 ..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 ..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 ..0954d606e44a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> > @@ -0,0 +1,230 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#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(,
> > +* K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> > +* in_bits = 16,
> > +* in_legal = False,
> > +* in_int = True,
> > +* out_bits = 8,
> > +* out_legal = False,
> > +*
Re: [PATCH v5 14/16] drm/vkms: Create KUnit tests for YUV conversions
Le 25/03/24 - 11:34, Maíra Canal a écrit :
> On 3/13/24 14:45, Louis Chauvet wrote:
> > From: Arthur Grillo
> >
> > 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
> > [Louis Chauvet:
> > - fix minor formating issues (whitespace, double line)
> > - change expected alpha from 0x to 0x
> > - 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
> > ---
> > 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 ..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 ..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 ..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(,
> > +* 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(,
* K=color.WEIGHTS_YCBCR[""],
* [...],
* out_leg
Re: [PATCH v5 14/16] drm/vkms: Create KUnit tests for YUV conversions
On 3/13/24 14:45, Louis Chauvet wrote:
From: Arthur Grillo
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
[Louis Chauvet:
- fix minor formating issues (whitespace, double line)
- change expected alpha from 0x to 0x
- 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
---
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 ..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 ..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 ..0954d606e44a
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+
+#include
+#include
+#include
+
+#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(,
+* 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 }, { 0x, 0x,
0x, 0x }},
+ { "gray", { 0x80, 0x80, 0x80 }, { 0x, 0x8080,
0x8080, 0x8080 }},
+ { "blac
[PATCH v5 14/16] drm/vkms: Create KUnit tests for YUV conversions
From: Arthur Grillo
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
[Louis Chauvet:
- fix minor formating issues (whitespace, double line)
- change expected alpha from 0x to 0x
- 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
---
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 ..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 ..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 ..0954d606e44a
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+
+#include
+#include
+#include
+
+#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(,
+* 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 }, { 0x, 0x,
0x, 0x }},
+ { "gray", { 0x80, 0x80, 0x80 }, { 0x, 0x8080,
0x8080, 0x8080 }},
+ { "black", { 0x00, 0x80, 0x80 }, { 0x, 0x,
0x, 0x }},
+ { "red", { 0x4c, 0x55, 0xff }, { 0x, 0x,
0x, 0x }},
+ { "green", { 0x96, 0x2c, 0x15 }
