Re: [Qemu-devel] [PATCH] console: Implementing blinking of cursor

[Alon Levy]

On Tue, Jul 03, 2012 at 11:05:50AM +0200, Jan Kiszka wrote:
> On 2012-07-03 10:59, Alon Levy wrote:
> > On Mon, Jul 02, 2012 at 10:20:17AM +0200, Jan Kiszka wrote:
> > 
> > One comment below.
> > 
> > Reviewed-by: Alon Levy <address@hidden>
> > 
> >> Let the text console cursor blink at 5 HZ.
> >>
> >> Signed-off-by: Jan Kiszka <address@hidden>
> >> ---
> >>  console.c |   26 +++++++++++++++++++++++++-
> >>  1 files changed, 25 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/console.c b/console.c
> >> index 6a463f5..29b0f1c 100644
> >> --- a/console.c
> >> +++ b/console.c
> >> @@ -28,6 +28,7 @@
> >>  //#define DEBUG_CONSOLE
> >>  #define DEFAULT_BACKSCROLL 512
> >>  #define MAX_CONSOLES 12
> >> +#define CONSOLE_CURSOR_PERIOD 200
> >>  
> >>  #define QEMU_RGBA(r, g, b, a) (((a) << 24) | ((r) << 16) | ((g) << 8) | 
> >> (b))
> >>  #define QEMU_RGB(r, g, b) QEMU_RGBA(r, g, b, 0xff)
> >> @@ -139,6 +140,8 @@ struct TextConsole {
> >>      TextCell *cells;
> >>      int text_x[2], text_y[2], cursor_invalidate;
> >>      int echo;
> >> +    int cursor_blink_state;
> >> +    QEMUTimer *cursor_timer;
> >>  
> >>      int update_x0;
> >>      int update_y0;
> >> @@ -615,7 +618,7 @@ static void console_show_cursor(TextConsole *s, int 
> >> show)
> >>              y += s->total_height;
> >>          if (y < s->height) {
> >>              c = &s->cells[y1 * s->width + x];
> >> -            if (show) {
> >> +            if (show && s->cursor_blink_state) {
> >>                  TextAttributes t_attrib = s->t_attrib_default;
> >>                  t_attrib.invers = !(t_attrib.invers); /* invert fg and bg 
> >> */
> >>                  vga_putcharxy(s->ds, x, y, c->ch, &t_attrib);
> >> @@ -1083,6 +1086,10 @@ void console_select(unsigned int index)
> >>      s = consoles[index];
> >>      if (s) {
> >>          DisplayState *ds = s->ds;
> >> +
> >> +        if (active_console->cursor_timer) {
> >> +            qemu_del_timer(active_console->cursor_timer);
> >> +        }
> >>          active_console = s;
> >>          if (ds_get_bits_per_pixel(s->ds)) {
> >>              ds->surface = qemu_resize_displaysurface(ds, s->g_width, 
> >> s->g_height);
> >> @@ -1090,6 +1097,10 @@ void console_select(unsigned int index)
> >>              s->ds->surface->width = s->width;
> >>              s->ds->surface->height = s->height;
> >>          }
> >> +        if (s->cursor_timer) {
> >> +            qemu_mod_timer(s->cursor_timer,
> >> +                   qemu_get_clock_ms(rt_clock) + CONSOLE_CURSOR_PERIOD);
> >> +        }
> >>          dpy_resize(s->ds);
> >>          vga_hw_invalidate();
> >>      }
> >> @@ -1454,6 +1465,16 @@ static void text_console_set_echo(CharDriverState 
> >> *chr, bool echo)
> >>      s->echo = echo;
> >>  }
> >>  
> >> +static void text_console_update_cursor(void *opaque)
> >> +{
> >> +    TextConsole *s = opaque;
> >> +
> >> +    s->cursor_blink_state ^= 1;
> >> +    vga_hw_invalidate();
> >> +    qemu_mod_timer(s->cursor_timer,
> >> +                   qemu_get_clock_ms(rt_clock) + CONSOLE_CURSOR_PERIOD);
> >> +}
> >> +
> >>  static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
> >>  {
> >>      TextConsole *s;
> >> @@ -1482,6 +1503,9 @@ static void text_console_do_init(CharDriverState 
> >> *chr, DisplayState *ds)
> >>          s->g_height = ds_get_height(s->ds);
> >>      }
> >>  
> >> +    s->cursor_timer =
> >> +        qemu_new_timer_ms(rt_clock, text_console_update_cursor, s);
> >> +
> > missing initialization of cursor_blink_state
> 
> TextConsole is zero-initialized, so it does not really matter. Can add
> it if preferred though.

I just thought because you initialized the same named field in the vga patch.
No need if everything is zero initialized, missed that.

> 
> Thanks for reviewing,
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux
> 
> 
>