Re: [PATCH v2 7/7] x86: qemu: Enable ramfb by default

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3267 bytes --]

On Mon, Aug 28, 2023 at 06:40:10PM +0300, Alper Nebi Yasak wrote:
> On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> > Hi Alper,
> > 
> > On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
> >>
> >> Now that we have everything in place to support ramfb, let's wire it up
> >> by default in the x86 QEMU targets. That way, we can use ramfb graphical
> >> console instead of the default by passing -vga none -device ramfb to the
> >> QEMU command line.
> >>
> >> Also increase SYS_MALLOC_F_LEN for QEMU x86_64 to be the same as its SPL
> >> counterpart, because we're running out of alloc space in pre-reloc stage
> >> with ramfb enabled.
> >>
> >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> >> ---
> >> This also suffers from the same issue with distros as the Bochs display
> >> driver [1], where it results in a hang after GRUB menu selection before
> >> the kernel can display anything. Couldn't reproduce on arm*/riscv*.
> > 
> > Yes I see that problem too. I wonder how we can debug it?
> 
> No idea, and I couldn't find a good commit to bisect from, tried as far
> back as v2021.10.
> 
> >> But just having it enabled doesn't seem to cause problems unless you run
> >> QEMU with -device ramfb, so this (unlike the Bochs video driver) can
> >> actually be co-enabled with VIDEO_VESA.
> > 
> > Indeed...which makes me wonder if we can do something similar with
> > Bochs, so that (from the cmdline) it is possible to chose ramfb, bochs
> > or vesa?
> 
> (Tried to answer video choice and DT concerns in another mail)
> 
> >> [1] https://lore.kernel.org/u-boot/20230724145210.304917-4-sjg@chromium.org/
> >>
> >> Changes in v2:
> >> - Add patch "x86: qemu: Enable ramfb by default"
> >>
> >>  arch/x86/cpu/qemu/Kconfig           |  4 +++
> >>  board/emulation/qemu-x86/qemu-x86.c | 47 +++++++++++++++++++++++++++++
> >>  configs/qemu-x86_64_defconfig       |  4 +--
> >>  configs/qemu-x86_defconfig          |  1 -
> >>  4 files changed, 52 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/x86/cpu/qemu/Kconfig b/arch/x86/cpu/qemu/Kconfig
> >> index f8f2f6473088..e0a57ac2d687 100644
> >> --- a/arch/x86/cpu/qemu/Kconfig
> >> +++ b/arch/x86/cpu/qemu/Kconfig
> >> @@ -13,6 +13,10 @@ config QEMU
> >>         imply USB
> >>         imply USB_EHCI_HCD
> >>         imply VIDEO_VESA
> >> +       imply VIDEO_RAMFB
> >> +       imply BOARD_EARLY_INIT_F
> >> +       imply BOARD_EARLY_INIT_R
> >> +       imply CMD_QFW
> >>
> >>  if QEMU
> >>
> >> [...]
> >> 
> >> @@ -59,7 +58,6 @@ CONFIG_CMD_USB=y
> >>  CONFIG_BOOTP_BOOTFILESIZE=y
> >>  CONFIG_CMD_EFIDEBUG=y
> >>  CONFIG_CMD_TIME=y
> >> -CONFIG_CMD_QFW=y
> > 
> > What is happening here? Why disable it?
> 
> I used `imply CMD_QFW` above to be consistent with previous patches, so
> it's no longer necessary in defconfigs. Should I drop the imply and keep
> these in defconfig?

I think our Kconfig logic around the QFW options is a bit backwards,
given that it's universal to the "qemu" boards and not used on
qemu-emulates-known-hardware platforms.  The qemu boards should be
select'ing QFW, and CMD_QFW should be depends on QFW and default y,
rather than selects.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]