From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UagFq-0005li-J9 for qemu-devel@nongnu.org; Fri, 10 May 2013 01:52:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UagFp-0003pH-HG for qemu-devel@nongnu.org; Fri, 10 May 2013 01:52:30 -0400 Received: from cantor2.suse.de ([195.135.220.15]:41653 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UagFp-0003pC-7V for qemu-devel@nongnu.org; Fri, 10 May 2013 01:52:29 -0400 Message-ID: <518C8B16.4060108@suse.de> Date: Fri, 10 May 2013 07:52:22 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Frediano Ziglio Cc: Peter Maydell , Anthony Liguori , xen-devel@lists.xensource.com, 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) >=20 > Qemu VGA emulation require width resolution to be multiple of 8. "QEMU" >=20 > VNC implementation require width resolution to be multiple of 16. "requires" or "implementations" >=20 > 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. >=20 > 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 ---. :) >=20 > Signed-off-by: Frediano Ziglio >=20 > --- > 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 <=3D VBE_DISPI_MAX_XRES) && ((val & 7) =3D=3D 0))= { > + if ((val <=3D VBE_DISPI_MAX_XRES) && ((val & 1) =3D=3D 0))= { > s->vbe_regs[s->vbe_index] =3D 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 ha= s_dirty) > for (y =3D 0; y < height; y++) { > int x; > int last_x =3D -1; > - for (x =3D 0; x < width / 16; x++) { > - if (test_and_clear_bit(x, vs->dirty[y])) { > + for (x =3D 0; x < width; x +=3D 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 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Subject: Re: [RFC PATCH] vga: Start supporting resolution not multiple of 16 correctly. Date: Fri, 10 May 2013 07:52:22 +0200 Message-ID: <518C8B16.4060108@suse.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org To: Frediano Ziglio Cc: Peter Maydell , Anthony Liguori , xen-devel@lists.xensource.com, Fabio Fantoni , Stefano Stabellini , qemu-devel , Alon Levy , Gerd Hoffmann List-Id: xen-devel@lists.xenproject.org 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) >=20 > Qemu VGA emulation require width resolution to be multiple of 8. "QEMU" >=20 > VNC implementation require width resolution to be multiple of 16. "requires" or "implementations" >=20 > 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. >=20 > 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 ---. :) >=20 > Signed-off-by: Frediano Ziglio >=20 > --- > 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 <=3D VBE_DISPI_MAX_XRES) && ((val & 7) =3D=3D 0))= { > + if ((val <=3D VBE_DISPI_MAX_XRES) && ((val & 1) =3D=3D 0))= { > s->vbe_regs[s->vbe_index] =3D 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 ha= s_dirty) > for (y =3D 0; y < height; y++) { > int x; > int last_x =3D -1; > - for (x =3D 0; x < width / 16; x++) { > - if (test_and_clear_bit(x, vs->dirty[y])) { > + for (x =3D 0; x < width; x +=3D 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 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg