From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCWMo-00051V-VC for qemu-devel@nongnu.org; Sun, 09 Feb 2014 10:32:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCWMj-0005Zq-Re for qemu-devel@nongnu.org; Sun, 09 Feb 2014 10:32:22 -0500 Received: from cantor2.suse.de ([195.135.220.15]:54893 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCWMj-0005Zc-In for qemu-devel@nongnu.org; Sun, 09 Feb 2014 10:32:17 -0500 Message-ID: <52F79F7B.30000@suse.de> Date: Sun, 09 Feb 2014 16:32:11 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1391877522-17254-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1391877522-17254-3-git-send-email-mark.cave-ayland@ilande.co.uk> In-Reply-To: <1391877522-17254-3-git-send-email-mark.cave-ayland@ilande.co.uk> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv2 2/2] sun4m: Add Sun CG3 framebuffer initialisation function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland , qemu-devel@nongnu.org Cc: Peter Maydell , Blue Swirl , Bob Breuer , Gerd Hoffmann , Anthony Liguori , Alon Levy , Artyom Tarasenko Am 08.02.2014 17:38, schrieb Mark Cave-Ayland: > In order to allow the user to choose the framebuffer for sparc-softmmu,= add > -vga tcx and -vga cg3 options to the QEMU command line. If no option is > specified, the default TCX framebuffer is used. >=20 > Signed-off-by: Mark Cave-Ayland > CC: Blue Swirl > CC: Anthony Liguori > CC: Peter Maydell > CC: Bob Breuer > CC: Artyom Tarasenko IIUC SUNW,cgthree is an optional device, so it's not covered by my qom-test. Please follow-up with a tests/cg3-test.c so that it gets covered. Compare my recent PCI NIC series for how such a stub could look like, in particular vmxnet3-test.c since this will be sparc-only. > --- > hw/sparc/sun4m.c | 60 +++++++++++++++++++++++++++++++++++++++= ++++++-- > include/sysemu/sysemu.h | 1 + > vl.c | 24 +++++++++++++++++++ > 3 files changed, 83 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c > index 94f7950..4c6c450 100644 > --- a/hw/sparc/sun4m.c > +++ b/hw/sparc/sun4m.c > @@ -561,6 +561,31 @@ static void tcx_init(hwaddr addr, int vram_size, i= nt width, > } > } > =20 > +static void cg3_init(hwaddr addr, qemu_irq irq, int vram_size, int wid= th, > + int height, int depth) Indentation? > +{ > + DeviceState *dev; > + SysBusDevice *s; > + > + dev =3D qdev_create(NULL, "SUNW,cgthree"); > + qdev_prop_set_uint32(dev, "vram_size", vram_size); > + qdev_prop_set_uint16(dev, "width", width); > + qdev_prop_set_uint16(dev, "height", height); > + qdev_prop_set_uint16(dev, "depth", depth); > + qdev_prop_set_uint64(dev, "prom_addr", addr); > + qdev_init_nofail(dev); > + s =3D SYS_BUS_DEVICE(dev); > + > + /* FCode ROM */ > + sysbus_mmio_map(s, 0, addr); > + /* DAC */ > + sysbus_mmio_map(s, 1, addr + 0x400000ULL); > + /* 8-bit plane */ > + sysbus_mmio_map(s, 2, addr + 0x800000ULL); > + > + sysbus_connect_irq(s, 0, irq); > +} > + > /* NCR89C100/MACIO Internal ID register */ > =20 > #define TYPE_MACIO_ID_REGISTER "macio_idreg" > @@ -918,8 +943,39 @@ static void sun4m_hw_init(const struct sun4m_hwdef= *hwdef, > } > num_vsimms =3D 0; > if (num_vsimms =3D=3D 0) { > - tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_h= eight, > - graphic_depth); > + if (vga_interface_type =3D=3D VGA_CG3) { > + if (graphic_depth !=3D 8) { > + fprintf(stderr, "qemu: Unsupported depth: %d\n", graph= ic_depth); error_report() please - without "qemu: " and without trailing \n then. > + exit(1); > + } > + > + if (!(graphic_width =3D=3D 1024 && graphic_height =3D=3D 7= 68) && > + !(graphic_width =3D=3D 1152 && graphic_height =3D=3D 9= 00)) { > + fprintf(stderr, "qemu: Unsupported resolution: %d x %d= \n", > + graphic_width, graphic_height); Dito. > + exit(1); > + } > + > + /* sbus irq 5 */ > + cg3_init(hwdef->tcx_base, slavio_irq[11], 0x00100000, > + graphic_width, graphic_height, graphic_depth); > + } else { > + /* If no display specified, default to TCX */ > + if (graphic_depth !=3D 8 && graphic_depth !=3D 24) { > + fprintf(stderr, "qemu: Unsupported depth: %d\n", > + graphic_depth); Dito. > + exit(1); > + } > + > + if (!(graphic_width =3D=3D 1024 && graphic_height =3D=3D 7= 68)) { > + fprintf(stderr, "qemu: Unsupported resolution: %d x %d= \n", > + graphic_width, graphic_height); Dito. > + exit(1); > + } These checks are new, right? Would deserve a mention in the commit messag= e. > + > + tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graph= ic_height, > + graphic_depth); > + } > } > =20 > for (i =3D num_vsimms; i < MAX_VSIMMS; i++) { > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 495dae8..b90df9a 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -104,6 +104,7 @@ extern int autostart; > =20 > typedef enum { > VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL, > + VGA_TCX, VGA_CG3, > } VGAInterfaceType; > =20 > extern int vga_interface_type; > diff --git a/vl.c b/vl.c > index 383be1b..61c8212 100644 > --- a/vl.c > +++ b/vl.c > @@ -2084,6 +2084,16 @@ static bool qxl_vga_available(void) > return object_class_by_name("qxl-vga"); > } > =20 > +static bool tcx_vga_available(void) > +{ > + return object_class_by_name("SUNW,tcx"); > +} > + > +static bool cg3_vga_available(void) > +{ > + return object_class_by_name("SUNW,cgthree"); > +} > + > static void select_vgahw (const char *p) > { > const char *opts; > @@ -2119,6 +2129,20 @@ static void select_vgahw (const char *p) > fprintf(stderr, "Error: QXL VGA not available\n"); > exit(0); > } > + } else if (strstart(p, "tcx", &opts)) { > + if (tcx_vga_available()) { > + vga_interface_type =3D VGA_TCX; > + } else { > + fprintf(stderr, "Error: TCX framebuffer not available\n"); > + exit(0); I note that these two and the below two are copied from QXL above, but shouldn't that be an exit(1) for such errors? error_report() would also be good. > + } > + } else if (strstart(p, "cg3", &opts)) { > + if (cg3_vga_available()) { > + vga_interface_type =3D VGA_CG3; > + } else { > + fprintf(stderr, "Error: CG3 framebuffer not available\n"); > + exit(0); > + } > } else if (!strstart(p, "none", &opts)) { > invalid_vga: > fprintf(stderr, "Unknown vga type: %s\n", p); Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg