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

["Arnd Bergmann" <arnd@arndb.de>]

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