Message ID | 1330033159-15860-2-git-send-email-hpoussin@reactos.org |
---|---|
State | New |
Headers | show |
On 02/23/2012 03:39 PM, Hervé Poussineau wrote: > 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. 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. > --- > console.c | 3 +++ > vl.c | 30 +++++++++++++++++++++--------- > 2 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/console.c b/console.c > index 135394f..2c413a7 100644 > --- a/console.c > +++ b/console.c > @@ -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; > } > > diff --git a/vl.c b/vl.c > index 7a8cc08..98e0091 100644 > --- a/vl.c > +++ b/vl.c > @@ -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; The fact that this works at all really surprises me. You're registering double input event handlers and doing a number of things that have a global state. sdl_display_init() isn't meant to be called twice. I really think more substantial refactoring is needed such that we're not maintaining the UI state as globals and can independently instantiate a backend for a given DisplayState. I had some patches that I posted a bit ago that started down this direction. Regards, Anthony Liguori > + } > 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); >
On Thu, 23 Feb 2012, Anthony Liguori wrote: > > diff --git a/console.c b/console.c > > index 135394f..2c413a7 100644 > > --- a/console.c > > +++ b/console.c > > @@ -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; > > } 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. > > diff --git a/vl.c b/vl.c > > index 7a8cc08..98e0091 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -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; > > The fact that this works at all really surprises me. You're registering double > input event handlers and doing a number of things that have a global state. > > sdl_display_init() isn't meant to be called twice. I really think more > substantial refactoring is needed such that we're not maintaining the UI state > as globals and can independently instantiate a backend for a given DisplayState. > > I had some patches that I posted a bit ago that started down this direction. 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.
diff --git a/console.c b/console.c index 135394f..2c413a7 100644 --- a/console.c +++ b/console.c @@ -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; } diff --git a/vl.c b/vl.c index 7a8cc08..98e0091 100644 --- a/vl.c +++ b/vl.c @@ -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);