Re: [PATCH 3/4] drm/pixel: Link panic helpers into drm_pixel_helper.ko

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] drm: missing DRM_PIXEL_HELPER select in DRM_KMS_HELPER for drm_crtc_init()
- [Low] drm/tests: link failure in drm_panic_helper_test when CONFIG_DRM_PANIC=n
- [Low] drm/xe: missing DRM_PIXEL_HELPER dependency when DRM_PANIC=y
--

commit d010f0f67d242682650aa0651dfd5a1e57612943
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/pixel: Link panic helpers into drm_pixel_helper.ko

This commit moves the DRM panic helpers from drm_kms_helper into
drm_pixel_helper.ko, drops the DRM_PANIC_HELPER Kconfig option, and
selects DRM_PIXEL_HELPER in drivers using the panic helpers.

> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 8e4a588e9111d..bf7c599443403 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -46,13 +46,14 @@ config DRM_MIPI_DSI
>  config DRM_KMS_HELPER
>  	tristate
>  	depends on DRM
> -	select DRM_PANIC_HELPER if DRM_PANIC
>  	help
>  	  CRTC helpers for KMS drivers.

[Severity: Low]
Does DRM_KMS_HELPER also need to select DRM_PIXEL_HELPER if DRM_PANIC?

drivers/gpu/drm/drm_modeset_helper.c is unconditionally compiled into
drm_kms_helper, and its primary_plane_funcs structure (used by
drm_crtc_init()) includes DRM_PANIC_PLANE_FUNCS:

drivers/gpu/drm/drm_modeset_helper.c:
    static const struct drm_plane_funcs primary_plane_funcs = {
            DRM_PLANE_NON_ATOMIC_FUNCS,
            DRM_PANIC_PLANE_FUNCS,
    };

When CONFIG_DRM_PANIC is enabled, DRM_PANIC_PLANE_FUNCS expands in
include/drm/drm_panic_helper.h to reference
drm_plane_helper_display_panic_screen, which is now built into
drm_pixel_helper. If CONFIG_DRM_PANIC=y and DRM_KMS_HELPER is enabled
without a matching built-in or modular DRM_PIXEL_HELPER, linking fails
with an undefined reference to drm_plane_helper_display_panic_screen.

[ ... ]

> diff --git a/drivers/gpu/drm/Kconfig.debug b/drivers/gpu/drm/Kconfig.debug
> index 1afb9cad850f6..4573cad368a28 100644
> --- a/drivers/gpu/drm/Kconfig.debug
> +++ b/drivers/gpu/drm/Kconfig.debug
> @@ -69,7 +69,6 @@ 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_PIXEL_HELPER
>  	select DRM_SYSFB_HELPER
>  	select PRIME_NUMBERS

[Severity: Low]
Can removing select DRM_PANIC_HELPER from DRM_KUNIT_TEST cause a link
failure when CONFIG_DRM_PANIC is disabled?

With this change, drm_panic_helper.o is only built into drm_pixel_helper
when CONFIG_DRM_PANIC is enabled:

drivers/gpu/drm/Makefile:
    drm_pixel_helper-$(CONFIG_DRM_PANIC) += drm_panic_helper.o

However, drivers/gpu/drm/tests/Makefile still builds
drm_panic_helper_test.o unconditionally under obj-$(CONFIG_DRM_KUNIT_TEST),
and functions such as drm_panic_helper_draw_screen_user_default() in
drivers/gpu/drm/tests/drm_panic_helper_test.c call
drm_panic_helper_draw_screen_user(), drm_panic_helper_draw_screen_kmsg(),
drm_panic_helper_set_description(), and
drm_panic_helper_clear_description(). When CONFIG_DRM_KUNIT_TEST is enabled
and CONFIG_DRM_PANIC=n, linking fails with undefined references to those
symbols.

[ ... ]

> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 71426b9c91f00..f698d3c8daa9e 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -19,6 +19,7 @@ config DRM_I915
>  	select DRM_KMS_HELPER
>  	select DRM_PANEL
>  	select DRM_MIPI_DSI
> +	select DRM_PIXEL_HELPER if DRM_PANIC
>  	select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
>  	select RELAY
>  	select I2C

[Severity: Low]
Should DRM_XE or DRM_XE_DISPLAY in drivers/gpu/drm/xe/Kconfig also select
DRM_PIXEL_HELPER if DRM_PANIC?

When CONFIG_DRM_XE_DISPLAY=y, drivers/gpu/drm/xe/Makefile compiles
drivers/gpu/drm/i915/display/skl_universal_plane.c into xe.ko, where
skl_plane_funcs, icl_plane_funcs, and tgl_plane_funcs use
DRM_PANIC_PLANE_FUNCS:

drivers/gpu/drm/i915/display/skl_universal_plane.c:
    static const struct drm_plane_funcs skl_plane_funcs = {
            ...
            DRM_PANIC_PLANE_FUNCS,
    };

If CONFIG_DRM_XE=m, CONFIG_DRM_XE_DISPLAY=y, and CONFIG_DRM_PANIC=y are
enabled while CONFIG_DRM_PIXEL_HELPER=n, modpost fails with an undefined
reference to drm_plane_helper_display_panic_screen in xe.ko.

[ ... ]

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