[PATCH 00/11] drm/fbdev: Remove DRM's helpers for fbdev I/O

[Thomas Zimmermann]

Hi Sam

Am 12.05.23 um 12:29 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, May 12, 2023 at 10:41:41AM +0200, Thomas Zimmermann wrote:
>> DRM provides a number of wrappers around fbdev cfb_() sys_(), fb_io_()
>> and fb_sys_() helpers. The DRM functions don't provide any additional
>> functionality for most DRM drivers. So remove them and call the fbdev
>> I/O helpers directly.
>>
>> The DRM fbdev I/O wrappers were originally added because <linux/fb.h>
>> does not protect its content with CONFIG_FB. DRM fbdev emulation did
>> not build if the the config option had been disabled. This has been
>> fixed. For fbdev-generic and i915, the wrappers added support for damage
>> handling. But this is better handled within the two callers, as each
>> is special in its damage handling.
>>
>> Patches 1 to 8 replace the DRM wrappers in a number of fbdev emulations.
>> Patch 9 exports two helpers for damage handling. Patches 10 and 11
>> update fbdev-generic and i915 with the help of the exported functions.
>> The patches also remove DRM's fbdev I/O helpers, which are now unused.
>>
>> DRM's fbdev helpers had to select fbdev I/O helpers for I/O and for
>> system memory. Each fbdev emulation now selects the correct helpers
>> for itself. Depending on the selected DRM drivers, kernel builds will
>> now only contain the necessary fbdev I/O helpers and might be slightly
>> smaller in size.
> 
> Nice cleanup.
> 
>  From one of the patches:
> 
>> +config DRM_ARMADA_FBDEV_EMULATION
>> +     bool
>> +     depends on DRM_ARMADA
>> +     select FB_CFB_COPYAREA
>> +     select FB_CFB_FILLRECT
>> +     select FB_CFB_IMAGEBLIT
> 
> This seems like a hard to maintain way to select a few helper functions.
> Today we have LD_DEAD_CODE_DATA_ELIMINATION for the configs that care
> about size - and that should work here as well.

I wasn't too happy about this solution either as it is quite verbose. 
But I don't want to rely on the linker either. It certainly cannot 
remove exported symbols.

But the pattern is very common among the fbdev drivers. We could 
introduce common Kconfig options in fbdev and selcet those instead. Like 
this:

const FB_IO_HELPERS
	bool
	depends on FB
	select FB_CFB_COPYAREA
	select FB_CFB_FILLRECT
	select FB_CFB_IMAGEBLIT

const FB_SYS_HELPERS
	bool
	depends on FB
	select FB_SYS_COPYAREA
	select FB_SYS_FILLRECT
	select FB_SYS_FOPS
	select FB_SYS_IMAGEBLIT

Apart from DRM, most of the fbdev drivers could use these as well.

Best regards
Thomas

> 
> I understand where this comes from and I am not against the
> solution, but wanted to point at a more modern approach to deal with the
> bloat.
> 
> Maybe some of the embbedded folks can tell if LD_DEAD_CODE_DATA_ELIMINATION
> can be trusted yet or that is something for the future.
> 
> In barebox -ffunction-section (what LD_DEAD_CODE_DATA_ELIMINATION
> enables) is used with success - there it really helps when generating
> different barebox images where size matters a lot.
> 
> 	Sam

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230512/391a22fd/attachment.sig>