* [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. @ 2013-03-15 18:14 Frediano Ziglio 2013-03-19 13:56 ` Stefano Stabellini ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Frediano Ziglio @ 2013-03-15 18:14 UTC (permalink / raw) To: Anthony Liguori, Stefano Stabellini, Fabio Fantoni; +Cc: xen-devel, qemu-devel Modern notebook support 136x768 resolution. The resolution width is not multiple of 16 causing some problems. Qemu VGA emulation require width resolution to be multiple of 8. VNC implementation require width resolution to be multiple of 16. This patch remove these limits. Was tested with a Windows machine with standard vga and 1366x768 as resolution. I had to update vgabios as version in qemu (pc-bios/vgabios-stdvga.bin) is quite old. I also had to add some patches on top of VGABIOS 0.7a to add some new resolutions. I have some doubt about this patch - are other UI (sdl, cocoa, qxl) happy if resolution is not multiple of 16 ? - scanline is computed exactly without any alignment (so 1366 8 bit is 1366 bytes) while getting vesa information from a laptop it seems to use some kind of alignment (if became 0x580 which is 1408 bytes). Perhaps should I change either VGABIOS and Qemu to make this alignment? Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> --- hw/vga.c | 2 +- ui/vnc.c | 27 +++++++++++++-------------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index 1caf23d..d229f06 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -651,7 +651,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) } break; case VBE_DISPI_INDEX_XRES: - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { s->vbe_regs[s->vbe_index] = val; } break; diff --git a/ui/vnc.c b/ui/vnc.c index ff4e2ae..328d14d 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -907,26 +907,27 @@ static int vnc_update_client(VncState *vs, int has_dirty) for (y = 0; y < height; y++) { int x; int last_x = -1; - for (x = 0; x < width / 16; x++) { - if (test_and_clear_bit(x, vs->dirty[y])) { + for (x = 0; x < width; x += 16) { + if (test_and_clear_bit(x/16, vs->dirty[y])) { if (last_x == -1) { last_x = x; } } else { if (last_x != -1) { - int h = find_and_clear_dirty_height(vs, y, last_x, x, + int h = find_and_clear_dirty_height(vs, y, last_x/16, x/16, height); - n += vnc_job_add_rect(job, last_x * 16, y, - (x - last_x) * 16, h); + n += vnc_job_add_rect(job, last_x, y, + (x - last_x), h); } last_x = -1; } } if (last_x != -1) { - int h = find_and_clear_dirty_height(vs, y, last_x, x, height); - n += vnc_job_add_rect(job, last_x * 16, y, - (x - last_x) * 16, h); + int h = find_and_clear_dirty_height(vs, y, last_x/16, x/16, height); + if (x > width) x = width; + n += vnc_job_add_rect(job, last_x, y, + (x - last_x), h); } } @@ -1771,7 +1772,7 @@ static void framebuffer_update_request(VncState *vs, int incremental, int w, int h) { int i; - const size_t width = ds_get_width(vs->ds) / 16; + const size_t width = (ds_get_width(vs->ds)+15) / 16; if (y_position > ds_get_height(vs->ds)) y_position = ds_get_height(vs->ds); @@ -2595,10 +2596,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd) * Check and copy modified bits from guest to server surface. * Update server dirty map. */ - cmp_bytes = 64; - if (cmp_bytes > vnc_server_fb_stride(vd)) { - cmp_bytes = vnc_server_fb_stride(vd); - } if (vd->guest.format != VNC_SERVER_FB_FORMAT) { int width = pixman_image_get_width(vd->server); tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width); @@ -2619,8 +2616,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd) } server_ptr = server_row; - for (x = 0; x + 15 < width; + cmp_bytes = 64; + for (x = 0; x < width; x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) { + if (width - x < 16) cmp_bytes = 4 * (width - x); if (!test_and_clear_bit((x / 16), vd->guest.dirty[y])) continue; if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-03-15 18:14 [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly Frediano Ziglio @ 2013-03-19 13:56 ` Stefano Stabellini 2013-05-09 14:20 ` [Qemu-devel] [Xen-devel] " Pasi Kärkkäinen 2013-05-10 5:52 ` [Qemu-devel] " Andreas Färber 2 siblings, 0 replies; 10+ messages in thread From: Stefano Stabellini @ 2013-03-19 13:56 UTC (permalink / raw) To: Frediano Ziglio Cc: qemu-devel, Anthony Liguori, Fabio Fantoni, xen-devel@lists.xensource.com, Stefano Stabellini On Fri, 15 Mar 2013, Frediano Ziglio wrote: > Modern notebook support 136x768 resolution. The resolution width is ^this can't be right > not multiple of 16 causing some problems. > > Qemu VGA emulation require width resolution to be multiple of 8. > > VNC implementation require width resolution to be multiple of 16. > > This patch remove these limits. Was tested with a Windows machine with > standard vga and 1366x768 as resolution. I had to update vgabios as > version in qemu (pc-bios/vgabios-stdvga.bin) is quite old. I also had > to add some patches on top of VGABIOS 0.7a to add some new > resolutions. > > I have some doubt about this patch > - are other UI (sdl, cocoa, qxl) happy if resolution is not multiple of 16 ? It's worth testing at least sdl and gtk > - scanline is computed exactly without any alignment (so 1366 8 bit is > 1366 bytes) while getting vesa information from a laptop it seems to > use some kind of alignment (if became 0x580 which is 1408 bytes). > Perhaps should I change either VGABIOS and Qemu to make this > alignment? I don't think there are any requirements to set the scanline bigger than width*bpp > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > --- > hw/vga.c | 2 +- > ui/vnc.c | 27 +++++++++++++-------------- > 2 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/hw/vga.c b/hw/vga.c > index 1caf23d..d229f06 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -651,7 +651,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t > addr, uint32_t val) > } > break; > case VBE_DISPI_INDEX_XRES: > - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > s->vbe_regs[s->vbe_index] = val; > } > break; > diff --git a/ui/vnc.c b/ui/vnc.c > index ff4e2ae..328d14d 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -907,26 +907,27 @@ static int vnc_update_client(VncState *vs, int has_dirty) > for (y = 0; y < height; y++) { > int x; > int last_x = -1; > - for (x = 0; x < width / 16; x++) { > - if (test_and_clear_bit(x, vs->dirty[y])) { > + for (x = 0; x < width; x += 16) { > + if (test_and_clear_bit(x/16, vs->dirty[y])) { > if (last_x == -1) { > last_x = x; > } > } else { > if (last_x != -1) { > - int h = find_and_clear_dirty_height(vs, y, last_x, x, > + int h = find_and_clear_dirty_height(vs, y, > last_x/16, x/16, your mail client must have messed up this patch > height); > > - n += vnc_job_add_rect(job, last_x * 16, y, > - (x - last_x) * 16, h); > + n += vnc_job_add_rect(job, last_x, y, > + (x - last_x), h); > } > last_x = -1; > } > } > if (last_x != -1) { > - int h = find_and_clear_dirty_height(vs, y, last_x, x, height); > - n += vnc_job_add_rect(job, last_x * 16, y, > - (x - last_x) * 16, h); > + int h = find_and_clear_dirty_height(vs, y, last_x/16, > x/16, height); > + if (x > width) x = width; > + n += vnc_job_add_rect(job, last_x, y, > + (x - last_x), h); > } > } > > @@ -1771,7 +1772,7 @@ static void framebuffer_update_request(VncState > *vs, int incremental, > int w, int h) > { > int i; > - const size_t width = ds_get_width(vs->ds) / 16; > + const size_t width = (ds_get_width(vs->ds)+15) / 16; > > if (y_position > ds_get_height(vs->ds)) > y_position = ds_get_height(vs->ds); > @@ -2595,10 +2596,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd) > * Check and copy modified bits from guest to server surface. > * Update server dirty map. > */ > - cmp_bytes = 64; > - if (cmp_bytes > vnc_server_fb_stride(vd)) { > - cmp_bytes = vnc_server_fb_stride(vd); > - } > if (vd->guest.format != VNC_SERVER_FB_FORMAT) { > int width = pixman_image_get_width(vd->server); > tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width); > @@ -2619,8 +2616,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd) > } > server_ptr = server_row; > > - for (x = 0; x + 15 < width; > + cmp_bytes = 64; > + for (x = 0; x < width; > x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) { > + if (width - x < 16) cmp_bytes = 4 * (width - x); > if (!test_and_clear_bit((x / 16), vd->guest.dirty[y])) > continue; > if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) > -- > 1.7.10.4 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-03-15 18:14 [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly Frediano Ziglio 2013-03-19 13:56 ` Stefano Stabellini @ 2013-05-09 14:20 ` Pasi Kärkkäinen 2013-05-10 5:52 ` [Qemu-devel] " Andreas Färber 2 siblings, 0 replies; 10+ messages in thread From: Pasi Kärkkäinen @ 2013-05-09 14:20 UTC (permalink / raw) To: Frediano Ziglio Cc: qemu-devel, Anthony Liguori, Fabio Fantoni, xen-devel, Stefano Stabellini On Fri, Mar 15, 2013 at 06:14:30PM +0000, Frediano Ziglio wrote: > Modern notebook support 136x768 resolution. The resolution width is > not multiple of 16 causing some problems. > > Qemu VGA emulation require width resolution to be multiple of 8. > > VNC implementation require width resolution to be multiple of 16. > > This patch remove these limits. Was tested with a Windows machine with > standard vga and 1366x768 as resolution. I had to update vgabios as > version in qemu (pc-bios/vgabios-stdvga.bin) is quite old. I also had > to add some patches on top of VGABIOS 0.7a to add some new > resolutions. > > I have some doubt about this patch > - are other UI (sdl, cocoa, qxl) happy if resolution is not multiple of 16 ? > - scanline is computed exactly without any alignment (so 1366 8 bit is > 1366 bytes) while getting vesa information from a laptop it seems to > use some kind of alignment (if became 0x580 which is 1408 bytes). > Perhaps should I change either VGABIOS and Qemu to make this > alignment? > Any plans to re-send the patch with Stefano's comments addressed? Thanks, -- Pasi > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > --- > hw/vga.c | 2 +- > ui/vnc.c | 27 +++++++++++++-------------- > 2 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/hw/vga.c b/hw/vga.c > index 1caf23d..d229f06 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -651,7 +651,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t > addr, uint32_t val) > } > break; > case VBE_DISPI_INDEX_XRES: > - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > s->vbe_regs[s->vbe_index] = val; > } > break; > diff --git a/ui/vnc.c b/ui/vnc.c > index ff4e2ae..328d14d 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -907,26 +907,27 @@ static int vnc_update_client(VncState *vs, int has_dirty) > for (y = 0; y < height; y++) { > int x; > int last_x = -1; > - for (x = 0; x < width / 16; x++) { > - if (test_and_clear_bit(x, vs->dirty[y])) { > + for (x = 0; x < width; x += 16) { > + if (test_and_clear_bit(x/16, vs->dirty[y])) { > if (last_x == -1) { > last_x = x; > } > } else { > if (last_x != -1) { > - int h = find_and_clear_dirty_height(vs, y, last_x, x, > + int h = find_and_clear_dirty_height(vs, y, > last_x/16, x/16, > height); > > - n += vnc_job_add_rect(job, last_x * 16, y, > - (x - last_x) * 16, h); > + n += vnc_job_add_rect(job, last_x, y, > + (x - last_x), h); > } > last_x = -1; > } > } > if (last_x != -1) { > - int h = find_and_clear_dirty_height(vs, y, last_x, x, height); > - n += vnc_job_add_rect(job, last_x * 16, y, > - (x - last_x) * 16, h); > + int h = find_and_clear_dirty_height(vs, y, last_x/16, > x/16, height); > + if (x > width) x = width; > + n += vnc_job_add_rect(job, last_x, y, > + (x - last_x), h); > } > } > > @@ -1771,7 +1772,7 @@ static void framebuffer_update_request(VncState > *vs, int incremental, > int w, int h) > { > int i; > - const size_t width = ds_get_width(vs->ds) / 16; > + const size_t width = (ds_get_width(vs->ds)+15) / 16; > > if (y_position > ds_get_height(vs->ds)) > y_position = ds_get_height(vs->ds); > @@ -2595,10 +2596,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd) > * Check and copy modified bits from guest to server surface. > * Update server dirty map. > */ > - cmp_bytes = 64; > - if (cmp_bytes > vnc_server_fb_stride(vd)) { > - cmp_bytes = vnc_server_fb_stride(vd); > - } > if (vd->guest.format != VNC_SERVER_FB_FORMAT) { > int width = pixman_image_get_width(vd->server); > tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width); > @@ -2619,8 +2616,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd) > } > server_ptr = server_row; > > - for (x = 0; x + 15 < width; > + cmp_bytes = 64; > + for (x = 0; x < width; > x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) { > + if (width - x < 16) cmp_bytes = 4 * (width - x); > if (!test_and_clear_bit((x / 16), vd->guest.dirty[y])) > continue; > if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) > -- > 1.7.10.4 > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-03-15 18:14 [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly Frediano Ziglio 2013-03-19 13:56 ` Stefano Stabellini 2013-05-09 14:20 ` [Qemu-devel] [Xen-devel] " Pasi Kärkkäinen @ 2013-05-10 5:52 ` Andreas Färber 2013-06-18 10:17 ` Frediano Ziglio 2 siblings, 1 reply; 10+ messages in thread From: Andreas Färber @ 2013-05-10 5:52 UTC (permalink / raw) To: Frediano Ziglio Cc: Peter Maydell, Anthony Liguori, xen-devel, Fabio Fantoni, Stefano Stabellini, qemu-devel, Alon Levy, Gerd Hoffmann Am 15.03.2013 19:14, schrieb Frediano Ziglio: > Modern notebook support 136x768 resolution. The resolution width is 1366? > not multiple of 16 causing some problems. "a multiple"? (me not a native English speaker) > > Qemu VGA emulation require width resolution to be multiple of 8. "QEMU" > > VNC implementation require width resolution to be multiple of 16. "requires" or "implementations" > > This patch remove these limits. Was tested with a Windows machine with > standard vga and 1366x768 as resolution. I had to update vgabios as > version in qemu (pc-bios/vgabios-stdvga.bin) is quite old. I also had > to add some patches on top of VGABIOS 0.7a to add some new > resolutions. > > I have some doubt about this patch > - are other UI (sdl, cocoa, qxl) happy if resolution is not multiple of 16 ? SDL and Gtk+ should be easily testable; if you CC Peter Maydell or me we can try to test Cocoa. CC'ing QXL guys. > - scanline is computed exactly without any alignment (so 1366 8 bit is > 1366 bytes) while getting vesa information from a laptop it seems to > use some kind of alignment (if became 0x580 which is 1408 bytes). > Perhaps should I change either VGABIOS and Qemu to make this > alignment? Concerns and personal comments are better placed below ---. :) > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > --- > hw/vga.c | 2 +- File has moved to hw/display/. > ui/vnc.c | 27 +++++++++++++-------------- > 2 files changed, 14 insertions(+), 15 deletions(-) I don't see VGABIOS being updated here despite being mentioned above? Is that done in a different patch? Still needed? > diff --git a/hw/vga.c b/hw/vga.c > index 1caf23d..d229f06 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -651,7 +651,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t > addr, uint32_t val) > } > break; > case VBE_DISPI_INDEX_XRES: > - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > s->vbe_regs[s->vbe_index] = val; > } > break; > diff --git a/ui/vnc.c b/ui/vnc.c > index ff4e2ae..328d14d 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -907,26 +907,27 @@ static int vnc_update_client(VncState *vs, int has_dirty) > for (y = 0; y < height; y++) { > int x; > int last_x = -1; > - for (x = 0; x < width / 16; x++) { > - if (test_and_clear_bit(x, vs->dirty[y])) { > + for (x = 0; x < width; x += 16) { > + if (test_and_clear_bit(x/16, vs->dirty[y])) { [snip] Please check if scripts/checkpatch.pl complains about missing spaces around operators. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-05-10 5:52 ` [Qemu-devel] " Andreas Färber @ 2013-06-18 10:17 ` Frediano Ziglio 2013-07-23 9:29 ` Fabio Fantoni 0 siblings, 1 reply; 10+ messages in thread From: Frediano Ziglio @ 2013-06-18 10:17 UTC (permalink / raw) To: Andreas Färber Cc: Peter Maydell, Anthony Liguori, xen-devel, Fabio Fantoni, Stefano Stabellini, qemu-devel, Alon Levy, Gerd Hoffmann [-- Attachment #1.1: Type: text/plain, Size: 7847 bytes --] Modern notebook support 1366x768 resolution. The resolution width is not multiple of 16 causing some problems. QEMU VGA emulation requires width resolution to be multiple of 8. VNC implementation requires width resolution to be multiple of 16. Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> --- hw/display/vga.c | 2 +- ui/vnc.c | 34 +++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 16 deletions(-) Updates: - rebased - fixed style problems - fixed typos in comment Attached patch for last vgabios in order to get some new resolutions. Still had no time to test deeply. diff --git a/hw/display/vga.c b/hw/display/vga.c index 21a108d..0053b0f 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -648,7 +648,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) } break; case VBE_DISPI_INDEX_XRES: - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { s->vbe_regs[s->vbe_index] = val; } break; diff --git a/ui/vnc.c b/ui/vnc.c index dfc7459..2a2bb90 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -911,26 +911,30 @@ static int vnc_update_client(VncState *vs, int has_dirty) for (y = 0; y < height; y++) { int x; int last_x = -1; - for (x = 0; x < width / 16; x++) { - if (test_and_clear_bit(x, vs->dirty[y])) { + for (x = 0; x < width; x += 16) { + if (test_and_clear_bit(x/16, vs->dirty[y])) { if (last_x == -1) { last_x = x; } } else { if (last_x != -1) { - int h = find_and_clear_dirty_height(vs, y, last_x, x, - height); + int h = find_and_clear_dirty_height(vs, y, last_x/16, + x/16, height); - n += vnc_job_add_rect(job, last_x * 16, y, - (x - last_x) * 16, h); + n += vnc_job_add_rect(job, last_x, y, + (x - last_x), h); } last_x = -1; } } if (last_x != -1) { - int h = find_and_clear_dirty_height(vs, y, last_x, x, height); - n += vnc_job_add_rect(job, last_x * 16, y, - (x - last_x) * 16, h); + int h = find_and_clear_dirty_height(vs, y, last_x/16, x/16, + height); + if (x > width) { + x = width; + } + n += vnc_job_add_rect(job, last_x, y, + (x - last_x), h); } } @@ -1861,7 +1865,7 @@ static void framebuffer_update_request(VncState *vs, int incremental, int w, int h) { int i; - const size_t width = surface_width(vs->vd->ds) / 16; + const size_t width = (surface_width(vs->vd->ds)+15) / 16; const size_t height = surface_height(vs->vd->ds); if (y_position > height) { @@ -2686,10 +2690,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd) * Check and copy modified bits from guest to server surface. * Update server dirty map. */ - cmp_bytes = 64; - if (cmp_bytes > vnc_server_fb_stride(vd)) { - cmp_bytes = vnc_server_fb_stride(vd); - } if (vd->guest.format != VNC_SERVER_FB_FORMAT) { int width = pixman_image_get_width(vd->server); tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width); @@ -2710,8 +2710,12 @@ static int vnc_refresh_server_surface(VncDisplay *vd) } server_ptr = server_row; - for (x = 0; x + 15 < width; + cmp_bytes = 64; + for (x = 0; x < width; x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) { + if (width - x < 16) { + cmp_bytes = 4 * (width - x); + } if (!test_and_clear_bit((x / 16), vd->guest.dirty[y])) continue; if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) -- 1.7.10.4 2013/5/10 Andreas Färber <afaerber@suse.de> > Am 15.03.2013 19:14, schrieb Frediano Ziglio: > > Modern notebook support 136x768 resolution. The resolution width is > > 1366? > > > not multiple of 16 causing some problems. > > "a multiple"? (me not a native English speaker) > > > > > Qemu VGA emulation require width resolution to be multiple of 8. > > "QEMU" > > > > > VNC implementation require width resolution to be multiple of 16. > > "requires" or "implementations" > > > > > This patch remove these limits. Was tested with a Windows machine with > > standard vga and 1366x768 as resolution. I had to update vgabios as > > version in qemu (pc-bios/vgabios-stdvga.bin) is quite old. I also had > > to add some patches on top of VGABIOS 0.7a to add some new > > resolutions. > > > > I have some doubt about this patch > > - are other UI (sdl, cocoa, qxl) happy if resolution is not multiple of > 16 ? > > SDL and Gtk+ should be easily testable; if you CC Peter Maydell or me we > can try to test Cocoa. CC'ing QXL guys. > > > - scanline is computed exactly without any alignment (so 1366 8 bit is > > 1366 bytes) while getting vesa information from a laptop it seems to > > use some kind of alignment (if became 0x580 which is 1408 bytes). > > Perhaps should I change either VGABIOS and Qemu to make this > > alignment? > > Concerns and personal comments are better placed below ---. :) > > > > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > > > --- > > hw/vga.c | 2 +- > > File has moved to hw/display/. > > > ui/vnc.c | 27 +++++++++++++-------------- > > 2 files changed, 14 insertions(+), 15 deletions(-) > > I don't see VGABIOS being updated here despite being mentioned above? Is > that done in a different patch? Still needed? > > > diff --git a/hw/vga.c b/hw/vga.c > > index 1caf23d..d229f06 100644 > > --- a/hw/vga.c > > +++ b/hw/vga.c > > @@ -651,7 +651,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t > > addr, uint32_t val) > > } > > break; > > case VBE_DISPI_INDEX_XRES: > > - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > > + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > > s->vbe_regs[s->vbe_index] = val; > > } > > break; > > diff --git a/ui/vnc.c b/ui/vnc.c > > index ff4e2ae..328d14d 100644 > > --- a/ui/vnc.c > > +++ b/ui/vnc.c > > @@ -907,26 +907,27 @@ static int vnc_update_client(VncState *vs, int > has_dirty) > > for (y = 0; y < height; y++) { > > int x; > > int last_x = -1; > > - for (x = 0; x < width / 16; x++) { > > - if (test_and_clear_bit(x, vs->dirty[y])) { > > + for (x = 0; x < width; x += 16) { > > + if (test_and_clear_bit(x/16, vs->dirty[y])) { > [snip] > > Please check if scripts/checkpatch.pl complains about missing spaces > around operators. > > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg > [-- Attachment #1.2: Type: text/html, Size: 10805 bytes --] [-- Attachment #2: 0001-Add-new-resolutions-to-VGABIOS.patch --] [-- Type: application/octet-stream, Size: 958 bytes --] From d272d21b34ed01b7bca28263a3dd47fc866d6839 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio <frediano.ziglio@citrix.com> Date: Tue, 18 Jun 2013 10:43:46 +0100 Subject: [PATCH 1/2] Add new resolutions to VGABIOS Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> --- vbetables-gen.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vbetables-gen.c b/vbetables-gen.c index b55c2d1..bc88e34 100644 --- a/vbetables-gen.c +++ b/vbetables-gen.c @@ -85,6 +85,12 @@ ModeInfo modes[] = { { 1920, 1080, 16 , 0x190}, { 1920, 1080, 24 , 0x191}, { 1920, 1080, 32 , 0x192}, +{ 1366, 768, 16 , 0x1a0}, +{ 1366, 768, 24 , 0x1a1}, +{ 1366, 768, 32 , 0x1a2}, +{ 1600, 900, 16 , 0x1a3}, +{ 1600, 900, 24 , 0x1a4}, +{ 1600, 900, 32 , 0x1a5}, { 0, }, }; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-06-18 10:17 ` Frediano Ziglio @ 2013-07-23 9:29 ` Fabio Fantoni 2013-07-23 11:28 ` Gerd Hoffmann 0 siblings, 1 reply; 10+ messages in thread From: Fabio Fantoni @ 2013-07-23 9:29 UTC (permalink / raw) To: Frediano Ziglio Cc: Peter Maydell, Anthony Liguori, xen-devel, Stefano Stabellini, qemu-devel, Alon Levy, Gerd Hoffmann, Andreas Färber [-- Attachment #1.1: Type: text/plain, Size: 8937 bytes --] Il 18/06/2013 12:17, Frediano Ziglio ha scritto: > Modern notebook support 1366x768 resolution. The resolution width is > not multiple of 16 causing some problems. > > QEMU VGA emulation requires width resolution to be multiple of 8. > > VNC implementation requires width resolution to be multiple of 16. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com > <mailto:frediano.ziglio@citrix.com>> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz> I tested it for a long time with spice on xen (because qxl will be fully working only after adding SSE support on hvm domUs). It works, I think it is good to add this and the respective vgabios patch on upstream. > --- > hw/display/vga.c | 2 +- > ui/vnc.c | 34 +++++++++++++++++++--------------- > 2 files changed, 20 insertions(+), 16 deletions(-) > > Updates: > - rebased > - fixed style problems > - fixed typos in comment > > Attached patch for last vgabios in order to get some new resolutions. > Still had no time to test deeply. > > diff --git a/hw/display/vga.c b/hw/display/vga.c > index 21a108d..0053b0f 100644 > --- a/hw/display/vga.c > +++ b/hw/display/vga.c > @@ -648,7 +648,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t > addr, uint32_t val) > } > break; > case VBE_DISPI_INDEX_XRES: > - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > s->vbe_regs[s->vbe_index] = val; > } > break; > diff --git a/ui/vnc.c b/ui/vnc.c > index dfc7459..2a2bb90 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -911,26 +911,30 @@ static int vnc_update_client(VncState *vs, int > has_dirty) > for (y = 0; y < height; y++) { > int x; > int last_x = -1; > - for (x = 0; x < width / 16; x++) { > - if (test_and_clear_bit(x, vs->dirty[y])) { > + for (x = 0; x < width; x += 16) { > + if (test_and_clear_bit(x/16, vs->dirty[y])) { > if (last_x == -1) { > last_x = x; > } > } else { > if (last_x != -1) { > - int h = find_and_clear_dirty_height(vs, y, > last_x, x, > - height); > + int h = find_and_clear_dirty_height(vs, y, > last_x/16, > + x/16, height); > > - n += vnc_job_add_rect(job, last_x * 16, y, > - (x - last_x) * 16, h); > + n += vnc_job_add_rect(job, last_x, y, > + (x - last_x), h); > } > last_x = -1; > } > } > if (last_x != -1) { > - int h = find_and_clear_dirty_height(vs, y, last_x, x, > height); > - n += vnc_job_add_rect(job, last_x * 16, y, > - (x - last_x) * 16, h); > + int h = find_and_clear_dirty_height(vs, y, last_x/16, > x/16, > + height); > + if (x > width) { > + x = width; > + } > + n += vnc_job_add_rect(job, last_x, y, > + (x - last_x), h); > } > } > > @@ -1861,7 +1865,7 @@ static void framebuffer_update_request(VncState > *vs, int incremental, > int w, int h) > { > int i; > - const size_t width = surface_width(vs->vd->ds) / 16; > + const size_t width = (surface_width(vs->vd->ds)+15) / 16; > const size_t height = surface_height(vs->vd->ds); > > if (y_position > height) { > @@ -2686,10 +2690,6 @@ static int > vnc_refresh_server_surface(VncDisplay *vd) > * Check and copy modified bits from guest to server surface. > * Update server dirty map. > */ > - cmp_bytes = 64; > - if (cmp_bytes > vnc_server_fb_stride(vd)) { > - cmp_bytes = vnc_server_fb_stride(vd); > - } > if (vd->guest.format != VNC_SERVER_FB_FORMAT) { > int width = pixman_image_get_width(vd->server); > tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width); > @@ -2710,8 +2710,12 @@ static int > vnc_refresh_server_surface(VncDisplay *vd) > } > server_ptr = server_row; > > - for (x = 0; x + 15 < width; > + cmp_bytes = 64; > + for (x = 0; x < width; > x += 16, guest_ptr += cmp_bytes, server_ptr += > cmp_bytes) { > + if (width - x < 16) { > + cmp_bytes = 4 * (width - x); > + } > if (!test_and_clear_bit((x / 16), vd->guest.dirty[y])) > continue; > if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) > -- > 1.7.10.4 > > > > 2013/5/10 Andreas Färber <afaerber@suse.de <mailto:afaerber@suse.de>> > > Am 15.03.2013 19:14, schrieb Frediano Ziglio: > > Modern notebook support 136x768 resolution. The resolution width is > > 1366? > > > not multiple of 16 causing some problems. > > "a multiple"? (me not a native English speaker) > > > > > Qemu VGA emulation require width resolution to be multiple of 8. > > "QEMU" > > > > > VNC implementation require width resolution to be multiple of 16. > > "requires" or "implementations" > > > > > This patch remove these limits. Was tested with a Windows > machine with > > standard vga and 1366x768 as resolution. I had to update vgabios as > > version in qemu (pc-bios/vgabios-stdvga.bin) is quite old. I > also had > > to add some patches on top of VGABIOS 0.7a to add some new > > resolutions. > > > > I have some doubt about this patch > > - are other UI (sdl, cocoa, qxl) happy if resolution is not > multiple of 16 ? > > SDL and Gtk+ should be easily testable; if you CC Peter Maydell or > me we > can try to test Cocoa. CC'ing QXL guys. > > > - scanline is computed exactly without any alignment (so 1366 8 > bit is > > 1366 bytes) while getting vesa information from a laptop it seems to > > use some kind of alignment (if became 0x580 which is 1408 bytes). > > Perhaps should I change either VGABIOS and Qemu to make this > > alignment? > > Concerns and personal comments are better placed below ---. :) > > > > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com > <mailto:frediano.ziglio@citrix.com>> > > > > --- > > hw/vga.c | 2 +- > > File has moved to hw/display/. > > > ui/vnc.c | 27 +++++++++++++-------------- > > 2 files changed, 14 insertions(+), 15 deletions(-) > > I don't see VGABIOS being updated here despite being mentioned > above? Is > that done in a different patch? Still needed? > > > diff --git a/hw/vga.c b/hw/vga.c > > index 1caf23d..d229f06 100644 > > --- a/hw/vga.c > > +++ b/hw/vga.c > > @@ -651,7 +651,7 @@ void vbe_ioport_write_data(void *opaque, > uint32_t > > addr, uint32_t val) > > } > > break; > > case VBE_DISPI_INDEX_XRES: > > - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > > + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > > s->vbe_regs[s->vbe_index] = val; > > } > > break; > > diff --git a/ui/vnc.c b/ui/vnc.c > > index ff4e2ae..328d14d 100644 > > --- a/ui/vnc.c > > +++ b/ui/vnc.c > > @@ -907,26 +907,27 @@ static int vnc_update_client(VncState *vs, > int has_dirty) > > for (y = 0; y < height; y++) { > > int x; > > int last_x = -1; > > - for (x = 0; x < width / 16; x++) { > > - if (test_and_clear_bit(x, vs->dirty[y])) { > > + for (x = 0; x < width; x += 16) { > > + if (test_and_clear_bit(x/16, vs->dirty[y])) { > [snip] > > Please check if scripts/checkpatch.pl <http://checkpatch.pl> > complains about missing spaces > around operators. > > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG > Nürnberg > > > > > Nessun virus nel messaggio. > Controllato da AVG - www.avg.com <http://www.avg.com> > Versione: 2013.0.3345 / Database dei virus: 3199/6420 - Data di > rilascio: 18/06/2013 > [-- Attachment #1.2: Type: text/html, Size: 17146 bytes --] [-- Attachment #2: Firma crittografica S/MIME --] [-- Type: application/pkcs7-signature, Size: 4510 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-07-23 9:29 ` Fabio Fantoni @ 2013-07-23 11:28 ` Gerd Hoffmann 2013-07-28 16:56 ` Frediano Ziglio 0 siblings, 1 reply; 10+ messages in thread From: Gerd Hoffmann @ 2013-07-23 11:28 UTC (permalink / raw) To: fantonifabio Cc: Peter Maydell, Anthony Liguori, xen-devel, Stefano Stabellini, qemu-devel, Alon Levy, Frediano Ziglio, Andreas Färber Hi, > Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz> > > I tested it for a long time with spice on xen (because qxl will be fully > working only after adding SSE support on hvm domUs). It works, I think > it is good to add this and the respective vgabios patch on upstream. >> case VBE_DISPI_INDEX_XRES: >> - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { >> + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { >> s->vbe_regs[s->vbe_index] = val; >> } >> break; It's not that simple. With 32bit depths common today it will work fine, but for lower depths (especially those lower than 8bit) this will give you broken scanline alignment. cheers, Gerd ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-07-23 11:28 ` Gerd Hoffmann @ 2013-07-28 16:56 ` Frediano Ziglio 2013-09-25 16:12 ` [Qemu-devel] [Xen-devel] " Pasi Kärkkäinen 0 siblings, 1 reply; 10+ messages in thread From: Frediano Ziglio @ 2013-07-28 16:56 UTC (permalink / raw) To: Gerd Hoffmann Cc: Peter Maydell, Anthony Liguori, xen-devel, Stefano Stabellini, Fabio Fantoni, qemu-devel, Alon Levy, Andreas Färber [-- Attachment #1: Type: text/plain, Size: 981 bytes --] 2013/7/23 Gerd Hoffmann <kraxel@redhat.com> > Hi, > > > Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz> > > > > I tested it for a long time with spice on xen (because qxl will be fully > > working only after adding SSE support on hvm domUs). It works, I think > > it is good to add this and the respective vgabios patch on upstream. > > >> case VBE_DISPI_INDEX_XRES: > >> - if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) { > >> + if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == 0)) { > >> s->vbe_regs[s->vbe_index] = val; > >> } > >> break; > > It's not that simple. With 32bit depths common today it will work fine, > but for lower depths (especially those lower than 8bit) this will give > you broken scanline alignment. > > cheers, > Gerd > > > In the card I tested the scanline is keep aligned but for this reason is not directly computed by maxx * bits but is something bigger. Frediano [-- Attachment #2: Type: text/html, Size: 1638 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-07-28 16:56 ` Frediano Ziglio @ 2013-09-25 16:12 ` Pasi Kärkkäinen 2013-10-03 12:09 ` Fabio Fantoni 0 siblings, 1 reply; 10+ messages in thread From: Pasi Kärkkäinen @ 2013-09-25 16:12 UTC (permalink / raw) To: Frediano Ziglio Cc: Peter Maydell, Anthony Liguori, xen-devel, Fabio Fantoni, Stefano Stabellini, qemu-devel, Alon Levy, Gerd Hoffmann, Andreas Färber On Sun, Jul 28, 2013 at 05:56:01PM +0100, Frediano Ziglio wrote: > 2013/7/23 Gerd Hoffmann <[1]kraxel@redhat.com> > > Â Hi, > > Tested-by: Fabio Fantoni <[2]fabio.fantoni@m2r.biz> > > > > I tested it for a long time with spice on xen (because qxl will be > fully > > working only after adding SSE support on hvm domUs). It works, I think > > it is good to add this and the respective vgabios patch on upstream. > > >> Â Â Â Â Â case VBE_DISPI_INDEX_XRES: > >> - Â Â Â Â Â Â if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == > 0)) { > >> + Â Â Â Â Â Â if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == > 0)) { > >> Â Â Â Â Â Â Â Â Â s->vbe_regs[s->vbe_index] = val; > >> Â Â Â Â Â Â Â } > >> Â Â Â Â Â Â Â break; > > It's not that simple. Â With 32bit depths common today it will work > fine, > but for lower depths (especially those lower than 8bit) this will give > you broken scanline alignment. > > cheers, > Â Gerd > > In the card I tested the scanline is keep aligned but for this reason is > not directly computed by maxx * bits but is something bigger. > Hmm.. so is an update needed for this patch? It'd be nice to get this patch in! -- Pasi > Frediano > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. 2013-09-25 16:12 ` [Qemu-devel] [Xen-devel] " Pasi Kärkkäinen @ 2013-10-03 12:09 ` Fabio Fantoni 0 siblings, 0 replies; 10+ messages in thread From: Fabio Fantoni @ 2013-10-03 12:09 UTC (permalink / raw) To: Pasi Kärkkäinen Cc: Peter Maydell, Anthony Liguori, xen-devel, Stefano Stabellini, Fabio Fantoni, qemu-devel, Alon Levy, Frediano Ziglio, Andreas Färber, Gerd Hoffmann Il 25/09/2013 18:12, Pasi Kärkkäinen ha scritto: > On Sun, Jul 28, 2013 at 05:56:01PM +0100, Frediano Ziglio wrote: >> 2013/7/23 Gerd Hoffmann <[1]kraxel@redhat.com> >> >> Â Hi, >> > Tested-by: Fabio Fantoni <[2]fabio.fantoni@m2r.biz> >> > >> > I tested it for a long time with spice on xen (because qxl will be >> fully >> > working only after adding SSE support on hvm domUs). It works, I think >> > it is good to add this and the respective vgabios patch on upstream. >> >> >> Â Â Â Â Â case VBE_DISPI_INDEX_XRES: >> >> - Â Â Â Â Â Â if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == >> 0)) { >> >> + Â Â Â Â Â Â if ((val <= VBE_DISPI_MAX_XRES) && ((val & 1) == >> 0)) { >> >> Â Â Â Â Â Â Â Â Â s->vbe_regs[s->vbe_index] = val; >> >> Â Â Â Â Â Â Â } >> >> Â Â Â Â Â Â Â break; >> >> It's not that simple. Â With 32bit depths common today it will work >> fine, >> but for lower depths (especially those lower than 8bit) this will give >> you broken scanline alignment. >> >> cheers, >> Â Gerd >> >> In the card I tested the scanline is keep aligned but for this reason is >> not directly computed by maxx * bits but is something bigger. >> > Hmm.. so is an update needed for this patch? It'd be nice to get this patch in! Ping Some statistics for evidence the importance to add support for 1366x768 and other resolutions: http://www.rapidtables.com/web/dev/screen-resolution-statistics.htm http://www.geek.com/chips/most-popular-screen-resolution-increases-to-1366-x-768-1482311/ > > -- Pasi > >> Frediano >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-10-03 12:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-15 18:14 [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly Frediano Ziglio 2013-03-19 13:56 ` Stefano Stabellini 2013-05-09 14:20 ` [Qemu-devel] [Xen-devel] " Pasi Kärkkäinen 2013-05-10 5:52 ` [Qemu-devel] " Andreas Färber 2013-06-18 10:17 ` Frediano Ziglio 2013-07-23 9:29 ` Fabio Fantoni 2013-07-23 11:28 ` Gerd Hoffmann 2013-07-28 16:56 ` Frediano Ziglio 2013-09-25 16:12 ` [Qemu-devel] [Xen-devel] " Pasi Kärkkäinen 2013-10-03 12:09 ` Fabio Fantoni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).