| Message ID | CAHt6W4fxEv9bMQHjSA=O3vuHxaH+sL9raaj9OsD2b-j4=3aYRw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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 >
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
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
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); }
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(-) } @@ -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)