On 2023-08-22 21:56 +03:00, Simon Glass wrote: > Hi Alper, > > On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak <alpernebiya...@gmail.com> > wrote: >> >> From: Alexander Graf <ag...@csgraf.de> >> >> Now that we have everything in place to support ramfb, let's wire it up >> by default in the ARM QEMU targets. That way, you can easily use a >> graphical console by just passing -device ramfb to the QEMU command line. >> >> Signed-off-by: Alexander Graf <ag...@csgraf.de> >> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] >> Co-developed-by: Alper Nebi Yasak <alpernebiya...@gmail.com> >> Signed-off-by: Alper Nebi Yasak <alpernebiya...@gmail.com> >> --- >> >> Changes in v2: >> - Rebase on "qemu: arm: Enable Bochs, console buffering, USB keyboard" >> - Drop env changes from ARM (necessary but in prerequisite series) >> - Drop imply VIDEO, SYS_CONSOLE_IN_ENV changes from ARM (in prereq.) >> - Probe QFW in ARM QEMU board_early_init_f to bind ramfb pre-reloc >> - Add IS_ENABLED(CONFIG_QFW) check and error handling to ARM QEMU >> >> arch/arm/Kconfig | 3 +++ >> board/emulation/qemu-arm/qemu-arm.c | 41 +++++++++++++++++++++++++++++ >> 2 files changed, 44 insertions(+) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 1fd3ccd1607f..7afe26ac804f 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -1046,6 +1046,9 @@ config ARCH_QEMU >> imply USB_XHCI_PCI >> imply USB_KEYBOARD >> imply CMD_USB >> + imply VIDEO_RAMFB >> + imply BOARD_EARLY_INIT_F >> + imply BOARD_EARLY_INIT_R >> >> config ARCH_RMOBILE >> bool "Renesas ARM SoCs" >> diff --git a/board/emulation/qemu-arm/qemu-arm.c >> b/board/emulation/qemu-arm/qemu-arm.c >> index 942f1fff5717..23ef31cb7feb 100644 >> --- a/board/emulation/qemu-arm/qemu-arm.c >> +++ b/board/emulation/qemu-arm/qemu-arm.c >> @@ -11,6 +11,7 @@ >> #include <fdtdec.h> >> #include <init.h> >> #include <log.h> >> +#include <qfw.h> >> #include <usb.h> >> #include <virtio_types.h> >> #include <virtio.h> >> @@ -102,6 +103,46 @@ static struct mm_region qemu_arm64_mem_map[] = { >> struct mm_region *mem_map = qemu_arm64_mem_map; >> #endif >> >> +int board_early_init_f(void) >> +{ >> + struct udevice *dev; >> + int ret; >> + >> + /* >> + * Make sure we enumerate the QEMU Firmware device to bind ramfb >> + * so video_reserve() can reserve memory for it. >> + */ >> + if (IS_ENABLED(CONFIG_QFW)) { >> + ret = qfw_get_dev(&dev); >> + if (ret) { >> + log_err("Failed to get QEMU FW device: %d\n", ret); > > We should only present an error if the device is present but > failed...so if the user doesn't provide the flag, all should be well.
I don't understand what you mean by "user doesn't provide the flag". But I assume this should ignore the error (unless what?) so that we can continue to boot. Would that apply for board_early_init_r below as well? >> + return ret; >> + } >> + } >> + >> + return 0; >> +} >> + >> +int board_early_init_r(void) >> +{ >> + struct udevice *dev; >> + int ret; >> + >> + /* >> + * Make sure we enumerate the QEMU Firmware device to find ramfb >> + * before console_init. >> + */ >> + if (IS_ENABLED(CONFIG_QFW)) { >> + ret = qfw_get_dev(&dev); >> + if (ret) { >> + log_err("Failed to get QEMU FW device: %d\n", ret); >> + return ret; >> + } >> + } >> + >> + return 0; >> +} >> + >> int board_init(void) >> { >> return 0; >> -- >> 2.40.1 >> > > The glue here feels like a bit of a hack...we should rely on normal DT > mechanisms here. > > Regards, > Simon