[Qemu-devel] [PATCH 0/3] console: text terminal updates

[Qemu-devel] [PATCH 0/3] console: text terminal updates

[Gerd Hoffmann]

  Hi,

These tree patches make the text terminal consoles support multiple
windows, i.e. work correctly in case a displaychangelistener binds
a text terminal explicitly to a fixed QemuConsole by setting dcl->con.

Use case: https://www.kraxel.org/cgit/qemu/commit/?h=rebase/ui-gtk-next&id=dc0bdb463d0b9cb0f3f2bd898ced9b9322f7d3ce

please review,
  Gerd

Gerd Hoffmann (3):
  console: update text terminal surface unconditionally
  console: rework text terminal cursor logic
  console: add kbd_put_keysym_console

 include/ui/console.h |   1 +
 ui/console.c         | 184 ++++++++++++++++++++++++++-------------------------
 2 files changed, 94 insertions(+), 91 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] console: update text terminal surface unconditionally

[Gerd Hoffmann]

These days each QemuConsole has its own private DisplaySurface,
so we can simply render updates all the time.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/console.c | 127 ++++++++++++++++++++++++++---------------------------------
 1 file changed, 56 insertions(+), 71 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index c6087d8..85f46ae 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -475,6 +475,9 @@ static inline void text_update_xy(QemuConsole *s, int x, int y)
 
 static void invalidate_xy(QemuConsole *s, int x, int y)
 {
+    if (!qemu_console_is_visible(s)) {
+        return;
+    }
     if (s->update_x0 > x * FONT_WIDTH)
         s->update_x0 = x * FONT_WIDTH;
     if (s->update_y0 > y * FONT_HEIGHT)
@@ -490,25 +493,20 @@ static void update_xy(QemuConsole *s, int x, int y)
     TextCell *c;
     int y1, y2;
 
-    if (!qemu_console_is_visible(s)) {
-        return;
-    }
-
     if (s->ds->have_text) {
         text_update_xy(s, x, y);
     }
 
-    if (s->ds->have_gfx) {
-        y1 = (s->y_base + y) % s->total_height;
-        y2 = y1 - s->y_displayed;
-        if (y2 < 0)
-            y2 += s->total_height;
-        if (y2 < s->height) {
-            c = &s->cells[y1 * s->width + x];
-            vga_putcharxy(s, x, y2, c->ch,
-                          &(c->t_attrib));
-            invalidate_xy(s, x, y2);
-        }
+    y1 = (s->y_base + y) % s->total_height;
+    y2 = y1 - s->y_displayed;
+    if (y2 < 0) {
+        y2 += s->total_height;
+    }
+    if (y2 < s->height) {
+        c = &s->cells[y1 * s->width + x];
+        vga_putcharxy(s, x, y2, c->ch,
+                      &(c->t_attrib));
+        invalidate_xy(s, x, y2);
     }
 }
 
@@ -518,33 +516,28 @@ static void console_show_cursor(QemuConsole *s, int show)
     int y, y1;
     int x = s->x;
 
-    if (!qemu_console_is_visible(s)) {
-        return;
-    }
-
     if (s->ds->have_text) {
         s->cursor_invalidate = 1;
     }
 
-    if (s->ds->have_gfx) {
-        if (x >= s->width) {
-            x = s->width - 1;
-        }
-        y1 = (s->y_base + s->y) % s->total_height;
-        y = y1 - s->y_displayed;
-        if (y < 0)
-            y += s->total_height;
-        if (y < s->height) {
-            c = &s->cells[y1 * s->width + x];
-            if (show && s->cursor_visible_phase) {
-                TextAttributes t_attrib = s->t_attrib_default;
-                t_attrib.invers = !(t_attrib.invers); /* invert fg and bg */
-                vga_putcharxy(s, x, y, c->ch, &t_attrib);
-            } else {
-                vga_putcharxy(s, x, y, c->ch, &(c->t_attrib));
-            }
-            invalidate_xy(s, x, y);
+    if (x >= s->width) {
+        x = s->width - 1;
+    }
+    y1 = (s->y_base + s->y) % s->total_height;
+    y = y1 - s->y_displayed;
+    if (y < 0) {
+        y += s->total_height;
+    }
+    if (y < s->height) {
+        c = &s->cells[y1 * s->width + x];
+        if (show && s->cursor_visible_phase) {
+            TextAttributes t_attrib = s->t_attrib_default;
+            t_attrib.invers = !(t_attrib.invers); /* invert fg and bg */
+            vga_putcharxy(s, x, y, c->ch, &t_attrib);
+        } else {
+            vga_putcharxy(s, x, y, c->ch, &(c->t_attrib));
         }
+        invalidate_xy(s, x, y);
     }
 }
 
@@ -554,10 +547,6 @@ static void console_refresh(QemuConsole *s)
     TextCell *c;
     int x, y, y1;
 
-    if (!qemu_console_is_visible(s)) {
-        return;
-    }
-
     if (s->ds->have_text) {
         s->text_x[0] = 0;
         s->text_y[0] = 0;
@@ -566,25 +555,23 @@ static void console_refresh(QemuConsole *s)
         s->cursor_invalidate = 1;
     }
 
-    if (s->ds->have_gfx) {
-        vga_fill_rect(s, 0, 0, surface_width(surface), surface_height(surface),
-                      color_table_rgb[0][COLOR_BLACK]);
-        y1 = s->y_displayed;
-        for (y = 0; y < s->height; y++) {
-            c = s->cells + y1 * s->width;
-            for (x = 0; x < s->width; x++) {
-                vga_putcharxy(s, x, y, c->ch,
-                              &(c->t_attrib));
-                c++;
-            }
-            if (++y1 == s->total_height) {
-                y1 = 0;
-            }
+    vga_fill_rect(s, 0, 0, surface_width(surface), surface_height(surface),
+                  color_table_rgb[0][COLOR_BLACK]);
+    y1 = s->y_displayed;
+    for (y = 0; y < s->height; y++) {
+        c = s->cells + y1 * s->width;
+        for (x = 0; x < s->width; x++) {
+            vga_putcharxy(s, x, y, c->ch,
+                          &(c->t_attrib));
+            c++;
+        }
+        if (++y1 == s->total_height) {
+            y1 = 0;
         }
-        console_show_cursor(s, 1);
-        dpy_gfx_update(s, 0, 0,
-                       surface_width(surface), surface_height(surface));
     }
+    console_show_cursor(s, 1);
+    dpy_gfx_update(s, 0, 0,
+                   surface_width(surface), surface_height(surface));
 }
 
 static void console_scroll(QemuConsole *s, int ydelta)
@@ -640,7 +627,7 @@ static void console_put_lf(QemuConsole *s)
             c->t_attrib = s->t_attrib_default;
             c++;
         }
-        if (qemu_console_is_visible(s) && s->y_displayed == s->y_base) {
+        if (s->y_displayed == s->y_base) {
             if (s->ds->have_text) {
                 s->text_x[0] = 0;
                 s->text_y[0] = 0;
@@ -648,18 +635,16 @@ static void console_put_lf(QemuConsole *s)
                 s->text_y[1] = s->height - 1;
             }
 
-            if (s->ds->have_gfx) {
-                vga_bitblt(s, 0, FONT_HEIGHT, 0, 0,
-                           s->width * FONT_WIDTH,
-                           (s->height - 1) * FONT_HEIGHT);
-                vga_fill_rect(s, 0, (s->height - 1) * FONT_HEIGHT,
-                              s->width * FONT_WIDTH, FONT_HEIGHT,
-                              color_table_rgb[0][s->t_attrib_default.bgcol]);
-                s->update_x0 = 0;
-                s->update_y0 = 0;
-                s->update_x1 = s->width * FONT_WIDTH;
-                s->update_y1 = s->height * FONT_HEIGHT;
-            }
+            vga_bitblt(s, 0, FONT_HEIGHT, 0, 0,
+                       s->width * FONT_WIDTH,
+                       (s->height - 1) * FONT_HEIGHT);
+            vga_fill_rect(s, 0, (s->height - 1) * FONT_HEIGHT,
+                          s->width * FONT_WIDTH, FONT_HEIGHT,
+                          color_table_rgb[0][s->t_attrib_default.bgcol]);
+            s->update_x0 = 0;
+            s->update_y0 = 0;
+            s->update_x1 = s->width * FONT_WIDTH;
+            s->update_y1 = s->height * FONT_HEIGHT;
         }
     }
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] console: rework text terminal cursor logic

[Gerd Hoffmann]

Have a global timer.  Update all visible terminal windows syncronously.
Right now this can be the active_console only, but that will change
soon.  The global timer will disable itself if not needed, so we only
have to care start it if needed.  Which might be at console switch time
or when a new displaychangelistener is registered.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/console.c | 50 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 85f46ae..f6ce0ef 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -143,8 +143,6 @@ struct QemuConsole {
     TextCell *cells;
     int text_x[2], text_y[2], cursor_invalidate;
     int echo;
-    bool cursor_visible_phase;
-    QEMUTimer *cursor_timer;
 
     int update_x0;
     int update_y0;
@@ -177,10 +175,14 @@ static DisplayState *display_state;
 static QemuConsole *active_console;
 static QemuConsole *consoles[MAX_CONSOLES];
 static int nb_consoles = 0;
+static bool cursor_visible_phase;
+static QEMUTimer *cursor_timer;
 
 static void text_console_do_init(CharDriverState *chr, DisplayState *ds);
 static void dpy_refresh(DisplayState *s);
 static DisplayState *get_alloc_displaystate(void);
+static void text_console_update_cursor_timer(void);
+static void text_console_update_cursor(void *opaque);
 
 static void gui_update(void *opaque)
 {
@@ -530,7 +532,7 @@ static void console_show_cursor(QemuConsole *s, int show)
     }
     if (y < s->height) {
         c = &s->cells[y1 * s->width + x];
-        if (show && s->cursor_visible_phase) {
+        if (show && cursor_visible_phase) {
             TextAttributes t_attrib = s->t_attrib_default;
             t_attrib.invers = !(t_attrib.invers); /* invert fg and bg */
             vga_putcharxy(s, x, y, c->ch, &t_attrib);
@@ -989,9 +991,6 @@ void console_select(unsigned int index)
     if (s) {
         DisplayState *ds = s->ds;
 
-        if (active_console && active_console->cursor_timer) {
-            timer_del(active_console->cursor_timer);
-        }
         active_console = s;
         if (ds->have_gfx) {
             QLIST_FOREACH(dcl, &ds->listeners, next) {
@@ -1008,10 +1007,7 @@ void console_select(unsigned int index)
         if (ds->have_text) {
             dpy_text_resize(s, s->width, s->height);
         }
-        if (s->cursor_timer) {
-            timer_mod(s->cursor_timer,
-                   qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + CONSOLE_CURSOR_PERIOD / 2);
-        }
+        text_console_update_cursor(NULL);
     }
 }
 
@@ -1314,6 +1310,7 @@ void register_displaychangelistener(DisplayChangeListener *dcl)
             dcl->ops->dpy_gfx_switch(dcl, dummy);
         }
     }
+    text_console_update_cursor(NULL);
 }
 
 void update_displaychangelistener(DisplayChangeListener *dcl,
@@ -1537,6 +1534,8 @@ static DisplayState *get_alloc_displaystate(void)
 {
     if (!display_state) {
         display_state = g_new0(DisplayState, 1);
+        cursor_timer = timer_new_ms(QEMU_CLOCK_REALTIME,
+                                    text_console_update_cursor, NULL);
     }
     return display_state;
 }
@@ -1696,14 +1695,32 @@ static void text_console_set_echo(CharDriverState *chr, bool echo)
     s->echo = echo;
 }
 
+static void text_console_update_cursor_timer(void)
+{
+    timer_mod(cursor_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
+              + CONSOLE_CURSOR_PERIOD / 2);
+}
+
 static void text_console_update_cursor(void *opaque)
 {
-    QemuConsole *s = opaque;
+    QemuConsole *s;
+    int i, count = 0;
+
+    cursor_visible_phase = !cursor_visible_phase;
 
-    s->cursor_visible_phase = !s->cursor_visible_phase;
-    graphic_hw_invalidate(s);
-    timer_mod(s->cursor_timer,
-                   qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + CONSOLE_CURSOR_PERIOD / 2);
+    for (i = 0; i < nb_consoles; i++) {
+        s = consoles[i];
+        if (qemu_console_is_graphic(s) ||
+            !qemu_console_is_visible(s)) {
+            continue;
+        }
+        count++;
+        graphic_hw_invalidate(s);
+    }
+
+    if (count) {
+        text_console_update_cursor_timer();
+    }
 }
 
 static const GraphicHwOps text_console_ops = {
@@ -1739,9 +1756,6 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
         s->surface = qemu_create_displaysurface(g_width, g_height);
     }
 
-    s->cursor_timer =
-        timer_new_ms(QEMU_CLOCK_REALTIME, text_console_update_cursor, s);
-
     s->hw_ops = &text_console_ops;
     s->hw = s;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] console: add kbd_put_keysym_console

[Gerd Hoffmann]

So you can send keysyms to a specific (text terminal) console.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/console.h | 1 +
 ui/console.c         | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 8a86617..b513e20 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -81,6 +81,7 @@ void do_mouse_set(Monitor *mon, const QDict *qdict);
 #define QEMU_KEY_CTRL_PAGEUP     0xe406
 #define QEMU_KEY_CTRL_PAGEDOWN   0xe407
 
+void kbd_put_keysym_console(QemuConsole *s, int keysym);
 void kbd_put_keysym(int keysym);
 
 /* consoles */
diff --git a/ui/console.c b/ui/console.c
index f6ce0ef..75ec3af 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1056,13 +1056,11 @@ static void kbd_send_chars(void *opaque)
 }
 
 /* called when an ascii key is pressed */
-void kbd_put_keysym(int keysym)
+void kbd_put_keysym_console(QemuConsole *s, int keysym)
 {
-    QemuConsole *s;
     uint8_t buf[16], *q;
     int c;
 
-    s = active_console;
     if (!s || (s->console_type == GRAPHIC_CONSOLE))
         return;
 
@@ -1111,6 +1109,11 @@ void kbd_put_keysym(int keysym)
     }
 }
 
+void kbd_put_keysym(int keysym)
+{
+    kbd_put_keysym_console(active_console, keysym);
+}
+
 static void text_console_invalidate(void *opaque)
 {
     QemuConsole *s = (QemuConsole *) opaque;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/3] console: rework text terminal cursor logic

[Ian Campbell]

On Thu, 2014-05-22 at 13:00 +0200, Gerd Hoffmann wrote:
> Have a global timer.  Update all visible terminal windows syncronously.
> Right now this can be the active_console only, but that will change
> soon.  The global timer will disable itself if not needed, so we only
> have to care start it if needed.  Which might be at console switch time
> or when a new displaychangelistener is registered.

Running current master (d7d3d6092cb7) I'm seeing a segmentation fault
while running:
        ./aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57
        -sdl
which goes away if I revert this patch. qemu is configured with
        ./configure --target-list=aarch64-softmmu  --enable-sdl

The backtrace shows that the timer is NULL.

Program received signal SIGSEGV, Segmentation fault.
0x0000555555778204 in timer_mod (ts=0x0, expire_time=662170703) at qemu-timer.c:442
442	    timer_mod_ns(ts, expire_time * ts->scale);
(gdb) bt
#0  0x0000555555778204 in timer_mod (ts=0x0, expire_time=662170703) at qemu-timer.c:442
#1  0x0000555555796130 in text_console_update_cursor_timer () at ui/console.c:1703
#2  text_console_update_cursor (opaque=opaque@entry=0x0) at ui/console.c:1725
#3  0x0000555555798feb in register_displaychangelistener (dcl=<optimized out>) at ui/console.c:1316
#4  0x00005555557a250f in sdl_display_init (ds=ds@entry=0x5555564c0d00, full_screen=-446133248, no_frame=<optimized out>)
    at ui/sdl.c:946
#5  0x00005555555f50a7 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4475

Cheers,
Ian.

Re: [Qemu-devel] [PATCH 2/3] console: rework text terminal cursor logic

[Gerd Hoffmann]

On So, 2014-06-08 at 11:30 +0100, Ian Campbell wrote:
> On Thu, 2014-05-22 at 13:00 +0200, Gerd Hoffmann wrote:
> > Have a global timer.  Update all visible terminal windows syncronously.
> > Right now this can be the active_console only, but that will change
> > soon.  The global timer will disable itself if not needed, so we only
> > have to care start it if needed.  Which might be at console switch time
> > or when a new displaychangelistener is registered.
> 
> Running current master (d7d3d6092cb7) I'm seeing a segmentation fault
> while running:
>         ./aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57
>         -sdl
> which goes away if I revert this patch. qemu is configured with
>         ./configure --target-list=aarch64-softmmu  --enable-sdl
> 
> The backtrace shows that the timer is NULL.

http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00106.html

High time to get the pull request out of the door ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] console: rework text terminal cursor logic

[Ian Campbell]

On Tue, 2014-06-10 at 09:44 +0200, Gerd Hoffmann wrote:
> On So, 2014-06-08 at 11:30 +0100, Ian Campbell wrote:
> > On Thu, 2014-05-22 at 13:00 +0200, Gerd Hoffmann wrote:
> > > Have a global timer.  Update all visible terminal windows syncronously.
> > > Right now this can be the active_console only, but that will change
> > > soon.  The global timer will disable itself if not needed, so we only
> > > have to care start it if needed.  Which might be at console switch time
> > > or when a new displaychangelistener is registered.
> > 
> > Running current master (d7d3d6092cb7) I'm seeing a segmentation fault
> > while running:
> >         ./aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57
> >         -sdl
> > which goes away if I revert this patch. qemu is configured with
> >         ./configure --target-list=aarch64-softmmu  --enable-sdl
> > 
> > The backtrace shows that the timer is NULL.
> 
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00106.html

Aha, thanks!

> High time to get the pull request out of the door ...

Please ;-)

Ian.