Multiple graphical consoles are broken (black screen)

Hi,

MIPS Magnum machine emulation is quite specific because it instanciates two graphical consoles.

This has worked for a while, up to following commit which breaks it badly (qemu crashes):

commit 3023f3329d87a6203d03a0e9ccb948772940da96

Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Fri Jan 16 19:04:14 2009 +0000

graphical_console_init change (Stefano Stabellini)

Patch 5/7

This patch changes the graphical_console_init function to return an allocated DisplayState instead of a QEMUConsole.

This patch contains just the graphical_console_init change and few other modifications mainly in console.c and vl.c.
It was necessary to move the display frontends (e.g. sdl and vnc) initialization after machine->init in vl.c.

This patch does *not* include any required changes to any device, these changes come with the following patches.

Patch 6/7

This patch changes the QEMUMachine init functions not to take a DisplayState as an argument because is not needed any more;

In few places the graphic hardware initialization function was called only if DisplayState was not NULL, now they are always called. Apart from these cases, the rest are all mechanical substitutions.

Patch 7/7

This patch updates the graphic device code to use the new graphical_console_init function.

As for the previous patch, in few places graphical_console_init was called only if DisplayState was not NULL, now it is always called. Apart from these cases, the rest are all mechanical substitutions.

Few commits later, the following commit fixed the crash, but still kept the second graphical console unusable: it stays blacks and the window size is not right.

commit 42aa98e8843aa8e3e1f35991f4be63eab2417e94

Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Fri Jan 16 21:01:48 2009 +0000

Remove assumption about a single graphic console.

This fixes a fault with the jazz_led since it has two graphic consoles.

Following patch is an attempt to work-around the problem, at least for SDL display.

I hope someone will be able to pin point the problem, and find the right solution. To reproduce the problem, run:

mips64el-softmmu -M magnum -bios /path/to/NTPROM.raw

and try to switch to second console with Ctrl-Alt-2. You should see a small console with some lines on it.

Hervé Poussineau (1):
sdl: initialize all graphic consoles

 console.c |    3 +++
 vl.c      |   30 +++++++++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

1.7.9

MIPS Jazz emulation registers two graphical consoles, but second one stays black. This patch repairs it.

Other display methods (cocoa, vnc...) also probably require the same kind of fix.

---
 console.c |    3 +++
 vl.c      |   30 +++++++++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

index 135394f..2c413a7 100644
@@ -1376,6 +1376,9 @@ DisplayState *get_displaystate(void)
     if (!display_state) {
         dumb_display_init ();
     }
+    if (active_console && active_console->ds) {
+        return active_console->ds;
+    }
     return display_state;
 }
 

index 7a8cc08..98e0091 100644
@@ -3451,8 +3451,14 @@ int main(int argc, char **argv, char **envp)
 #endif
 #if defined(CONFIG_SDL)
     case DT_SDL:
-        sdl_display_init(ds, full_screen, no_frame);
+    {
+        DisplayState *ds2 = ds;
+        while (ds2) {
+            sdl_display_init(ds2, full_screen, no_frame);
+            ds2 = ds2->next;
+        }
         break;
+    }
 #elif defined(CONFIG_COCOA)
     case DT_SDL:
         cocoa_display_init(ds, full_screen);

@@ -3484,15 +3490,21 @@ int main(int argc, char **argv, char **envp)
 #endif
 
     /* display setup */
-    dpy_resize(ds);
-    dcl = ds->listeners;
-    while (dcl != NULL) {
-        if (dcl->dpy_refresh != NULL) {
-            ds->gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds);
-            qemu_mod_timer(ds->gui_timer, qemu_get_clock_ms(rt_clock));
-            break;
+    {
+        DisplayState *ds2 = ds;
+        while (ds2 != NULL) {
+            dpy_resize(ds2);
+            dcl = ds2->listeners;
+            while (dcl != NULL) {
+                if (dcl->dpy_refresh != NULL) {
+                    ds2->gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds2);
+                    qemu_mod_timer(ds2->gui_timer, qemu_get_clock_ms(rt_clock));
+                    break;
+                }
+                dcl = dcl->next;
+            }
+            ds2 = ds2->next;
         }
-        dcl = dcl->next;
     }
     text_consoles_set_display(ds);
 

1.7.9

I don't think this is really the right way to solve this problem.

A bunch of assumptions are being made here and since this has been "broken" for some years now, I'm not sure that I really view this as a bug.

You should be wary of all the callers of this function because they probably assume that there is just one DisplayState and may not cope well with a multiple DisplayState scenario.
It might be better to rename this function "get_active_displaystate" or "get_current_displaystate". Even better would be to replace it entirely with a for_each_display_state type of iterator, see for example pci_for_each_device or irq_domain_for_each_irq in Linux.

Like Anthony wrote, you probably need a more substantial refactoring to make this work, but the basic idea that you can call graphic_console_init twice to instantiate two DisplayState instances is correct.
Then you should be able to call sdl_display_init on all of them independently. If sdl_display_init (or any other display_init function) modifies a single global state, that needs to be fixed.