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

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

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?