[PATCH] ioemu: improve colordepth negociation

[PATCH] ioemu: improve colordepth negociation

[Samuel Thibault]

ioemu: improve colordepth negociation
By moving the colourdepth callback a bit earlier, we can let the display
decide the actual depth to be used before the draw and whether sharing
is possible or not.

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r a9b63386b636 tools/ioemu/hw/vga.c
--- a/tools/ioemu/hw/vga.c	Mon Mar 03 17:24:25 2008 +0000
+++ b/tools/ioemu/hw/vga.c	Mon Mar 03 17:32:49 2008 +0000
@@ -1061,6 +1061,92 @@ static const uint8_t cursor_glyph[32 * 4
     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
 };    
 
+enum {
+    VGA_DRAW_LINE2,
+    VGA_DRAW_LINE2D2,
+    VGA_DRAW_LINE4,
+    VGA_DRAW_LINE4D2,
+    VGA_DRAW_LINE8D2,
+    VGA_DRAW_LINE8,
+    VGA_DRAW_LINE15,
+    VGA_DRAW_LINE16,
+    VGA_DRAW_LINE24,
+    VGA_DRAW_LINE32,
+    VGA_DRAW_LINE_NB,
+};
+
+static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
+    vga_draw_line2_8,
+    vga_draw_line2_16,
+    vga_draw_line2_16,
+    vga_draw_line2_32,
+    vga_draw_line2_32,
+
+    vga_draw_line2d2_8,
+    vga_draw_line2d2_16,
+    vga_draw_line2d2_16,
+    vga_draw_line2d2_32,
+    vga_draw_line2d2_32,
+
+    vga_draw_line4_8,
+    vga_draw_line4_16,
+    vga_draw_line4_16,
+    vga_draw_line4_32,
+    vga_draw_line4_32,
+
+    vga_draw_line4d2_8,
+    vga_draw_line4d2_16,
+    vga_draw_line4d2_16,
+    vga_draw_line4d2_32,
+    vga_draw_line4d2_32,
+
+    vga_draw_line8d2_8,
+    vga_draw_line8d2_16,
+    vga_draw_line8d2_16,
+    vga_draw_line8d2_32,
+    vga_draw_line8d2_32,
+
+    vga_draw_line8_8,
+    vga_draw_line8_16,
+    vga_draw_line8_16,
+    vga_draw_line8_32,
+    vga_draw_line8_32,
+
+    vga_draw_line15_8,
+    vga_draw_line15_15,
+    vga_draw_line15_16,
+    vga_draw_line15_32,
+    vga_draw_line15_32bgr,
+
+    vga_draw_line16_8,
+    vga_draw_line16_15,
+    vga_draw_line16_16,
+    vga_draw_line16_32,
+    vga_draw_line16_32bgr,
+
+    vga_draw_line24_8,
+    vga_draw_line24_15,
+    vga_draw_line24_16,
+    vga_draw_line24_32,
+    vga_draw_line24_32bgr,
+
+    vga_draw_line32_8,
+    vga_draw_line32_15,
+    vga_draw_line32_16,
+    vga_draw_line32_32,
+    vga_draw_line32_32bgr,
+};
+
+typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g, unsigned b);
+
+static rgb_to_pixel_dup_func *rgb_to_pixel_dup_table[NB_DEPTHS] = {
+    rgb_to_pixel8_dup,
+    rgb_to_pixel15_dup,
+    rgb_to_pixel16_dup,
+    rgb_to_pixel32_dup,
+    rgb_to_pixel32bgr_dup,
+};
+
 /* 
  * Text mode update 
  * Missing:
@@ -1081,6 +1167,12 @@ static void vga_draw_text(VGAState *s, i
     uint32_t *ch_attr_ptr;
     vga_draw_glyph8_func *vga_draw_glyph8;
     vga_draw_glyph9_func *vga_draw_glyph9;
+
+    depth = s->get_bpp(s);
+    if (s->ds->dpy_colourdepth != NULL && s->ds->depth != depth)
+        s->ds->dpy_colourdepth(s->ds, depth);
+    s->rgb_to_pixel = 
+        rgb_to_pixel_dup_table[get_depth_index(s->ds)];
 
     full_update |= update_palette16(s);
     palette = s->last_palette;
@@ -1134,9 +1226,6 @@ static void vga_draw_text(VGAState *s, i
         return;
     }
 
-    depth = s->get_bpp(s);
-    if (s->ds->dpy_colourdepth != NULL && s->ds->depth != depth)
-        s->ds->dpy_colourdepth(s->ds, depth);
     if (width != s->last_width || height != s->last_height ||
         cw != s->last_cw || cheight != s->last_ch) {
         s->last_scr_width = width * cw;
@@ -1242,92 +1331,6 @@ static void vga_draw_text(VGAState *s, i
         s1 += line_offset;
     }
 }
-
-enum {
-    VGA_DRAW_LINE2,
-    VGA_DRAW_LINE2D2,
-    VGA_DRAW_LINE4,
-    VGA_DRAW_LINE4D2,
-    VGA_DRAW_LINE8D2,
-    VGA_DRAW_LINE8,
-    VGA_DRAW_LINE15,
-    VGA_DRAW_LINE16,
-    VGA_DRAW_LINE24,
-    VGA_DRAW_LINE32,
-    VGA_DRAW_LINE_NB,
-};
-
-static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
-    vga_draw_line2_8,
-    vga_draw_line2_16,
-    vga_draw_line2_16,
-    vga_draw_line2_32,
-    vga_draw_line2_32,
-
-    vga_draw_line2d2_8,
-    vga_draw_line2d2_16,
-    vga_draw_line2d2_16,
-    vga_draw_line2d2_32,
-    vga_draw_line2d2_32,
-
-    vga_draw_line4_8,
-    vga_draw_line4_16,
-    vga_draw_line4_16,
-    vga_draw_line4_32,
-    vga_draw_line4_32,
-
-    vga_draw_line4d2_8,
-    vga_draw_line4d2_16,
-    vga_draw_line4d2_16,
-    vga_draw_line4d2_32,
-    vga_draw_line4d2_32,
-
-    vga_draw_line8d2_8,
-    vga_draw_line8d2_16,
-    vga_draw_line8d2_16,
-    vga_draw_line8d2_32,
-    vga_draw_line8d2_32,
-
-    vga_draw_line8_8,
-    vga_draw_line8_16,
-    vga_draw_line8_16,
-    vga_draw_line8_32,
-    vga_draw_line8_32,
-
-    vga_draw_line15_8,
-    vga_draw_line15_15,
-    vga_draw_line15_16,
-    vga_draw_line15_32,
-    vga_draw_line15_32bgr,
-
-    vga_draw_line16_8,
-    vga_draw_line16_15,
-    vga_draw_line16_16,
-    vga_draw_line16_32,
-    vga_draw_line16_32bgr,
-
-    vga_draw_line24_8,
-    vga_draw_line24_15,
-    vga_draw_line24_16,
-    vga_draw_line24_32,
-    vga_draw_line24_32bgr,
-
-    vga_draw_line32_8,
-    vga_draw_line32_15,
-    vga_draw_line32_16,
-    vga_draw_line32_32,
-    vga_draw_line32_32bgr,
-};
-
-typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g, unsigned b);
-
-static rgb_to_pixel_dup_func *rgb_to_pixel_dup_table[NB_DEPTHS] = {
-    rgb_to_pixel8_dup,
-    rgb_to_pixel15_dup,
-    rgb_to_pixel16_dup,
-    rgb_to_pixel32_dup,
-    rgb_to_pixel32bgr_dup,
-};
 
 static int vga_get_bpp(VGAState *s)
 {
@@ -1494,6 +1497,16 @@ static void vga_draw_graphic(VGAState *s
     s->get_resolution(s, &width, &height);
     disp_width = width;
 
+    changed_flag = 0;
+    depth = s->get_bpp(s);
+    if (s->ds->dpy_colourdepth != NULL && 
+            (s->ds->depth != depth || !s->ds->shared_buf)) {
+        s->ds->dpy_colourdepth(s->ds, depth);
+        changed_flag = 1;
+    }
+    s->rgb_to_pixel = 
+        rgb_to_pixel_dup_table[get_depth_index(s->ds)];
+
     shift_control = (s->gr[0x05] >> 5) & 3;
     double_scan = (s->cr[0x09] >> 7);
     if (shift_control != 1) {
@@ -1552,15 +1565,8 @@ static void vga_draw_graphic(VGAState *s
             break;
         }
     }
+
     vga_draw_line = vga_draw_line_table[v * NB_DEPTHS + get_depth_index(s->ds)];
-
-    changed_flag = 0;
-    depth = s->get_bpp(s);
-    if (s->ds->dpy_colourdepth != NULL && 
-            (s->ds->depth != depth || !s->ds->shared_buf)) {
-        s->ds->dpy_colourdepth(s->ds, depth);
-        changed_flag = 1;
-    }
     if (disp_width != s->last_width ||
         height != s->last_height) {
         dpy_resize(s->ds, disp_width, height, disp_width * (depth / 8));
@@ -1674,6 +1680,8 @@ static void vga_draw_blank(VGAState *s, 
         return;
     if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
         return;
+    s->rgb_to_pixel = 
+        rgb_to_pixel_dup_table[get_depth_index(s->ds)];
     if (s->ds->depth == 8) 
         val = s->rgb_to_pixel(0, 0, 0);
     else
@@ -1700,9 +1708,6 @@ static void vga_update_display(void *opa
     if (s->ds->depth == 0) {
         /* nothing to do */
     } else {
-        s->rgb_to_pixel = 
-            rgb_to_pixel_dup_table[get_depth_index(s->ds)];
-        
         full_update = 0;
         if (!(s->ar_index & 0x20)) {
             graphic_mode = GMODE_BLANK;
diff -r a9b63386b636 tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c	Mon Mar 03 17:24:25 2008 +0000
+++ b/tools/ioemu/hw/xenfb.c	Mon Mar 03 17:32:49 2008 +0000
@@ -1238,10 +1238,16 @@ static void xenfb_pv_resize(DisplayState
     fbfront_update(fb_dev, 0, 0, WIDTH, HEIGHT);
 }
 
-static void xenfb_pv_colourdepth(DisplayState *s, int depth)
+static void xenfb_pv_colourdepth(DisplayState *ds, int depth)
 {
     /* TODO: send redepth event if supported */
-    fprintf(stderr,"redepth to %d required\n", depth);
+    static int lastdepth = -1;
+    if (depth != lastdepth) {
+        fprintf(stderr,"redepth to %d required\n", depth);
+        lastdepth = depth;
+    }
+    /* We can't redepth for now */
+    ds->depth = DEPTH;
 }
 
 static void xenfb_kbd_handler(void *opaque)
@@ -1334,6 +1340,8 @@ static void xenfb_kbd_handler(void *opaq
 
 static void xenfb_pv_refresh(DisplayState *ds)
 {
+    /* always request negociation */
+    ds->depth = -1;
     vga_hw_update();
 }
 
@@ -1387,7 +1395,7 @@ int xenfb_pv_display_init(DisplayState *
     ds->height = HEIGHT;
     ds->dpy_update = xenfb_pv_update;
     ds->dpy_resize = xenfb_pv_resize;
-    ds->dpy_colourdepth = NULL; //xenfb_pv_colourdepth;
+    ds->dpy_colourdepth = xenfb_pv_colourdepth;
     ds->dpy_refresh = xenfb_pv_refresh;
     ds->opaque = fb_dev;
     return 0;

Re: [PATCH] ioemu: improve colordepth negociation

[Ian Jackson]

Samuel Thibault writes ("[Xen-devel] [PATCH] ioemu: improve colordepth negociation"):
> +static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
> +    vga_draw_line2_8,
> +    vga_draw_line2_16,
> +    vga_draw_line2_16,
> +    vga_draw_line2_32,
> +    vga_draw_line2_32,
...
> -static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
> -    vga_draw_line2_8,
> -    vga_draw_line2_16,
> -    vga_draw_line2_16,
> -    vga_draw_line2_32,
> -    vga_draw_line2_32,

Could I suggest that it would be a good idea to avoid enormous code
rearrangements like this in ioemu ?  That's only going to make the
ongoing merge situation with qemu worse.

If you were moving this up the file so that you could refer to it
later, you could provide a forward declaration.

Ian.

Re: [PATCH] ioemu: improve colordepth negociation

[Samuel Thibault]

Ian Jackson, le Tue 04 Mar 2008 11:38:44 +0000, a écrit :
> Samuel Thibault writes ("[Xen-devel] [PATCH] ioemu: improve colordepth negociation"):
> > +static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
> > +    vga_draw_line2_8,
> > +    vga_draw_line2_16,
> > +    vga_draw_line2_16,
> > +    vga_draw_line2_32,
> > +    vga_draw_line2_32,
> ...
> > -static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
> > -    vga_draw_line2_8,
> > -    vga_draw_line2_16,
> > -    vga_draw_line2_16,
> > -    vga_draw_line2_32,
> > -    vga_draw_line2_32,
> 
> Could I suggest that it would be a good idea to avoid enormous code
> rearrangements like this in ioemu ?  That's only going to make the
> ongoing merge situation with qemu worse.
> 
> If you were moving this up the file so that you could refer to it
> later, you could provide a forward declaration.

I was wondering about it indeed.  I'll provide another patch.

Samuel

Re: [PATCH] ioemu: improve colordepth negociation

[Ian Jackson]

Samuel Thibault writes ("Re: [Xen-devel] [PATCH] ioemu: improve colordepth negociation"):
> Ian Jackson, le Tue 04 Mar 2008 11:38:44 +0000, a écrit :
> > If you were moving this up the file so that you could refer to it
> > later, you could provide a forward declaration.
> 
> I was wondering about it indeed.  I'll provide another patch.

Thanks,
Ian.

[PATCH2] ioemu: improve colordepth negociation

[Samuel Thibault]

Samuel Thibault, le Tue 04 Mar 2008 11:44:10 +0000, a écrit :
> Ian Jackson, le Tue 04 Mar 2008 11:38:44 +0000, a écrit :
> > Samuel Thibault writes ("[Xen-devel] [PATCH] ioemu: improve colordepth negociation"):
> > > +static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
> > > +    vga_draw_line2_8,
> > > +    vga_draw_line2_16,
> > > +    vga_draw_line2_16,
> > > +    vga_draw_line2_32,
> > > +    vga_draw_line2_32,
> > ...
> > > -static vga_draw_line_func *vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_NB] = {
> > > -    vga_draw_line2_8,
> > > -    vga_draw_line2_16,
> > > -    vga_draw_line2_16,
> > > -    vga_draw_line2_32,
> > > -    vga_draw_line2_32,
> > 
> > Could I suggest that it would be a good idea to avoid enormous code
> > rearrangements like this in ioemu ?
> 
> I was wondering about it indeed.  I'll provide another patch.

Here it is:

ioemu: improve colordepth negociation
By moving the colourdepth callback a bit earlier, we can let the display
decide the actual depth to be used before the draw and whether sharing
is possible or not.

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r a9b63386b636 tools/ioemu/hw/vga.c
--- a/tools/ioemu/hw/vga.c	Mon Mar 03 17:24:25 2008 +0000
+++ b/tools/ioemu/hw/vga.c	Tue Mar 04 11:47:11 2008 +0000
@@ -1061,6 +1061,10 @@ static const uint8_t cursor_glyph[32 * 4
     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
 };    
 
+typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g, unsigned b);
+
+static rgb_to_pixel_dup_func *rgb_to_pixel_dup_table[NB_DEPTHS];
+
 /* 
  * Text mode update 
  * Missing:
@@ -1081,6 +1085,12 @@ static void vga_draw_text(VGAState *s, i
     uint32_t *ch_attr_ptr;
     vga_draw_glyph8_func *vga_draw_glyph8;
     vga_draw_glyph9_func *vga_draw_glyph9;
+
+    depth = s->get_bpp(s);
+    if (s->ds->dpy_colourdepth != NULL && s->ds->depth != depth)
+        s->ds->dpy_colourdepth(s->ds, depth);
+    s->rgb_to_pixel = 
+        rgb_to_pixel_dup_table[get_depth_index(s->ds)];
 
     full_update |= update_palette16(s);
     palette = s->last_palette;
@@ -1134,9 +1144,6 @@ static void vga_draw_text(VGAState *s, i
         return;
     }
 
-    depth = s->get_bpp(s);
-    if (s->ds->dpy_colourdepth != NULL && s->ds->depth != depth)
-        s->ds->dpy_colourdepth(s->ds, depth);
     if (width != s->last_width || height != s->last_height ||
         cw != s->last_cw || cheight != s->last_ch) {
         s->last_scr_width = width * cw;
@@ -1318,8 +1325,6 @@ static vga_draw_line_func *vga_draw_line
     vga_draw_line32_32,
     vga_draw_line32_32bgr,
 };
-
-typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g, unsigned b);
 
 static rgb_to_pixel_dup_func *rgb_to_pixel_dup_table[NB_DEPTHS] = {
     rgb_to_pixel8_dup,
@@ -1494,6 +1499,16 @@ static void vga_draw_graphic(VGAState *s
     s->get_resolution(s, &width, &height);
     disp_width = width;
 
+    changed_flag = 0;
+    depth = s->get_bpp(s);
+    if (s->ds->dpy_colourdepth != NULL && 
+            (s->ds->depth != depth || !s->ds->shared_buf)) {
+        s->ds->dpy_colourdepth(s->ds, depth);
+        changed_flag = 1;
+    }
+    s->rgb_to_pixel = 
+        rgb_to_pixel_dup_table[get_depth_index(s->ds)];
+
     shift_control = (s->gr[0x05] >> 5) & 3;
     double_scan = (s->cr[0x09] >> 7);
     if (shift_control != 1) {
@@ -1552,15 +1567,8 @@ static void vga_draw_graphic(VGAState *s
             break;
         }
     }
+
     vga_draw_line = vga_draw_line_table[v * NB_DEPTHS + get_depth_index(s->ds)];
-
-    changed_flag = 0;
-    depth = s->get_bpp(s);
-    if (s->ds->dpy_colourdepth != NULL && 
-            (s->ds->depth != depth || !s->ds->shared_buf)) {
-        s->ds->dpy_colourdepth(s->ds, depth);
-        changed_flag = 1;
-    }
     if (disp_width != s->last_width ||
         height != s->last_height) {
         dpy_resize(s->ds, disp_width, height, disp_width * (depth / 8));
@@ -1674,6 +1682,8 @@ static void vga_draw_blank(VGAState *s, 
         return;
     if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
         return;
+    s->rgb_to_pixel = 
+        rgb_to_pixel_dup_table[get_depth_index(s->ds)];
     if (s->ds->depth == 8) 
         val = s->rgb_to_pixel(0, 0, 0);
     else
@@ -1700,9 +1710,6 @@ static void vga_update_display(void *opa
     if (s->ds->depth == 0) {
         /* nothing to do */
     } else {
-        s->rgb_to_pixel = 
-            rgb_to_pixel_dup_table[get_depth_index(s->ds)];
-        
         full_update = 0;
         if (!(s->ar_index & 0x20)) {
             graphic_mode = GMODE_BLANK;
diff -r a9b63386b636 tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c	Mon Mar 03 17:24:25 2008 +0000
+++ b/tools/ioemu/hw/xenfb.c	Tue Mar 04 11:47:11 2008 +0000
@@ -1238,10 +1238,16 @@ static void xenfb_pv_resize(DisplayState
     fbfront_update(fb_dev, 0, 0, WIDTH, HEIGHT);
 }
 
-static void xenfb_pv_colourdepth(DisplayState *s, int depth)
+static void xenfb_pv_colourdepth(DisplayState *ds, int depth)
 {
     /* TODO: send redepth event if supported */
-    fprintf(stderr,"redepth to %d required\n", depth);
+    static int lastdepth = -1;
+    if (depth != lastdepth) {
+        fprintf(stderr,"redepth to %d required\n", depth);
+        lastdepth = depth;
+    }
+    /* We can't redepth for now */
+    ds->depth = DEPTH;
 }
 
 static void xenfb_kbd_handler(void *opaque)
@@ -1334,6 +1340,8 @@ static void xenfb_kbd_handler(void *opaq
 
 static void xenfb_pv_refresh(DisplayState *ds)
 {
+    /* always request negociation */
+    ds->depth = -1;
     vga_hw_update();
 }
 
@@ -1387,7 +1395,7 @@ int xenfb_pv_display_init(DisplayState *
     ds->height = HEIGHT;
     ds->dpy_update = xenfb_pv_update;
     ds->dpy_resize = xenfb_pv_resize;
-    ds->dpy_colourdepth = NULL; //xenfb_pv_colourdepth;
+    ds->dpy_colourdepth = xenfb_pv_colourdepth;
     ds->dpy_refresh = xenfb_pv_refresh;
     ds->opaque = fb_dev;
     return 0;