| Message ID | 20230822121026.1007105-5-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: > > QEMU firmware config drivers need to be probed to bind the ramfb device. > The ramfb driver needs to be bound before relocation to properly reserve > video memory for it, otherwise it cannot be probed after relocation. Add > the flag to probe QEMU firmware config drivers before relocation so that > ramfb can work as an initial vidconsole. > > Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > --- > Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a > higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers > cannot be bound before relocation unless they are mentioned in the > devicetree" and qfw is in the QEMU-generated devicetree unlike those, so > I assumed this would be the preferred way. > > Changes in v2: > - Add patch "qfw: Add flag to allow probing before relocation" > > drivers/misc/qfw.c | 1 + > drivers/misc/qfw_mmio.c | 1 + > drivers/misc/qfw_pio.c | 1 + > drivers/misc/qfw_sandbox.c | 1 + > 4 files changed, 4 insertions(+) > > diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c > index 4e4260982cce..265f45290011 100644 > --- a/drivers/misc/qfw.c > +++ b/drivers/misc/qfw.c > @@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = { > .name = "qfw", > .post_bind = qfw_post_bind, > .per_device_auto = sizeof(struct qfw_dev), > + .flags = DM_FLAG_PRE_RELOC, > }; Should we add this to the DT instead? In the case where it isn't present it can return -EPERM. 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: >> >> QEMU firmware config drivers need to be probed to bind the ramfb device. >> The ramfb driver needs to be bound before relocation to properly reserve >> video memory for it, otherwise it cannot be probed after relocation. Add >> the flag to probe QEMU firmware config drivers before relocation so that >> ramfb can work as an initial vidconsole. >> >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >> --- >> Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a >> higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers >> cannot be bound before relocation unless they are mentioned in the >> devicetree" and qfw is in the QEMU-generated devicetree unlike those, so >> I assumed this would be the preferred way. >> >> Changes in v2: >> - Add patch "qfw: Add flag to allow probing before relocation" >> >> drivers/misc/qfw.c | 1 + >> drivers/misc/qfw_mmio.c | 1 + >> drivers/misc/qfw_pio.c | 1 + >> drivers/misc/qfw_sandbox.c | 1 + >> 4 files changed, 4 insertions(+) >> >> diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c >> index 4e4260982cce..265f45290011 100644 >> --- a/drivers/misc/qfw.c >> +++ b/drivers/misc/qfw.c >> @@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = { >> .name = "qfw", >> .post_bind = qfw_post_bind, >> .per_device_auto = sizeof(struct qfw_dev), >> + .flags = DM_FLAG_PRE_RELOC, >> }; > > Should we add this to the DT instead? I've experimented a bit, and got something that works nice on x86 because we have the device-trees in-tree. So I can add something to arch/x86/dts/qemu-*.dts like: / { fw-cfg { compatible = "qemu,fw-cfg-pio"; bootph-some-ram; qfw-ramfb { compatible = "qemu,qfw-ramfb"; bootph-some-ram; }; }; }; Then add the compatibles as .of_match to qfw_pio and ramfb drivers, call dm_scan_fdt_dev(dev) in qfw_post_bind(), merge qfw_bind_ramfb() into the ramfb driver probe, and it will work without the board_early_init_[fr] hooks or these flags. That all seems to be fine. The problem is other arches where QEMU generates a device-tree at runtime and passes it to U-Boot with OF_BOARD. It has, on arm64 (and with @10100000 instead on riscv64): fw-cfg@9020000 { dma-coherent; reg = <0x00 0x9020000 0x00 0x18>; compatible = "qemu,fw-cfg-mmio"; }; As a proof of concept, I used `qemu-system-* -M dumpdtb=file.dtb` and `dtc -I dtb -O dts` to replace the empty qemu dts files, then set OF_BOARD=n and OF_OMIT_DTB=n, and added similar modifications to qemu-*-u-boot.dtsi. And everything seems fine like that as well. I think it can be automated in Makefile, but I don't think disabling OF_BOARD is right here. I see there's OF_BOARD_FIXUP, so I guess I'll try finding the fw-cfg node there and adding the bootph props and ramfb nodes... Is this going the right way? (And the pio/ramfb compatibles don't exist, do we have to use u-boot,*?) > In the case where it isn't present it can return -EPERM. > > Regards, > Simon
On Mon, Aug 28, 2023 at 06:22:31PM +0300, Alper Nebi Yasak 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: > >> > >> QEMU firmware config drivers need to be probed to bind the ramfb device. > >> The ramfb driver needs to be bound before relocation to properly reserve > >> video memory for it, otherwise it cannot be probed after relocation. Add > >> the flag to probe QEMU firmware config drivers before relocation so that > >> ramfb can work as an initial vidconsole. > >> > >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > >> --- > >> Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a > >> higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers > >> cannot be bound before relocation unless they are mentioned in the > >> devicetree" and qfw is in the QEMU-generated devicetree unlike those, so > >> I assumed this would be the preferred way. > >> > >> Changes in v2: > >> - Add patch "qfw: Add flag to allow probing before relocation" > >> > >> drivers/misc/qfw.c | 1 + > >> drivers/misc/qfw_mmio.c | 1 + > >> drivers/misc/qfw_pio.c | 1 + > >> drivers/misc/qfw_sandbox.c | 1 + > >> 4 files changed, 4 insertions(+) > >> > >> diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c > >> index 4e4260982cce..265f45290011 100644 > >> --- a/drivers/misc/qfw.c > >> +++ b/drivers/misc/qfw.c > >> @@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = { > >> .name = "qfw", > >> .post_bind = qfw_post_bind, > >> .per_device_auto = sizeof(struct qfw_dev), > >> + .flags = DM_FLAG_PRE_RELOC, > >> }; > > > > Should we add this to the DT instead? > > I've experimented a bit, and got something that works nice on x86 > because we have the device-trees in-tree. So I can add something to > arch/x86/dts/qemu-*.dts like: > > / { > fw-cfg { > compatible = "qemu,fw-cfg-pio"; > bootph-some-ram; > > qfw-ramfb { > compatible = "qemu,qfw-ramfb"; > bootph-some-ram; > }; > }; > }; > > Then add the compatibles as .of_match to qfw_pio and ramfb drivers, call > dm_scan_fdt_dev(dev) in qfw_post_bind(), merge qfw_bind_ramfb() into the > ramfb driver probe, and it will work without the board_early_init_[fr] > hooks or these flags. That all seems to be fine. > > The problem is other arches where QEMU generates a device-tree at > runtime and passes it to U-Boot with OF_BOARD. It has, on arm64 (and > with @10100000 instead on riscv64): > > fw-cfg@9020000 { > dma-coherent; > reg = <0x00 0x9020000 0x00 0x18>; > compatible = "qemu,fw-cfg-mmio"; > }; > > As a proof of concept, I used `qemu-system-* -M dumpdtb=file.dtb` and > `dtc -I dtb -O dts` to replace the empty qemu dts files, then set > OF_BOARD=n and OF_OMIT_DTB=n, and added similar modifications to > qemu-*-u-boot.dtsi. And everything seems fine like that as well. > > I think it can be automated in Makefile, but I don't think disabling > OF_BOARD is right here. I see there's OF_BOARD_FIXUP, so I guess I'll > try finding the fw-cfg node there and adding the bootph props and ramfb > nodes... Is this going the right way? > > (And the pio/ramfb compatibles don't exist, do we have to use u-boot,*?) This seems like a whole lot of work to avoid using the flags mechanism that we have, intentionally.
diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c index 4e4260982cce..265f45290011 100644 --- a/drivers/misc/qfw.c +++ b/drivers/misc/qfw.c @@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = { .name = "qfw", .post_bind = qfw_post_bind, .per_device_auto = sizeof(struct qfw_dev), + .flags = DM_FLAG_PRE_RELOC, }; struct bootdev_ops qfw_bootdev_ops = { diff --git a/drivers/misc/qfw_mmio.c b/drivers/misc/qfw_mmio.c index f397384054a6..a5274982711c 100644 --- a/drivers/misc/qfw_mmio.c +++ b/drivers/misc/qfw_mmio.c @@ -116,4 +116,5 @@ U_BOOT_DRIVER(qfw_mmio) = { .of_to_plat = qfw_mmio_of_to_plat, .probe = qfw_mmio_probe, .ops = &qfw_mmio_ops, + .flags = DM_FLAG_PRE_RELOC, }; diff --git a/drivers/misc/qfw_pio.c b/drivers/misc/qfw_pio.c index e2f628d33830..dae809bd3fea 100644 --- a/drivers/misc/qfw_pio.c +++ b/drivers/misc/qfw_pio.c @@ -66,4 +66,5 @@ U_BOOT_DRIVER(qfw_pio) = { .id = UCLASS_QFW, .probe = qfw_pio_probe, .ops = &qfw_pio_ops, + .flags = DM_FLAG_PRE_RELOC, }; diff --git a/drivers/misc/qfw_sandbox.c b/drivers/misc/qfw_sandbox.c index 1002df75339e..0bd36acd1a09 100644 --- a/drivers/misc/qfw_sandbox.c +++ b/drivers/misc/qfw_sandbox.c @@ -120,6 +120,7 @@ U_BOOT_DRIVER(qfw_sandbox) = { .plat_auto = sizeof(struct qfw_sandbox_plat), .probe = qfw_sandbox_probe, .ops = &qfw_sandbox_ops, + .flags = DM_FLAG_PRE_RELOC, }; U_BOOT_DRVINFO(qfw_sandbox) = {
QEMU firmware config drivers need to be probed to bind the ramfb device. The ramfb driver needs to be bound before relocation to properly reserve video memory for it, otherwise it cannot be probed after relocation. Add the flag to probe QEMU firmware config drivers before relocation so that ramfb can work as an initial vidconsole. Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> --- Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers cannot be bound before relocation unless they are mentioned in the devicetree" and qfw is in the QEMU-generated devicetree unlike those, so I assumed this would be the preferred way. Changes in v2: - Add patch "qfw: Add flag to allow probing before relocation" drivers/misc/qfw.c | 1 + drivers/misc/qfw_mmio.c | 1 + drivers/misc/qfw_pio.c | 1 + drivers/misc/qfw_sandbox.c | 1 + 4 files changed, 4 insertions(+)