Hi Alper,
On Mon, 28 Aug 2023 at 09:46, Alper Nebi Yasak <alpernebiya...@gmail.com> 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 <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?
I thought you said there was a qemu flag to enable ramfb? So add it to
the DT and then the device can (in its bind routine) decide not to be
bound by returning -ENODEV
>
> >> + 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
Regards.
Simon
