Re: [PATCH] x86: sysfb: fool-proof CONFIG_X86_SYSFB

From: Ingo Molnar
Date: Thu Dec 19 2013 - 10:59:31 EST



* David Herrmann <dh.herrmann@xxxxxxxxx> wrote:

> Hi
>
> On Thu, Dec 19, 2013 at 4:40 PM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > * David Herrmann <dh.herrmann@xxxxxxxxx> wrote:
> >
> >> Turns out, people do not read help-texts of new config-options and
> >> enable them nonetheless. [...]
> >
> > Yeah, I too don't read them either, whenever an option name seems
> > obvious to enable, so this is really something that happens frequently
> > ;-)
> >
> >> [...] So several reports came in with X86_SYSFB=y and FB_SIMPLE=n,
> >> which in almost all situations prevents firmware-fbs from being
> >> probed.
> >>
> >> X86_SYSFB clearly states that it turns legacy vesa/efi framebuffers
> >> into a format compatible to simplefb (and does nothing else..). So
> >> to avoid further complaints about missing gfx-support during boot,
> >> simply depend on FB_SIMPLE now.
> >>
> >> As FB_SIMPLE is disabled by default and usually only enabled on
> >> selected ARM architectures, x86 users should thus never see the
> >> X86_SYSFB config-option. And if they do, everything is fine as
> >> simplefb will be available.
> >>
> >> Note that most of the sysfb code is enabled independently of
> >> X86_SYSFB. The config option only selects a compatibility mode for
> >> simplefb. It was introduced to ease the transition to SimpleDRM and
> >> disabling fbdev. As this is still ongoing, there's no need for
> >> non-developers to care for X86_SYSFB so just change the help-text
> >> recommendation to "n".
> >>
> >> Cc: <stable@xxxxxxxxxxxxxxx> # 3.11+
> >> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
> >> ---
> >> arch/x86/Kconfig | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >> index e903c71..9317ede 100644
> >> --- a/arch/x86/Kconfig
> >> +++ b/arch/x86/Kconfig
> >> @@ -2298,6 +2298,7 @@ source "drivers/rapidio/Kconfig"
> >>
> >> config X86_SYSFB
> >> bool "Mark VGA/VBE/EFI FB as generic system framebuffer"
> >> + depends on (FB_SIMPLE = y)
> >
> > Could that be written as:
> >
> > depends on FB_SIMPLE
> >
> > Or is there some complication with modular builds?
>
> simplefb is actually "bool" so yeah, I can remove the "= y".
>
> Note that *if* it ever is built as module, it will not get loaded
> during boot, thus not showing any boot-messages. But that's true for
> vesafb/efifb, too, so we're fine.
>
> >> help
> >> Firmwares often provide initial graphics framebuffers so the BIOS,
> >> bootloader or kernel can show basic video-output during boot for
> >> @@ -2320,7 +2321,7 @@ config X86_SYSFB
> >> and others enabled as fallback if a system framebuffer is
> >> incompatible with simplefb.
> >>
> >> - If unsure, say Y.
> >> + If unsure, say N.
> >
> > We might in fact leave this bit alone and suggest 'Y' - with the
> > robustification fixes it's not possible anymore to crash the boot or
> > to create an unintentionally headless system, right?
>
> Nope, we will not cause a headless system, but the handover to other
> fbdev/DRM drivers (other than the common nouveau/radeon/i915) may
> still fail without the other patch (x86: sysfb: remove sysfb when
> probing real hw).
>
> I will gladly keep the Y, if no-one disagrees. Given that the other
> patch is rather complex (and a no-op with X86_SYSFB=n) I wasn't
> quite sure. But as SYSFB is set to 'n' now if FB_SIMPLE wasn't
> explicitly selected, I think we're fine both ways.

Well, as long as fixes keep coming when people report them, I see no
problem with keeping the 'Y'. Bugs happen.

And I think we want to apply the other fix regardless of the default.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/