mbox series

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

Message ID 20230512084152.31233-1-tzimmermann@suse.de
Headers show
Series drm/fbdev: Remove DRM's helpers for fbdev I/O | expand

Message

Thomas Zimmermann May 12, 2023, 8:41 a.m. UTC 
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.

Thomas Zimmermann (11):
  drm/armada: Use regular fbdev I/O helpers
  drm/exynos: Use regular fbdev I/O helpers
  drm/gma500: Use regular fbdev I/O helpers
  drm/radeon: Use regular fbdev I/O helpers
  drm/fbdev-dma: Use regular fbdev I/O helpers
  drm/msm: Use regular fbdev I/O helpers
  drm/omapdrm: Use regular fbdev I/O helpers
  drm/tegra: Use regular fbdev I/O helpers
  drm/fb-helper: Export helpers for marking damage areas
  drm/fbdev-generic: Implement dedicated fbdev I/O helpers
  drm/i915: Implement dedicated fbdev I/O helpers

 drivers/gpu/drm/Kconfig                       |  27 +-
 drivers/gpu/drm/Makefile                      |   7 +-
 drivers/gpu/drm/armada/Kconfig                |   8 +
 drivers/gpu/drm/armada/Makefile               |   2 +-
 drivers/gpu/drm/armada/armada_drm.h           |   2 +-
 drivers/gpu/drm/armada/armada_fbdev.c         |   9 +-
 drivers/gpu/drm/drm_fb_helper.c               | 233 ++----------------
 drivers/gpu/drm/drm_fbdev_dma.c               |  12 +-
 drivers/gpu/drm/drm_fbdev_generic.c           |  47 +++-
 drivers/gpu/drm/exynos/Kconfig                |   8 +
 drivers/gpu/drm/exynos/Makefile               |   2 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c     |  10 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.h     |   2 +-
 drivers/gpu/drm/gma500/Kconfig                |   8 +
 drivers/gpu/drm/gma500/Makefile               |   2 +-
 drivers/gpu/drm/gma500/fbdev.c                |   9 +-
 drivers/gpu/drm/gma500/psb_drv.h              |   2 +-
 drivers/gpu/drm/i915/Kconfig                  |   8 +
 drivers/gpu/drm/i915/Makefile                 |   2 +-
 .../drm/i915/display/intel_display_debugfs.c  |   2 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c    |  51 +++-
 drivers/gpu/drm/i915/display/intel_fbdev.h    |   2 +-
 drivers/gpu/drm/msm/Kconfig                   |   9 +
 drivers/gpu/drm/msm/Makefile                  |   2 +-
 drivers/gpu/drm/msm/msm_drv.h                 |   2 +-
 drivers/gpu/drm/msm/msm_fbdev.c               |  12 +-
 drivers/gpu/drm/omapdrm/Kconfig               |   9 +
 drivers/gpu/drm/omapdrm/Makefile              |   2 +-
 drivers/gpu/drm/omapdrm/omap_debugfs.c        |   4 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c          |  12 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.h          |   2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c            |   4 +-
 drivers/gpu/drm/radeon/Kconfig                |   8 +
 drivers/gpu/drm/radeon/Makefile               |   2 +-
 drivers/gpu/drm/radeon/radeon_fbdev.c         |   9 +-
 drivers/gpu/drm/radeon/radeon_mode.h          |   2 +-
 drivers/gpu/drm/tegra/Kconfig                 |   9 +
 drivers/gpu/drm/tegra/Makefile                |   2 +-
 drivers/gpu/drm/tegra/drm.h                   |   2 +-
 drivers/gpu/drm/tegra/fbdev.c                 |  11 +-
 include/drm/drm_fb_helper.h                   |  84 +------
 include/drm/drm_fbdev_dma.h                   |   2 +-
 include/drm/drm_fbdev_generic.h               |   2 +-
 43 files changed, 264 insertions(+), 382 deletions(-)


base-commit: 451e49cfbaa90720149e63f4fa9c7824013c783d

Comments

Sam Ravnborg May 12, 2023, 10:29 a.m. UTC | #1 
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.
Sam Ravnborg May 12, 2023, 1:41 p.m. UTC | #2 
Hi Thomas,
> > 
> > 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.
I forgot about exported symbols - that makes the idea futile.

> 
> 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.
That's a much nicer way to express it - and with this we do not introduce
the IMO confusing CFB (Color Frame Buffer) abbreviation in every driver.

	Sam