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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Mismatched parameter order between declaration and definition for `drm_panic_helper_draw_screen_qr_code`.
- [Low] Unmet direct dependency warning when DRM_PANIC_HELPER is selected without DRM_TTM.
- [Low] Missing indirect dependencies on DRM_DRAW and FONT_SUPPORT leading to linker errors.
--

--- Patch [12]: [PATCH 12/12] drm/panic: Compile KUnit tests as module ---
commit a56a5f0ffd0c185768aecc54426d672e40b1b0ae
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/panic: Compile KUnit tests as module

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.

> diff --git a/drivers/gpu/drm/Kconfig.debug b/drivers/gpu/drm/Kconfig.debug
> index 1f4c98cbf0906..b909cd0a619fe 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

[Severity: Low]
When DRM_KUNIT_TEST is enabled on a configuration where DRM_TTM is disabled,
does this cause an unmet direct dependency warning? DRM_PANIC_HELPER has a
depends on DRM_TTM requirement in drivers/gpu/drm/Kconfig.

Also, does selecting DRM_PANIC_HELPER without ensuring DRM_DRAW and
FONT_SUPPORT are available cause linker errors? 

The drm_panic_helper.c file unconditionally calls functions from the DRM
draw library (drm_draw_color_from_xrgb8888) and the font library
(get_default_font). Normally, DRM_PANIC selects DRM_DRAW and FONT_SUPPORT,
ensuring these symbols are available, but selecting DRM_PANIC_HELPER
directly bypasses these.

[ ... ]

> diff --git a/include/drm/drm_panic_helper.h b/include/drm/drm_panic_helper.h
> index 6dc63288bc2ab..76ff3a83e07b7 100644
> --- a/include/drm/drm_panic_helper.h
> +++ b/include/drm/drm_panic_helper.h
[ ... ]
> @@ -15,6 +16,13 @@ void 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, unsigned int qr_version,
> +					 u32 fg_color, u32 bg_color);

[Severity: Medium]
Does this prototype match the definition in
drivers/gpu/drm/drm_panic_helper.c?

Here it is declared as:
int drm_panic_helper_draw_screen_qr_code(struct drm_scanout_buffer *sb,
                                         unsigned int qr_version,
                                         u32 fg_color,
                                         u32 bg_color);

But the implementation is defined as:
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)

Because u32 and unsigned int are functionally equivalent types, the compiler
silently accepts this mismatch without any warnings. Will any caller
relying on the header's parameter order inadvertently swap the qr_version
and fg_color arguments, causing the function to fail with -ENOSPC or render
with corrupted colors?

> +void drm_panic_helper_set_description(const char *description);
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818125012.468092-1-tzimmermann@suse.de?part=12