| Message ID | 20230822121026.1007105-4-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 a ramfb device driver, let's add the necessary glueing > magic to also spawn it when we find its qfw file node. So then how do we select which video driver is used? I think we should have this in the DT so there is some control. > > Signed-off-by: Alexander Graf <agraf@csgraf.de> > [Alper: Use if IS_ENABLED() instead of #ifdef] > Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > --- > > Changes in v2: > - Use if (IS_ENABLED(CONFIG_VIDEO_RAMFB)) instead of #ifdef > > drivers/misc/qfw.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c > index 7c01bf23d53b..4e4260982cce 100644 > --- a/drivers/misc/qfw.c > +++ b/drivers/misc/qfw.c > @@ -16,6 +16,7 @@ > #include <malloc.h> > #include <qfw.h> > #include <dm.h> > +#include <dm/lists.h> > #include <misc.h> > #include <tables_csum.h> > #include <asm/acpi_table.h> > @@ -307,6 +308,27 @@ void qfw_read_entry(struct udevice *dev, u16 entry, u32 size, void *address) > qfw_read_entry_io(qdev, entry, size, address); > } > > +static void qfw_bind_ramfb(struct udevice *dev) > +{ > + struct fw_file *file; > + int ret; > + > + if (!IS_ENABLED(CONFIG_VIDEO_RAMFB)) > + return; Needs an error-code return > + > + ret = qfw_read_firmware_list(dev); > + if (ret) > + return; > + > + file = qfw_find_file(dev, "etc/ramfb"); > + if (!file) { > + /* No ramfb available. */ > + return; > + } > + > + device_bind_driver(dev, "ramfb", "qfw-ramfb", NULL); check error > +} > + > int qfw_register(struct udevice *dev) > { > struct qfw_dev *qdev = dev_get_uclass_priv(dev); > @@ -323,6 +345,8 @@ int qfw_register(struct udevice *dev) > if (dma_enabled & FW_CFG_DMA_ENABLED) > qdev->dma_present = true; > > + qfw_bind_ramfb(dev); > + > return 0; > } > > -- > 2.40.1 > 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 a ramfb device driver, let's add the necessary glueing >> magic to also spawn it when we find its qfw file node. > > So then how do we select which video driver is used? I think we should > have this in the DT so there is some control. I think you mean choosing between multiple potential video devices for a video console, but just in case: AFAIK only the ramfb driver can be used with -device ramfb, unlike how either vesa or bochs can be used for -device VGA. This series is less about being able to use multiple video devices, but more about supporting by default different configurations in which QEMU can be launched: with "-device VGA" or "-vga none -device ramfb". That being said, I had some success with multiple video console attempts. For "-device VGA -device ramfb", we need to make `*addrp -= CONFIG_VAL(VIDEO_PCI_DEFAULT_FB_SIZE);` unconditional in video_reserve() to correctly allocate for both vesa and ramfb. Then we can run `env set serial,vidconsole,vidconsole1` which sets up both video/vidconsole devices with same text content. Regardless of this series, with VIDEO_PCI_DEFAULT_FB_SIZE=0x3000000 and "-device VGA -device VGA -device VGA" we get two broken and one empty display, and `env set serial,vidconsole,vidconsole1,vidconsole2` puts a console to the third display. I think it's a bug the first two are broken, and it's initially using the first broken display as vidconsole. Anyway, my opinion is: if stdout/stderr has "vidconsole", we should enable a vidconsole on all consoles, but if it has "vidconsole#" we should only enable the one on that video# device. Then device-tree could have /aliases { video0 = "..."; video1 = &...; } or something to have a stable order? Or, you might mean adding a ramfb node to the device-tree, in which case that's going to reopen a can of OF_BOARD worms... (Same as what applies to putting bootph,* into qfw device-tree node, so I'll reply there.) >> [...] >> >> +static void qfw_bind_ramfb(struct udevice *dev) >> +{ >> + struct fw_file *file; >> + int ret; >> + >> + if (!IS_ENABLED(CONFIG_VIDEO_RAMFB)) >> + return; > > Needs an error-code return > >> + >> + ret = qfw_read_firmware_list(dev); >> + if (ret) >> + return; >> + >> + file = qfw_find_file(dev, "etc/ramfb"); >> + if (!file) { >> + /* No ramfb available. */ >> + return; >> + } >> + >> + device_bind_driver(dev, "ramfb", "qfw-ramfb", NULL); > > check error > Will try to fix both, thanks.
diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c index 7c01bf23d53b..4e4260982cce 100644 --- a/drivers/misc/qfw.c +++ b/drivers/misc/qfw.c @@ -16,6 +16,7 @@ #include <malloc.h> #include <qfw.h> #include <dm.h> +#include <dm/lists.h> #include <misc.h> #include <tables_csum.h> #include <asm/acpi_table.h> @@ -307,6 +308,27 @@ void qfw_read_entry(struct udevice *dev, u16 entry, u32 size, void *address) qfw_read_entry_io(qdev, entry, size, address); } +static void qfw_bind_ramfb(struct udevice *dev) +{ + struct fw_file *file; + int ret; + + if (!IS_ENABLED(CONFIG_VIDEO_RAMFB)) + return; + + ret = qfw_read_firmware_list(dev); + if (ret) + return; + + file = qfw_find_file(dev, "etc/ramfb"); + if (!file) { + /* No ramfb available. */ + return; + } + + device_bind_driver(dev, "ramfb", "qfw-ramfb", NULL); +} + int qfw_register(struct udevice *dev) { struct qfw_dev *qdev = dev_get_uclass_priv(dev); @@ -323,6 +345,8 @@ int qfw_register(struct udevice *dev) if (dma_enabled & FW_CFG_DMA_ENABLED) qdev->dma_present = true; + qfw_bind_ramfb(dev); + return 0; }