2023-09-12 07:29:43

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] drm: fix up fbdev Kconfig defaults

Hi Arnd,

On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <[email protected]> wrote:
> From: Arnd Bergmann <[email protected]>
>
> As a result of the recent Kconfig reworks, the default settings for the
> framebuffer interfaces changed in unexpected ways:
>
> Configurations that leave CONFIG_FB disabled but use DRM now get
> DRM_FBDEV_EMULATION by default. This also turns on the deprecated /dev/fb
> device nodes for machines that don't actually want it.
>
> In turn, configurations that previously had DRM_FBDEV_EMULATION enabled
> now only get the /dev/fb front-end but not the more useful framebuffer
> console, which is not selected any more.
>
> We had previously decided that any combination of the three frontends
> (FB_DEVICE, FRAMEBUFFER_CONSOLE and LOGO) should be selectable, but the
> new default settings mean that a lot of defconfig files would have to
> get adapted.
>
> Change the defaults back to what they were in Linux 6.5:
>
> - Leave DRM_FBDEV_EMULATION turned off unless CONFIG_FB
> is enabled. Previously this was a hard dependency but now the two are
> independent. However, configurations that enable CONFIG_FB probably
> also want to keep the emulation for DRM, while those without FB
> presumably did that intentionally in the past.
>
> - Leave FB_DEVICE turned off for FB=n. Following the same
> logic, the deprecated option should not automatically get enabled
> here, most users that had FB turned off in the past do not want it,
> even if they want the console
>
> - Turn the FRAMEBUFFER_CONSOLE option on if
> DRM_FBDEV_EMULATION is set to avoid having to change defconfig
> files that relied on it being selected unconditionally in the past.
> This also makes sense since both LOGO and FB_DEVICE are now disabled
> by default for builds without CONFIG_FB, but DRM_FBDEV_EMULATION
> would make no sense if all three are disabled.
>
> Fixes: a5ae331edb02b ("drm: Drop select FRAMEBUFFER_CONSOLE for DRM_FBDEV_EMULATION")
> Fixes: 701d2054fa317 ("fbdev: Make support for userspace interfaces configurable")
> Reported-by: Geert Uytterhoeven <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>

Thanks for your patch!

> --- 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.

> help
> Choose this option if you have a need for the legacy fbdev
> support. Note that this support also provides the linux console
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index b575cf54174af..83c2d7329ca58 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -74,6 +74,7 @@ config DUMMY_CONSOLE_ROWS
> config FRAMEBUFFER_CONSOLE
> bool "Framebuffer Console support"
> depends on FB_CORE && !UML
> + default DRM_FBDEV_EMULATION

Sounds good to me, although it looks a bit strange at first sight
(FRAMEBUFFER_CONSOLE defaults to n on a system with real fbdev, but
y on emulated fbdev?).
So this is the fix for commit a5ae331edb02b ("drm: Drop select
FRAMEBUFFER_CONSOLE for DRM_FBDEV_EMULATION").

> select VT_HW_CONSOLE_BINDING
> select CRC32
> select FONT_SUPPORT
> diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig
> index 114cb8aa6c8fd..804c2bec9b43c 100644
> --- a/drivers/video/fbdev/core/Kconfig
> +++ b/drivers/video/fbdev/core/Kconfig
> @@ -28,7 +28,7 @@ config FIRMWARE_EDID
> config FB_DEVICE
> bool "Provide legacy /dev/fb* device"
> depends on FB_CORE
> - default y
> + default FB

Changing this means possibly causing regressions on systems running
an fbdev userspace.

> help
> Say Y here if you want the legacy /dev/fb* device file and
> interfaces within sysfs anc procfs. It is only required if you

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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


2023-09-12 19:49:52

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] drm: fix up fbdev Kconfig defaults

Geert Uytterhoeven <[email protected]> writes:

Hello Geert,

> Hi Arnd,
>
> On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <[email protected]> wrote:

[...]

>> --- 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.
>

Maybe default y if (FB_DEVICE || FRAMEBUFFER_CONSOLE) ?

>> help
>> Choose this option if you have a need for the legacy fbdev
>> support. Note that this support also provides the linux console
>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>> index b575cf54174af..83c2d7329ca58 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -74,6 +74,7 @@ config DUMMY_CONSOLE_ROWS
>> config FRAMEBUFFER_CONSOLE
>> bool "Framebuffer Console support"
>> depends on FB_CORE && !UML
>> + default DRM_FBDEV_EMULATION
>
> Sounds good to me, although it looks a bit strange at first sight
> (FRAMEBUFFER_CONSOLE defaults to n on a system with real fbdev, but
> y on emulated fbdev?).

And there Maybe default y if (FB || DRM_FBDEV_EMULATION) ?

> So this is the fix for commit a5ae331edb02b ("drm: Drop select
> FRAMEBUFFER_CONSOLE for DRM_FBDEV_EMULATION").
>
>> select VT_HW_CONSOLE_BINDING
>> select CRC32
>> select FONT_SUPPORT
>> diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig
>> index 114cb8aa6c8fd..804c2bec9b43c 100644
>> --- a/drivers/video/fbdev/core/Kconfig
>> +++ b/drivers/video/fbdev/core/Kconfig
>> @@ -28,7 +28,7 @@ config FIRMWARE_EDID
>> config FB_DEVICE
>> bool "Provide legacy /dev/fb* device"
>> depends on FB_CORE
>> - default y
>> + default FB
>
> Changing this means possibly causing regressions on systems running
> an fbdev userspace.
>

Right, specially if using DRM fbdev emulation since then the default will
be different between v6.5 and v6.6 (that's what this patch tries to avoid).

So probably we could keept that default as 'y'.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-09-12 20:54:18

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH] drm: fix up fbdev Kconfig defaults

Hi

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

So if any graphics subsystems are enabled, FRAMEBUFFER_CONSOLE is
select-able. But for DRM, FBDEV_EMULATION disables the console. That
option remains more for historical reasons than actual usefulness.

Best regards
Thomas

>
>> help
>> Choose this option if you have a need for the legacy fbdev
>> support. Note that this support also provides the linux console
>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>> index b575cf54174af..83c2d7329ca58 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -74,6 +74,7 @@ config DUMMY_CONSOLE_ROWS
>> config FRAMEBUFFER_CONSOLE
>> bool "Framebuffer Console support"
>> depends on FB_CORE && !UML
>> + default DRM_FBDEV_EMULATION
>
> Sounds good to me, although it looks a bit strange at first sight
> (FRAMEBUFFER_CONSOLE defaults to n on a system with real fbdev, but
> y on emulated fbdev?).
> So this is the fix for commit a5ae331edb02b ("drm: Drop select
> FRAMEBUFFER_CONSOLE for DRM_FBDEV_EMULATION").
>
>> select VT_HW_CONSOLE_BINDING
>> select CRC32
>> select FONT_SUPPORT
>> diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig
>> index 114cb8aa6c8fd..804c2bec9b43c 100644
>> --- a/drivers/video/fbdev/core/Kconfig
>> +++ b/drivers/video/fbdev/core/Kconfig
>> @@ -28,7 +28,7 @@ config FIRMWARE_EDID
>> config FB_DEVICE
>> bool "Provide legacy /dev/fb* device"
>> depends on FB_CORE
>> - default y
>> + default FB
>
> Changing this means possibly causing regressions on systems running
> an fbdev userspace.
>
>> help
>> Say Y here if you want the legacy /dev/fb* device file and
>> interfaces within sysfs anc procfs. It is only required if you
>
> Gr{oetje,eeting}s,
>
> Geert
>

--
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)


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2023-09-13 03:28:33

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] drm: fix up fbdev Kconfig defaults

On Tue, Sep 12, 2023, at 09:14, Geert Uytterhoeven wrote:
> On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <[email protected]> wrote:
>> --- 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.

I think it still makes some sense though, as configs that have
both DRM and FB enabled almost certainly want this enabled.

>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>> index b575cf54174af..83c2d7329ca58 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -74,6 +74,7 @@ config DUMMY_CONSOLE_ROWS
>> config FRAMEBUFFER_CONSOLE
>> bool "Framebuffer Console support"
>> depends on FB_CORE && !UML
>> + default DRM_FBDEV_EMULATION
>
> Sounds good to me, although it looks a bit strange at first sight
> (FRAMEBUFFER_CONSOLE defaults to n on a system with real fbdev, but
> y on emulated fbdev?).
> So this is the fix for commit a5ae331edb02b ("drm: Drop select
> FRAMEBUFFER_CONSOLE for DRM_FBDEV_EMULATION").

Correct, this should restore the console on configs that
accidentally lost it. The real problem here is much older,
the assymetry between framebuffer-only configs (with console
default off) and DRM configs (with console selected
unconditionally) started back in 2009 with commit 6fcefd56f5060
("drm/kms: fix kms helper license + Kconfig").

I think that was a mistake, but there is little we can do
to fix that now without breaking users.

The only alternative I can think of would be to default-enable
or force-enable FRAMEBUFFER_CONSOLE for any config that includes
both VT_CONSOLE and FB_CORE. This would increase defconfig
builds for systems that currently only want CONFIG_FB for
either FB_DEVICE or LOGO but don't care about
FRAMEBUFFER_CONSOLE. I have no idea who uses such a config,
but I think Javier previously said this was an important
use case.

>> diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig
>> index 114cb8aa6c8fd..804c2bec9b43c 100644
>> --- a/drivers/video/fbdev/core/Kconfig
>> +++ b/drivers/video/fbdev/core/Kconfig
>> @@ -28,7 +28,7 @@ config FIRMWARE_EDID
>> config FB_DEVICE
>> bool "Provide legacy /dev/fb* device"
>> depends on FB_CORE
>> - default y
>> + default FB
>
> Changing this means possibly causing regressions on systems running
> an fbdev userspace.

How? FB_DEVICE is a new config that was just split out from
CONFIG_FB in 6.6-rc1, so nobody should have any defconfig
that disables CONFIG_FB but relies on the FB_DEVICE default yet.

Arnd