[Qemu-devel] [PATCH 1/2][RESEND] vl: initialize all displaystates

[Qemu-devel] [PATCH 1/2][RESEND] vl: initialize all displaystates

[Dmitry Eremin-Solenikov]

Init not only first displaystate, but all. Otherwise machines with
multiple display devices (e.g. tosa, as it exists now) will just
segfault on ds switch.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 vl.c |  104 +++++++++++++++++++++++++++++++++---------------------------------
 1 files changed, 52 insertions(+), 52 deletions(-)

 Basically this patch is equal to:
 @@ -3009,9 +3009,7 @@ int main(int argc, char **argv, char **envp)
  
      net_check_clients();
  
 -    /* just use the first displaystate for the moment */
 -    ds = get_displaystate();
 -
 +    for (ds = get_displaystate(); ds; ds = ds->next) {
      if (using_spice)
          display_remote++;
      if (display_type == DT_DEFAULT && !display_remote) {
 @@ -3077,7 +3075,9 @@ int main(int argc, char **argv, char **envp)
          nographic_timer = qemu_new_timer(rt_clock, nographic_update, NULL);
          qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
      }
 -    text_consoles_set_display(ds);
 +    }
 +
 +    text_consoles_set_display(get_displaystate());
  
      if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
          fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",

diff --git a/vl.c b/vl.c
index 14255c4..b8cd455 100644
--- a/vl.c
+++ b/vl.c
@@ -3009,75 +3009,75 @@ int main(int argc, char **argv, char **envp)
 
     net_check_clients();
 
-    /* just use the first displaystate for the moment */
-    ds = get_displaystate();
-
-    if (using_spice)
-        display_remote++;
-    if (display_type == DT_DEFAULT && !display_remote) {
+    for (ds = get_displaystate(); ds; ds = ds->next) {
+        if (using_spice)
+            display_remote++;
+        if (display_type == DT_DEFAULT && !display_remote) {
 #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
-        display_type = DT_SDL;
+            display_type = DT_SDL;
 #else
-        vnc_display = "localhost:0,to=99";
-        show_vnc_port = 1;
+            vnc_display = "localhost:0,to=99";
+            show_vnc_port = 1;
 #endif
-    }
-        
+        }
+
 
-    /* init local displays */
-    switch (display_type) {
-    case DT_NOGRAPHIC:
-        break;
+        /* init local displays */
+        switch (display_type) {
+        case DT_NOGRAPHIC:
+            break;
 #if defined(CONFIG_CURSES)
-    case DT_CURSES:
-        curses_display_init(ds, full_screen);
-        break;
+        case DT_CURSES:
+            curses_display_init(ds, full_screen);
+            break;
 #endif
 #if defined(CONFIG_SDL)
-    case DT_SDL:
-        sdl_display_init(ds, full_screen, no_frame);
-        break;
+        case DT_SDL:
+            sdl_display_init(ds, full_screen, no_frame);
+            break;
 #elif defined(CONFIG_COCOA)
-    case DT_SDL:
-        cocoa_display_init(ds, full_screen);
-        break;
+        case DT_SDL:
+            cocoa_display_init(ds, full_screen);
+            break;
 #endif
-    default:
-        break;
-    }
+        default:
+            break;
+        }
 
-    /* init remote displays */
-    if (vnc_display) {
-        vnc_display_init(ds);
-        if (vnc_display_open(ds, vnc_display) < 0)
-            exit(1);
+        /* init remote displays */
+        if (vnc_display) {
+            vnc_display_init(ds);
+            if (vnc_display_open(ds, vnc_display) < 0)
+                exit(1);
 
-        if (show_vnc_port) {
-            printf("VNC server running on `%s'\n", vnc_display_local_addr(ds));
+            if (show_vnc_port) {
+                printf("VNC server running on `%s'\n", vnc_display_local_addr(ds));
+            }
         }
-    }
 #ifdef CONFIG_SPICE
-    if (using_spice && !qxl_enabled) {
-        qemu_spice_display_init(ds);
-    }
+        if (using_spice && !qxl_enabled) {
+            qemu_spice_display_init(ds);
+        }
 #endif
 
-    /* display setup */
-    dpy_resize(ds);
-    dcl = ds->listeners;
-    while (dcl != NULL) {
-        if (dcl->dpy_refresh != NULL) {
-            ds->gui_timer = qemu_new_timer(rt_clock, gui_update, ds);
-            qemu_mod_timer(ds->gui_timer, qemu_get_clock(rt_clock));
-            break;
+        /* display setup */
+        dpy_resize(ds);
+        dcl = ds->listeners;
+        while (dcl != NULL) {
+            if (dcl->dpy_refresh != NULL) {
+                ds->gui_timer = qemu_new_timer(rt_clock, gui_update, ds);
+                qemu_mod_timer(ds->gui_timer, qemu_get_clock(rt_clock));
+                break;
+            }
+            dcl = dcl->next;
+        }
+        if (ds->gui_timer == NULL) {
+            nographic_timer = qemu_new_timer(rt_clock, nographic_update, NULL);
+            qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
         }
-        dcl = dcl->next;
-    }
-    if (ds->gui_timer == NULL) {
-        nographic_timer = qemu_new_timer(rt_clock, nographic_update, NULL);
-        qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
     }
-    text_consoles_set_display(ds);
+
+    text_consoles_set_display(get_displaystate());
 
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
         fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
-- 
1.7.2.3

[Qemu-devel] [PATCH 2/2][RESEND] tc6393xb: add support for invalidate display callback

[Dmitry Eremin-Solenikov]

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 hw/tc6393xb.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c
index ed49e94..14847a9 100644
--- a/hw/tc6393xb.c
+++ b/hw/tc6393xb.c
@@ -126,7 +126,8 @@ struct TC6393xbState {
     uint32_t scr_width, scr_height; /* in pixels */
     qemu_irq l3v;
     unsigned blank : 1,
-             blanked : 1;
+             blanked : 1,
+             invalidated : 1;
 };
 
 qemu_irq *tc6393xb_gpio_in_get(TC6393xbState *s)
@@ -483,7 +484,9 @@ static void tc6393xb_update_display(void *opaque)
         s->blanked = s->blank;
         full_update = 1;
     }
-    if (s->scr_width != ds_get_width(s->ds) || s->scr_height != ds_get_height(s->ds)) {
+    if (s->scr_width != ds_get_width(s->ds) ||
+            s->scr_height != ds_get_height(s->ds) ||
+            s->invalidated) {
         qemu_console_resize(s->ds, s->scr_width, s->scr_height);
         full_update = 1;
     }
@@ -491,6 +494,14 @@ static void tc6393xb_update_display(void *opaque)
         tc6393xb_draw_blank(s, full_update);
     else
         tc6393xb_draw_graphic(s, full_update);
+
+    s->invalidated = 0;
+}
+
+static void tc6393xb_invalidate_display(void *opaque)
+{
+    TC6393xbState *s = opaque;
+    s->invalidated = 1;
 }
 
 
@@ -598,7 +609,7 @@ TC6393xbState *tc6393xb_init(uint32_t base, qemu_irq irq)
     s->scr_width = 480;
     s->scr_height = 640;
     s->ds = graphic_console_init(tc6393xb_update_display,
-            NULL, /* invalidate */
+            tc6393xb_invalidate_display, /* invalidate */
             NULL, /* screen_dump */
             NULL, /* text_update */
             s);
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH 1/2][RESEND] vl: initialize all displaystates

[andrzej zaborowski]

On 4 March 2011 01:48, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:
> Init not only first displaystate, but all. Otherwise machines with
> multiple display devices (e.g. tosa, as it exists now) will just
> segfault on ds switch.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  vl.c |  104 +++++++++++++++++++++++++++++++++---------------------------------
>  1 files changed, 52 insertions(+), 52 deletions(-)
>
>  Basically this patch is equal to:
>  @@ -3009,9 +3009,7 @@ int main(int argc, char **argv, char **envp)
>
>      net_check_clients();
>
>  -    /* just use the first displaystate for the moment */
>  -    ds = get_displaystate();
>  -
>  +    for (ds = get_displaystate(); ds; ds = ds->next) {
>      if (using_spice)
>          display_remote++;
>      if (display_type == DT_DEFAULT && !display_remote) {
>  @@ -3077,7 +3075,9 @@ int main(int argc, char **argv, char **envp)
>          nographic_timer = qemu_new_timer(rt_clock, nographic_update, NULL);
>          qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
>      }
>  -    text_consoles_set_display(ds);
>  +    }
>  +
>  +    text_consoles_set_display(get_displaystate());
>
>      if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
>          fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
>
> diff --git a/vl.c b/vl.c
> index 14255c4..b8cd455 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3009,75 +3009,75 @@ int main(int argc, char **argv, char **envp)
>
>     net_check_clients();
>
> -    /* just use the first displaystate for the moment */
> -    ds = get_displaystate();
> -
> -    if (using_spice)
> -        display_remote++;
> -    if (display_type == DT_DEFAULT && !display_remote) {
> +    for (ds = get_displaystate(); ds; ds = ds->next) {
> +        if (using_spice)
> +            display_remote++;
> +        if (display_type == DT_DEFAULT && !display_remote) {
>  #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
> -        display_type = DT_SDL;
> +            display_type = DT_SDL;
>  #else
> -        vnc_display = "localhost:0,to=99";
> -        show_vnc_port = 1;
> +            vnc_display = "localhost:0,to=99";
> +            show_vnc_port = 1;
>  #endif
> -    }
> -
> +        }
> +
>
> -    /* init local displays */
> -    switch (display_type) {
> -    case DT_NOGRAPHIC:
> -        break;
> +        /* init local displays */
> +        switch (display_type) {
> +        case DT_NOGRAPHIC:
> +            break;
>  #if defined(CONFIG_CURSES)
> -    case DT_CURSES:
> -        curses_display_init(ds, full_screen);
> -        break;
> +        case DT_CURSES:
> +            curses_display_init(ds, full_screen);
> +            break;
>  #endif
>  #if defined(CONFIG_SDL)
> -    case DT_SDL:
> -        sdl_display_init(ds, full_screen, no_frame);
> -        break;
> +        case DT_SDL:
> +            sdl_display_init(ds, full_screen, no_frame);
> +            break;
>  #elif defined(CONFIG_COCOA)
> -    case DT_SDL:
> -        cocoa_display_init(ds, full_screen);
> -        break;
> +        case DT_SDL:
> +            cocoa_display_init(ds, full_screen);
> +            break;

I'm not sure this will work as intended, I think we shouldn't call
curses/sdl/cocoa_display_init() for every display state, we should
just call register_displaychangelistener() etc. for each display
state.  My assumption is that we want each ds to appear as a graphical
console in the same window, not open N SDL windows / VNC servers (for
curses that would break completely I think).

Either way I'd like more people to comment on this.

Cheers

Re: [Qemu-devel] [PATCH 1/2][RESEND] vl: initialize all displaystates

[Dmitry Eremin-Solenikov]

Hello,


On 3/10/11, andrzej zaborowski <balrogg@gmail.com> wrote:
> On 4 March 2011 01:48, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:
>> Init not only first displaystate, but all. Otherwise machines with
>> multiple display devices (e.g. tosa, as it exists now) will just
>> segfault on ds switch.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> ---
>>  vl.c |  104
>> +++++++++++++++++++++++++++++++++---------------------------------
>>  1 files changed, 52 insertions(+), 52 deletions(-)
>>
>>  Basically this patch is equal to:
>>  @@ -3009,9 +3009,7 @@ int main(int argc, char **argv, char **envp)
>>
>>      net_check_clients();
>>
>>  -    /* just use the first displaystate for the moment */
>>  -    ds = get_displaystate();
>>  -
>>  +    for (ds = get_displaystate(); ds; ds = ds->next) {
>>      if (using_spice)
>>          display_remote++;
>>      if (display_type == DT_DEFAULT && !display_remote) {
>>  @@ -3077,7 +3075,9 @@ int main(int argc, char **argv, char **envp)
>>          nographic_timer = qemu_new_timer(rt_clock, nographic_update,
>> NULL);
>>          qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
>>      }
>>  -    text_consoles_set_display(ds);
>>  +    }
>>  +
>>  +    text_consoles_set_display(get_displaystate());
>>
>>      if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
>>          fprintf(stderr, "qemu: could not open gdbserver on device
>> '%s'\n",
>>
>> diff --git a/vl.c b/vl.c
>> index 14255c4..b8cd455 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3009,75 +3009,75 @@ int main(int argc, char **argv, char **envp)
>>
>>     net_check_clients();
>>
>> -    /* just use the first displaystate for the moment */
>> -    ds = get_displaystate();
>> -
>> -    if (using_spice)
>> -        display_remote++;
>> -    if (display_type == DT_DEFAULT && !display_remote) {
>> +    for (ds = get_displaystate(); ds; ds = ds->next) {
>> +        if (using_spice)
>> +            display_remote++;
>> +        if (display_type == DT_DEFAULT && !display_remote) {
>>  #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
>> -        display_type = DT_SDL;
>> +            display_type = DT_SDL;
>>  #else
>> -        vnc_display = "localhost:0,to=99";
>> -        show_vnc_port = 1;
>> +            vnc_display = "localhost:0,to=99";
>> +            show_vnc_port = 1;
>>  #endif
>> -    }
>> -
>> +        }
>> +
>>
>> -    /* init local displays */
>> -    switch (display_type) {
>> -    case DT_NOGRAPHIC:
>> -        break;
>> +        /* init local displays */
>> +        switch (display_type) {
>> +        case DT_NOGRAPHIC:
>> +            break;
>>  #if defined(CONFIG_CURSES)
>> -    case DT_CURSES:
>> -        curses_display_init(ds, full_screen);
>> -        break;
>> +        case DT_CURSES:
>> +            curses_display_init(ds, full_screen);
>> +            break;
>>  #endif
>>  #if defined(CONFIG_SDL)
>> -    case DT_SDL:
>> -        sdl_display_init(ds, full_screen, no_frame);
>> -        break;
>> +        case DT_SDL:
>> +            sdl_display_init(ds, full_screen, no_frame);
>> +            break;
>>  #elif defined(CONFIG_COCOA)
>> -    case DT_SDL:
>> -        cocoa_display_init(ds, full_screen);
>> -        break;
>> +        case DT_SDL:
>> +            cocoa_display_init(ds, full_screen);
>> +            break;
>
> I'm not sure this will work as intended, I think we shouldn't call
> curses/sdl/cocoa_display_init() for every display state, we should
> just call register_displaychangelistener() etc. for each display
> state.  My assumption is that we want each ds to appear as a graphical
> console in the same window, not open N SDL windows / VNC servers (for
> curses that would break completely I think).

I've not tested VNC/curses (will do this later), but for SDL this
works as expected:
on tosa, where qemu registers both PXA and TC6393xb displays, I've
exactly one window.
Moreover w/o this patch qemu crashes if I try to switch to secondary
graphic console (tc6393).

-- 
With best wishes
Dmitry