Re: [PATCH v2 3/7] qfw: Spawn ramfb device if its file is present
On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> Hi Alper,
>
> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
> wrote:
>>
>> From: Alexander Graf
>>
>> 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=0x300 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.
Re: [PATCH v2 3/7] qfw: Spawn ramfb device if its file is present
Hi Alper,
On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak wrote:
>
> From: Alexander Graf
>
> 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
> [Alper: Use if IS_ENABLED() instead of #ifdef]
> Signed-off-by: Alper Nebi Yasak
> ---
>
> 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
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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
[PATCH v2 3/7] qfw: Spawn ramfb device if its file is present
From: Alexander Graf
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.
Signed-off-by: Alexander Graf
[Alper: Use if IS_ENABLED() instead of #ifdef]
Signed-off-by: Alper Nebi Yasak
---
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
#include
#include
+#include
#include
#include
#include
@@ -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;
}
--
2.40.1
