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

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 19.10.26 um 18:19 schrieb 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()

I see this is still an issue. The fix here is to move drm_crtc_init() 
into drivers. The only remaining callers of drm_crtc_init() are in 
radeon and amdgpu. There was recently a patch to move the helper there, 
[1] but the effort appears to have stalled.

[1] 
https://lore.kernel.org/dri-devel/20260903071409.427010-1-oushixiong1025@163.com/

I'll do this before continuing with the pixel helpers.


> - [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.
>
> [ ... ]
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)