[PATCH v2 3/7] qfw: Spawn ramfb device if its file is present

[Alper Nebi Yasak]

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 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.