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

Message ID 20230701214503.550549-2-javierm@redhat.com
State New
Headers
Series Allow disabling all native fbdev drivers and only keeping DRM emulation |

Commit Message

Javier Martinez Canillas July 1, 2023, 9:44 p.m. UTC
  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(-)
  

Comments

Randy Dunlap July 1, 2023, 10:20 p.m. UTC | #1
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.
  
Arnd Bergmann July 1, 2023, 10:24 p.m. UTC | #2
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
  
Geert Uytterhoeven July 2, 2023, 9:07 a.m. UTC | #3
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
  
Javier Martinez Canillas July 2, 2023, 10:19 a.m. UTC | #4
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.
  
Thomas Zimmermann July 3, 2023, 6:53 a.m. UTC | #5
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)
  
Javier Martinez Canillas July 3, 2023, 7:46 a.m. UTC | #6
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
>
  
Thomas Zimmermann July 3, 2023, 7:52 a.m. UTC | #7
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)
  
Javier Martinez Canillas July 3, 2023, 8:49 a.m. UTC | #8
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
>
  

Patch

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 \