Re: [PATCH] drm: fix up fbdev Kconfig defaults

[Arnd Bergmann]

On Tue, Sep 12, 2023, at 04:18, Geert Uytterhoeven wrote:
> On Tue, Sep 12, 2023 at 10:11 AM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>> Am 12.09.23 um 09:14 schrieb Geert Uytterhoeven:
>> [...]
>> >> --- a/drivers/gpu/drm/Kconfig
>> >> +++ b/drivers/gpu/drm/Kconfig
>> >> @@ -135,7 +135,7 @@ config DRM_FBDEV_EMULATION
>> >>          bool "Enable legacy fbdev support for your modesetting driver"
>> >>          depends on DRM
>> >>          select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE
>> >> -       default y
>> >> +       default FB
>> >
>> > While this is true for existing configs, it is no longer true in general,
>> > as DRM_FBDEV_EMULATION is no longer related to FB.
>>
>> Would it make sense to make FRAMEBUFFER_CONSOLE an independent option
>> and have FBDEV_EMULATION depend on it? Something like this:
>>
>> FRAMEBUFFER_CONSOLE
>>         depends on DRM || FB
>>         select FB_CORE
>>
>> FBDEV_EMULATION
>>         depends on DRM
>>         depends on FRAMEBUFFER_CONSOLE
>>         default y
>
> Oops, now you can no longer have FBDEV_EMULATION without
> FRAMEBUFFER_CONSOLE, which is useful to be able to enable
> FB_DEVICE...
>
> And what's the point (if DRM is enabled) of having FB_CORE with
> FBDEV_EMULATION disabled?

I think we technically have the choice between selecting FB_CORE from
all the providers (FB and DRM_FBDEV_EMULATION) or from all the
consumers (FRAMEBUFFER_CONSOLE, FB_DEVICE and LOGO). We chose
to have it the former way at the moment, and I think what Thomas
suggests here is to change it to the latter.

The downside of the current approach is that you can have
pointless configuration with

CONFIG_FB=y
CONFIG_DRM=y
CONFIG_DRM_FBDEV_EMULATION=y
CONFIG_FRAMEBUFFER_CONSOLE=n
CONFIG_FB_DEVICE=n
CONFIG_LOGO=n

where the fb_core module gets initialized but it is impossible
for anything to draw on it.

Another option we have would be to try to separate the Kconfig
options for DRM and FB a little further, in anticipation of
reducing the amount of shared code in the long run (a lot of
FB code is built but never used when enabling console on
DRM).

So adding a new

config DRM_CONSOLE
     bool "enable console on DRM devices"
     depends on DRM
     depends on VT_CONSOLE
     depends on !UML
     select FB_CORE
     select FRAMEBUFFER_CONSOLE_CORE # new helper sym
     select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY
     default y

would give us a path to minimize the shared code
further, allowing the console code to be reused as
before, but without having to pull in the code that
is now only needed for FB_DEVICE. In this scenario,
we'd still have CONFIG_DRM_FBDEV_EMULATION like

config DRM_FBDEV_EMULATION
      bool "full emulation of fbdev layer for DRM"
      select FB_CORE
      select FB_DEVICE
      default FB # if enabled already, use it

but only use that if we actually want FB_DEVICE.

      Arnd