| Message ID | 20230911205338.2385278-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm: fix up fbdev Kconfig defaults | expand |
Arnd Bergmann <arnd@kernel.org> writes: Hello Arnd, > From: Arnd Bergmann <arnd@arndb.de> > > 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 <geert@linux-m68k.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- Thanks for fixing this and sorry that I missed the defaults changed. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Arnd, On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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 <geert@linux-m68k.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> 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
On Tue, Sep 12, 2023, at 09:14, Geert Uytterhoeven wrote: > On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> 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
Geert Uytterhoeven <geert@linux-m68k.org> writes: Hello Geert, > Hi Arnd, > > On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> 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'.
Hi Arnd, On Tue, Sep 12, 2023 at 9:40 AM Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Sep 12, 2023, at 09:14, Geert Uytterhoeven wrote: > > On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> 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. OK. People who use DRM drivers only, and now realize they no longer need CONFIG_FB, may need to enable CONFIG_DRM_FBDEV_EMULATION manually when disabling CONFIG_FB, depending on their use case. > >> 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. Right... > > 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. Right, from the PoV of running "make oldconfig" that is true. (I was incorrectly considering the defconfigs which I maintain, and where I already disabled CONFIG_FB after the FB/FB_CORE split in linux-next). Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Javier, On Tue, Sep 12, 2023 at 9:48 AM Javier Martinez Canillas <javierm@redhat.com> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> 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) ? Which is almost if FB_CORE? ;-) Won't a dependency on FRAMEBUFFER_CONSOLE here introduce a circular dependency, when combined with the hunk below? > >> 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"). Gr{oetje,eeting}s, Geert
"Arnd Bergmann" <arnd@arndb.de> writes: > On Tue, Sep 12, 2023, at 09:14, Geert Uytterhoeven wrote: >> On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> wrote: [...] > > 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. > Yes, IMO that should be a possible combination. >>> 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. > Ah, scratch my previous comment about making this default 'y' then. For some reasons I thought that FB_DEVICE was added in v6.5 but see now that commit 701d2054fa31 ("fbdev: Make support for userspace interfaces configurable") landed in v6.6-rc1: $ git tag --contains 701d2054fa31 | tail -1 v6.6-rc1
On Tue, Sep 12, 2023, at 09:48, Javier Martinez Canillas wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: >> On Mon, Sep 11, 2023 at 10:53 PM Arnd Bergmann <arnd@kernel.org> 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) ? That wouldn't work unless we swap around the 'select DRM_CORE', which currently gets selected when DRM_FBDEV_EMULATION is turned on. >>> 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) ? That would be the same as a plain 'default y' based on the dependencies. We can definitely do that, but it does change the behavior for FB-only users. At the moment, we have 21 defconfig files in the kernel that enable CONFIG_FB but not CONFIG_FRAMEBUFFER_CONSOLE: $ git grep -l CONFIG_FB=y arch/*configs/ | xargs grep -L "FRAMEBUFFER_CONSOLE=\|DRM=" arch/arm/configs/am200epdkit_defconfig arch/arm/configs/assabet_defconfig arch/arm/configs/clps711x_defconfig arch/arm/configs/ep93xx_defconfig arch/arm/configs/footbridge_defconfig arch/arm/configs/h3600_defconfig arch/arm/configs/multi_v4t_defconfig arch/arm/configs/mvebu_v5_defconfig arch/arm/configs/pxa910_defconfig arch/arm/configs/s3c6400_defconfig arch/arm/configs/wpcm450_defconfig arch/microblaze/configs/mmu_defconfig arch/mips/configs/cobalt_defconfig arch/mips/configs/generic/board-ranchu.config arch/mips/configs/malta_qemu_32r6_defconfig arch/mips/configs/maltaaprp_defconfig arch/mips/configs/maltasmvp_defconfig arch/mips/configs/maltasmvp_eva_defconfig arch/mips/configs/maltaup_defconfig arch/sh/configs/r7785rp_defconfig arch/sh/configs/se7343_defconfig >> 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'. I really don't want to start enabling this for configs that didn't have it in the past. Arnd
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 >
Hi Thomas, On Tue, Sep 12, 2023 at 10:11 AM Thomas Zimmermann <tzimmermann@suse.de> 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? > So if any graphics subsystems are enabled, FRAMEBUFFER_CONSOLE is > select-able. But for DRM, FBDEV_EMULATION disables the console. That Huh? /me looks at his morning coffee, and confirms the cup is empty... > option remains more for historical reasons than actual usefulness. > >> --- 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
Hi Geert Am 12.09.23 um 10:18 schrieb Geert Uytterhoeven: > Hi Thomas, > > On Tue, Sep 12, 2023 at 10:11 AM Thomas Zimmermann <tzimmermann@suse.de> 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 if it depends on FB_CORE instead of FRAMEBUFFER_CONSOLE? I'm aware that this would require more Kconfig changes than outlined here. > > And what's the point (if DRM is enabled) of having FB_CORE with > FBDEV_EMULATION disabled? > >> So if any graphics subsystems are enabled, FRAMEBUFFER_CONSOLE is >> select-able. But for DRM, FBDEV_EMULATION disables the console. That > > Huh? > > /me looks at his morning coffee, and confirms the cup is empty... Decaf maybe? But there's really no need to get snarky. My though is that FRAMEBUFFER_CONSOLE configures an end-user feature. The user sits there an thinks "I want a console". FBDEV_EMULATION controls a driver functionality. It's not useful by itself, but enables the enduser feature. The features would be FRAMEBUFFER_CONSOLE and FRAMEBUFFER_DEVICE. Best regards Thomas > >> option remains more for historical reasons than actual usefulness. > >>>> --- 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 >
Hi Thomas, On Tue, Sep 12, 2023 at 10:38 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Am 12.09.23 um 10:18 schrieb Geert Uytterhoeven: > > On Tue, Sep 12, 2023 at 10:11 AM Thomas Zimmermann <tzimmermann@suse.de> 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 if it depends on FB_CORE instead of FRAMEBUFFER_CONSOLE? I'm aware > that this would require more Kconfig changes than outlined here. > > > > > > And what's the point (if DRM is enabled) of having FB_CORE with > > FBDEV_EMULATION disabled? > > > >> So if any graphics subsystems are enabled, FRAMEBUFFER_CONSOLE is > >> select-able. But for DRM, FBDEV_EMULATION disables the console. That > > > > Huh? > > > > /me looks at his morning coffee, and confirms the cup is empty... > > Decaf maybe? > > But there's really no need to get snarky. My though is that Sorry, I was surprised by "FBDEV_EMULATION disables the console", which is not what the Kconfig snippet you suggested does? > FRAMEBUFFER_CONSOLE configures an end-user feature. The user sits there > an thinks "I want a console". FBDEV_EMULATION controls a driver > functionality. It's not useful by itself, but enables the enduser > feature. The features would be FRAMEBUFFER_CONSOLE and FRAMEBUFFER_DEVICE. The latter is currently called FB_DEVICE. If you want to have this controlled by user-visible features, then either FRAMEBUFFER_CONSOLE and FRAMEBUFFER_DEVICE should "select FBDEV_EMULATION if DRM", right? Gr{oetje,eeting}s, Geert
On Tue, Sep 12, 2023, at 04:18, Geert Uytterhoeven wrote: > On Tue, Sep 12, 2023 at 10:11 AM Thomas Zimmermann <tzimmermann@suse.de> 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
Hi Am 11.09.23 um 22:52 schrieb Arnd Bergmann: > From: Arnd Bergmann <arnd@arndb.de> > > 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 <geert@linux-m68k.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/Kconfig | 2 +- > drivers/video/console/Kconfig | 1 + > drivers/video/fbdev/core/Kconfig | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 7df8b6875b121..144a9a1d31cea 100644 > --- 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 > 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 > 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 > 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
Thomas Zimmermann <tzimmermann@suse.de> writes: > Hi > > Am 11.09.23 um 22:52 schrieb Arnd Bergmann: >> From: Arnd Bergmann <arnd@arndb.de> >> >> 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 <geert@linux-m68k.org> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > Pushed to drm-misc (drm-misc-fixes). Thanks!
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 7df8b6875b121..144a9a1d31cea 100644 --- 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 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 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 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