| Message ID | 20230629225113.297512-2-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allow disabling all native fbdev drivers and only keeping DRM emulation | expand |
On Fri, Jun 30, 2023, at 00:51, Javier Martinez Canillas wrote: > Currently the CONFIG_FB option has to be enabled even if no legacy fbdev > drivers are needed (e.g: only to have support for framebuffer consoles). > > The DRM subsystem has a fbdev emulation layer, but depends on CONFIG_FB > and so it can only be enabled if that dependency is enabled as well. > > That means fbdev drivers have to be explicitly disabled if users want to > enable CONFIG_FB, only to use fbcon and/or the DRM fbdev emulation layer. > > This patch introduces a CONFIG_FB_CORE option that could be enabled just > to have the core support needed for CONFIG_DRM_FBDEV_EMULATION, allowing > CONFIG_FB to be disabled (and automatically disabling all fbdev drivers). > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- This looks really nice! I tried to do something like this a few years ago, but failed as I did too much at once by attempting to cut out msot of the fb core support that is not actually used by DRM at the same time. Doing just the Kconfig bits as you do here is probably better anyway, cutting out unneeded bits into separate modules or #ifdef sections can come later. > @@ -59,7 +71,7 @@ config FIRMWARE_EDID > > config FB_DEVICE > bool "Provide legacy /dev/fb* device" > - depends on FB > + depends on FB_CORE > default y > help > Say Y here if you want the legacy /dev/fb* device file and I don't see this symbol in linux-next yet, what tree are you using as a base? > @@ -69,13 +81,13 @@ config FB_DEVICE > > config FB_DDC > tristate > - depends on FB > + depends on FB_CORE > select I2C_ALGOBIT > select I2C This seems to only be used by actual fbdev drivers, so maybe don't change the dependency here. > @@ -162,22 +174,22 @@ endchoice > > config FB_SYS_FOPS > tristate > - depends on FB > + depends on FB_CORE Same for this one > config FB_HECUBA > tristate > - depends on FB > + depends on FB_CORE > depends on FB_DEFERRED_IO > > config FB_SVGALIB > tristate > - depends on FB > + depends on FB_CORE > help > Common utility functions useful to fbdev drivers of VGA-based > cards. > config FB_MACMODES > tristate > - depends on FB > + depends on FB_CORE > These three seem to actually be part of fbdev drivers rather than the core, and it may be best to move them into drviers/video/fbdev/ as standalone modules. That would be a separate patch of course. > config FB_BACKLIGHT > tristate > - depends on FB > + depends on FB_CORE > select BACKLIGHT_CLASS_DEVICE Separating this one from FB_CORE would help avoid circular dependencies, this one keeps causing issues. > @@ -1,22 +1,22 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_FB_NOTIFY) += fb_notify.o > -obj-$(CONFIG_FB) += fb.o > -fb-y := fb_backlight.o \ > +obj-$(CONFIG_FB_CORE) += fb_core.o > +fb_core-y := fb_backlight.o \ > fb_info.o \ > fbmem.o fbmon.o fbcmap.o \ > modedb.o fbcvt.o fb_cmdline.o I would not bother renaming the module itself here, that might cause problems if anything relies on loading the module by name or a named module parameter. Arnd
"Arnd Bergmann" <arnd@arndb.de> writes: Hello Arnd, Thanks for your review! > On Fri, Jun 30, 2023, at 00:51, Javier Martinez Canillas wrote: >> Currently the CONFIG_FB option has to be enabled even if no legacy fbdev >> drivers are needed (e.g: only to have support for framebuffer consoles). >> >> The DRM subsystem has a fbdev emulation layer, but depends on CONFIG_FB >> and so it can only be enabled if that dependency is enabled as well. >> >> That means fbdev drivers have to be explicitly disabled if users want to >> enable CONFIG_FB, only to use fbcon and/or the DRM fbdev emulation layer. >> >> This patch introduces a CONFIG_FB_CORE option that could be enabled just >> to have the core support needed for CONFIG_DRM_FBDEV_EMULATION, allowing >> CONFIG_FB to be disabled (and automatically disabling all fbdev drivers). >> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> >> --- > > This looks really nice! > > I tried to do something like this a few years ago, but failed as Yes, I also tried to do this before some time ago [0]: [0]: https://lore.kernel.org/lkml/20210827100027.1577561-1-javierm@redhat.com/t/#mc8bb6cda8c2d795673618049b6c834b34bf86162 and at the time required some code refactoring but now thanks to all the cleanups that Thomas has been doing over, I could just do it with Kconfig. > I did too much at once by attempting to cut out msot of the fb core > support that is not actually used by DRM at the same time. > > Doing just the Kconfig bits as you do here is probably better > anyway, cutting out unneeded bits into separate modules or #ifdef > sections can come later. > Exactly, that was my thought too. Glad that you agree with the approach. >> @@ -59,7 +71,7 @@ config FIRMWARE_EDID >> >> config FB_DEVICE >> bool "Provide legacy /dev/fb* device" >> - depends on FB >> + depends on FB_CORE >> default y >> help >> Say Y here if you want the legacy /dev/fb* device file and > > I don't see this symbol in linux-next yet, what tree are you using > as a base? > It's now in the drm-misc/drm-misc-next branch [1]. It's not in -next yet because it just landed a few days ago [2]. [1]: https://cgit.freedesktop.org/drm/drm-misc/log/?h=drm-misc-next [2]: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=701d2054fa3 In fact, that's the reason why I rebased my previous attempt [0]. >> @@ -69,13 +81,13 @@ config FB_DEVICE >> >> config FB_DDC >> tristate >> - depends on FB >> + depends on FB_CORE >> select I2C_ALGOBIT >> select I2C > > This seems to only be used by actual fbdev drivers, so maybe > don't change the dependency here. > Indeed. >> @@ -162,22 +174,22 @@ endchoice >> >> config FB_SYS_FOPS >> tristate >> - depends on FB >> + depends on FB_CORE > > Same for this one > Ok. >> config FB_HECUBA >> tristate >> - depends on FB >> + depends on FB_CORE >> depends on FB_DEFERRED_IO >> >> config FB_SVGALIB >> tristate >> - depends on FB >> + depends on FB_CORE >> help >> Common utility functions useful to fbdev drivers of VGA-based >> cards. >> config FB_MACMODES >> tristate >> - depends on FB >> + depends on FB_CORE >> > > These three seem to actually be part of fbdev drivers rather > than the core, and it may be best to move them into > drviers/video/fbdev/ as standalone modules. That would be a > separate patch of course. > Agreed. I'll then also don't change the dependency on these ones. >> config FB_BACKLIGHT >> tristate >> - depends on FB >> + depends on FB_CORE >> select BACKLIGHT_CLASS_DEVICE > > Separating this one from FB_CORE would help avoid circular dependencies, > this one keeps causing issues. > You mean separating from FB or should I keep the existing depends on FB? It seems this is only used by fbdev drivers so probably the latter? >> @@ -1,22 +1,22 @@ >> # SPDX-License-Identifier: GPL-2.0 >> obj-$(CONFIG_FB_NOTIFY) += fb_notify.o >> -obj-$(CONFIG_FB) += fb.o >> -fb-y := fb_backlight.o \ >> +obj-$(CONFIG_FB_CORE) += fb_core.o >> +fb_core-y := fb_backlight.o \ >> fb_info.o \ >> fbmem.o fbmon.o fbcmap.o \ >> modedb.o fbcvt.o fb_cmdline.o > > I would not bother renaming the module itself here, that > might cause problems if anything relies on loading the > module by name or a named module parameter. > I was actually not sure about this, but then thought that someone could had complained that the Kconfig symbol and module name wouldn't match :) I'll just keep the existing module name then and drop the rename. > Arnd >
On Fri, Jun 30, 2023, at 12:51, Javier Martinez Canillas wrote: > "Arnd Bergmann" <arnd@arndb.de> writes: > >>> @@ -59,7 +71,7 @@ config FIRMWARE_EDID >>> >>> config FB_DEVICE >>> bool "Provide legacy /dev/fb* device" >>> - depends on FB >>> + depends on FB_CORE >>> default y >>> help >>> Say Y here if you want the legacy /dev/fb* device file and >> >> I don't see this symbol in linux-next yet, what tree are you using >> as a base? >> > > It's now in the drm-misc/drm-misc-next branch [1]. It's not in -next yet > because it just landed a few days ago [2]. > > [1]: https://cgit.freedesktop.org/drm/drm-misc/log/?h=drm-misc-next > [2]: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=701d2054fa3 > > In fact, that's the reason why I rebased my previous attempt [0]. Ok. I wonder if it would make sense to also make FB_DEVICE depend on FB instead of FB_CORE then. I don't think there is any actual dependency between the two, but as I understand we want modern distros to use neither FB nor FB_DEVICE, so tying them together with a dependency may be desirable anyway. >>> config FB_BACKLIGHT >>> tristate >>> - depends on FB >>> + depends on FB_CORE >>> select BACKLIGHT_CLASS_DEVICE >> >> Separating this one from FB_CORE would help avoid circular dependencies, >> this one keeps causing issues. >> > > You mean separating from FB or should I keep the existing depends on FB? > > It seems this is only used by fbdev drivers so probably the latter? Right, I meant keeping the dependency on FB. Ideally we'd turn this all into a set of 'depends on' instead of 'select', but that is a completely separate topic. Arnd
"Arnd Bergmann" <arnd@arndb.de> writes: > On Fri, Jun 30, 2023, at 12:51, Javier Martinez Canillas wrote: >> "Arnd Bergmann" <arnd@arndb.de> writes: >> >>>> @@ -59,7 +71,7 @@ config FIRMWARE_EDID >>>> >>>> config FB_DEVICE >>>> bool "Provide legacy /dev/fb* device" >>>> - depends on FB >>>> + depends on FB_CORE >>>> default y >>>> help >>>> Say Y here if you want the legacy /dev/fb* device file and >>> >>> I don't see this symbol in linux-next yet, what tree are you using >>> as a base? >>> >> >> It's now in the drm-misc/drm-misc-next branch [1]. It's not in -next yet >> because it just landed a few days ago [2]. >> >> [1]: https://cgit.freedesktop.org/drm/drm-misc/log/?h=drm-misc-next >> [2]: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=701d2054fa3 >> >> In fact, that's the reason why I rebased my previous attempt [0]. > > Ok. > > I wonder if it would make sense to also make FB_DEVICE depend on FB > instead of FB_CORE then. I don't think there is any actual dependency No, because that wouldn't allow to have /dev/fb* devices when using the DRM fbdev emulation layer. It could be the case that users have an old fbdev user-space but the platform only has a DRM driver, in that case we want to disable all native fbdev drivers (CONFIG_FB not set) but still have FB_CORE, FB_DEVICE and DRM_FBDEV_EMULATION options enabled. > between the two, but as I understand we want modern distros to use > neither FB nor FB_DEVICE, so tying them together with a dependency > may be desirable anyway. > As said, modern distros would want to disable both FB and FB_DEVICE, but we want to allow for these two options to be {en,dis}abled independently. > >>>> config FB_BACKLIGHT >>>> tristate >>>> - depends on FB >>>> + depends on FB_CORE >>>> select BACKLIGHT_CLASS_DEVICE >>> >>> Separating this one from FB_CORE would help avoid circular dependencies, >>> this one keeps causing issues. >>> >> >> You mean separating from FB or should I keep the existing depends on FB? >> >> It seems this is only used by fbdev drivers so probably the latter? > > Right, I meant keeping the dependency on FB. Ideally we'd > turn this all into a set of 'depends on' instead of 'select', > but that is a completely separate topic. > Ok. > Arnd >
diff --git a/arch/x86/Makefile b/arch/x86/Makefile index b39975977c03..89a02e69be5f 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -259,7 +259,7 @@ drivers-$(CONFIG_PCI) += arch/x86/pci/ # suspend and hibernation support drivers-$(CONFIG_PM) += arch/x86/power/ -drivers-$(CONFIG_FB) += arch/x86/video/ +drivers-$(CONFIG_FB_CORE) += arch/x86/video/ #### # boot loader support. Several targets are kept for legacy purposes diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile index 11640c116115..5ebe48752ffc 100644 --- a/arch/x86/video/Makefile +++ b/arch/x86/video/Makefile @@ -1,2 +1,2 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB) += fbdev.o +obj-$(CONFIG_FB_CORE) += fbdev.o diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig index a2a88d42edf0..1b5a319971ed 100644 --- a/drivers/video/console/Kconfig +++ b/drivers/video/console/Kconfig @@ -72,7 +72,7 @@ config DUMMY_CONSOLE_ROWS config FRAMEBUFFER_CONSOLE bool "Framebuffer Console support" - depends on FB && !UML + depends on FB_CORE && !UML select VT_HW_CONSOLE_BINDING select CRC32 select FONT_SUPPORT diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index cecf15418632..74d7a305657a 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -6,10 +6,8 @@ config FB_NOTIFY bool -menuconfig FB - tristate "Support for frame buffer devices" - select FB_NOTIFY - select VIDEO_CMDLINE +menuconfig FB_CORE + tristate "Core support for frame buffer devices" help The frame buffer device provides an abstraction for the graphics hardware. It represents the frame buffer of some video hardware and @@ -33,6 +31,20 @@ menuconfig FB <http://www.munted.org.uk/programming/Framebuffer-HOWTO-1.3.html> for more information. + This options enables the core support for frame buffer devices. + +menuconfig FB + tristate "Support for frame buffer device drivers" + depends on FB_CORE + select FB_NOTIFY + select VIDEO_CMDLINE + help + This enables support for native frame buffer device (fbdev) drivers. + + The DRM subsystem provides support for emulated frame buffer devices + on top of KMS drivers, but this option allows legacy fbdev drivers to + be enabled as well. + Say Y here and to the driver for your graphics board below if you are compiling a kernel for a non-x86 architecture. @@ -44,7 +56,7 @@ menuconfig FB config FIRMWARE_EDID bool "Enable firmware EDID" - depends on FB + depends on FB_CORE help This enables access to the EDID transferred from the firmware. On the i386, this is from the Video BIOS. Enable this if DDC/I2C @@ -59,7 +71,7 @@ config FIRMWARE_EDID config FB_DEVICE bool "Provide legacy /dev/fb* device" - depends on FB + depends on FB_CORE default y help Say Y here if you want the legacy /dev/fb* device file and @@ -69,13 +81,13 @@ config FB_DEVICE config FB_DDC tristate - depends on FB + depends on FB_CORE select I2C_ALGOBIT select I2C config FB_CFB_FILLRECT tristate - depends on FB + depends on FB_CORE help Include the cfb_fillrect function for generic software rectangle filling. This is used by drivers that don't provide their own @@ -83,7 +95,7 @@ config FB_CFB_FILLRECT config FB_CFB_COPYAREA tristate - depends on FB + depends on FB_CORE help Include the cfb_copyarea function for generic software area copying. This is used by drivers that don't provide their own (accelerated) @@ -91,7 +103,7 @@ config FB_CFB_COPYAREA config FB_CFB_IMAGEBLIT tristate - depends on FB + depends on FB_CORE help Include the cfb_imageblit function for generic software image blitting. This is used by drivers that don't provide their own @@ -99,7 +111,7 @@ config FB_CFB_IMAGEBLIT config FB_CFB_REV_PIXELS_IN_BYTE bool - depends on FB + depends on FB_CORE help Allow generic frame-buffer functions to work on displays with 1, 2 and 4 bits per pixel depths which has opposite order of pixels in @@ -107,7 +119,7 @@ config FB_CFB_REV_PIXELS_IN_BYTE config FB_SYS_FILLRECT tristate - depends on FB + depends on FB_CORE help Include the sys_fillrect function for generic software rectangle filling. This is used by drivers that don't provide their own @@ -115,7 +127,7 @@ config FB_SYS_FILLRECT config FB_SYS_COPYAREA tristate - depends on FB + depends on FB_CORE help Include the sys_copyarea function for generic software area copying. This is used by drivers that don't provide their own (accelerated) @@ -123,7 +135,7 @@ config FB_SYS_COPYAREA config FB_SYS_IMAGEBLIT tristate - depends on FB + depends on FB_CORE help Include the sys_imageblit function for generic software image blitting. This is used by drivers that don't provide their own @@ -162,22 +174,22 @@ endchoice config FB_SYS_FOPS tristate - depends on FB + depends on FB_CORE config FB_DEFERRED_IO bool - depends on FB + depends on FB_CORE config FB_IO_HELPERS bool - depends on FB + depends on FB_CORE select FB_CFB_COPYAREA select FB_CFB_FILLRECT select FB_CFB_IMAGEBLIT config FB_SYS_HELPERS bool - depends on FB + depends on FB_CORE select FB_SYS_COPYAREA select FB_SYS_FILLRECT select FB_SYS_FOPS @@ -185,34 +197,34 @@ config FB_SYS_HELPERS config FB_SYS_HELPERS_DEFERRED bool - depends on FB + depends on FB_CORE select FB_DEFERRED_IO select FB_SYS_HELPERS config FB_HECUBA tristate - depends on FB + depends on FB_CORE depends on FB_DEFERRED_IO config FB_SVGALIB tristate - depends on FB + depends on FB_CORE help Common utility functions useful to fbdev drivers of VGA-based cards. config FB_MACMODES tristate - depends on FB + depends on FB_CORE config FB_BACKLIGHT tristate - depends on FB + depends on FB_CORE select BACKLIGHT_CLASS_DEVICE config FB_MODE_HELPERS bool "Enable Video Mode Handling Helpers" - depends on FB + depends on FB_CORE help This enables functions for handling video modes using the Generalized Timing Formula and the EDID parser. A few drivers rely @@ -222,7 +234,7 @@ config FB_MODE_HELPERS config FB_TILEBLITTING bool "Enable Tile Blitting Support" - depends on FB + depends on FB_CORE help This enables tile blitting. Tile blitting is a drawing technique where the screen is divided into rectangular sections (tiles), whereas diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile index 9150bafd9e89..88ef39dad03b 100644 --- a/drivers/video/fbdev/core/Makefile +++ b/drivers/video/fbdev/core/Makefile @@ -1,22 +1,22 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_FB_NOTIFY) += fb_notify.o -obj-$(CONFIG_FB) += fb.o -fb-y := fb_backlight.o \ +obj-$(CONFIG_FB_CORE) += fb_core.o +fb_core-y := fb_backlight.o \ fb_info.o \ fbmem.o fbmon.o fbcmap.o \ modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o -fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o -fb-$(CONFIG_FB_DEVICE) += fb_chrdev.o \ +fb_core-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o +fb_core-$(CONFIG_FB_DEVICE) += fb_chrdev.o \ fb_procfs.o \ fbsysfs.o ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y) -fb-y += fbcon.o bitblit.o softcursor.o +fb_core-y += fbcon.o bitblit.o softcursor.o ifeq ($(CONFIG_FB_TILEBLITTING),y) -fb-y += tileblit.o +fb_core-y += tileblit.o endif ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y) -fb-y += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \ +fb_core-y += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \ fbcon_ccw.o endif endif
Currently the CONFIG_FB option has to be enabled even if no legacy fbdev drivers are needed (e.g: only to have support for framebuffer consoles). The DRM subsystem has a fbdev emulation layer, but depends on CONFIG_FB and so it can only be enabled if that dependency is enabled as well. That means fbdev drivers have to be explicitly disabled if users want to enable CONFIG_FB, only to use fbcon and/or the DRM fbdev emulation layer. This patch introduces a CONFIG_FB_CORE option that could be enabled just to have the core support needed for CONFIG_DRM_FBDEV_EMULATION, allowing CONFIG_FB to be disabled (and automatically disabling all fbdev drivers). Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> --- arch/x86/Makefile | 2 +- arch/x86/video/Makefile | 2 +- drivers/video/console/Kconfig | 2 +- drivers/video/fbdev/Kconfig | 62 ++++++++++++++++++------------- drivers/video/fbdev/core/Makefile | 14 +++---- 5 files changed, 47 insertions(+), 35 deletions(-)