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
