| Message ID | 1391877522-17254-3-git-send-email-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
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. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > CC: Blue Swirl <blauwirbel@gmail.com> > CC: Anthony Liguori <aliguori@amazon.com> > CC: Peter Maydell <peter.maydell@linaro.org> > CC: Bob Breuer <breuerr@mc.net> > CC: Artyom Tarasenko <atar4qemu@gmail.com> 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(-) > > 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, int width, > } > } > > +static void cg3_init(hwaddr addr, qemu_irq irq, int vram_size, int width, > + int height, int depth) Indentation? > +{ > + DeviceState *dev; > + SysBusDevice *s; > + > + dev = 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 = 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 */ > > #define TYPE_MACIO_ID_REGISTER "macio_idreg" > @@ -918,8 +943,39 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, > } > num_vsimms = 0; > if (num_vsimms == 0) { > - tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_height, > - graphic_depth); > + if (vga_interface_type == VGA_CG3) { > + if (graphic_depth != 8) { > + fprintf(stderr, "qemu: Unsupported depth: %d\n", graphic_depth); error_report() please - without "qemu: " and without trailing \n then. > + exit(1); > + } > + > + if (!(graphic_width == 1024 && graphic_height == 768) && > + !(graphic_width == 1152 && graphic_height == 900)) { > + 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 != 8 && graphic_depth != 24) { > + fprintf(stderr, "qemu: Unsupported depth: %d\n", > + graphic_depth); Dito. > + exit(1); > + } > + > + if (!(graphic_width == 1024 && graphic_height == 768)) { > + 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 message. > + > + tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_height, > + graphic_depth); > + } > } > > for (i = 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; > > typedef enum { > VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL, > + VGA_TCX, VGA_CG3, > } VGAInterfaceType; > > 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"); > } > > +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 = 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 = 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
On 09/02/14 15:32, Andreas Färber wrote: > 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. Okay - should this be a totally separate patch or appended to the end of this patchset? >> --- >> hw/sparc/sun4m.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-- >> include/sysemu/sysemu.h | 1 + >> vl.c | 24 +++++++++++++++++++ >> 3 files changed, 83 insertions(+), 2 deletions(-) >> >> 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, int width, >> } >> } >> >> +static void cg3_init(hwaddr addr, qemu_irq irq, int vram_size, int width, >> + int height, int depth) > > Indentation? Fixed. >> +{ >> + DeviceState *dev; >> + SysBusDevice *s; >> + >> + dev = 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 = 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 */ >> >> #define TYPE_MACIO_ID_REGISTER "macio_idreg" >> @@ -918,8 +943,39 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, >> } >> num_vsimms = 0; >> if (num_vsimms == 0) { >> - tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_height, >> - graphic_depth); >> + if (vga_interface_type == VGA_CG3) { >> + if (graphic_depth != 8) { >> + fprintf(stderr, "qemu: Unsupported depth: %d\n", graphic_depth); > > error_report() please - without "qemu: " and without trailing \n then. Fixed. >> + exit(1); >> + } >> + >> + if (!(graphic_width == 1024&& graphic_height == 768)&& >> + !(graphic_width == 1152&& graphic_height == 900)) { >> + fprintf(stderr, "qemu: Unsupported resolution: %d x %d\n", >> + graphic_width, graphic_height); > > Dito. Fixed. >> + 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 != 8&& graphic_depth != 24) { >> + fprintf(stderr, "qemu: Unsupported depth: %d\n", >> + graphic_depth); > > Dito. Fixed. >> + exit(1); >> + } >> + >> + if (!(graphic_width == 1024&& graphic_height == 768)) { >> + fprintf(stderr, "qemu: Unsupported resolution: %d x %d\n", >> + graphic_width, graphic_height); > > Dito. Fixed. >> + exit(1); >> + } > > These checks are new, right? Would deserve a mention in the commit message. Yes that's a good point. Sun's OBP boots in the higher resolution so I've altered the commit message to make this clearer. >> + >> + tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_height, >> + graphic_depth); >> + } >> } >> >> for (i = 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; >> >> typedef enum { >> VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL, >> + VGA_TCX, VGA_CG3, >> } VGAInterfaceType; >> >> 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"); >> } >> >> +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 = 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. I'm not sure about this because all of the existing -vga options use fprintf(stderr ... ) and exit(0). Changing this behaviour for just CG3 seems to be the worst solution because then it may cause confusion for programs that (incorrectly) monitor stderr to detect these errors on startup as CG3 would have a different behaviour to everything else. For bisectability I think that this would best be handled by a separate commit to change all of the -vga devices over to error_report()/exit(1) if/when people decide that this is the correct behaviour. ATB, Mark.
On 17/02/14 12:30, Mark Cave-Ayland wrote: > On 09/02/14 15:32, Andreas Färber wrote: > >> 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. > > Okay - should this be a totally separate patch or appended to the end of > this patchset? So I tried this, and if I launch with -device cgthree then I get an error about the device not being pluggable (which I'm guessing is because the device is attached to sysbus). Some further poking shows that several other of the -vga devices appear to be marked as non-pluggable too, so I'm not sure this could work with QOM tests based upon the current implementation. ATB, Mark.
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, int width, } } +static void cg3_init(hwaddr addr, qemu_irq irq, int vram_size, int width, + int height, int depth) +{ + DeviceState *dev; + SysBusDevice *s; + + dev = 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 = 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 */ #define TYPE_MACIO_ID_REGISTER "macio_idreg" @@ -918,8 +943,39 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, } num_vsimms = 0; if (num_vsimms == 0) { - tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_height, - graphic_depth); + if (vga_interface_type == VGA_CG3) { + if (graphic_depth != 8) { + fprintf(stderr, "qemu: Unsupported depth: %d\n", graphic_depth); + exit(1); + } + + if (!(graphic_width == 1024 && graphic_height == 768) && + !(graphic_width == 1152 && graphic_height == 900)) { + fprintf(stderr, "qemu: Unsupported resolution: %d x %d\n", + graphic_width, graphic_height); + 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 != 8 && graphic_depth != 24) { + fprintf(stderr, "qemu: Unsupported depth: %d\n", + graphic_depth); + exit(1); + } + + if (!(graphic_width == 1024 && graphic_height == 768)) { + fprintf(stderr, "qemu: Unsupported resolution: %d x %d\n", + graphic_width, graphic_height); + exit(1); + } + + tcx_init(hwdef->tcx_base, 0x00100000, graphic_width, graphic_height, + graphic_depth); + } } for (i = 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; typedef enum { VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL, + VGA_TCX, VGA_CG3, } VGAInterfaceType; 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"); } +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 = VGA_TCX; + } else { + fprintf(stderr, "Error: TCX framebuffer not available\n"); + exit(0); + } + } else if (strstart(p, "cg3", &opts)) { + if (cg3_vga_available()) { + vga_interface_type = 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);
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. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> CC: Blue Swirl <blauwirbel@gmail.com> CC: Anthony Liguori <aliguori@amazon.com> CC: Peter Maydell <peter.maydell@linaro.org> CC: Bob Breuer <breuerr@mc.net> CC: Artyom Tarasenko <atar4qemu@gmail.com> --- hw/sparc/sun4m.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-- include/sysemu/sysemu.h | 1 + vl.c | 24 +++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-)