| Message ID | 20230822121026.1007105-6-alpernebiyasak@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | Add support for QEMU's ramfb display | expand |
Hi Alper, On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: > > From: Alexander Graf <agraf@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@csgraf.de> > [Alper: Rebase on bochs changes, add pre-reloc init, error handling] > Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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. > + 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
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: >> >> From: Alexander Graf <agraf@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@csgraf.de> >> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] >> Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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
Hi Alper, On Mon, 28 Aug 2023 at 09:46, Alper Nebi Yasak <alpernebiyasak@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@gmail.com> wrote: > >> > >> From: Alexander Graf <agraf@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@csgraf.de> > >> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] > >> Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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
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@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@gmail.com> wrote: >>>> >>>> From: Alexander Graf <agraf@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@csgraf.de> >>>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] >>>> Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >>>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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, ...))?
On 28.08.23 20:28, Alper Nebi Yasak 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@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@gmail.com> wrote: >>>>> From: Alexander Graf <agraf@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@csgraf.de> >>>>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] >>>>> Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >>>>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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. You bring up a great point here. I don't think we should model the actual ramfb device as device tree (platform) device. It's 100% enumerable from QFW which means we should treat it as a dynamic entity that gets created by QFW instead of from device tree, just like USB or PCI devices. And when you do that, everything should fall into place nicely: You don't need to dynamically modify device trees on boot, the DT you get from QEMU is sufficient to enumerate everything, etc etc. Alex
Hi Alper, On Mon, 28 Aug 2023 at 12:29, Alper Nebi Yasak <alpernebiyasak@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@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@gmail.com> wrote: > >>>> > >>>> From: Alexander Graf <agraf@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@csgraf.de> > >>>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] > >>>> Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > >>>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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? Perhaps another way would be to update the logic in stdio_init_tables() to find them all (which I believe it already does?) and then select one? Regards, Simon
On 28.08.23 22:38, Simon Glass wrote: > Hi Alper, > > On Mon, 28 Aug 2023 at 12:29, Alper Nebi Yasak <alpernebiyasak@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@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@gmail.com> wrote: >>>>>> From: Alexander Graf <agraf@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@csgraf.de> >>>>>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling] >>>>>> Co-developed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >>>>>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@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
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); + 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;