Re: [PATCH v3 14/14] drm/panic: Compile KUnit tests as module

[Jocelyn Falempe <jfalempe@redhat.com>]

On 9/2/26 14:13, Thomas Zimmermann wrote:
> Fix dependency of the panic-helper KUnit tests on CONFIG_DRM_KUNIT_TEST=y
> and make them build as a module. Export the necessary interfaces from the
> panic helpers iff the KUnit framework has been enabled.
> 
> Select DRM_PANIC_HELPER from DRM_KUNIT_TEST to meet the dependencies of
> the panic-helper tests.
> 
> It is now possible to run the tests by modprobe-ing drm_panic_helper_test
> at runtime.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> 
> v3:
> - clean up after each test case (Sashiko)
> v2:
> - fix parameter order in decl of drm_panic_helper_draw_screen_qr()
>    (Sashiko)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/Kconfig.debug                 |  1 +
>   drivers/gpu/drm/drm_panic_helper.c            | 26 ++++++++++---------
>   drivers/gpu/drm/tests/Makefile                |  1 +
>   drivers/gpu/drm/tests/drm_panic_helper_test.c | 21 ++++++++++++---
>   include/drm/drm_panic_helper.h                |  8 ++++++
>   5 files changed, 41 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig.debug b/drivers/gpu/drm/Kconfig.debug
> index 1f4c98cbf090..b909cd0a619f 100644
> --- a/drivers/gpu/drm/Kconfig.debug
> +++ b/drivers/gpu/drm/Kconfig.debug
> @@ -69,6 +69,7 @@ config DRM_KUNIT_TEST
>   	select DRM_EXPORT_FOR_TESTS if m
>   	select DRM_GEM_SHMEM_HELPER
>   	select DRM_KUNIT_TEST_HELPERS
> +	select DRM_PANIC_HELPER
>   	select DRM_SYSFB_HELPER
>   	select PRIME_NUMBERS
>   	default KUNIT_ALL_TESTS
> diff --git a/drivers/gpu/drm/drm_panic_helper.c b/drivers/gpu/drm/drm_panic_helper.c
> index f64a3ca38467..683af48de38c 100644
> --- a/drivers/gpu/drm/drm_panic_helper.c
> +++ b/drivers/gpu/drm/drm_panic_helper.c
> @@ -8,6 +8,8 @@
>   #include <linux/utsname.h>
>   #include <linux/zlib.h>
>   
> +#include <kunit/visibility.h>
> +
>   #include <drm/drm_fourcc.h>
>   #include <drm/drm_framebuffer.h>
>   #include <drm/drm_modeset_helper_vtables.h>
> @@ -433,8 +435,8 @@ static void drm_panic_helper_logo_draw(struct drm_scanout_buffer *sb, struct drm
>   				   fg_color);
>   }
>   
> -static int drm_panic_helper_draw_screen_user(struct drm_scanout_buffer *sb,
> -					     u32 fg_color, u32 bg_color)
> +VISIBLE_IF_KUNIT int drm_panic_helper_draw_screen_user(struct drm_scanout_buffer *sb,
> +						       u32 fg_color, u32 bg_color)
>   {
>   	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
>   	struct drm_rect r_screen, r_logo, r_msg;
> @@ -466,6 +468,7 @@ static int drm_panic_helper_draw_screen_user(struct drm_scanout_buffer *sb,
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL_IF_KUNIT(drm_panic_helper_draw_screen_user);
>   
>   /*
>    * Draw one line of kmsg, and handle wrapping if it won't fit in the screen width.
> @@ -504,8 +507,8 @@ static int draw_line_with_wrap(struct drm_scanout_buffer *sb, const struct font_
>    * Draw the kmsg buffer to the screen, starting from the youngest message at the bottom,
>    * and going up until reaching the top of the screen.
>    */
> -static int drm_panic_helper_draw_screen_kmsg(struct drm_scanout_buffer *sb,
> -					     u32 fg_color, u32 bg_color)
> +VISIBLE_IF_KUNIT int drm_panic_helper_draw_screen_kmsg(struct drm_scanout_buffer *sb,
> +						       u32 fg_color, u32 bg_color)
>   {
>   	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
>   	struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
> @@ -549,6 +552,7 @@ static int drm_panic_helper_draw_screen_kmsg(struct drm_scanout_buffer *sb,
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL_IF_KUNIT(drm_panic_helper_draw_screen_kmsg);
>   
>   #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
>   /*
> @@ -691,9 +695,9 @@ static int drm_panic_helper_get_qr_code(u8 **qr_image, unsigned int qr_version)
>   /*
>    * Draw the panic message at the center of the screen, with a QR Code
>    */
> -static int drm_panic_helper_draw_screen_qr_code(struct drm_scanout_buffer *sb,
> -						u32 fg_color, u32 bg_color,
> -						unsigned int qr_version)
> +VISIBLE_IF_KUNIT int drm_panic_helper_draw_screen_qr_code(struct drm_scanout_buffer *sb,
> +							  u32 fg_color, u32 bg_color,
> +							  unsigned int qr_version)
>   {
>   	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
>   	struct drm_rect r_screen, r_logo, r_msg, r_qr, r_qr_canvas;
> @@ -763,6 +767,7 @@ static int drm_panic_helper_draw_screen_qr_code(struct drm_scanout_buffer *sb,
>   	drm_panic_helper_blit(sb, &r_qr, qr_image, qr_pitch, scale, fg_color);
>   	return 0;
>   }
> +EXPORT_SYMBOL_IF_KUNIT(drm_panic_helper_draw_screen_qr_code);
>   #else
>   static void __init drm_panic_helper_qr_init(void) { };
>   static void __exit drm_panic_helper_qr_exit(void) { };
> @@ -815,7 +820,7 @@ static int draw_panic_helper_dispatch(struct drm_scanout_buffer *sb,
>   	return ret;
>   }
>   
> -static void drm_panic_helper_set_description(const char *description)
> +VISIBLE_IF_KUNIT void drm_panic_helper_set_description(const char *description)
>   {
>   	u32 len;
>   
> @@ -830,6 +835,7 @@ static void drm_panic_helper_set_description(const char *description)
>   		desc_line->len = len;
>   	}
>   }
> +EXPORT_SYMBOL_IF_KUNIT(drm_panic_helper_set_description);
>   
>   static void drm_panic_helper_clear_description(void)
>   {
> @@ -910,7 +916,3 @@ void __exit drm_panic_helper_exit(void)
>   {
>   	drm_panic_helper_qr_exit();
>   }
> -
> -#ifdef CONFIG_DRM_KUNIT_TEST
> -#include "tests/drm_panic_helper_test.c"
> -#endif
> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
> index e7193b49cf91..42207df1d1c8 100644
> --- a/drivers/gpu/drm/tests/Makefile
> +++ b/drivers/gpu/drm/tests/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
>   	drm_managed_test.o \
>   	drm_mm_test.o \
>   	drm_modes_test.o \
> +	drm_panic_helper_test.o \
>   	drm_plane_helper_test.o \
>   	drm_probe_helper_test.o \
>   	drm_rect_test.o \
> diff --git a/drivers/gpu/drm/tests/drm_panic_helper_test.c b/drivers/gpu/drm/tests/drm_panic_helper_test.c
> index e6befe0f1c83..6184a6fc7eb8 100644
> --- a/drivers/gpu/drm/tests/drm_panic_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_panic_helper_test.c
> @@ -6,13 +6,17 @@
>    * KUNIT tests for DRM panic helpers
>    */
>   
> -#include <drm/drm_fourcc.h>
> -#include <drm/drm_panic.h>
> +#include <linux/highmem.h>
> +#include <linux/units.h>
> +#include <linux/vmalloc.h>
>   
>   #include <kunit/test.h>
>   
> -#include <linux/units.h>
> -#include <linux/vmalloc.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_panic.h>
> +#include <drm/drm_panic_helper.h>
> +
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
>   
>   static void drm_panic_check_color_byte(struct kunit *test, u8 b)
>   {
> @@ -86,6 +90,11 @@ static int drm_test_panic_init(struct kunit *test)
>   	return 0;
>   }
>   
> +static void drm_test_panic_exit(struct kunit *test)
> +{
> +	drm_panic_helper_set_description(NULL);
> +}
> +
>   /*
>    * Test drawing the panic screen, using a memory mapped framebuffer
>    * Set the whole buffer to 0xa5, and then check that all pixels have been
> @@ -229,7 +238,11 @@ static struct kunit_case drm_panic_screen_user_test[] = {
>   static struct kunit_suite drm_panic_helper_suite = {
>   	.name = "drm_panic_helper",
>   	.init = drm_test_panic_init,
> +	.exit = drm_test_panic_exit,
>   	.test_cases = drm_panic_screen_user_test,
>   };
>   
>   kunit_test_suite(drm_panic_helper_suite);
> +
> +MODULE_DESCRIPTION("KUnit test suite for DRM panic handling");
> +MODULE_LICENSE("GPL");
> diff --git a/include/drm/drm_panic_helper.h b/include/drm/drm_panic_helper.h
> index 6cc9a0eba077..6b19607e0784 100644
> --- a/include/drm/drm_panic_helper.h
> +++ b/include/drm/drm_panic_helper.h
> @@ -8,6 +8,7 @@
>   enum drm_panic_type;
>   
>   struct drm_plane;
> +struct drm_scanout_buffer;
>   
>   /* drm_panic_helper.c */
>   int drm_plane_helper_display_panic_screen(struct drm_plane *plane,
> @@ -15,6 +16,13 @@ int drm_plane_helper_display_panic_screen(struct drm_plane *plane,
>   					  enum drm_panic_type panic_type,
>   					  u32 fg_color, u32 bg_color,
>   					  unsigned int qr_version);
> +#if IS_ENABLED(CONFIG_KUNIT)
> +int drm_panic_helper_draw_screen_user(struct drm_scanout_buffer *sb, u32 fg_color, u32 bg_color);
> +int drm_panic_helper_draw_screen_kmsg(struct drm_scanout_buffer *sb, u32 fg_color, u32 bg_color);
> +int drm_panic_helper_draw_screen_qr_code(struct drm_scanout_buffer *sb,
> +					 u32 fg_color, u32 bg_color, unsigned int qr_version);
> +void drm_panic_helper_set_description(const char *description);
> +#endif
>   
>   /* drm_panic_helper_qr.rs */
>   size_t drm_panic_helper_qr_max_data_size(u8 version, size_t url_len);