Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-499-gf27bbf33e2-fm-20230619.001-gf27bbf33
Mime-Version: 1.0
Message-Id: <393ca142-5226-4779-a963-c34fb0464c59@app.fastmail.com>
In-Reply-To: <20230701214503.550549-2-javierm@redhat.com>
References: <20230701214503.550549-1-javierm@redhat.com>
 <20230701214503.550549-2-javierm@redhat.com>
Date:   Sun, 02 Jul 2023 00:24:49 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Javier Martinez Canillas" <javierm@redhat.com>,
        linux-kernel@vger.kernel.org
Cc:     "Thomas Zimmermann" <tzimmermann@suse.de>,
        "Geert Uytterhoeven" <geert@linux-m68k.org>,
        "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
        "Borislav Petkov" <bp@alien8.de>,
        "Daniel Vetter" <daniel@ffwll.ch>,
        "Dave Hansen" <dave.hansen@linux.intel.com>,
        "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
        "H. Peter Anvin" <hpa@zytor.com>, "Helge Deller" <deller@gmx.de>,
        "Ingo Molnar" <mingo@redhat.com>,
        "Randy Dunlap" <rdunlap@infradead.org>,
        "Sam Ravnborg" <sam@ravnborg.org>,
        "Thomas Gleixner" <tglx@linutronix.de>,
        dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
        x86@kernel.org
Subject: Re: [PATCH v2 1/2] fbdev: Split frame buffer support in FB and FB_CORE symbols
Content-Type: text/plain
Precedence: bulk

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