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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux