All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] KUnit tests for drm_format_helper
@ 2022-06-13 17:17 ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: dri-devel, davidgow, airlied, dlatypov, linux-kernel,
	tzimmermann, José Expósito, kunit-dev

Hello everyone,

Here is the v3 of the series, including the documentation, previously
sent as a standalone patch [1], and changes suggested during review.

Thanks a lot,
José Expósito

RFC -> v1: https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposito89@gmail.com/T/

 - Add .kunitconfig (Maxime Ripard)
 - Fix memory leak (Daniel Latypov)
 - Make config option generic (Javier Martinez Canillas):
   DRM_FORMAR_HELPER_TEST -> DRM_KUNIT_TEST
 - Remove DISABLE_STRUCTLEAK_PLUGIN (Daniel Latypov)

v1 -> v2: https://lore.kernel.org/dri-devel/20220606095516.938934-1-jose.exposito89@gmail.com/T/

 Thomas Zimmermann:
 - Add DRM_RECT_INIT() macro
 - Move tests to drivers/gpu/drm/kunit
 - Improve test documentation

v2 -> v3: https://lore.kernel.org/dri-devel/20220612161248.271590-1-jose.exposito89@gmail.com/T/

 - Use designated initializer in DRM_RECT_INIT (Jani Nikula)
 - Simplify the "conversion_buf_size" helper

[1] https://lore.kernel.org/dri-devel/20220606180940.43371-1-jose.exposito89@gmail.com/T/

José Expósito (3):
  drm/rect: Add DRM_RECT_INIT() macro
  drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
  drm/doc: Add KUnit documentation

 Documentation/gpu/drm-internals.rst           |  32 ++++
 drivers/gpu/drm/Kconfig                       |  16 ++
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/kunit/.kunitconfig            |   3 +
 drivers/gpu/drm/kunit/Makefile                |   3 +
 .../gpu/drm/kunit/drm_format_helper_test.c    | 160 ++++++++++++++++++
 include/drm/drm_rect.h                        |  16 ++
 7 files changed, 231 insertions(+)
 create mode 100644 drivers/gpu/drm/kunit/.kunitconfig
 create mode 100644 drivers/gpu/drm/kunit/Makefile
 create mode 100644 drivers/gpu/drm/kunit/drm_format_helper_test.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 0/3] KUnit tests for drm_format_helper
@ 2022-06-13 17:17 ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	José Expósito

Hello everyone,

Here is the v3 of the series, including the documentation, previously
sent as a standalone patch [1], and changes suggested during review.

Thanks a lot,
José Expósito

RFC -> v1: https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposito89@gmail.com/T/

 - Add .kunitconfig (Maxime Ripard)
 - Fix memory leak (Daniel Latypov)
 - Make config option generic (Javier Martinez Canillas):
   DRM_FORMAR_HELPER_TEST -> DRM_KUNIT_TEST
 - Remove DISABLE_STRUCTLEAK_PLUGIN (Daniel Latypov)

v1 -> v2: https://lore.kernel.org/dri-devel/20220606095516.938934-1-jose.exposito89@gmail.com/T/

 Thomas Zimmermann:
 - Add DRM_RECT_INIT() macro
 - Move tests to drivers/gpu/drm/kunit
 - Improve test documentation

v2 -> v3: https://lore.kernel.org/dri-devel/20220612161248.271590-1-jose.exposito89@gmail.com/T/

 - Use designated initializer in DRM_RECT_INIT (Jani Nikula)
 - Simplify the "conversion_buf_size" helper

[1] https://lore.kernel.org/dri-devel/20220606180940.43371-1-jose.exposito89@gmail.com/T/

José Expósito (3):
  drm/rect: Add DRM_RECT_INIT() macro
  drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
  drm/doc: Add KUnit documentation

 Documentation/gpu/drm-internals.rst           |  32 ++++
 drivers/gpu/drm/Kconfig                       |  16 ++
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/kunit/.kunitconfig            |   3 +
 drivers/gpu/drm/kunit/Makefile                |   3 +
 .../gpu/drm/kunit/drm_format_helper_test.c    | 160 ++++++++++++++++++
 include/drm/drm_rect.h                        |  16 ++
 7 files changed, 231 insertions(+)
 create mode 100644 drivers/gpu/drm/kunit/.kunitconfig
 create mode 100644 drivers/gpu/drm/kunit/Makefile
 create mode 100644 drivers/gpu/drm/kunit/drm_format_helper_test.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 1/3] drm/rect: Add DRM_RECT_INIT() macro
  2022-06-13 17:17 ` José Expósito
@ 2022-06-13 17:17   ` José Expósito
  -1 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: dri-devel, davidgow, airlied, dlatypov, linux-kernel,
	tzimmermann, José Expósito, kunit-dev

Add a helper macro to initialize a rectangle from x, y, width and
height information.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 include/drm/drm_rect.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 6f6e19bd4dac..e8d94fca2703 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -47,6 +47,22 @@ struct drm_rect {
 	int x1, y1, x2, y2;
 };
 
+/**
+ * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
+ * @x: x coordinate
+ * @y: y coordinate
+ * @w: width
+ * @h: height
+ *
+ * RETURNS:
+ * A new rectangle of the specified size.
+ */
+#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
+		.x1 = (x), \
+		.y1 = (y), \
+		.x2 = (x) + (w), \
+		.y2 = (y) + (h) })
+
 /**
  * DRM_RECT_FMT - printf string for &struct drm_rect
  */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 1/3] drm/rect: Add DRM_RECT_INIT() macro
@ 2022-06-13 17:17   ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	José Expósito

Add a helper macro to initialize a rectangle from x, y, width and
height information.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 include/drm/drm_rect.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 6f6e19bd4dac..e8d94fca2703 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -47,6 +47,22 @@ struct drm_rect {
 	int x1, y1, x2, y2;
 };
 
+/**
+ * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
+ * @x: x coordinate
+ * @y: y coordinate
+ * @w: width
+ * @h: height
+ *
+ * RETURNS:
+ * A new rectangle of the specified size.
+ */
+#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
+		.x1 = (x), \
+		.y1 = (y), \
+		.x2 = (x) + (w), \
+		.y2 = (y) + (h) })
+
 /**
  * DRM_RECT_FMT - printf string for &struct drm_rect
  */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 2/3] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
  2022-06-13 17:17 ` José Expósito
@ 2022-06-13 17:17   ` José Expósito
  -1 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: dri-devel, davidgow, airlied, dlatypov, linux-kernel,
	tzimmermann, José Expósito, kunit-dev

Test the conversion from XRGB8888 to RGB332.

What is tested?

 - Different values for the X in XRGB8888 to make sure it is ignored
 - Different clip values: Single pixel and full and partial buffer
 - Well known colors: White, black, red, green, blue, magenta, yellow
   and cyan
 - Other colors: Randomly picked
 - Destination pitch

How to run the tests?

 $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
         --kconfig_add CONFIG_VIRTIO_UML=y \
         --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y

Suggested-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/gpu/drm/Kconfig                       |  16 ++
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/kunit/.kunitconfig            |   3 +
 drivers/gpu/drm/kunit/Makefile                |   3 +
 .../gpu/drm/kunit/drm_format_helper_test.c    | 160 ++++++++++++++++++
 5 files changed, 183 insertions(+)
 create mode 100644 drivers/gpu/drm/kunit/.kunitconfig
 create mode 100644 drivers/gpu/drm/kunit/Makefile
 create mode 100644 drivers/gpu/drm/kunit/drm_format_helper_test.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 22e7fa48d693..6c2256e8474b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -70,6 +70,22 @@ config DRM_DEBUG_SELFTEST
 
 	  If in doubt, say "N".
 
+config DRM_KUNIT_TEST
+	tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
+	depends on DRM && KUNIT=y
+	select DRM_KMS_HELPER
+	default KUNIT_ALL_TESTS
+	help
+	  This builds unit tests for DRM. This option is not useful for
+	  distributions or general kernels, but only for kernel
+	  developers working on DRM and associated drivers.
+
+	  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".
+
 config DRM_KMS_HELPER
 	tristate
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 13ef240b3d2b..3171437d74f8 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 #
 
 obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
+obj-$(CONFIG_DRM_KUNIT_TEST) += kunit/
 
 obj-$(CONFIG_DRM_MIPI_DBI) += drm_mipi_dbi.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
diff --git a/drivers/gpu/drm/kunit/.kunitconfig b/drivers/gpu/drm/kunit/.kunitconfig
new file mode 100644
index 000000000000..6ec04b4c979d
--- /dev/null
+++ b/drivers/gpu/drm/kunit/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_DRM=y
+CONFIG_DRM_KUNIT_TEST=y
diff --git a/drivers/gpu/drm/kunit/Makefile b/drivers/gpu/drm/kunit/Makefile
new file mode 100644
index 000000000000..2c8273796d9d
--- /dev/null
+++ b/drivers/gpu/drm/kunit/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_DRM_KUNIT_TEST) += drm_format_helper_test.o
diff --git a/drivers/gpu/drm/kunit/drm_format_helper_test.c b/drivers/gpu/drm/kunit/drm_format_helper_test.c
new file mode 100644
index 000000000000..f8f2351f3449
--- /dev/null
+++ b/drivers/gpu/drm/kunit/drm_format_helper_test.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <kunit/test.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_mode.h>
+#include <drm/drm_print.h>
+#include <drm/drm_rect.h>
+
+#include "../drm_crtc_internal.h"
+
+#define TEST_BUF_SIZE 50
+
+struct xrgb8888_to_rgb332_case {
+	const char *name;
+	unsigned int pitch;
+	unsigned int dst_pitch;
+	struct drm_rect clip;
+	const u32 xrgb8888[TEST_BUF_SIZE];
+	const u8 expected[4 * TEST_BUF_SIZE];
+};
+
+static struct xrgb8888_to_rgb332_case xrgb8888_to_rgb332_cases[] = {
+	{
+		.name = "Single pixel source buffer",
+		.pitch = 1 * 4,
+		.dst_pitch = 0,
+		.clip = DRM_RECT_INIT(0, 0, 1, 1),
+		.xrgb8888 = { 0x01FF0000 },
+		.expected = { 0xE0 },
+	},
+	{
+		.name = "Single pixel clip rectangle",
+		.pitch = 2 * 4,
+		.dst_pitch = 0,
+		.clip = DRM_RECT_INIT(1, 1, 1, 1),
+		.xrgb8888 = {
+			0x00000000, 0x00000000,
+			0x00000000, 0x10FF0000,
+		},
+		.expected = { 0xE0 },
+	},
+	{
+		/* Well known colors, different values for the X in XRGB8888 to
+		 * make sure it is ignored. Partial clip area.
+		 */
+		.name = "White, black, red, green, blue, magenta, yellow, cyan",
+		.pitch = 4 * 4,
+		.dst_pitch = 0,
+		.clip = DRM_RECT_INIT(1, 1, 2, 4),
+		.xrgb8888 = {
+			0x00000000, 0x00000000, 0x00000000, 0x00000000,
+			0x00000000, 0x11FFFFFF, 0x22000000, 0x00000000,
+			0x00000000, 0x33FF0000, 0x4400FF00, 0x00000000,
+			0x00000000, 0x550000FF, 0x66FF00FF, 0x00000000,
+			0x00000000, 0x77FFFF00, 0x8800FFFF, 0x00000000,
+		},
+		.expected = {
+			0xFF, 0x00,
+			0xE0, 0x1C,
+			0x03, 0xE3,
+			0xFC, 0x1F,
+		},
+	},
+	{
+		/* Randomly picked colors. Full buffer within the clip area. */
+		.name = "Destination pitch",
+		.pitch = 3 * 4,
+		.dst_pitch = 5,
+		.clip = DRM_RECT_INIT(0, 0, 3, 3),
+		.xrgb8888 = {
+			0xA10E449C, 0xB1114D05, 0xC1A80303,
+			0xD16C7073, 0xA20E449C, 0xB2114D05,
+			0xC2A80303, 0xD26C7073, 0xA30E449C,
+		},
+		.expected = {
+			0x0A, 0x08, 0xA0, 0x00, 0x00,
+			0x6D, 0x0A, 0x08, 0x00, 0x00,
+			0xA0, 0x6D, 0x0A, 0x00, 0x00,
+		},
+	},
+};
+
+/*
+ * conversion_buf_size - Return the destination buffer size required to convert
+ * between formats.
+ * @dst_format: destination buffer pixel format (DRM_FORMAT_*)
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
+ * @clip: Clip rectangle area to convert
+ *
+ * Returns:
+ * The size of the destination buffer or negative value on error.
+ */
+static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
+				  const struct drm_rect *clip)
+{
+	const struct drm_format_info *dst_fi = drm_format_info(dst_format);
+
+	if (!dst_fi)
+		return -EINVAL;
+
+	if (!dst_pitch)
+		dst_pitch = drm_rect_width(clip) * dst_fi->cpp[0];
+
+	return dst_pitch * drm_rect_height(clip);
+}
+
+static void xrgb8888_to_rgb332_case_desc(struct xrgb8888_to_rgb332_case *t,
+					 char *desc)
+{
+	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(xrgb8888_to_rgb332, xrgb8888_to_rgb332_cases,
+		  xrgb8888_to_rgb332_case_desc);
+
+static void xrgb8888_to_rgb332_test(struct kunit *test)
+{
+	const struct xrgb8888_to_rgb332_case *params = test->param_value;
+	size_t dst_size;
+	__u8 *dst = NULL;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_RGB332, params->dst_pitch,
+				       &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);
+
+	drm_fb_xrgb8888_to_rgb332(dst, params->dst_pitch, params->xrgb8888,
+				  &fb, &params->clip);
+	KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
+}
+
+static struct kunit_case drm_format_helper_test_cases[] = {
+	KUNIT_CASE_PARAM(xrgb8888_to_rgb332_test,
+			 xrgb8888_to_rgb332_gen_params),
+	{}
+};
+
+static struct kunit_suite drm_format_helper_test_suite = {
+	.name = "drm-format-helper-test",
+	.test_cases = drm_format_helper_test_cases,
+};
+
+kunit_test_suite(drm_format_helper_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the drm_format_helper APIs");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("José Expósito <jose.exposito89@gmail.com>");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 2/3] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
@ 2022-06-13 17:17   ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	José Expósito

Test the conversion from XRGB8888 to RGB332.

What is tested?

 - Different values for the X in XRGB8888 to make sure it is ignored
 - Different clip values: Single pixel and full and partial buffer
 - Well known colors: White, black, red, green, blue, magenta, yellow
   and cyan
 - Other colors: Randomly picked
 - Destination pitch

How to run the tests?

 $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
         --kconfig_add CONFIG_VIRTIO_UML=y \
         --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y

Suggested-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/gpu/drm/Kconfig                       |  16 ++
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/kunit/.kunitconfig            |   3 +
 drivers/gpu/drm/kunit/Makefile                |   3 +
 .../gpu/drm/kunit/drm_format_helper_test.c    | 160 ++++++++++++++++++
 5 files changed, 183 insertions(+)
 create mode 100644 drivers/gpu/drm/kunit/.kunitconfig
 create mode 100644 drivers/gpu/drm/kunit/Makefile
 create mode 100644 drivers/gpu/drm/kunit/drm_format_helper_test.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 22e7fa48d693..6c2256e8474b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -70,6 +70,22 @@ config DRM_DEBUG_SELFTEST
 
 	  If in doubt, say "N".
 
+config DRM_KUNIT_TEST
+	tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS
+	depends on DRM && KUNIT=y
+	select DRM_KMS_HELPER
+	default KUNIT_ALL_TESTS
+	help
+	  This builds unit tests for DRM. This option is not useful for
+	  distributions or general kernels, but only for kernel
+	  developers working on DRM and associated drivers.
+
+	  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".
+
 config DRM_KMS_HELPER
 	tristate
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 13ef240b3d2b..3171437d74f8 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 #
 
 obj-$(CONFIG_DRM_DEBUG_SELFTEST) += selftests/
+obj-$(CONFIG_DRM_KUNIT_TEST) += kunit/
 
 obj-$(CONFIG_DRM_MIPI_DBI) += drm_mipi_dbi.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
diff --git a/drivers/gpu/drm/kunit/.kunitconfig b/drivers/gpu/drm/kunit/.kunitconfig
new file mode 100644
index 000000000000..6ec04b4c979d
--- /dev/null
+++ b/drivers/gpu/drm/kunit/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_DRM=y
+CONFIG_DRM_KUNIT_TEST=y
diff --git a/drivers/gpu/drm/kunit/Makefile b/drivers/gpu/drm/kunit/Makefile
new file mode 100644
index 000000000000..2c8273796d9d
--- /dev/null
+++ b/drivers/gpu/drm/kunit/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_DRM_KUNIT_TEST) += drm_format_helper_test.o
diff --git a/drivers/gpu/drm/kunit/drm_format_helper_test.c b/drivers/gpu/drm/kunit/drm_format_helper_test.c
new file mode 100644
index 000000000000..f8f2351f3449
--- /dev/null
+++ b/drivers/gpu/drm/kunit/drm_format_helper_test.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <kunit/test.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_mode.h>
+#include <drm/drm_print.h>
+#include <drm/drm_rect.h>
+
+#include "../drm_crtc_internal.h"
+
+#define TEST_BUF_SIZE 50
+
+struct xrgb8888_to_rgb332_case {
+	const char *name;
+	unsigned int pitch;
+	unsigned int dst_pitch;
+	struct drm_rect clip;
+	const u32 xrgb8888[TEST_BUF_SIZE];
+	const u8 expected[4 * TEST_BUF_SIZE];
+};
+
+static struct xrgb8888_to_rgb332_case xrgb8888_to_rgb332_cases[] = {
+	{
+		.name = "Single pixel source buffer",
+		.pitch = 1 * 4,
+		.dst_pitch = 0,
+		.clip = DRM_RECT_INIT(0, 0, 1, 1),
+		.xrgb8888 = { 0x01FF0000 },
+		.expected = { 0xE0 },
+	},
+	{
+		.name = "Single pixel clip rectangle",
+		.pitch = 2 * 4,
+		.dst_pitch = 0,
+		.clip = DRM_RECT_INIT(1, 1, 1, 1),
+		.xrgb8888 = {
+			0x00000000, 0x00000000,
+			0x00000000, 0x10FF0000,
+		},
+		.expected = { 0xE0 },
+	},
+	{
+		/* Well known colors, different values for the X in XRGB8888 to
+		 * make sure it is ignored. Partial clip area.
+		 */
+		.name = "White, black, red, green, blue, magenta, yellow, cyan",
+		.pitch = 4 * 4,
+		.dst_pitch = 0,
+		.clip = DRM_RECT_INIT(1, 1, 2, 4),
+		.xrgb8888 = {
+			0x00000000, 0x00000000, 0x00000000, 0x00000000,
+			0x00000000, 0x11FFFFFF, 0x22000000, 0x00000000,
+			0x00000000, 0x33FF0000, 0x4400FF00, 0x00000000,
+			0x00000000, 0x550000FF, 0x66FF00FF, 0x00000000,
+			0x00000000, 0x77FFFF00, 0x8800FFFF, 0x00000000,
+		},
+		.expected = {
+			0xFF, 0x00,
+			0xE0, 0x1C,
+			0x03, 0xE3,
+			0xFC, 0x1F,
+		},
+	},
+	{
+		/* Randomly picked colors. Full buffer within the clip area. */
+		.name = "Destination pitch",
+		.pitch = 3 * 4,
+		.dst_pitch = 5,
+		.clip = DRM_RECT_INIT(0, 0, 3, 3),
+		.xrgb8888 = {
+			0xA10E449C, 0xB1114D05, 0xC1A80303,
+			0xD16C7073, 0xA20E449C, 0xB2114D05,
+			0xC2A80303, 0xD26C7073, 0xA30E449C,
+		},
+		.expected = {
+			0x0A, 0x08, 0xA0, 0x00, 0x00,
+			0x6D, 0x0A, 0x08, 0x00, 0x00,
+			0xA0, 0x6D, 0x0A, 0x00, 0x00,
+		},
+	},
+};
+
+/*
+ * conversion_buf_size - Return the destination buffer size required to convert
+ * between formats.
+ * @dst_format: destination buffer pixel format (DRM_FORMAT_*)
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
+ * @clip: Clip rectangle area to convert
+ *
+ * Returns:
+ * The size of the destination buffer or negative value on error.
+ */
+static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
+				  const struct drm_rect *clip)
+{
+	const struct drm_format_info *dst_fi = drm_format_info(dst_format);
+
+	if (!dst_fi)
+		return -EINVAL;
+
+	if (!dst_pitch)
+		dst_pitch = drm_rect_width(clip) * dst_fi->cpp[0];
+
+	return dst_pitch * drm_rect_height(clip);
+}
+
+static void xrgb8888_to_rgb332_case_desc(struct xrgb8888_to_rgb332_case *t,
+					 char *desc)
+{
+	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(xrgb8888_to_rgb332, xrgb8888_to_rgb332_cases,
+		  xrgb8888_to_rgb332_case_desc);
+
+static void xrgb8888_to_rgb332_test(struct kunit *test)
+{
+	const struct xrgb8888_to_rgb332_case *params = test->param_value;
+	size_t dst_size;
+	__u8 *dst = NULL;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_RGB332, params->dst_pitch,
+				       &params->clip);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	dst = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dst);
+
+	drm_fb_xrgb8888_to_rgb332(dst, params->dst_pitch, params->xrgb8888,
+				  &fb, &params->clip);
+	KUNIT_EXPECT_EQ(test, memcmp(dst, params->expected, dst_size), 0);
+}
+
+static struct kunit_case drm_format_helper_test_cases[] = {
+	KUNIT_CASE_PARAM(xrgb8888_to_rgb332_test,
+			 xrgb8888_to_rgb332_gen_params),
+	{}
+};
+
+static struct kunit_suite drm_format_helper_test_suite = {
+	.name = "drm-format-helper-test",
+	.test_cases = drm_format_helper_test_cases,
+};
+
+kunit_test_suite(drm_format_helper_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the drm_format_helper APIs");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("José Expósito <jose.exposito89@gmail.com>");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-13 17:17 ` José Expósito
@ 2022-06-13 17:17   ` José Expósito
  -1 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, José Expósito, kunit-dev

Explain how to run the KUnit tests present in the DRM subsystem and
clarify why the UML-only options were not added to the configuration
file present in drivers/gpu/drm/.kunitconfig [1] [2].

[1] https://lore.kernel.org/dri-devel/CABVgOSn8i=LO5p7830h2XU1Jgg0KrN0qTnxkOMhf1oTgxjaKKw@mail.gmail.com/
[2] https://lore.kernel.org/dri-devel/CAGS_qxqpiCim_sy1LDK7PLwVgWf-LKW+uNFTGM=T7ydk-dYcEw@mail.gmail.com/

Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 Documentation/gpu/drm-internals.rst | 32 +++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index 38afed24a75c..f1d97e80ca29 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -207,6 +207,38 @@ Utilities
    :internal:
 
 
+Unit testing
+============
+
+KUnit
+-----
+
+KUnit (Kernel unit testing framework) provides a common framework for unit tests
+within the Linux kernel.
+
+This section covers the specifics for the DRM subsystem. For general information
+about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
+
+How to run the tests?
+~~~~~~~~~~~~~~~~~~~~~
+
+In order to facilitate running the test suite, a configuration file is present
+in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
+follows:
+
+.. code-block:: bash
+
+	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
+		--kconfig_add CONFIG_VIRTIO_UML=y \
+		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
+
+.. note::
+	The configuration included in ``.kunitconfig`` should be as generic as
+	possible.
+	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
+	included in it because they are only required for User Mode Linux.
+
+
 Legacy Support Code
 ===================
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-13 17:17   ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-13 17:17 UTC (permalink / raw)
  To: javierm
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	José Expósito, Maxime Ripard

Explain how to run the KUnit tests present in the DRM subsystem and
clarify why the UML-only options were not added to the configuration
file present in drivers/gpu/drm/.kunitconfig [1] [2].

[1] https://lore.kernel.org/dri-devel/CABVgOSn8i=LO5p7830h2XU1Jgg0KrN0qTnxkOMhf1oTgxjaKKw@mail.gmail.com/
[2] https://lore.kernel.org/dri-devel/CAGS_qxqpiCim_sy1LDK7PLwVgWf-LKW+uNFTGM=T7ydk-dYcEw@mail.gmail.com/

Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 Documentation/gpu/drm-internals.rst | 32 +++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index 38afed24a75c..f1d97e80ca29 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -207,6 +207,38 @@ Utilities
    :internal:
 
 
+Unit testing
+============
+
+KUnit
+-----
+
+KUnit (Kernel unit testing framework) provides a common framework for unit tests
+within the Linux kernel.
+
+This section covers the specifics for the DRM subsystem. For general information
+about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
+
+How to run the tests?
+~~~~~~~~~~~~~~~~~~~~~
+
+In order to facilitate running the test suite, a configuration file is present
+in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
+follows:
+
+.. code-block:: bash
+
+	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
+		--kconfig_add CONFIG_VIRTIO_UML=y \
+		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
+
+.. note::
+	The configuration included in ``.kunitconfig`` should be as generic as
+	possible.
+	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
+	included in it because they are only required for User Mode Linux.
+
+
 Legacy Support Code
 ===================
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 1/3] drm/rect: Add DRM_RECT_INIT() macro
  2022-06-13 17:17   ` José Expósito
@ 2022-06-13 18:31     ` Jani Nikula
  -1 siblings, 0 replies; 31+ messages in thread
From: Jani Nikula @ 2022-06-13 18:31 UTC (permalink / raw)
  To: José Expósito, javierm
  Cc: davidgow, airlied, dlatypov, linux-kernel, dri-devel,
	tzimmermann, José Expósito, kunit-dev

On Mon, 13 Jun 2022, José Expósito <jose.exposito89@gmail.com> wrote:
> Add a helper macro to initialize a rectangle from x, y, width and
> height information.
>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  include/drm/drm_rect.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index 6f6e19bd4dac..e8d94fca2703 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -47,6 +47,22 @@ struct drm_rect {
>  	int x1, y1, x2, y2;
>  };
>  
> +/**
> + * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
> + * @x: x coordinate
> + * @y: y coordinate
> + * @w: width
> + * @h: height
> + *
> + * RETURNS:
> + * A new rectangle of the specified size.
> + */
> +#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
> +		.x1 = (x), \
> +		.y1 = (y), \
> +		.x2 = (x) + (w), \
> +		.y2 = (y) + (h) })
> +
>  /**
>   * DRM_RECT_FMT - printf string for &struct drm_rect
>   */

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 1/3] drm/rect: Add DRM_RECT_INIT() macro
@ 2022-06-13 18:31     ` Jani Nikula
  0 siblings, 0 replies; 31+ messages in thread
From: Jani Nikula @ 2022-06-13 18:31 UTC (permalink / raw)
  To: José Expósito, javierm
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, dri-devel, kunit-dev, linux-kernel,
	José Expósito

On Mon, 13 Jun 2022, José Expósito <jose.exposito89@gmail.com> wrote:
> Add a helper macro to initialize a rectangle from x, y, width and
> height information.
>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  include/drm/drm_rect.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index 6f6e19bd4dac..e8d94fca2703 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -47,6 +47,22 @@ struct drm_rect {
>  	int x1, y1, x2, y2;
>  };
>  
> +/**
> + * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
> + * @x: x coordinate
> + * @y: y coordinate
> + * @w: width
> + * @h: height
> + *
> + * RETURNS:
> + * A new rectangle of the specified size.
> + */
> +#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
> +		.x1 = (x), \
> +		.y1 = (y), \
> +		.x2 = (x) + (w), \
> +		.y2 = (y) + (h) })
> +
>  /**
>   * DRM_RECT_FMT - printf string for &struct drm_rect
>   */

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
  2022-06-13 17:17 ` José Expósito
@ 2022-06-14  7:08   ` Thomas Zimmermann
  -1 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2022-06-14  7:08 UTC (permalink / raw)
  To: José Expósito, javierm
  Cc: davidgow, dlatypov, maarten.lankhorst, mripard, airlied, daniel,
	jani.nikula, dri-devel, kunit-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2484 bytes --]

Hi Jose,

for the whole patchset:

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

One small detail on licensing: drm_format_helper.c is licensed under 
GPL2 or MIT. Maybe consider adding 'or MIT' to drm_format_helper_test.c 
as well.

Best regards
Thomas

Am 13.06.22 um 19:17 schrieb José Expósito:
> Hello everyone,
> 
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
> 
> Thanks a lot,
> José Expósito
> 
> RFC -> v1: https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposito89@gmail.com/T/
> 
>   - Add .kunitconfig (Maxime Ripard)
>   - Fix memory leak (Daniel Latypov)
>   - Make config option generic (Javier Martinez Canillas):
>     DRM_FORMAR_HELPER_TEST -> DRM_KUNIT_TEST
>   - Remove DISABLE_STRUCTLEAK_PLUGIN (Daniel Latypov)
> 
> v1 -> v2: https://lore.kernel.org/dri-devel/20220606095516.938934-1-jose.exposito89@gmail.com/T/
> 
>   Thomas Zimmermann:
>   - Add DRM_RECT_INIT() macro
>   - Move tests to drivers/gpu/drm/kunit
>   - Improve test documentation
> 
> v2 -> v3: https://lore.kernel.org/dri-devel/20220612161248.271590-1-jose.exposito89@gmail.com/T/
> 
>   - Use designated initializer in DRM_RECT_INIT (Jani Nikula)
>   - Simplify the "conversion_buf_size" helper
> 
> [1] https://lore.kernel.org/dri-devel/20220606180940.43371-1-jose.exposito89@gmail.com/T/
> 
> José Expósito (3):
>    drm/rect: Add DRM_RECT_INIT() macro
>    drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
>    drm/doc: Add KUnit documentation
> 
>   Documentation/gpu/drm-internals.rst           |  32 ++++
>   drivers/gpu/drm/Kconfig                       |  16 ++
>   drivers/gpu/drm/Makefile                      |   1 +
>   drivers/gpu/drm/kunit/.kunitconfig            |   3 +
>   drivers/gpu/drm/kunit/Makefile                |   3 +
>   .../gpu/drm/kunit/drm_format_helper_test.c    | 160 ++++++++++++++++++
>   include/drm/drm_rect.h                        |  16 ++
>   7 files changed, 231 insertions(+)
>   create mode 100644 drivers/gpu/drm/kunit/.kunitconfig
>   create mode 100644 drivers/gpu/drm/kunit/Makefile
>   create mode 100644 drivers/gpu/drm/kunit/drm_format_helper_test.c
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
@ 2022-06-14  7:08   ` Thomas Zimmermann
  0 siblings, 0 replies; 31+ messages in thread
From: Thomas Zimmermann @ 2022-06-14  7:08 UTC (permalink / raw)
  To: José Expósito, javierm
  Cc: dri-devel, davidgow, airlied, dlatypov, linux-kernel, kunit-dev


[-- Attachment #1.1: Type: text/plain, Size: 2484 bytes --]

Hi Jose,

for the whole patchset:

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

One small detail on licensing: drm_format_helper.c is licensed under 
GPL2 or MIT. Maybe consider adding 'or MIT' to drm_format_helper_test.c 
as well.

Best regards
Thomas

Am 13.06.22 um 19:17 schrieb José Expósito:
> Hello everyone,
> 
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
> 
> Thanks a lot,
> José Expósito
> 
> RFC -> v1: https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposito89@gmail.com/T/
> 
>   - Add .kunitconfig (Maxime Ripard)
>   - Fix memory leak (Daniel Latypov)
>   - Make config option generic (Javier Martinez Canillas):
>     DRM_FORMAR_HELPER_TEST -> DRM_KUNIT_TEST
>   - Remove DISABLE_STRUCTLEAK_PLUGIN (Daniel Latypov)
> 
> v1 -> v2: https://lore.kernel.org/dri-devel/20220606095516.938934-1-jose.exposito89@gmail.com/T/
> 
>   Thomas Zimmermann:
>   - Add DRM_RECT_INIT() macro
>   - Move tests to drivers/gpu/drm/kunit
>   - Improve test documentation
> 
> v2 -> v3: https://lore.kernel.org/dri-devel/20220612161248.271590-1-jose.exposito89@gmail.com/T/
> 
>   - Use designated initializer in DRM_RECT_INIT (Jani Nikula)
>   - Simplify the "conversion_buf_size" helper
> 
> [1] https://lore.kernel.org/dri-devel/20220606180940.43371-1-jose.exposito89@gmail.com/T/
> 
> José Expósito (3):
>    drm/rect: Add DRM_RECT_INIT() macro
>    drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
>    drm/doc: Add KUnit documentation
> 
>   Documentation/gpu/drm-internals.rst           |  32 ++++
>   drivers/gpu/drm/Kconfig                       |  16 ++
>   drivers/gpu/drm/Makefile                      |   1 +
>   drivers/gpu/drm/kunit/.kunitconfig            |   3 +
>   drivers/gpu/drm/kunit/Makefile                |   3 +
>   .../gpu/drm/kunit/drm_format_helper_test.c    | 160 ++++++++++++++++++
>   include/drm/drm_rect.h                        |  16 ++
>   7 files changed, 231 insertions(+)
>   create mode 100644 drivers/gpu/drm/kunit/.kunitconfig
>   create mode 100644 drivers/gpu/drm/kunit/Makefile
>   create mode 100644 drivers/gpu/drm/kunit/drm_format_helper_test.c
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-13 17:17   ` José Expósito
@ 2022-06-14 12:58     ` Javier Martinez Canillas
  -1 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-14 12:58 UTC (permalink / raw)
  To: José Expósito
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, kunit-dev

Hello José,

On 6/13/22 19:17, José Expósito wrote:

[snip]

> +KUnit (Kernel unit testing framework) provides a common framework for unit tests
> +within the Linux kernel.
> +

I think that it will be useful to have a reference to the KUnit kernel doc here,
something like the following:

`KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...

> +This section covers the specifics for the DRM subsystem. For general information
> +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
> +
> +How to run the tests?
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +In order to facilitate running the test suite, a configuration file is present
> +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
> +follows:
> +
> +.. code-block:: bash
> +
> +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> +		--kconfig_add CONFIG_VIRTIO_UML=y \
> +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> +
> +.. note::
> +	The configuration included in ``.kunitconfig`` should be as generic as
> +	possible.
> +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
> +	included in it because they are only required for User Mode Linux.
> +
> +

Maybe also add something like this ?

For example, the following command can be used to run the test for x86_64:

	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
		--arch=x86_64

Regardless, the patch looks good to me:

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-14 12:58     ` Javier Martinez Canillas
  0 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-14 12:58 UTC (permalink / raw)
  To: José Expósito
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	Maxime Ripard

Hello José,

On 6/13/22 19:17, José Expósito wrote:

[snip]

> +KUnit (Kernel unit testing framework) provides a common framework for unit tests
> +within the Linux kernel.
> +

I think that it will be useful to have a reference to the KUnit kernel doc here,
something like the following:

`KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...

> +This section covers the specifics for the DRM subsystem. For general information
> +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
> +
> +How to run the tests?
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +In order to facilitate running the test suite, a configuration file is present
> +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
> +follows:
> +
> +.. code-block:: bash
> +
> +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> +		--kconfig_add CONFIG_VIRTIO_UML=y \
> +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> +
> +.. note::
> +	The configuration included in ``.kunitconfig`` should be as generic as
> +	possible.
> +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
> +	included in it because they are only required for User Mode Linux.
> +
> +

Maybe also add something like this ?

For example, the following command can be used to run the test for x86_64:

	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
		--arch=x86_64

Regardless, the patch looks good to me:

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
  2022-06-13 17:17 ` José Expósito
@ 2022-06-14 13:03   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-14 13:03 UTC (permalink / raw)
  To: José Expósito
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel

On 6/13/22 19:17, José Expósito wrote:
> Hello everyone,
> 
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
> 
> Thanks a lot,
> José Expósito
> 

Before merging this, could you please reach the folks working on [0] ?

I think that would be good to have some consistency with regard to KUnit
tests from the start to avoid future refactorings. For instance, you are
adding the tests under a `kunit` sub-directory while they are doing it
in a `tests` sub-dir.

Also there may be other things that could be made more consistent, like
the naming conventions for the tests, suites, etc.

[0]: https://lore.kernel.org/all/20220608010709.272962-4-maira.canal@usp.br/T/

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
@ 2022-06-14 13:03   ` Javier Martinez Canillas
  0 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-14 13:03 UTC (permalink / raw)
  To: José Expósito
  Cc: dri-devel, davidgow, airlied, dlatypov, linux-kernel,
	tzimmermann, kunit-dev

On 6/13/22 19:17, José Expósito wrote:
> Hello everyone,
> 
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
> 
> Thanks a lot,
> José Expósito
> 

Before merging this, could you please reach the folks working on [0] ?

I think that would be good to have some consistency with regard to KUnit
tests from the start to avoid future refactorings. For instance, you are
adding the tests under a `kunit` sub-directory while they are doing it
in a `tests` sub-dir.

Also there may be other things that could be made more consistent, like
the naming conventions for the tests, suites, etc.

[0]: https://lore.kernel.org/all/20220608010709.272962-4-maira.canal@usp.br/T/

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-14 12:58     ` Javier Martinez Canillas
@ 2022-06-14 18:09       ` José Expósito
  -1 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-14 18:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	Maxime Ripard

Hi Javier,

On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
> Hello José,
> 
> On 6/13/22 19:17, José Expósito wrote:
> 
> [snip]
> 
> > +KUnit (Kernel unit testing framework) provides a common framework for unit tests
> > +within the Linux kernel.
> > +
> 
> I think that it will be useful to have a reference to the KUnit kernel doc here,
> something like the following:
> 
> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...

There is a link in the next paragraph. Once the documentation is
generated the path "Documentation/dev-tools/kunit/start.rst" is
transformed into a link.
 
> > +This section covers the specifics for the DRM subsystem. For general information
> > +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
> > +
> > +How to run the tests?
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +In order to facilitate running the test suite, a configuration file is present
> > +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
> > +follows:
> > +
> > +.. code-block:: bash
> > +
> > +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> > +		--kconfig_add CONFIG_VIRTIO_UML=y \
> > +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> > +
> > +.. note::
> > +	The configuration included in ``.kunitconfig`` should be as generic as
> > +	possible.
> > +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
> > +	included in it because they are only required for User Mode Linux.
> > +
> > +
> 
> Maybe also add something like this ?
> 
> For example, the following command can be used to run the test for x86_64:
> 
> 	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> 		--arch=x86_64

I didn't want to go into much detail because the KUnit docs are
a very good resource and already explain how to run the tests in your
favorite architecture.

Since running the test on x86_64 should not change the results, I'd
prefer to keep it simple and trust the KUnit docs for the "advanced"
options.
 
> Regardless, the patch looks good to me:
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>


Thanks a lot for taking the time to review it. I'll add the tag if a v4
is required after chatting with the guys working on the AMDGPU tests.

Jose

> -- 
> Best regards,
> 
> Javier Martinez Canillas
> Linux Engineering
> Red Hat
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-14 18:09       ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-14 18:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, kunit-dev

Hi Javier,

On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
> Hello José,
> 
> On 6/13/22 19:17, José Expósito wrote:
> 
> [snip]
> 
> > +KUnit (Kernel unit testing framework) provides a common framework for unit tests
> > +within the Linux kernel.
> > +
> 
> I think that it will be useful to have a reference to the KUnit kernel doc here,
> something like the following:
> 
> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...

There is a link in the next paragraph. Once the documentation is
generated the path "Documentation/dev-tools/kunit/start.rst" is
transformed into a link.
 
> > +This section covers the specifics for the DRM subsystem. For general information
> > +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
> > +
> > +How to run the tests?
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +In order to facilitate running the test suite, a configuration file is present
> > +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
> > +follows:
> > +
> > +.. code-block:: bash
> > +
> > +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> > +		--kconfig_add CONFIG_VIRTIO_UML=y \
> > +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> > +
> > +.. note::
> > +	The configuration included in ``.kunitconfig`` should be as generic as
> > +	possible.
> > +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
> > +	included in it because they are only required for User Mode Linux.
> > +
> > +
> 
> Maybe also add something like this ?
> 
> For example, the following command can be used to run the test for x86_64:
> 
> 	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> 		--arch=x86_64

I didn't want to go into much detail because the KUnit docs are
a very good resource and already explain how to run the tests in your
favorite architecture.

Since running the test on x86_64 should not change the results, I'd
prefer to keep it simple and trust the KUnit docs for the "advanced"
options.
 
> Regardless, the patch looks good to me:
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>


Thanks a lot for taking the time to review it. I'll add the tag if a v4
is required after chatting with the guys working on the AMDGPU tests.

Jose

> -- 
> Best regards,
> 
> Javier Martinez Canillas
> Linux Engineering
> Red Hat
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-14 18:09       ` José Expósito
@ 2022-06-14 18:14         ` Javier Martinez Canillas
  -1 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-14 18:14 UTC (permalink / raw)
  To: José Expósito
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, daniel, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	Maxime Ripard

On 6/14/22 20:09, José Expósito wrote:
> Hi Javier,
> 
> On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
>> Hello José,
>>
>> On 6/13/22 19:17, José Expósito wrote:
>>
>> [snip]
>>
>>> +KUnit (Kernel unit testing framework) provides a common framework for unit tests
>>> +within the Linux kernel.
>>> +
>>
>> I think that it will be useful to have a reference to the KUnit kernel doc here,
>> something like the following:
>>
>> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...
> 
> There is a link in the next paragraph. Once the documentation is
> generated the path "Documentation/dev-tools/kunit/start.rst" is
> transformed into a link.
>  
Ah, I wasn't aware of that. Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-14 18:14         ` Javier Martinez Canillas
  0 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-14 18:14 UTC (permalink / raw)
  To: José Expósito
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, kunit-dev

On 6/14/22 20:09, José Expósito wrote:
> Hi Javier,
> 
> On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
>> Hello José,
>>
>> On 6/13/22 19:17, José Expósito wrote:
>>
>> [snip]
>>
>>> +KUnit (Kernel unit testing framework) provides a common framework for unit tests
>>> +within the Linux kernel.
>>> +
>>
>> I think that it will be useful to have a reference to the KUnit kernel doc here,
>> something like the following:
>>
>> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...
> 
> There is a link in the next paragraph. Once the documentation is
> generated the path "Documentation/dev-tools/kunit/start.rst" is
> transformed into a link.
>  
Ah, I wasn't aware of that. Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
  2022-06-13 17:17 ` José Expósito
@ 2022-06-16 14:44   ` David Gow
  -1 siblings, 0 replies; 31+ messages in thread
From: David Gow @ 2022-06-16 14:44 UTC (permalink / raw)
  To: José Expósito, Maíra Canal, Isabella Basso,
	magalilemes00, tales.aparecida
  Cc: Javier Martinez Canillas, Daniel Latypov, tzimmermann,
	maarten.lankhorst, mripard, David Airlie, Daniel Vetter,
	Jani Nikula, dri-devel, KUnit Development,
	Linux Kernel Mailing List

On Tue, Jun 14, 2022 at 1:17 AM José Expósito <jose.exposito89@gmail.com> wrote:
>
> Hello everyone,
>
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
>
> Thanks a lot,
> José Expósito
>

[+Maíra, Isabella, Tales, Magali for other drm,amdgpu,KUnit work.]

These seem pretty good to me, but I'd echo Javier's comments about
consistency with other DRM tests.

In particular, we now have three concurrently developed DRM-related
test suites, each doing things slightly differently:
- This series is putting tests in drm/kunit, and providing a
.kunitconfig in that directory,
- The selftest ports here[1] are putting tests in drm/tests, and
provide a separate Kconfig file, as well as a .kunitconfig
- And the AMDGPU tests[2] are doing something totally different, with
their own tests in drm/amd/display/amdgpu_dm/tests, which get compiled
directly into the amdgpu module (and, at present, can't be run at all
via kunit_tool)

Certainly the general DRM tests should be in the same place, and use
the same Kconfig entries, etc. A mix of the separate Kconfig file from
[1] (if there's enough benefit to having the ability to turn on and
off suites individually, which seems plausible) and the documentation
from this series seems good to me.

There's some basic guidelines around test nomenclature in
Documentation/dev-tools/kunit/style.rst[3], though all of these
patches seem pretty consistent with that. Either 'kunit' or 'tests'
would work as a directory name: given the AMDGPU patches are using
'tests', maybe that's easier to stick with.

Cheers,
-- David

[1]: https://lore.kernel.org/linux-kselftest/20220615135824.15522-1-maira.canal@usp.br/
[2]: https://lore.kernel.org/dri-devel/20220608010709.272962-1-maira.canal@usp.br/
[3]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
@ 2022-06-16 14:44   ` David Gow
  0 siblings, 0 replies; 31+ messages in thread
From: David Gow @ 2022-06-16 14:44 UTC (permalink / raw)
  To: José Expósito, Maíra Canal, Isabella Basso,
	magalilemes00, tales.aparecida
  Cc: dri-devel, David Airlie, Daniel Latypov,
	Javier Martinez Canillas, Linux Kernel Mailing List, tzimmermann,
	KUnit Development

On Tue, Jun 14, 2022 at 1:17 AM José Expósito <jose.exposito89@gmail.com> wrote:
>
> Hello everyone,
>
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
>
> Thanks a lot,
> José Expósito
>

[+Maíra, Isabella, Tales, Magali for other drm,amdgpu,KUnit work.]

These seem pretty good to me, but I'd echo Javier's comments about
consistency with other DRM tests.

In particular, we now have three concurrently developed DRM-related
test suites, each doing things slightly differently:
- This series is putting tests in drm/kunit, and providing a
.kunitconfig in that directory,
- The selftest ports here[1] are putting tests in drm/tests, and
provide a separate Kconfig file, as well as a .kunitconfig
- And the AMDGPU tests[2] are doing something totally different, with
their own tests in drm/amd/display/amdgpu_dm/tests, which get compiled
directly into the amdgpu module (and, at present, can't be run at all
via kunit_tool)

Certainly the general DRM tests should be in the same place, and use
the same Kconfig entries, etc. A mix of the separate Kconfig file from
[1] (if there's enough benefit to having the ability to turn on and
off suites individually, which seems plausible) and the documentation
from this series seems good to me.

There's some basic guidelines around test nomenclature in
Documentation/dev-tools/kunit/style.rst[3], though all of these
patches seem pretty consistent with that. Either 'kunit' or 'tests'
would work as a directory name: given the AMDGPU patches are using
'tests', maybe that's easier to stick with.

Cheers,
-- David

[1]: https://lore.kernel.org/linux-kselftest/20220615135824.15522-1-maira.canal@usp.br/
[2]: https://lore.kernel.org/dri-devel/20220608010709.272962-1-maira.canal@usp.br/
[3]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
  2022-06-16 14:44   ` David Gow
@ 2022-06-16 18:38     ` José Expósito
  -1 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-16 18:38 UTC (permalink / raw)
  To: David Gow
  Cc: Maíra Canal, Isabella Basso, magalilemes00, tales.aparecida,
	Javier Martinez Canillas, Daniel Latypov, tzimmermann,
	maarten.lankhorst, mripard, David Airlie, Daniel Vetter,
	Jani Nikula, dri-devel, KUnit Development,
	Linux Kernel Mailing List

Hi!

Javier Martinez Canillas wrote:
> Before merging this, could you please reach the folks working on [0] ?
> I think that would be good to have some consistency with regard to KUnit
> tests from the start to avoid future refactorings. For instance, you are
> adding the tests under a `kunit` sub-directory while they are doing it
> in a `tests` sub-dir.
>
> Also there may be other things that could be made more consistent, like
> the naming conventions for the tests, suites, etc.
>
> [0]: https://lore.kernel.org/all/20220608010709.272962-4-maira.canal@usp.br/T/

David Gow wrote:
> [+Maíra, Isabella, Tales, Magali for other drm,amdgpu,KUnit work.]
> 
> These seem pretty good to me, but I'd echo Javier's comments about
> consistency with other DRM tests.

I agree, I'd need to look with more detail into the selftest conversion
and the AMD series, but it'd be nice to avoid unnecessary refactors.
 
> In particular, we now have three concurrently developed DRM-related
> test suites, each doing things slightly differently:
> - This series is putting tests in drm/kunit, and providing a
> .kunitconfig in that directory,
>
> - The selftest ports here[1] are putting tests in drm/tests, and
> provide a separate Kconfig file, as well as a .kunitconfig

Now that "selftests/" is going to be removed, "tests/" is a good name
for the folder, I'll rename it in v4.

> - And the AMDGPU tests[2] are doing something totally different, with
> their own tests in drm/amd/display/amdgpu_dm/tests, which get compiled
> directly into the amdgpu module (and, at present, can't be run at all
> via kunit_tool)
> 
> Certainly the general DRM tests should be in the same place, and use
> the same Kconfig entries, etc. A mix of the separate Kconfig file from
> [1] (if there's enough benefit to having the ability to turn on and
> off suites individually, which seems plausible) and the documentation
> from this series seems good to me.

I agree about having the DRM-core tests in drm/tests/ and driver tests
in drm/driver/tests/.

About allowing turning on or off test suites, it was agreed to use a
generic symbol to group them (DRM_KUNIT_TEST) [1].
I won't have time merge all patches toghether and run them until next
week, but if it takes too long to run them or you find it beneficial to
split the symbols, I'll change my patch.

[1] https://lore.kernel.org/dri-devel/e26de140-afb7-7b1b-4826-6ac4f3a4fe02@redhat.com/
 
> There's some basic guidelines around test nomenclature in
> Documentation/dev-tools/kunit/style.rst[3], though all of these
> patches seem pretty consistent with that. Either 'kunit' or 'tests'
> would work as a directory name: given the AMDGPU patches are using
> 'tests', maybe that's easier to stick with.

I'll have to rename my kunit_suite to use underscores, as well as the
test cases, that at the moment are using English sentences.

Maíra: We'd also need to agree on the file names used, the
documentation [2] suggest to use *_test.c, it'd need to be changed in
the selftest to KUnit series.

Best wishes,
Jose

[2] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names 
> Cheers,
> -- David
> 
> [1]: https://lore.kernel.org/linux-kselftest/20220615135824.15522-1-maira.canal@usp.br/
> [2]: https://lore.kernel.org/dri-devel/20220608010709.272962-1-maira.canal@usp.br/
> [3]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
@ 2022-06-16 18:38     ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-16 18:38 UTC (permalink / raw)
  To: David Gow
  Cc: dri-devel, magalilemes00, David Airlie, Maíra Canal,
	Daniel Latypov, Javier Martinez Canillas,
	Linux Kernel Mailing List, tzimmermann, tales.aparecida,
	Isabella Basso, KUnit Development

Hi!

Javier Martinez Canillas wrote:
> Before merging this, could you please reach the folks working on [0] ?
> I think that would be good to have some consistency with regard to KUnit
> tests from the start to avoid future refactorings. For instance, you are
> adding the tests under a `kunit` sub-directory while they are doing it
> in a `tests` sub-dir.
>
> Also there may be other things that could be made more consistent, like
> the naming conventions for the tests, suites, etc.
>
> [0]: https://lore.kernel.org/all/20220608010709.272962-4-maira.canal@usp.br/T/

David Gow wrote:
> [+Maíra, Isabella, Tales, Magali for other drm,amdgpu,KUnit work.]
> 
> These seem pretty good to me, but I'd echo Javier's comments about
> consistency with other DRM tests.

I agree, I'd need to look with more detail into the selftest conversion
and the AMD series, but it'd be nice to avoid unnecessary refactors.
 
> In particular, we now have three concurrently developed DRM-related
> test suites, each doing things slightly differently:
> - This series is putting tests in drm/kunit, and providing a
> .kunitconfig in that directory,
>
> - The selftest ports here[1] are putting tests in drm/tests, and
> provide a separate Kconfig file, as well as a .kunitconfig

Now that "selftests/" is going to be removed, "tests/" is a good name
for the folder, I'll rename it in v4.

> - And the AMDGPU tests[2] are doing something totally different, with
> their own tests in drm/amd/display/amdgpu_dm/tests, which get compiled
> directly into the amdgpu module (and, at present, can't be run at all
> via kunit_tool)
> 
> Certainly the general DRM tests should be in the same place, and use
> the same Kconfig entries, etc. A mix of the separate Kconfig file from
> [1] (if there's enough benefit to having the ability to turn on and
> off suites individually, which seems plausible) and the documentation
> from this series seems good to me.

I agree about having the DRM-core tests in drm/tests/ and driver tests
in drm/driver/tests/.

About allowing turning on or off test suites, it was agreed to use a
generic symbol to group them (DRM_KUNIT_TEST) [1].
I won't have time merge all patches toghether and run them until next
week, but if it takes too long to run them or you find it beneficial to
split the symbols, I'll change my patch.

[1] https://lore.kernel.org/dri-devel/e26de140-afb7-7b1b-4826-6ac4f3a4fe02@redhat.com/
 
> There's some basic guidelines around test nomenclature in
> Documentation/dev-tools/kunit/style.rst[3], though all of these
> patches seem pretty consistent with that. Either 'kunit' or 'tests'
> would work as a directory name: given the AMDGPU patches are using
> 'tests', maybe that's easier to stick with.

I'll have to rename my kunit_suite to use underscores, as well as the
test cases, that at the moment are using English sentences.

Maíra: We'd also need to agree on the file names used, the
documentation [2] suggest to use *_test.c, it'd need to be changed in
the selftest to KUnit series.

Best wishes,
Jose

[2] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names 
> Cheers,
> -- David
> 
> [1]: https://lore.kernel.org/linux-kselftest/20220615135824.15522-1-maira.canal@usp.br/
> [2]: https://lore.kernel.org/dri-devel/20220608010709.272962-1-maira.canal@usp.br/
> [3]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-14 12:58     ` Javier Martinez Canillas
@ 2022-06-24 21:01       ` Daniel Vetter
  -1 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2022-06-24 21:01 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: José Expósito, davidgow, dlatypov, tzimmermann,
	maarten.lankhorst, mripard, airlied, daniel, jani.nikula,
	dri-devel, kunit-dev, linux-kernel, Maxime Ripard

On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
> Hello José,
> 
> On 6/13/22 19:17, José Expósito wrote:
> 
> [snip]
> 
> > +KUnit (Kernel unit testing framework) provides a common framework for unit tests
> > +within the Linux kernel.
> > +
> 
> I think that it will be useful to have a reference to the KUnit kernel doc here,
> something like the following:
> 
> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...
> 
> > +This section covers the specifics for the DRM subsystem. For general information
> > +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
> > +
> > +How to run the tests?
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +In order to facilitate running the test suite, a configuration file is present
> > +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
> > +follows:
> > +
> > +.. code-block:: bash
> > +
> > +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> > +		--kconfig_add CONFIG_VIRTIO_UML=y \
> > +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> > +
> > +.. note::
> > +	The configuration included in ``.kunitconfig`` should be as generic as
> > +	possible.
> > +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
> > +	included in it because they are only required for User Mode Linux.
> > +
> > +
> 
> Maybe also add something like this ?
> 
> For example, the following command can be used to run the test for x86_64:
> 
> 	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> 		--arch=x86_64
> 
> Regardless, the patch looks good to me:
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Hey so since you have a bunch of patches merged into drm already but seem
to lack drm-misc commit rights to push these yourself I think it's time to
get those:

https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc

And I guess Javier can help you with any questions you might have and make
sure the request gets through by poking folks on #dri-devel irc?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-24 21:01       ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2022-06-24 21:01 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, José Expósito, kunit-dev

On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
> Hello José,
> 
> On 6/13/22 19:17, José Expósito wrote:
> 
> [snip]
> 
> > +KUnit (Kernel unit testing framework) provides a common framework for unit tests
> > +within the Linux kernel.
> > +
> 
> I think that it will be useful to have a reference to the KUnit kernel doc here,
> something like the following:
> 
> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...
> 
> > +This section covers the specifics for the DRM subsystem. For general information
> > +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
> > +
> > +How to run the tests?
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +In order to facilitate running the test suite, a configuration file is present
> > +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
> > +follows:
> > +
> > +.. code-block:: bash
> > +
> > +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> > +		--kconfig_add CONFIG_VIRTIO_UML=y \
> > +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> > +
> > +.. note::
> > +	The configuration included in ``.kunitconfig`` should be as generic as
> > +	possible.
> > +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
> > +	included in it because they are only required for User Mode Linux.
> > +
> > +
> 
> Maybe also add something like this ?
> 
> For example, the following command can be used to run the test for x86_64:
> 
> 	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
> 		--arch=x86_64
> 
> Regardless, the patch looks good to me:
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Hey so since you have a bunch of patches merged into drm already but seem
to lack drm-misc commit rights to push these yourself I think it's time to
get those:

https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc

And I guess Javier can help you with any questions you might have and make
sure the request gets through by poking folks on #dri-devel irc?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-24 21:01       ` Daniel Vetter
  (?)
@ 2022-06-24 21:18       ` Javier Martinez Canillas
  2022-06-27 12:36           ` José Expósito
  -1 siblings, 1 reply; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-24 21:18 UTC (permalink / raw)
  To: José Expósito, davidgow, dlatypov, tzimmermann,
	maarten.lankhorst, mripard, airlied, jani.nikula, dri-devel,
	kunit-dev, linux-kernel, Maxime Ripard

Hello Daniel,

On 6/24/22 23:01, Daniel Vetter wrote:
> On Tue, Jun 14, 2022 at 02:58:29PM +0200, Javier Martinez Canillas wrote:
>> Hello José,
>>
>> On 6/13/22 19:17, José Expósito wrote:
>>
>> [snip]
>>
>>> +KUnit (Kernel unit testing framework) provides a common framework for unit tests
>>> +within the Linux kernel.
>>> +
>>
>> I think that it will be useful to have a reference to the KUnit kernel doc here,
>> something like the following:
>>
>> `KUnit <https://docs.kernel.org/dev-tools/kunit/index.html>`_ (Kernel Unit...
>>
>>> +This section covers the specifics for the DRM subsystem. For general information
>>> +about KUnit, please refer to Documentation/dev-tools/kunit/start.rst.
>>> +
>>> +How to run the tests?
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +In order to facilitate running the test suite, a configuration file is present
>>> +in ``drivers/gpu/drm/kunit/.kunitconfig``. It can be used by ``kunit.py`` as
>>> +follows:
>>> +
>>> +.. code-block:: bash
>>> +
>>> +	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
>>> +		--kconfig_add CONFIG_VIRTIO_UML=y \
>>> +		--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
>>> +
>>> +.. note::
>>> +	The configuration included in ``.kunitconfig`` should be as generic as
>>> +	possible.
>>> +	``CONFIG_VIRTIO_UML`` and ``CONFIG_UML_PCI_OVER_VIRTIO`` are not
>>> +	included in it because they are only required for User Mode Linux.
>>> +
>>> +
>>
>> Maybe also add something like this ?
>>
>> For example, the following command can be used to run the test for x86_64:
>>
>> 	$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/kunit \
>> 		--arch=x86_64
>>
>> Regardless, the patch looks good to me:
>>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> Hey so since you have a bunch of patches merged into drm already but seem
> to lack drm-misc commit rights to push these yourself I think it's time to
> get those:
> 
> https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc
>
> And I guess Javier can help you with any questions you might have and make
> sure the request gets through by poking folks on #dri-devel irc?
>

Yes, he already requested commit access and got the acks, so I think is
just a matter of time until he has this sorted out.

José, please don't hesitate to ask if you need any help or clarification
once you have a setup to push your patches. The dim documentation [0] is
superb but I know that it can be somewhat stressful the first time :)

[0]: https://drm.pages.freedesktop.org/maintainer-tools/dim.html
 -- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-24 21:18       ` Javier Martinez Canillas
@ 2022-06-27 12:36           ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-27 12:36 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	Maxime Ripard

Hi Javier, Daniel,

On Fri, Jun 24, 2022 at 11:18:40PM +0200, Javier Martinez Canillas wrote:
> Hello Daniel,
> 
> On 6/24/22 23:01, Daniel Vetter wrote:
> >
> > [...]
> > 
> > Hey so since you have a bunch of patches merged into drm already but seem
> > to lack drm-misc commit rights to push these yourself I think it's time to
> > get those:
> > 
> > https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc
> >
> > And I guess Javier can help you with any questions you might have and make
> > sure the request gets through by poking folks on #dri-devel irc?
> >
> 
> Yes, he already requested commit access and got the acks, so I think is
> just a matter of time until he has this sorted out.
> 
> José, please don't hesitate to ask if you need any help or clarification
> once you have a setup to push your patches. The dim documentation [0] is
> superb but I know that it can be somewhat stressful the first time :)

Thanks for creating my ssh account :D

The tool is really well documented, setting it up was a piece of cake.
I pushed the patches to drm-misc-next. The output didn't show any
errors or warnings, hopefully meaning that I didn't mess up.

Jose

> [0]: https://drm.pages.freedesktop.org/maintainer-tools/dim.html
>  -- 
> Best regards,
> 
> Javier Martinez Canillas
> Linux Engineering
> Red Hat
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-27 12:36           ` José Expósito
  0 siblings, 0 replies; 31+ messages in thread
From: José Expósito @ 2022-06-27 12:36 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, kunit-dev

Hi Javier, Daniel,

On Fri, Jun 24, 2022 at 11:18:40PM +0200, Javier Martinez Canillas wrote:
> Hello Daniel,
> 
> On 6/24/22 23:01, Daniel Vetter wrote:
> >
> > [...]
> > 
> > Hey so since you have a bunch of patches merged into drm already but seem
> > to lack drm-misc commit rights to push these yourself I think it's time to
> > get those:
> > 
> > https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc
> >
> > And I guess Javier can help you with any questions you might have and make
> > sure the request gets through by poking folks on #dri-devel irc?
> >
> 
> Yes, he already requested commit access and got the acks, so I think is
> just a matter of time until he has this sorted out.
> 
> José, please don't hesitate to ask if you need any help or clarification
> once you have a setup to push your patches. The dim documentation [0] is
> superb but I know that it can be somewhat stressful the first time :)

Thanks for creating my ssh account :D

The tool is really well documented, setting it up was a piece of cake.
I pushed the patches to drm-misc-next. The output didn't show any
errors or warnings, hopefully meaning that I didn't mess up.

Jose

> [0]: https://drm.pages.freedesktop.org/maintainer-tools/dim.html
>  -- 
> Best regards,
> 
> Javier Martinez Canillas
> Linux Engineering
> Red Hat
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
  2022-06-27 12:36           ` José Expósito
@ 2022-06-27 12:53             ` Javier Martinez Canillas
  -1 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-27 12:53 UTC (permalink / raw)
  To: José Expósito
  Cc: dri-devel, davidgow, Maxime Ripard, airlied, dlatypov,
	linux-kernel, tzimmermann, kunit-dev

Hello José,

On 6/27/22 14:36, José Expósito wrote:
> Hi Javier, Daniel,
> 
> On Fri, Jun 24, 2022 at 11:18:40PM +0200, Javier Martinez Canillas wrote:
>> Hello Daniel,
>>
>> On 6/24/22 23:01, Daniel Vetter wrote:
>>>
>>> [...]
>>>
>>> Hey so since you have a bunch of patches merged into drm already but seem
>>> to lack drm-misc commit rights to push these yourself I think it's time to
>>> get those:
>>>
>>> https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc
>>>
>>> And I guess Javier can help you with any questions you might have and make
>>> sure the request gets through by poking folks on #dri-devel irc?
>>>
>>
>> Yes, he already requested commit access and got the acks, so I think is
>> just a matter of time until he has this sorted out.
>>
>> José, please don't hesitate to ask if you need any help or clarification
>> once you have a setup to push your patches. The dim documentation [0] is
>> superb but I know that it can be somewhat stressful the first time :)
> 
> Thanks for creating my ssh account :D
> 
> The tool is really well documented, setting it up was a piece of cake.
> I pushed the patches to drm-misc-next. The output didn't show any
> errors or warnings, hopefully meaning that I didn't mess up.
>

Awesome! Yes, everything looks correct. Thanks a lot.
 -- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/3] drm/doc: Add KUnit documentation
@ 2022-06-27 12:53             ` Javier Martinez Canillas
  0 siblings, 0 replies; 31+ messages in thread
From: Javier Martinez Canillas @ 2022-06-27 12:53 UTC (permalink / raw)
  To: José Expósito
  Cc: davidgow, dlatypov, tzimmermann, maarten.lankhorst, mripard,
	airlied, jani.nikula, dri-devel, kunit-dev, linux-kernel,
	Maxime Ripard

Hello José,

On 6/27/22 14:36, José Expósito wrote:
> Hi Javier, Daniel,
> 
> On Fri, Jun 24, 2022 at 11:18:40PM +0200, Javier Martinez Canillas wrote:
>> Hello Daniel,
>>
>> On 6/24/22 23:01, Daniel Vetter wrote:
>>>
>>> [...]
>>>
>>> Hey so since you have a bunch of patches merged into drm already but seem
>>> to lack drm-misc commit rights to push these yourself I think it's time to
>>> get those:
>>>
>>> https://drm.pages.freedesktop.org/maintainer-tools/commit-access.html#drm-misc
>>>
>>> And I guess Javier can help you with any questions you might have and make
>>> sure the request gets through by poking folks on #dri-devel irc?
>>>
>>
>> Yes, he already requested commit access and got the acks, so I think is
>> just a matter of time until he has this sorted out.
>>
>> José, please don't hesitate to ask if you need any help or clarification
>> once you have a setup to push your patches. The dim documentation [0] is
>> superb but I know that it can be somewhat stressful the first time :)
> 
> Thanks for creating my ssh account :D
> 
> The tool is really well documented, setting it up was a piece of cake.
> I pushed the patches to drm-misc-next. The output didn't show any
> errors or warnings, hopefully meaning that I didn't mess up.
>

Awesome! Yes, everything looks correct. Thanks a lot.
 -- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2022-06-27 12:53 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 17:17 [PATCH v3 0/3] KUnit tests for drm_format_helper José Expósito
2022-06-13 17:17 ` José Expósito
2022-06-13 17:17 ` [PATCH v3 1/3] drm/rect: Add DRM_RECT_INIT() macro José Expósito
2022-06-13 17:17   ` José Expósito
2022-06-13 18:31   ` Jani Nikula
2022-06-13 18:31     ` Jani Nikula
2022-06-13 17:17 ` [PATCH v3 2/3] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332() José Expósito
2022-06-13 17:17   ` José Expósito
2022-06-13 17:17 ` [PATCH v3 3/3] drm/doc: Add KUnit documentation José Expósito
2022-06-13 17:17   ` José Expósito
2022-06-14 12:58   ` Javier Martinez Canillas
2022-06-14 12:58     ` Javier Martinez Canillas
2022-06-14 18:09     ` José Expósito
2022-06-14 18:09       ` José Expósito
2022-06-14 18:14       ` Javier Martinez Canillas
2022-06-14 18:14         ` Javier Martinez Canillas
2022-06-24 21:01     ` Daniel Vetter
2022-06-24 21:01       ` Daniel Vetter
2022-06-24 21:18       ` Javier Martinez Canillas
2022-06-27 12:36         ` José Expósito
2022-06-27 12:36           ` José Expósito
2022-06-27 12:53           ` Javier Martinez Canillas
2022-06-27 12:53             ` Javier Martinez Canillas
2022-06-14  7:08 ` [PATCH v3 0/3] KUnit tests for drm_format_helper Thomas Zimmermann
2022-06-14  7:08   ` Thomas Zimmermann
2022-06-14 13:03 ` Javier Martinez Canillas
2022-06-14 13:03   ` Javier Martinez Canillas
2022-06-16 14:44 ` David Gow
2022-06-16 14:44   ` David Gow
2022-06-16 18:38   ` José Expósito
2022-06-16 18:38     ` José Expósito

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.