[PATCH v2 0/2] Allow disabling all native fbdev drivers and only keeping DRM emulation

[PATCH v2 0/2] Allow disabling all native fbdev drivers and only keeping DRM emulation

[Javier Martinez Canillas]

This patch series splits the fbdev core support in two different Kconfig
symbols: FB and FB_CORE. The motivation for this is to allow CONFIG_FB to
be disabled, while still having the the core fbdev support needed for the
CONFIG_DRM_FBDEV_EMULATION to be enabled. The motivation is automatically
disabling all fbdev drivers instead of having to be disabled individually.

The reason for doing this is that now with simpledrm, there's no need for
the legacy fbdev (e.g: efifb or vesafb) drivers anymore and many distros
now disable them. But it would simplify the config a lot fo have a single
Kconfig symbol to disable all fbdev drivers.

I've built tested with possible combinations of CONFIG_FB, CONFIG_FB_CORE,
CONFIG_DRM_FBDEV_EMULATION and CONFIG_FB_DEVICE symbols set to 'y' or 'n'.

Patch 1/2 makes the CONFIG_FB split that is mentioned above and patch 2/2
makes the DRM fbdev emulation to select the new FB_CORE symbol instead of
depending on FB.

This is a v2 of the patch-set that addresses issues pointed out by Arnd
Bergmann and Thomas Zimmermann in the previous version:

https://lists.freedesktop.org/archives/dri-devel/2023-June/411435.html


Changes in v2:
- Keep "depends on FB" for FB_DDC, FB_HECUBA, FB_SVGALIB, FB_MACMODES,
  FB_BACKLIGHT, FB_MODE_HELPERS and FB_TILEBLITTING (Arnd Bergmann).
- Don't change the fb.o object name (Arnd Bergmann).
- Make FB_CORE a non-visible Kconfig symbol instead (Thomas Zimmermann).
- Make CONFIG_DRM_FBDEV_EMULATION to select FB_CORE (Thomas Zimmermann).

Javier Martinez Canillas (2):
  fbdev: Split frame buffer support in FB and FB_CORE symbols
  drm: Make fbdev emulation select FB_CORE instead of depends on FB

 arch/x86/Makefile                 |  2 +-
 arch/x86/video/Makefile           |  2 +-
 drivers/gpu/drm/Kconfig           |  2 +-
 drivers/video/console/Kconfig     |  2 +-
 drivers/video/fbdev/Kconfig       | 40 +++++++++++++++++++------------
 drivers/video/fbdev/core/Makefile |  2 +-
 6 files changed, 30 insertions(+), 20 deletions(-)


base-commit: 270689d257c88fd1ad7050041ed196a8188e6914
-- 
2.41.0

[PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Javier Martinez Canillas]

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 non-visible CONFIG_FB_CORE symbol that could be
enabled just to have core support needed for CONFIG_DRM_FBDEV_EMULATION,
allowing CONFIG_FB to be disabled (and automatically disabling all the
fbdev drivers).

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v2:
- Keep "depends on FB" for FB_DDC, FB_HECUBA, FB_SVGALIB, FB_MACMODES,
  FB_BACKLIGHT, FB_MODE_HELPERS and FB_TILEBLITTING (Arnd Bergmann).
- Don't change the fb.o object name (Arnd Bergmann).
- Make FB_CORE a non-visible Kconfig symbol instead (Thomas Zimmermann).

 arch/x86/Makefile                 |  2 +-
 arch/x86/video/Makefile           |  2 +-
 drivers/video/console/Kconfig     |  2 +-
 drivers/video/fbdev/Kconfig       | 40 +++++++++++++++++++------------
 drivers/video/fbdev/core/Makefile |  2 +-
 5 files changed, 29 insertions(+), 19 deletions(-)

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..da6f7d588f17 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -6,8 +6,12 @@
 config FB_NOTIFY
 	bool
 
+menuconfig FB_CORE
+	tristate "Core support for frame buffer devices"
+
 menuconfig FB
-	tristate "Support for frame buffer devices"
+	tristate "Support for frame buffer device drivers"
+	select FB_CORE
 	select FB_NOTIFY
 	select VIDEO_CMDLINE
 	help
@@ -33,6 +37,12 @@ menuconfig FB
 	  <http://www.munted.org.uk/programming/Framebuffer-HOWTO-1.3.html> for more
 	  information.
 
+	  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 +54,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 +69,7 @@ config FIRMWARE_EDID
 
 config FB_DEVICE
 	bool "Provide legacy /dev/fb* device"
-	depends on FB
+	select FB_CORE
 	default y
 	help
 	  Say Y here if you want the legacy /dev/fb* device file and
@@ -75,7 +85,7 @@ config FB_DDC
 
 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 +93,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 +101,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 +109,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 +117,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 +125,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 +133,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 +172,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,7 +195,7 @@ 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
 
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 9150bafd9e89..4c2e4a026d12 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
-obj-$(CONFIG_FB)                  += fb.o
+obj-$(CONFIG_FB_CORE)             += fb.o
 fb-y                              := fb_backlight.o \
                                      fb_info.o \
                                      fbmem.o fbmon.o fbcmap.o \
-- 
2.41.0

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Randy Dunlap]

Hi,


Does this series apply on top of the previous series or on what?


On 7/1/23 14:44, 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 non-visible CONFIG_FB_CORE symbol that could be
> enabled just to have core support needed for CONFIG_DRM_FBDEV_EMULATION,
> allowing CONFIG_FB to be disabled (and automatically disabling all the
> fbdev drivers).
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> Changes in v2:
> - Keep "depends on FB" for FB_DDC, FB_HECUBA, FB_SVGALIB, FB_MACMODES,
>   FB_BACKLIGHT, FB_MODE_HELPERS and FB_TILEBLITTING (Arnd Bergmann).
> - Don't change the fb.o object name (Arnd Bergmann).
> - Make FB_CORE a non-visible Kconfig symbol instead (Thomas Zimmermann).
> 
>  arch/x86/Makefile                 |  2 +-
>  arch/x86/video/Makefile           |  2 +-
>  drivers/video/console/Kconfig     |  2 +-
>  drivers/video/fbdev/Kconfig       | 40 +++++++++++++++++++------------
>  drivers/video/fbdev/core/Makefile |  2 +-
>  5 files changed, 29 insertions(+), 19 deletions(-)
> 

> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index cecf15418632..da6f7d588f17 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -6,8 +6,12 @@
>  config FB_NOTIFY
>  	bool
>  
> +menuconfig FB_CORE
> +	tristate "Core support for frame buffer devices"
> +

I could be reading this incorrectly, but FB_CORE does not appear to
be a non-visible Kconfig symbol here.

>  menuconfig FB
> -	tristate "Support for frame buffer devices"
> +	tristate "Support for frame buffer device drivers"
> +	select FB_CORE
>  	select FB_NOTIFY
>  	select VIDEO_CMDLINE
>  	help

thanks.
-- 
~Randy

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Arnd Bergmann]

On Sat, Jul 1, 2023, at 23:44, 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 non-visible CONFIG_FB_CORE symbol that could be
> enabled just to have core support needed for CONFIG_DRM_FBDEV_EMULATION,
> allowing CONFIG_FB to be disabled (and automatically disabling all the
> fbdev drivers).
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---

I found two more things now:

> 
> +menuconfig FB_CORE
> +	tristate "Core support for frame buffer devices"
> +

This is not actually a hidden option, since you left the prompt
after the 'tristate' keyword. There is also no pointn in having
it as a menu, just use the simpler

config FB_CORE
        tristate

or (as in my other email)

config FB_CORE
       def_tristate FB || (DRM && DRM_FBDEV_EMULATION)


> @@ -44,7 +54,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 +69,7 @@ config FIRMWARE_EDID
> 
>  config FB_DEVICE
>  	bool "Provide legacy /dev/fb* device"
> -	depends on FB
> +	select FB_CORE
>  	default y
>  	help
>  	  Say Y here if you want the legacy /dev/fb* device file and

These are now the only user visible sub-options when CONFIG_FB is
disabled. I missed FIRMWARE_EDID earlier, but this also looks like
it can clearly be left as depending on FB since nothing else calls
fb_firmware_edid. In fact, it looks like all of fbmon.c could be
left out since none of its exported symbols are needed for DRM.

That would leave CONFIG_FB_DEVICE as the only user visible option
for DRM-only configs, which is slightly odd for the menuconfig,
so I still wonder if that could be done differently.

Is there actually a point in configurations for kernels with FB=y,
DRM=n and FB_DEVICE=n? If we don't expect that to be a useful
configuration, an easier way would be to have CONFIG_FB turn it
on implicitly and instead have a user-visible Kconfig option
below CONFIG_DRM_FBDEV_EMULATION that allows controlling the
creation of /dev/fb*.

     Arnd

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Geert Uytterhoeven]

Hi Arnd,

On Sun, Jul 2, 2023 at 12:25 AM Arnd Bergmann <arnd@arndb.de> wrote:
> On Sat, Jul 1, 2023, at 23:44, 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 non-visible CONFIG_FB_CORE symbol that could be
> > enabled just to have core support needed for CONFIG_DRM_FBDEV_EMULATION,
> > allowing CONFIG_FB to be disabled (and automatically disabling all the
> > fbdev drivers).
> >
> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

> > @@ -59,7 +69,7 @@ config FIRMWARE_EDID
> >
> >  config FB_DEVICE
> >       bool "Provide legacy /dev/fb* device"
> > -     depends on FB
> > +     select FB_CORE
> >       default y
> >       help
> >         Say Y here if you want the legacy /dev/fb* device file and
>
> These are now the only user visible sub-options when CONFIG_FB is
> disabled. I missed FIRMWARE_EDID earlier, but this also looks like
> it can clearly be left as depending on FB since nothing else calls
> fb_firmware_edid. In fact, it looks like all of fbmon.c could be
> left out since none of its exported symbols are needed for DRM.
>
> That would leave CONFIG_FB_DEVICE as the only user visible option
> for DRM-only configs, which is slightly odd for the menuconfig,
> so I still wonder if that could be done differently.
>
> Is there actually a point in configurations for kernels with FB=y,
> DRM=n and FB_DEVICE=n? If we don't expect that to be a useful
> configuration, an easier way would be to have CONFIG_FB turn it
> on implicitly and instead have a user-visible Kconfig option
> below CONFIG_DRM_FBDEV_EMULATION that allows controlling the
> creation of /dev/fb*.

Such a combination would allow the user to still have a text console
on a legacy fbdev, while not having to worry about possible security
ramifications of providing fbdev userspace access.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Javier Martinez Canillas]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Arnd,
>

[...]

>>
>> That would leave CONFIG_FB_DEVICE as the only user visible option
>> for DRM-only configs, which is slightly odd for the menuconfig,
>> so I still wonder if that could be done differently.
>>
>> Is there actually a point in configurations for kernels with FB=y,
>> DRM=n and FB_DEVICE=n? If we don't expect that to be a useful
>> configuration, an easier way would be to have CONFIG_FB turn it
>> on implicitly and instead have a user-visible Kconfig option
>> below CONFIG_DRM_FBDEV_EMULATION that allows controlling the
>> creation of /dev/fb*.
>
> Such a combination would allow the user to still have a text console
> on a legacy fbdev, while not having to worry about possible security
> ramifications of providing fbdev userspace access.
>

Exactly, it may be a possible combination. Not sure how useful what would
be in practice but we shouldn't restrict that IMO.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 8189 bytes --]

Hi

Am 01.07.23 um 23:44 schrieb Javier Martinez Canillas:
> 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 non-visible CONFIG_FB_CORE symbol that could be
> enabled just to have core support needed for CONFIG_DRM_FBDEV_EMULATION,
> allowing CONFIG_FB to be disabled (and automatically disabling all the
> fbdev drivers).
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> Changes in v2:
> - Keep "depends on FB" for FB_DDC, FB_HECUBA, FB_SVGALIB, FB_MACMODES,
>    FB_BACKLIGHT, FB_MODE_HELPERS and FB_TILEBLITTING (Arnd Bergmann).
> - Don't change the fb.o object name (Arnd Bergmann).
> - Make FB_CORE a non-visible Kconfig symbol instead (Thomas Zimmermann).
> 
>   arch/x86/Makefile                 |  2 +-
>   arch/x86/video/Makefile           |  2 +-
>   drivers/video/console/Kconfig     |  2 +-
>   drivers/video/fbdev/Kconfig       | 40 +++++++++++++++++++------------
>   drivers/video/fbdev/core/Makefile |  2 +-
>   5 files changed, 29 insertions(+), 19 deletions(-)
> 
> 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..da6f7d588f17 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -6,8 +6,12 @@
>   config FB_NOTIFY
>   	bool
>   
> +menuconfig FB_CORE
> +	tristate "Core support for frame buffer devices"

With the text, this is visible; as others noted.

> +
>   menuconfig FB
> -	tristate "Support for frame buffer devices"
> +	tristate "Support for frame buffer device drivers"

Just keep the text as-is.

> +	select FB_CORE
>   	select FB_NOTIFY
>   	select VIDEO_CMDLINE
>   	help
> @@ -33,6 +37,12 @@ menuconfig FB
>   	  <http://www.munted.org.uk/programming/Framebuffer-HOWTO-1.3.html> for more
>   	  information.
>   
> +	  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 +54,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 +69,7 @@ config FIRMWARE_EDID
>   
>   config FB_DEVICE
>   	bool "Provide legacy /dev/fb* device"
> -	depends on FB
> +	select FB_CORE

This should depend on FB_CORE.

Best regards
Thomas

>   	default y
>   	help
>   	  Say Y here if you want the legacy /dev/fb* device file and
> @@ -75,7 +85,7 @@ config FB_DDC
>   
>   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 +93,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 +101,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 +109,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 +117,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 +125,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 +133,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 +172,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,7 +195,7 @@ 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
>   
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index 9150bafd9e89..4c2e4a026d12 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0
>   obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
> -obj-$(CONFIG_FB)                  += fb.o
> +obj-$(CONFIG_FB_CORE)             += fb.o
>   fb-y                              := fb_backlight.o \
>                                        fb_info.o \
>                                        fbmem.o fbmon.o fbcmap.o \

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

Thanks for your review.

> Hi
>
> Am 01.07.23 um 23:44 schrieb Javier Martinez Canillas:

[...]

>>   
>> +menuconfig FB_CORE
>> +	tristate "Core support for frame buffer devices"
>
> With the text, this is visible; as others noted.
>

Yes, I misremembered what made a Kconfig symbol non-visible, and thought
that was just the lack of a help section but forgot to remove the prompt.

This is already fixed in v3.

>> +
>>   menuconfig FB
>> -	tristate "Support for frame buffer devices"
>> +	tristate "Support for frame buffer device drivers"
>
> Just keep the text as-is.
>

I disagree. Because we are slightly changing the Kconfig symbol semantics
here, for instance CONFIG_FB_CORE + CONFIG_DRM_FBDEV_EMULATION will also
provide a frame buffer device (and with CONFIG_FB_DEVICE, will be exposed
to user-space as a /dev/fb? device).

So now CONFIG_FB is really about allowing the native fbdev drivers to be
enabled. That's why I'm changing the prompt text to make that more clear.

[...]

>>   config FB_DEVICE
>>   	bool "Provide legacy /dev/fb* device"
>> -	depends on FB
>> +	select FB_CORE
>
> This should depend on FB_CORE.
>

Yes, already fixed in v3 too. I did a select to prevent symbol circular
dependencies but doing that lead to CONFIG_FB_CORE=y even if CONFIG_DRM
was set as a module.

But with the "select FB_CORE if DRM_FBDEV_EMULATION" in the DRM symbol as
Arnd suggested, I was able to have FB_DEVICE to depend on FB_CORE again.

> Best regards
> Thomas
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2147 bytes --]

Hi

Am 03.07.23 um 09:46 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
> Hello Thomas,
> 
> Thanks for your review.
> 
>> Hi
>>
>> Am 01.07.23 um 23:44 schrieb Javier Martinez Canillas:
> 
> [...]
> 
>>>    
>>> +menuconfig FB_CORE
>>> +	tristate "Core support for frame buffer devices"
>>
>> With the text, this is visible; as others noted.
>>
> 
> Yes, I misremembered what made a Kconfig symbol non-visible, and thought
> that was just the lack of a help section but forgot to remove the prompt.
> 
> This is already fixed in v3.
> 
>>> +
>>>    menuconfig FB
>>> -	tristate "Support for frame buffer devices"
>>> +	tristate "Support for frame buffer device drivers"
>>
>> Just keep the text as-is.
>>
> 
> I disagree. Because we are slightly changing the Kconfig symbol semantics
> here, for instance CONFIG_FB_CORE + CONFIG_DRM_FBDEV_EMULATION will also
> provide a frame buffer device (and with CONFIG_FB_DEVICE, will be exposed
> to user-space as a /dev/fb? device).
> 
> So now CONFIG_FB is really about allowing the native fbdev drivers to be
> enabled. That's why I'm changing the prompt text to make that more clear.
> 
> [...]
> 
>>>    config FB_DEVICE
>>>    	bool "Provide legacy /dev/fb* device"
>>> -	depends on FB
>>> +	select FB_CORE
>>
>> This should depend on FB_CORE.
>>
> 
> Yes, already fixed in v3 too. I did a select to prevent symbol circular
> dependencies but doing that lead to CONFIG_FB_CORE=y even if CONFIG_DRM
> was set as a module.
> 
> But with the "select FB_CORE if DRM_FBDEV_EMULATION" in the DRM symbol as
> Arnd suggested, I was able to have FB_DEVICE to depend on FB_CORE again.

BTW, where does this item now show up in the menu? It used to be in the 
framebuffer menu. It's now in the graphics-drivers menu?

Best regards
Thomas

> 
>> Best regards
>> Thomas
>>
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

[...]

>>>>    config FB_DEVICE
>>>>    	bool "Provide legacy /dev/fb* device"
>>>> -	depends on FB
>>>> +	select FB_CORE
>>>
>>> This should depend on FB_CORE.
>>>
>> 
>> Yes, already fixed in v3 too. I did a select to prevent symbol circular
>> dependencies but doing that lead to CONFIG_FB_CORE=y even if CONFIG_DRM
>> was set as a module.
>> 
>> But with the "select FB_CORE if DRM_FBDEV_EMULATION" in the DRM symbol as
>> Arnd suggested, I was able to have FB_DEVICE to depend on FB_CORE again.
>
> BTW, where does this item now show up in the menu? It used to be in the 
> framebuffer menu. It's now in the graphics-drivers menu?
>

No, it's still in the framebuffer menu. But after the FB_CORE split the
menuconfig ends broken (no sub-level for fbdev drivers anymore).

I was talking with Arnd and Geert about this. I think that will pause this
series and instead first focus on cleaning up the fbdev Kconfig, then it
should be easier to add the FB_CORE on top of that.

> Best regards
> Thomas
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat