[PATCH v2 5/7] arm: qemu: Enable ramfb by default

[Alexander Graf]

On 28.08.23 22:38, Simon Glass wrote:
> Hi Alper,
>
> On Mon, 28 Aug 2023 at 12:29, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>> On 2023-08-28 20:54 +03:00, Simon Glass wrote:
>>> Hi Alper,
>>> On Mon, 28 Aug 2023 at 09:46, Alper Nebi Yasak <alpernebiyasak at 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 <alpernebiyasak at gmail.com> wrote:
>>>>>> From: Alexander Graf <agraf at 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 <agraf at csgraf.de>
>>>>>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling]
>>>>>> Co-developed-by: Alper Nebi Yasak <alpernebiyasak at gmail.com>
>>>>>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak at 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
>> Ah. I'm not used to calling them flags, especially those that have
>> values as in "-device ramfb". My mind jumped to DM_FLAG_PRE_RELOC /
>> bootph-some-ram.
>>
>> But note that what this code tries to probe here is not the ramfb
>> device, it's the bus/parent of that. If this fails, we can't know if the
>> user specified "-device ramfb" or not.
>>
>> Is the parent node's device probed by the time .bind() is called? Or,
>> can I have it probed by calling qfw_get_dev() (which is just
>> uclass_first_device_err(UCLASS_QFW, ...))?
> I suppose your problem might be that there are up to three possible
> video devices (vesa, bochs, ramfb) and one of them needs to return 0
> from bind() and the others need to return -ENODEV?


How do multiple Legacy VGA devices work in parallel in the first place? 
Do we access the legacy VGA PIO ports or memory ranges? If so, which of 
the VGA devices owns it at which point in time?

IIRC the only way you can actually multiplex real VGA is by putting a 
VGA arbiter[1] in between that literally maps the BARs in and out or 
enables/disables device mapping dynamically, no?

It probably makes most sense to make multiple video outputs work 
smoothly with at most 1 VGA device at a time for now to resolve any 
U-Boot internal infrastructure problems, then maybe later try multiple VGAs.


Alex

[1] https://docs.kernel.org/gpu/vgaarbiter.html