[Qemu-devel] [PATCHv3 0/2] sun4m: Implement Sun CG3 framebuffer for QEMU

[Qemu-devel] [PATCHv3 0/2] sun4m: Implement Sun CG3 framebuffer for QEMU

[Mark Cave-Ayland]

This patchset provides QEMU with an implementation of the Sun CG3 8-bit
framebuffer. It is based upon Bob Breuer's original work which has been
rebased onto git master, and is now capable of running with an OpenBIOS CG3 
FCode ROM instead of requiring copies of proprietary Sun ROMs.

The motivation behind this patch is that older operating systems such as
Debian Woody and Solaris (running OpenWindows) do not contain drivers for the
TCX framebuffer and as a result currently cannot run in graphical mode. The
screenshots linked below show qemu-system-sparc successfully running both 
Debian Woody and the Solaris 8 installer in graphical mode during testing:

http://www.ilande.co.uk/tmp/debian-woody.png
http://www.ilande.co.uk/tmp/sol8-1.png
http://www.ilande.co.uk/tmp/sol8-2.png

The CG3 framebuffer is selected by passing -vga cg3 on the command line to
qemu-system-sparc. If either -vga tcx is specified (or the -vga argument is
omitted) then qemu-system-sparc defaults to using the existing TCX
framebuffer to maintain compatibility.

v3:
    - Rebased to git master
    - Fix DEBUG_CG3 macro
    - Use register constants based upon Linux/BSD drivers
    - Use qemu_log(LOG_UNIMP ... ) to capture unrecognised register accesses
    - Rename device type from SUNW,cgthree to cgthree (matches OBP)
    - Use error_report() instead of fprintf(stderr ... )
    - Convert from init to realizefn

v2:
    - Rebased to git master
    - Updated QEMU,cgthree.bin ROM to latest OpenBIOS version
    - Added Peter Maydell to CC


Mark Cave-Ayland (2):
  sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
  sun4m: Add Sun CG3 framebuffer initialisation function

 Makefile                          |    2 +-
 default-configs/sparc-softmmu.mak |    1 +
 hw/display/Makefile.objs          |    1 +
 hw/display/cg3.c                  |  384 +++++++++++++++++++++++++++++++++++++
 hw/sparc/sun4m.c                  |   62 +++++-
 include/sysemu/sysemu.h           |    1 +
 pc-bios/QEMU,cgthree.bin          |  Bin 0 -> 850 bytes
 pc-bios/README                    |    4 +-
 vl.c                              |   24 +++
 9 files changed, 473 insertions(+), 6 deletions(-)
 create mode 100644 hw/display/cg3.c
 create mode 100644 pc-bios/QEMU,cgthree.bin

-- 
1.7.10.4

[Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Mark Cave-Ayland]

The CG3 framebuffer is a simple 8-bit framebuffer for use with operating
systems such as early Solaris that do not have drivers for TCX.

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>
---
 Makefile                          |    2 +-
 default-configs/sparc-softmmu.mak |    1 +
 hw/display/Makefile.objs          |    1 +
 hw/display/cg3.c                  |  384 +++++++++++++++++++++++++++++++++++++
 pc-bios/QEMU,cgthree.bin          |  Bin 0 -> 850 bytes
 pc-bios/README                    |    4 +-
 6 files changed, 389 insertions(+), 3 deletions(-)
 create mode 100644 hw/display/cg3.c
 create mode 100644 pc-bios/QEMU,cgthree.bin

diff --git a/Makefile b/Makefile
index 807054b..c3c7ccc 100644
--- a/Makefile
+++ b/Makefile
@@ -293,7 +293,7 @@ ifdef INSTALL_BLOBS
 BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
 vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
 acpi-dsdt.aml q35-acpi-dsdt.aml \
-ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin \
+ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin QEMU,cgthree.bin \
 pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
 pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
 efi-e1000.rom efi-eepro100.rom efi-ne2k_pci.rom \
diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak
index 8fc93dd..ab796b3 100644
--- a/default-configs/sparc-softmmu.mak
+++ b/default-configs/sparc-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_EMPTY_SLOT=y
 CONFIG_PCNET_COMMON=y
 CONFIG_LANCE=y
 CONFIG_TCX=y
+CONFIG_CG3=y
 CONFIG_SLAVIO=y
 CONFIG_CS4231=y
 CONFIG_GRLIB=y
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 540df82..7ed76a9 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -28,6 +28,7 @@ obj-$(CONFIG_OMAP) += omap_lcdc.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_lcd.o
 obj-$(CONFIG_SM501) += sm501.o
 obj-$(CONFIG_TCX) += tcx.o
+obj-$(CONFIG_CG3) += cg3.o
 
 obj-$(CONFIG_VGA) += vga.o
 
diff --git a/hw/display/cg3.c b/hw/display/cg3.c
new file mode 100644
index 0000000..b6bc0c2
--- /dev/null
+++ b/hw/display/cg3.c
@@ -0,0 +1,384 @@
+/*
+ * QEMU CG3 Frame buffer
+ *
+ * Copyright (c) 2012 Bob Breuer
+ * Copyright (c) 2013 Mark Cave-Ayland
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "ui/console.h"
+#include "hw/sysbus.h"
+#include "hw/loader.h"
+
+/* Change to 1 to enable debugging */
+#define DEBUG_CG3 0
+
+#define CG3_ROM_FILE  "QEMU,cgthree.bin"
+#define FCODE_MAX_ROM_SIZE 0x10000
+
+#define CG3_REG_SIZE            0x20
+
+#define CG3_REG_BT458_ADDR      0x0
+#define CG3_REG_BT458_COLMAP    0x4
+#define CG3_REG_FBC_CTRL        0x10
+#define CG3_REG_FBC_STATUS      0x11
+#define CG3_REG_FBC_CURSTART    0x12
+#define CG3_REG_FBC_CUREND      0x13
+#define CG3_REG_FBC_VCTRL       0x14
+
+/* Control register flags */
+#define CG3_CR_ENABLE_INTS      0x80
+
+/* Status register flags */
+#define CG3_SR_PENDING_INT      0x80
+#define CG3_SR_1152_900_76_B    0x60
+#define CG3_SR_ID_COLOR         0x01
+
+#define CG3_VRAM_SIZE 0x100000
+#define CG3_VRAM_OFFSET 0x800000
+
+#define DPRINTF(fmt, ...) do { \
+    if (DEBUG_CG3) { \
+        printf("CG3: " fmt , ## __VA_ARGS__); \
+    } \
+} while (0);
+
+#define TYPE_CG3 "cgthree"
+#define CG3(obj) OBJECT_CHECK(CG3State, (obj), TYPE_CG3)
+
+typedef struct CG3State {
+    SysBusDevice parent_obj;
+
+    QemuConsole *con;
+    qemu_irq irq;
+    hwaddr prom_addr;
+    MemoryRegion vram_mem;
+    MemoryRegion rom;
+    MemoryRegion reg;
+    uint32_t vram_size;
+    int full_update;
+    uint8_t regs[16];
+    uint8_t r[256], g[256], b[256];
+    uint16_t width, height, depth;
+    uint8_t dac_index, dac_state;
+} CG3State;
+
+static void cg3_update_display(void *opaque)
+{
+    CG3State *s = opaque;
+    DisplaySurface *surface = qemu_console_surface(s->con);
+    const uint8_t *pix;
+    uint32_t *data;
+    uint32_t dval;
+    int x, y, y_start;
+    unsigned int width, height;
+    ram_addr_t page, page_min, page_max;
+
+    if (surface_bits_per_pixel(surface) != 32) {
+        return;
+    }
+    width = s->width;
+    height = s->height;
+
+    y_start = -1;
+    page_min = -1;
+    page_max = 0;
+    page = 0;
+    pix = memory_region_get_ram_ptr(&s->vram_mem);
+    data = (uint32_t *)surface_data(surface);
+
+    for (y = 0; y < height; y++) {
+        int update = s->full_update;
+
+        page = (y * width) & TARGET_PAGE_MASK;
+        update |= memory_region_get_dirty(&s->vram_mem, page, page + width,
+                                          DIRTY_MEMORY_VGA);
+        if (update) {
+            if (y_start < 0) {
+                y_start = y;
+            }
+            if (page < page_min) {
+                page_min = page;
+            }
+            if (page > page_max) {
+                page_max = page;
+            }
+
+            for (x = 0; x < width; x++) {
+                dval = *pix++;
+                dval = (s->r[dval] << 16) | (s->g[dval] << 8) | s->b[dval];
+                *data++ = dval;
+            }
+        } else {
+            if (y_start >= 0) {
+                dpy_gfx_update(s->con, 0, y_start, s->width, y - y_start);
+                y_start = -1;
+            }
+            pix += width;
+            data += width;
+        }
+    }
+    s->full_update = 0;
+    if (y_start >= 0) {
+        dpy_gfx_update(s->con, 0, y_start, s->width, y - y_start);
+    }
+    if (page_max >= page_min) {
+        memory_region_reset_dirty(&s->vram_mem,
+                              page_min, page_max - page_min + TARGET_PAGE_SIZE,
+                              DIRTY_MEMORY_VGA);
+    }
+    /* vsync interrupt? */
+    if (s->regs[0] & CG3_CR_ENABLE_INTS) {
+        s->regs[1] |= CG3_SR_PENDING_INT;
+        qemu_irq_raise(s->irq);
+    }
+}
+
+static void cg3_invalidate_display(void *opaque)
+{
+    CG3State *s = opaque;
+
+    memory_region_set_dirty(&s->vram_mem, 0, CG3_VRAM_SIZE);
+}
+
+static uint64_t cg3_reg_read(void *opaque, hwaddr addr, unsigned size)
+{
+    CG3State *s = opaque;
+    int val;
+
+    switch (addr) {
+    case CG3_REG_BT458_ADDR:
+    case CG3_REG_BT458_COLMAP:
+        val = 0;
+        break;
+    case CG3_REG_FBC_CTRL:
+        val = s->regs[0];
+        break;
+    case CG3_REG_FBC_STATUS:
+        /* monitor ID 6, board type = 1 (color) */
+        val = s->regs[1] | CG3_SR_1152_900_76_B | CG3_SR_ID_COLOR;
+        break;
+    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
+        val = s->regs[addr - 0x10];
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                  "cg3: Unimplemented register read "
+                  "reg 0x%" HWADDR_PRIx " size 0x%x\n",
+                  addr, size);
+        val = 0;
+        break;
+    }
+    DPRINTF("read %02x from reg %x\n", val, (int)addr);
+    return val;
+}
+
+static void cg3_reg_write(void *opaque, hwaddr addr, uint64_t val,
+                          unsigned size)
+{
+    CG3State *s = opaque;
+    uint8_t regval;
+    int i;
+
+    DPRINTF("write %02lx to reg %x size %d\n", val, (int)addr, size);
+
+    switch (addr) {
+    case CG3_REG_BT458_ADDR:
+        s->dac_index = val;
+        s->dac_state = 0;
+        break;
+    case CG3_REG_BT458_COLMAP:
+        /* This register can be written to as either a long word or a byte */
+        if (size == 1) {
+            val <<= 24;
+        }
+
+        for (i = 0; i < size; i++) {
+            regval = val >> 24;
+
+            switch (s->dac_state) {
+            case 0:
+                s->r[s->dac_index] = regval;
+                s->dac_state++;
+                break;
+            case 1:
+                s->g[s->dac_index] = regval;
+                s->dac_state++;
+                break;
+            case 2:
+                s->b[s->dac_index] = regval;
+                /* Index autoincrement */
+                s->dac_index = (s->dac_index + 1) & 0xff;
+            default:
+                s->dac_state = 0;
+                break;
+            }
+            val <<= 8;
+        }
+        s->full_update = 1;
+        break;
+    case CG3_REG_FBC_CTRL:
+        s->regs[0] = val;
+        break;
+    case CG3_REG_FBC_STATUS:
+        if (s->regs[1] & CG3_SR_PENDING_INT) {
+            /* clear interrupt */
+            s->regs[1] &= ~CG3_SR_PENDING_INT;
+            qemu_irq_lower(s->irq);
+        }
+        break;
+    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
+        s->regs[addr - 0x10] = val;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                  "cg3: Unimplemented register write "
+                  "reg 0x%" HWADDR_PRIx " size 0x%x value 0x%" PRIx64 "\n",
+                  addr, size, val);
+        break;
+    }
+}
+
+static const MemoryRegionOps cg3_reg_ops = {
+    .read = cg3_reg_read,
+    .write = cg3_reg_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
+static const GraphicHwOps cg3_ops = {
+    .invalidate = cg3_invalidate_display,
+    .gfx_update = cg3_update_display,
+};
+
+static void cg3_realizefn(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    CG3State *s = CG3(dev);
+    int ret;
+    char *fcode_filename;
+
+    /* FCode ROM */
+    memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE);
+    vmstate_register_ram_global(&s->rom);
+    memory_region_set_readonly(&s->rom, true);
+    sysbus_init_mmio(sbd, &s->rom);
+
+    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
+    if (fcode_filename) {
+        ret = load_image_targphys(fcode_filename, s->prom_addr,
+                                  FCODE_MAX_ROM_SIZE);
+        if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
+            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
+        }
+    }
+
+    memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
+                          CG3_REG_SIZE);
+    sysbus_init_mmio(sbd, &s->reg);
+
+    memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size);
+    vmstate_register_ram_global(&s->vram_mem);
+    sysbus_init_mmio(sbd, &s->vram_mem);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    s->con = graphic_console_init(DEVICE(dev), &cg3_ops, s);
+    qemu_console_resize(s->con, s->width, s->height);
+}
+
+static int vmstate_cg3_post_load(void *opaque, int version_id)
+{
+    CG3State *s = opaque;
+
+    cg3_invalidate_display(s);
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_cg3 = {
+    .name = "cg3",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = vmstate_cg3_post_load,
+    .fields    = (VMStateField[]) {
+        VMSTATE_UINT16(height, CG3State),
+        VMSTATE_UINT16(width, CG3State),
+        VMSTATE_UINT16(depth, CG3State),
+        VMSTATE_BUFFER(r, CG3State),
+        VMSTATE_BUFFER(g, CG3State),
+        VMSTATE_BUFFER(b, CG3State),
+        VMSTATE_UINT8(dac_index, CG3State),
+        VMSTATE_UINT8(dac_state, CG3State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void cg3_reset(DeviceState *d)
+{
+    CG3State *s = CG3(d);
+
+    /* Initialize palette */
+    memset(s->r, 0, 256);
+    memset(s->g, 0, 256);
+    memset(s->b, 0, 256);
+
+    s->dac_state = 0;
+    s->full_update = 1;
+    qemu_irq_lower(s->irq);
+}
+
+static Property cg3_properties[] = {
+    DEFINE_PROP_UINT32("vram_size",    CG3State, vram_size, -1),
+    DEFINE_PROP_UINT16("width",        CG3State, width,     -1),
+    DEFINE_PROP_UINT16("height",       CG3State, height,    -1),
+    DEFINE_PROP_UINT16("depth",        CG3State, depth,     -1),
+    DEFINE_PROP_UINT64("prom_addr",    CG3State, prom_addr, -1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void cg3_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = cg3_realizefn;
+    dc->reset = cg3_reset;
+    dc->vmsd = &vmstate_cg3;
+    dc->props = cg3_properties;
+}
+
+static const TypeInfo cg3_info = {
+    .name          = TYPE_CG3,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(CG3State),
+    .class_init    = cg3_class_init,
+};
+
+static void cg3_register_types(void)
+{
+    type_register_static(&cg3_info);
+}
+
+type_init(cg3_register_types)
diff --git a/pc-bios/QEMU,cgthree.bin b/pc-bios/QEMU,cgthree.bin
new file mode 100644
index 0000000000000000000000000000000000000000..6fec9462070bf9a0f88024c7bf66f444d1d11d61
GIT binary patch
literal 850
zcmZ{i&2HL25XX10!NXT-76XB#Rk_y^s6|SoR%&la53Q7_>KX6`yTvxLO)GA>_ch=J
zP_0Ed5~+vw1^OC&f^?RT1C@IDd*?SZ|5@Af2Y<wjX;&#S`O9L)^D4_Nujb2jiXgca
zPDC*9!r1=eIU=;bdQRdZo4=yUd6i|Ci{<)%MIyz_ir4;eaD_K=6J(UtR=nVdN#fcA
zFNrruCp7i~VGm}B*rM#FYA_wy$!sE!rI=f#Xh;N$<uT(|S$=6UrZaVA++l5xwGGbi
zu)fC(Rdr#9vwOTXDN4*eUYqPSqhX~xGG|XyEYsmuks~^k)Zx)xil*!=AoCkEsCJ<O
zoLnnXbubf1(BxVqMqm=>vIAO|=luS}_JT~FP*rk6h2b=zc%GuQBB{~))g@X_rt6=%
zVK@$>Ha6q}>z8kpvySz{2N@kpEMXb>Jz5ksB_3_=s6dTCOX4v$*W4J65;qbe1Ke=D
zcrxzKpvBAAAKra@*6Vcb?u%{@nkk;p^!ZDR`PfpU9u9?JLjiUu4_oRe`bNojC4ddA
z-NOJr!DrG6H~Nkfi8uxm4a5uZ+ZQly!#DLmP9;_lsVKMI5>-P{cC&R96e!56_1J6&
zm}<Z|R2J&PbKMJ)NOcg^Z|U+yl{R+kL90s5Wj_qOB#i7>1hD{<>+03P;w8TyOmF(b
yWEu%F;rYw!_h)ClbGu8)^3d%^loP5i*^VudTX8sz;xLL`?}lgvPvCTor|d5SYsmWm

literal 0
HcmV?d00001

diff --git a/pc-bios/README b/pc-bios/README
index f190068..5914200 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -11,8 +11,8 @@
   firmware implementation. The goal is to implement a 100% IEEE
   1275-1994 (referred to as Open Firmware) compliant firmware.
   The included images for PowerPC (for 32 and 64 bit PPC CPUs),
-  Sparc32 (including QEMU,tcx.bin) and Sparc64 are built from OpenBIOS SVN
-  revision 1246.
+  Sparc32 (including QEMU,tcx.bin and QEMU,cgthree.bin) and Sparc64 are built
+  from OpenBIOS SVN revision 1246.
 
 - SLOF (Slimline Open Firmware) is a free IEEE 1275 Open Firmware
   implementation for certain IBM POWER hardware.  The sources are at
-- 
1.7.10.4

[Qemu-devel] [PATCHv3 2/2] sun4m: Add Sun CG3 framebuffer initialisation function

[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.

Since proprietary FCode ROMs use a resolution of 1152x900, slightly relax the
validation rules to allow both displays to be initiated at the higher
resolution used by these ROMs upon request (OpenBIOS FCode ROMs default to
the normal QEMU sun4m default resolution of 1024x768).

Finally move any fprintf(stderr ...) statements in the areas affected by this
patch over to the new error_report() function.

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        |   62 ++++++++++++++++++++++++++++++++++++++++++++---
 include/sysemu/sysemu.h |    1 +
 vl.c                    |   24 ++++++++++++++++++
 3 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 2957d90..65ea07b 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "hw/sysbus.h"
+#include "qemu/error-report.h"
 #include "qemu/timer.h"
 #include "hw/sparc/sun4m.h"
 #include "hw/timer/m48t59.h"
@@ -561,6 +562,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, "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"
@@ -914,13 +940,43 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                              slavio_irq[16], iommu, &ledma_irq, 1);
 
     if (graphic_depth != 8 && graphic_depth != 24) {
-        fprintf(stderr, "qemu: Unsupported depth: %d\n", graphic_depth);
+        error_report("Unsupported depth: %d", graphic_depth);
         exit (1);
     }
     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) {
+                error_report("Unsupported depth: %d", graphic_depth);
+                exit(1);
+            }
+
+            if (!(graphic_width == 1024 && graphic_height == 768) &&
+                !(graphic_width == 1152 && graphic_height == 900)) {
+                error_report("Unsupported resolution: %d x %d", 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) {
+                error_report("Unsupported depth: %d", graphic_depth);
+                exit(1);
+            }
+
+            if (!(graphic_width == 1024 && graphic_height == 768)) {
+                error_report("Unsupported resolution: %d x %d",
+                             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 316de54..10aca2b 100644
--- a/vl.c
+++ b/vl.c
@@ -2031,6 +2031,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("cgthree");
+}
+
 static void select_vgahw (const char *p)
 {
     const char *opts;
@@ -2066,6 +2076,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);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv3 0/2] sun4m: Implement Sun CG3 framebuffer for QEMU

[Andreas Färber]

Am 19.02.2014 10:05, schrieb Mark Cave-Ayland:
> This patchset provides QEMU with an implementation of the Sun CG3 8-bit
> framebuffer. It is based upon Bob Breuer's original work which has been
> rebased onto git master, and is now capable of running with an OpenBIOS CG3 
> FCode ROM instead of requiring copies of proprietary Sun ROMs.
> 
> The motivation behind this patch is that older operating systems such as
> Debian Woody and Solaris (running OpenWindows) do not contain drivers for the
> TCX framebuffer and as a result currently cannot run in graphical mode. The
> screenshots linked below show qemu-system-sparc successfully running both 
> Debian Woody and the Solaris 8 installer in graphical mode during testing:
> 
> http://www.ilande.co.uk/tmp/debian-woody.png
> http://www.ilande.co.uk/tmp/sol8-1.png
> http://www.ilande.co.uk/tmp/sol8-2.png
> 
> The CG3 framebuffer is selected by passing -vga cg3 on the command line to
> qemu-system-sparc. If either -vga tcx is specified (or the -vga argument is
> omitted) then qemu-system-sparc defaults to using the existing TCX
> framebuffer to maintain compatibility.
> 
> v3:
>     - Rebased to git master
>     - Fix DEBUG_CG3 macro
>     - Use register constants based upon Linux/BSD drivers
>     - Use qemu_log(LOG_UNIMP ... ) to capture unrecognised register accesses
>     - Rename device type from SUNW,cgthree to cgthree (matches OBP)
>     - Use error_report() instead of fprintf(stderr ... )
>     - Convert from init to realizefn
> 
> v2:
>     - Rebased to git master
>     - Updated QEMU,cgthree.bin ROM to latest OpenBIOS version
>     - Added Peter Maydell to CC
> 
> 
> Mark Cave-Ayland (2):
>   sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
>   sun4m: Add Sun CG3 framebuffer initialisation function

Reviewed-by: Andreas Färber <afaerber@suse.de>

There's some lines we could extract from the realizefn into an
instance_init, but can be done as follow-up when needed.

Two small questions, are vram_size and prom_addr for compatibility? New
convention for QOM properties would be dashes.

Regards,
Andreas

> 
>  Makefile                          |    2 +-
>  default-configs/sparc-softmmu.mak |    1 +
>  hw/display/Makefile.objs          |    1 +
>  hw/display/cg3.c                  |  384 +++++++++++++++++++++++++++++++++++++
>  hw/sparc/sun4m.c                  |   62 +++++-
>  include/sysemu/sysemu.h           |    1 +
>  pc-bios/QEMU,cgthree.bin          |  Bin 0 -> 850 bytes
>  pc-bios/README                    |    4 +-
>  vl.c                              |   24 +++
>  9 files changed, 473 insertions(+), 6 deletions(-)
>  create mode 100644 hw/display/cg3.c
>  create mode 100644 pc-bios/QEMU,cgthree.bin
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Leandro Dorileo]

Hi Mark,

On Wed, Feb 19, 2014 at 09:05:19AM +0000, Mark Cave-Ayland wrote:
> The CG3 framebuffer is a simple 8-bit framebuffer for use with operating
> systems such as early Solaris that do not have drivers for TCX.
> 
> 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>
> ---
>  Makefile                          |    2 +-
>  default-configs/sparc-softmmu.mak |    1 +
>  hw/display/Makefile.objs          |    1 +
>  hw/display/cg3.c                  |  384 +++++++++++++++++++++++++++++++++++++
>  pc-bios/QEMU,cgthree.bin          |  Bin 0 -> 850 bytes
>  pc-bios/README                    |    4 +-
>  6 files changed, 389 insertions(+), 3 deletions(-)
>  create mode 100644 hw/display/cg3.c
>  create mode 100644 pc-bios/QEMU,cgthree.bin
> 
> diff --git a/Makefile b/Makefile
> index 807054b..c3c7ccc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -293,7 +293,7 @@ ifdef INSTALL_BLOBS
>  BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
>  vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
>  acpi-dsdt.aml q35-acpi-dsdt.aml \
> -ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin \
> +ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin QEMU,cgthree.bin \
>  pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
>  pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
>  efi-e1000.rom efi-eepro100.rom efi-ne2k_pci.rom \
> diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak
> index 8fc93dd..ab796b3 100644
> --- a/default-configs/sparc-softmmu.mak
> +++ b/default-configs/sparc-softmmu.mak
> @@ -10,6 +10,7 @@ CONFIG_EMPTY_SLOT=y
>  CONFIG_PCNET_COMMON=y
>  CONFIG_LANCE=y
>  CONFIG_TCX=y
> +CONFIG_CG3=y
>  CONFIG_SLAVIO=y
>  CONFIG_CS4231=y
>  CONFIG_GRLIB=y
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index 540df82..7ed76a9 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -28,6 +28,7 @@ obj-$(CONFIG_OMAP) += omap_lcdc.o
>  obj-$(CONFIG_PXA2XX) += pxa2xx_lcd.o
>  obj-$(CONFIG_SM501) += sm501.o
>  obj-$(CONFIG_TCX) += tcx.o
> +obj-$(CONFIG_CG3) += cg3.o
>  
>  obj-$(CONFIG_VGA) += vga.o
>  
> diff --git a/hw/display/cg3.c b/hw/display/cg3.c
> new file mode 100644
> index 0000000..b6bc0c2
> --- /dev/null
> +++ b/hw/display/cg3.c
> @@ -0,0 +1,384 @@
> +/*
> + * QEMU CG3 Frame buffer
> + *
> + * Copyright (c) 2012 Bob Breuer
> + * Copyright (c) 2013 Mark Cave-Ayland
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu-common.h"
> +#include "qemu/error-report.h"
> +#include "ui/console.h"
> +#include "hw/sysbus.h"
> +#include "hw/loader.h"
> +
> +/* Change to 1 to enable debugging */
> +#define DEBUG_CG3 0
> +
> +#define CG3_ROM_FILE  "QEMU,cgthree.bin"
> +#define FCODE_MAX_ROM_SIZE 0x10000
> +
> +#define CG3_REG_SIZE            0x20
> +
> +#define CG3_REG_BT458_ADDR      0x0
> +#define CG3_REG_BT458_COLMAP    0x4
> +#define CG3_REG_FBC_CTRL        0x10
> +#define CG3_REG_FBC_STATUS      0x11
> +#define CG3_REG_FBC_CURSTART    0x12
> +#define CG3_REG_FBC_CUREND      0x13
> +#define CG3_REG_FBC_VCTRL       0x14
> +
> +/* Control register flags */
> +#define CG3_CR_ENABLE_INTS      0x80
> +
> +/* Status register flags */
> +#define CG3_SR_PENDING_INT      0x80
> +#define CG3_SR_1152_900_76_B    0x60
> +#define CG3_SR_ID_COLOR         0x01
> +
> +#define CG3_VRAM_SIZE 0x100000
> +#define CG3_VRAM_OFFSET 0x800000
> +
> +#define DPRINTF(fmt, ...) do { \
> +    if (DEBUG_CG3) { \
> +        printf("CG3: " fmt , ## __VA_ARGS__); \
> +    } \
> +} while (0);
> +
> +#define TYPE_CG3 "cgthree"
> +#define CG3(obj) OBJECT_CHECK(CG3State, (obj), TYPE_CG3)
> +
> +typedef struct CG3State {
> +    SysBusDevice parent_obj;
> +
> +    QemuConsole *con;
> +    qemu_irq irq;
> +    hwaddr prom_addr;
> +    MemoryRegion vram_mem;
> +    MemoryRegion rom;
> +    MemoryRegion reg;
> +    uint32_t vram_size;
> +    int full_update;
> +    uint8_t regs[16];
> +    uint8_t r[256], g[256], b[256];
> +    uint16_t width, height, depth;
> +    uint8_t dac_index, dac_state;
> +} CG3State;
> +
> +static void cg3_update_display(void *opaque)
> +{
> +    CG3State *s = opaque;
> +    DisplaySurface *surface = qemu_console_surface(s->con);
> +    const uint8_t *pix;
> +    uint32_t *data;
> +    uint32_t dval;
> +    int x, y, y_start;
> +    unsigned int width, height;
> +    ram_addr_t page, page_min, page_max;
> +
> +    if (surface_bits_per_pixel(surface) != 32) {
> +        return;
> +    }
> +    width = s->width;
> +    height = s->height;
> +
> +    y_start = -1;
> +    page_min = -1;
> +    page_max = 0;
> +    page = 0;
> +    pix = memory_region_get_ram_ptr(&s->vram_mem);
> +    data = (uint32_t *)surface_data(surface);
> +
> +    for (y = 0; y < height; y++) {
> +        int update = s->full_update;
> +
> +        page = (y * width) & TARGET_PAGE_MASK;
> +        update |= memory_region_get_dirty(&s->vram_mem, page, page + width,
> +                                          DIRTY_MEMORY_VGA);
> +        if (update) {
> +            if (y_start < 0) {
> +                y_start = y;
> +            }
> +            if (page < page_min) {
> +                page_min = page;
> +            }
> +            if (page > page_max) {
> +                page_max = page;
> +            }
> +
> +            for (x = 0; x < width; x++) {
> +                dval = *pix++;
> +                dval = (s->r[dval] << 16) | (s->g[dval] << 8) | s->b[dval];
> +                *data++ = dval;
> +            }
> +        } else {
> +            if (y_start >= 0) {
> +                dpy_gfx_update(s->con, 0, y_start, s->width, y - y_start);
> +                y_start = -1;
> +            }
> +            pix += width;
> +            data += width;
> +        }
> +    }
> +    s->full_update = 0;
> +    if (y_start >= 0) {
> +        dpy_gfx_update(s->con, 0, y_start, s->width, y - y_start);
> +    }
> +    if (page_max >= page_min) {
> +        memory_region_reset_dirty(&s->vram_mem,
> +                              page_min, page_max - page_min + TARGET_PAGE_SIZE,
> +                              DIRTY_MEMORY_VGA);
> +    }
> +    /* vsync interrupt? */
> +    if (s->regs[0] & CG3_CR_ENABLE_INTS) {
> +        s->regs[1] |= CG3_SR_PENDING_INT;
> +        qemu_irq_raise(s->irq);
> +    }
> +}
> +
> +static void cg3_invalidate_display(void *opaque)
> +{
> +    CG3State *s = opaque;
> +
> +    memory_region_set_dirty(&s->vram_mem, 0, CG3_VRAM_SIZE);
> +}
> +
> +static uint64_t cg3_reg_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    CG3State *s = opaque;
> +    int val;
> +
> +    switch (addr) {
> +    case CG3_REG_BT458_ADDR:
> +    case CG3_REG_BT458_COLMAP:
> +        val = 0;
> +        break;
> +    case CG3_REG_FBC_CTRL:
> +        val = s->regs[0];
> +        break;
> +    case CG3_REG_FBC_STATUS:
> +        /* monitor ID 6, board type = 1 (color) */
> +        val = s->regs[1] | CG3_SR_1152_900_76_B | CG3_SR_ID_COLOR;
> +        break;
> +    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
> +        val = s->regs[addr - 0x10];
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP,
> +                  "cg3: Unimplemented register read "
> +                  "reg 0x%" HWADDR_PRIx " size 0x%x\n",
> +                  addr, size);
> +        val = 0;
> +        break;
> +    }
> +    DPRINTF("read %02x from reg %x\n", val, (int)addr);
> +    return val;
> +}
> +
> +static void cg3_reg_write(void *opaque, hwaddr addr, uint64_t val,
> +                          unsigned size)
> +{
> +    CG3State *s = opaque;
> +    uint8_t regval;
> +    int i;
> +
> +    DPRINTF("write %02lx to reg %x size %d\n", val, (int)addr, size);
> +
> +    switch (addr) {
> +    case CG3_REG_BT458_ADDR:
> +        s->dac_index = val;
> +        s->dac_state = 0;
> +        break;
> +    case CG3_REG_BT458_COLMAP:
> +        /* This register can be written to as either a long word or a byte */
> +        if (size == 1) {
> +            val <<= 24;
> +        }
> +
> +        for (i = 0; i < size; i++) {
> +            regval = val >> 24;
> +
> +            switch (s->dac_state) {
> +            case 0:
> +                s->r[s->dac_index] = regval;
> +                s->dac_state++;
> +                break;
> +            case 1:
> +                s->g[s->dac_index] = regval;
> +                s->dac_state++;
> +                break;
> +            case 2:
> +                s->b[s->dac_index] = regval;
> +                /* Index autoincrement */
> +                s->dac_index = (s->dac_index + 1) & 0xff;
> +            default:
> +                s->dac_state = 0;
> +                break;
> +            }
> +            val <<= 8;
> +        }
> +        s->full_update = 1;
> +        break;
> +    case CG3_REG_FBC_CTRL:
> +        s->regs[0] = val;
> +        break;
> +    case CG3_REG_FBC_STATUS:
> +        if (s->regs[1] & CG3_SR_PENDING_INT) {
> +            /* clear interrupt */
> +            s->regs[1] &= ~CG3_SR_PENDING_INT;
> +            qemu_irq_lower(s->irq);
> +        }
> +        break;
> +    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
> +        s->regs[addr - 0x10] = val;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP,
> +                  "cg3: Unimplemented register write "
> +                  "reg 0x%" HWADDR_PRIx " size 0x%x value 0x%" PRIx64 "\n",
> +                  addr, size, val);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps cg3_reg_ops = {
> +    .read = cg3_reg_read,
> +    .write = cg3_reg_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static const GraphicHwOps cg3_ops = {
> +    .invalidate = cg3_invalidate_display,
> +    .gfx_update = cg3_update_display,
> +};
> +
> +static void cg3_realizefn(DeviceState *dev, Error **errp)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    CG3State *s = CG3(dev);
> +    int ret;
> +    char *fcode_filename;
> +
> +    /* FCode ROM */
> +    memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE);
> +    vmstate_register_ram_global(&s->rom);
> +    memory_region_set_readonly(&s->rom, true);
> +    sysbus_init_mmio(sbd, &s->rom);
> +


I think this initialization code could be done in a SysBusDeviceClass init operation,
don't you think?


> +    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
> +    if (fcode_filename) {
> +        ret = load_image_targphys(fcode_filename, s->prom_addr,
> +                                  FCODE_MAX_ROM_SIZE);
> +        if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
> +            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);


What happens if we fail to load the rom file? is the framebuffer supposed to work?


> +        }
> +    }
> +
> +    memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
> +                          CG3_REG_SIZE);
> +    sysbus_init_mmio(sbd, &s->reg);
> +
> +    memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size);
> +    vmstate_register_ram_global(&s->vram_mem);
> +    sysbus_init_mmio(sbd, &s->vram_mem);
> +
> +    sysbus_init_irq(sbd, &s->irq);
> +
> +    s->con = graphic_console_init(DEVICE(dev), &cg3_ops, s);
> +    qemu_console_resize(s->con, s->width, s->height);
> +}
> +
> +static int vmstate_cg3_post_load(void *opaque, int version_id)
> +{
> +    CG3State *s = opaque;
> +
> +    cg3_invalidate_display(s);
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_cg3 = {
> +    .name = "cg3",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = vmstate_cg3_post_load,
> +    .fields    = (VMStateField[]) {
> +        VMSTATE_UINT16(height, CG3State),
> +        VMSTATE_UINT16(width, CG3State),
> +        VMSTATE_UINT16(depth, CG3State),
> +        VMSTATE_BUFFER(r, CG3State),
> +        VMSTATE_BUFFER(g, CG3State),
> +        VMSTATE_BUFFER(b, CG3State),
> +        VMSTATE_UINT8(dac_index, CG3State),
> +        VMSTATE_UINT8(dac_state, CG3State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void cg3_reset(DeviceState *d)
> +{
> +    CG3State *s = CG3(d);
> +
> +    /* Initialize palette */
> +    memset(s->r, 0, 256);
> +    memset(s->g, 0, 256);
> +    memset(s->b, 0, 256);
> +
> +    s->dac_state = 0;
> +    s->full_update = 1;
> +    qemu_irq_lower(s->irq);
> +}
> +
> +static Property cg3_properties[] = {
> +    DEFINE_PROP_UINT32("vram_size",    CG3State, vram_size, -1),
> +    DEFINE_PROP_UINT16("width",        CG3State, width,     -1),
> +    DEFINE_PROP_UINT16("height",       CG3State, height,    -1),
> +    DEFINE_PROP_UINT16("depth",        CG3State, depth,     -1),
> +    DEFINE_PROP_UINT64("prom_addr",    CG3State, prom_addr, -1),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void cg3_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = cg3_realizefn;
> +    dc->reset = cg3_reset;
> +    dc->vmsd = &vmstate_cg3;
> +    dc->props = cg3_properties;
> +}
> +
> +static const TypeInfo cg3_info = {
> +    .name          = TYPE_CG3,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(CG3State),
> +    .class_init    = cg3_class_init,
> +};
> +
> +static void cg3_register_types(void)
> +{
> +    type_register_static(&cg3_info);
> +}
> +
> +type_init(cg3_register_types)
> diff --git a/pc-bios/QEMU,cgthree.bin b/pc-bios/QEMU,cgthree.bin
> new file mode 100644
> index 0000000000000000000000000000000000000000..6fec9462070bf9a0f88024c7bf66f444d1d11d61
> GIT binary patch
> literal 850
> zcmZ{i&2HL25XX10!NXT-76XB#Rk_y^s6|SoR%&la53Q7_>KX6`yTvxLO)GA>_ch=J
> zP_0Ed5~+vw1^OC&f^?RT1C@IDd*?SZ|5@Af2Y<wjX;&#S`O9L)^D4_Nujb2jiXgca
> zPDC*9!r1=eIU=;bdQRdZo4=yUd6i|Ci{<)%MIyz_ir4;eaD_K=6J(UtR=nVdN#fcA
> zFNrruCp7i~VGm}B*rM#FYA_wy$!sE!rI=f#Xh;N$<uT(|S$=6UrZaVA++l5xwGGbi
> zu)fC(Rdr#9vwOTXDN4*eUYqPSqhX~xGG|XyEYsmuks~^k)Zx)xil*!=AoCkEsCJ<O
> zoLnnXbubf1(BxVqMqm=>vIAO|=luS}_JT~FP*rk6h2b=zc%GuQBB{~))g@X_rt6=%
> zVK@$>Ha6q}>z8kpvySz{2N@kpEMXb>Jz5ksB_3_=s6dTCOX4v$*W4J65;qbe1Ke=D
> zcrxzKpvBAAAKra@*6Vcb?u%{@nkk;p^!ZDR`PfpU9u9?JLjiUu4_oRe`bNojC4ddA
> z-NOJr!DrG6H~Nkfi8uxm4a5uZ+ZQly!#DLmP9;_lsVKMI5>-P{cC&R96e!56_1J6&
> zm}<Z|R2J&PbKMJ)NOcg^Z|U+yl{R+kL90s5Wj_qOB#i7>1hD{<>+03P;w8TyOmF(b
> yWEu%F;rYw!_h)ClbGu8)^3d%^loP5i*^VudTX8sz;xLL`?}lgvPvCTor|d5SYsmWm
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/pc-bios/README b/pc-bios/README
> index f190068..5914200 100644
> --- a/pc-bios/README
> +++ b/pc-bios/README
> @@ -11,8 +11,8 @@
>    firmware implementation. The goal is to implement a 100% IEEE
>    1275-1994 (referred to as Open Firmware) compliant firmware.
>    The included images for PowerPC (for 32 and 64 bit PPC CPUs),
> -  Sparc32 (including QEMU,tcx.bin) and Sparc64 are built from OpenBIOS SVN
> -  revision 1246.
> +  Sparc32 (including QEMU,tcx.bin and QEMU,cgthree.bin) and Sparc64 are built
> +  from OpenBIOS SVN revision 1246.
>  
>  - SLOF (Slimline Open Firmware) is a free IEEE 1275 Open Firmware
>    implementation for certain IBM POWER hardware.  The sources are at
> -- 
> 1.7.10.4
> 
> 

-- 
Leandro Dorileo

Re: [Qemu-devel] [PATCHv3 0/2] sun4m: Implement Sun CG3 framebuffer for QEMU

[Mark Cave-Ayland]

On 19/02/14 13:01, Andreas Färber wrote:

> Am 19.02.2014 10:05, schrieb Mark Cave-Ayland:
>> This patchset provides QEMU with an implementation of the Sun CG3 8-bit
>> framebuffer. It is based upon Bob Breuer's original work which has been
>> rebased onto git master, and is now capable of running with an OpenBIOS CG3
>> FCode ROM instead of requiring copies of proprietary Sun ROMs.
>>
>> The motivation behind this patch is that older operating systems such as
>> Debian Woody and Solaris (running OpenWindows) do not contain drivers for the
>> TCX framebuffer and as a result currently cannot run in graphical mode. The
>> screenshots linked below show qemu-system-sparc successfully running both
>> Debian Woody and the Solaris 8 installer in graphical mode during testing:
>>
>> http://www.ilande.co.uk/tmp/debian-woody.png
>> http://www.ilande.co.uk/tmp/sol8-1.png
>> http://www.ilande.co.uk/tmp/sol8-2.png
>>
>> The CG3 framebuffer is selected by passing -vga cg3 on the command line to
>> qemu-system-sparc. If either -vga tcx is specified (or the -vga argument is
>> omitted) then qemu-system-sparc defaults to using the existing TCX
>> framebuffer to maintain compatibility.
>>
>> v3:
>>      - Rebased to git master
>>      - Fix DEBUG_CG3 macro
>>      - Use register constants based upon Linux/BSD drivers
>>      - Use qemu_log(LOG_UNIMP ... ) to capture unrecognised register accesses
>>      - Rename device type from SUNW,cgthree to cgthree (matches OBP)
>>      - Use error_report() instead of fprintf(stderr ... )
>>      - Convert from init to realizefn
>>
>> v2:
>>      - Rebased to git master
>>      - Updated QEMU,cgthree.bin ROM to latest OpenBIOS version
>>      - Added Peter Maydell to CC
>>
>>
>> Mark Cave-Ayland (2):
>>    sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
>>    sun4m: Add Sun CG3 framebuffer initialisation function
>
> Reviewed-by: Andreas Färber<afaerber@suse.de>

Thanks a lot!

> There's some lines we could extract from the realizefn into an
> instance_init, but can be done as follow-up when needed.

Okay great. I guess that it's always possible to move something from 
realizefn into instance_init, but not the other way around.

> Two small questions, are vram_size and prom_addr for compatibility? New
> convention for QOM properties would be dashes.

They are only named that way because that's the way they were in tcx.c. 
Do you want me to adjust and resend?


ATB,

Mark.

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Mark Cave-Ayland]

On 19/02/14 13:35, Leandro Dorileo wrote:

Hi Leandro,

>> +static void cg3_realizefn(DeviceState *dev, Error **errp)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    CG3State *s = CG3(dev);
>> +    int ret;
>> +    char *fcode_filename;
>> +
>> +    /* FCode ROM */
>> +    memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE);
>> +    vmstate_register_ram_global(&s->rom);
>> +    memory_region_set_readonly(&s->rom, true);
>> +    sysbus_init_mmio(sbd,&s->rom);
>> +
>
>
> I think this initialization code could be done in a SysBusDeviceClass init operation,
> don't you think?

I think it's possible since these MemoryRegions don't depend upon 
properties, but I leave that to Andres who seems reasonably happy with 
the patchset in its current form.

>
>> +    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
>> +    if (fcode_filename) {
>> +        ret = load_image_targphys(fcode_filename, s->prom_addr,
>> +                                  FCODE_MAX_ROM_SIZE);
>> +        if (ret<  0 || ret>  FCODE_MAX_ROM_SIZE) {
>> +            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
>
>
> What happens if we fail to load the rom file? is the framebuffer supposed to work?

I guess the framebuffer would still "work" although nothing would be 
able to find its address because the node wouldn't exist in the device tree?


ATB,

Mark.

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Leandro Dorileo]

On Wed, Feb 19, 2014 at 09:39:09PM +0000, Mark Cave-Ayland wrote:
> On 19/02/14 13:35, Leandro Dorileo wrote:
> 
> Hi Leandro,
> 
> >>+static void cg3_realizefn(DeviceState *dev, Error **errp)
> >>+{
> >>+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> >>+    CG3State *s = CG3(dev);
> >>+    int ret;
> >>+    char *fcode_filename;
> >>+
> >>+    /* FCode ROM */
> >>+    memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE);
> >>+    vmstate_register_ram_global(&s->rom);
> >>+    memory_region_set_readonly(&s->rom, true);
> >>+    sysbus_init_mmio(sbd,&s->rom);
> >>+
> >
> >
> >I think this initialization code could be done in a SysBusDeviceClass init operation,
> >don't you think?
> 
> I think it's possible since these MemoryRegions don't depend upon
> properties, but I leave that to Andres who seems reasonably happy with the
> patchset in its current form.


Yes, I just saw his comment in the patch 02...

> 
> >
> >>+    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
> >>+    if (fcode_filename) {
> >>+        ret = load_image_targphys(fcode_filename, s->prom_addr,
> >>+                                  FCODE_MAX_ROM_SIZE);
> >>+        if (ret<  0 || ret>  FCODE_MAX_ROM_SIZE) {
> >>+            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
> >
> >
> >What happens if we fail to load the rom file? is the framebuffer supposed to work?
> 
> I guess the framebuffer would still "work" although nothing would be able to
> find its address because the node wouldn't exist in the device tree?
> 

Ok, when we move this code to an instance init op we handle it a bit better.

> 
> ATB,
> 
> Mark.

-- 
Leandro Dorileo

Re: [Qemu-devel] [PATCHv3 0/2] sun4m: Implement Sun CG3 framebuffer for QEMU

[Mark Cave-Ayland]

On 19/02/14 21:35, Mark Cave-Ayland wrote:

> Thanks a lot!
>
>> There's some lines we could extract from the realizefn into an
>> instance_init, but can be done as follow-up when needed.
>
> Okay great. I guess that it's always possible to move something from
> realizefn into instance_init, but not the other way around.
>
>> Two small questions, are vram_size and prom_addr for compatibility? New
>> convention for QOM properties would be dashes.
>
> They are only named that way because that's the way they were in tcx.c.
> Do you want me to adjust and resend?

Hi Andreas,

Due to a lack of response, I've renamed the prom_addr and vram_size 
properties to prom-addr and vram-size as requested and pushed the result 
to my qemu-sparc branch. Hope this is okay in order to get the basic 
patch applied before freeze (I guess it's up to Peter whether a tidy-up 
patch for TCX could be applied after this point or not).


ATB,

Mark.

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Paolo Bonzini]

Il 19/02/2014 10:05, Mark Cave-Ayland ha scritto:
> +#define CG3_REG_SIZE            0x20
> +
> +#define CG3_REG_FBC_CTRL        0x10
> +#define CG3_REG_FBC_STATUS      0x11
> +#define CG3_REG_FBC_CURSTART    0x12
> +#define CG3_REG_FBC_CUREND      0x13
> +#define CG3_REG_FBC_VCTRL       0x14
> +
> +typedef struct CG3State {
...

> +    uint8_t regs[16];

...

> +    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
> +        val = s->regs[addr - 0x10];
> +        break;
> +    default:

Something weird here, you can access regs[16] if addr == CG3_REG_SIZE.

The same happens in the write path.

Paolo

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Paolo Bonzini]

Il 05/03/2014 11:05, Paolo Bonzini ha scritto:
> Il 19/02/2014 10:05, Mark Cave-Ayland ha scritto:
>> +#define CG3_REG_SIZE            0x20
>> +
>> +#define CG3_REG_FBC_CTRL        0x10
>> +#define CG3_REG_FBC_STATUS      0x11
>> +#define CG3_REG_FBC_CURSTART    0x12
>> +#define CG3_REG_FBC_CUREND      0x13
>> +#define CG3_REG_FBC_VCTRL       0x14
>> +
>> +typedef struct CG3State {
> ...
>
>> +    uint8_t regs[16];
>
> ...
>
>> +    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
>> +        val = s->regs[addr - 0x10];
>> +        break;
>> +    default:
>
> Something weird here, you can access regs[16] if addr == CG3_REG_SIZE.
>
> The same happens in the write path.

Ping.  I cannot fix it without access to the datasheet, though I suspect 
you want CG3_REG_SIZE - 1.

Paolo

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Andreas Färber]

Hi,

Am 19.02.2014 22:39, schrieb Mark Cave-Ayland:
> On 19/02/14 13:35, Leandro Dorileo wrote:
> 
> Hi Leandro,
> 
>>> +static void cg3_realizefn(DeviceState *dev, Error **errp)
>>> +{
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +    CG3State *s = CG3(dev);
>>> +    int ret;
>>> +    char *fcode_filename;
>>> +
>>> +    /* FCode ROM */
>>> +    memory_region_init_ram(&s->rom, NULL, "cg3.prom",
>>> FCODE_MAX_ROM_SIZE);
>>> +    vmstate_register_ram_global(&s->rom);
>>> +    memory_region_set_readonly(&s->rom, true);
>>> +    sysbus_init_mmio(sbd,&s->rom);
>>> +
>>
>>
>> I think this initialization code could be done in a SysBusDeviceClass
>> init operation,
>> don't you think?
> 
> I think it's possible since these MemoryRegions don't depend upon
> properties, but I leave that to Andres who seems reasonably happy with
> the patchset in its current form.

Just seeing this...

memory_region_init_ram() and sysbus_init_mmio() could indeed be moved to
an .instance_init function, given that FCODE_MAX_ROM_SIZE is constant.
The others no. It makes a difference when considering reentrancy of the
property code via qom-set (just posted a patchset that makes playing
with that easier), although there's probably more corner cases to
consider. Could either of you follow up with a cleanup?

Regards,
Andreas

>>> +    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
>>> +    if (fcode_filename) {
>>> +        ret = load_image_targphys(fcode_filename, s->prom_addr,
>>> +                                  FCODE_MAX_ROM_SIZE);
>>> +        if (ret<  0 || ret>  FCODE_MAX_ROM_SIZE) {
>>> +            error_report("cg3: could not load prom '%s'",
>>> CG3_ROM_FILE);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Mark Cave-Ayland]

On 07/05/14 20:56, Paolo Bonzini wrote:

> Il 05/03/2014 11:05, Paolo Bonzini ha scritto:
>> Il 19/02/2014 10:05, Mark Cave-Ayland ha scritto:
>>> +#define CG3_REG_SIZE            0x20
>>> +
>>> +#define CG3_REG_FBC_CTRL        0x10
>>> +#define CG3_REG_FBC_STATUS      0x11
>>> +#define CG3_REG_FBC_CURSTART    0x12
>>> +#define CG3_REG_FBC_CUREND      0x13
>>> +#define CG3_REG_FBC_VCTRL       0x14
>>> +
>>> +typedef struct CG3State {
>> ...
>>
>>> +    uint8_t regs[16];
>>
>> ...
>>
>>> +    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
>>> +        val = s->regs[addr - 0x10];
>>> +        break;
>>> +    default:
>>
>> Something weird here, you can access regs[16] if addr == CG3_REG_SIZE.
>>
>> The same happens in the write path.
>
> Ping.  I cannot fix it without access to the datasheet, though I suspect
> you want CG3_REG_SIZE - 1.

Hi Paolo,

Sorry I didn't think you could access regs[16] since the MemoryRegion 
size is set to CG3_REG_SIZE too (and so I hope should only handle 
accesses from 0 to CG3_REG_SIZE - 1).

Anyway, I've quickly tried a Solaris 8 boot test replacing CG3_REG_SIZE 
with CG3_REG_SIZE - 1 for the case statements in both the read and write 
paths and everything still works, so happy for you to go ahead and fix it.


ATB,

Mark.

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Paolo Bonzini]

Il 08/05/2014 16:44, Mark Cave-Ayland ha scritto:
>>>
>>>> +    case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE:
>>>> +        val = s->regs[addr - 0x10];
>>>> +        break;
>>>> +    default:
>>>
>>> Something weird here, you can access regs[16] if addr == CG3_REG_SIZE.
>>>
>>> The same happens in the write path.
>>
>> Ping.  I cannot fix it without access to the datasheet, though I suspect
>> you want CG3_REG_SIZE - 1.
>
> Hi Paolo,
>
> Sorry I didn't think you could access regs[16] since the MemoryRegion
> size is set to CG3_REG_SIZE too (and so I hope should only handle
> accesses from 0 to CG3_REG_SIZE - 1).
>
> Anyway, I've quickly tried a Solaris 8 boot test replacing CG3_REG_SIZE
> with CG3_REG_SIZE - 1 for the case statements in both the read and write
> paths and everything still works, so happy for you to go ahead and fix it.

Ah okay so it's a false positive.  But yes, it's better to fix it.  I'll 
try to send a patch for qemu-trivial.

Paolo

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Mark Cave-Ayland]

On 08/05/14 15:34, Andreas Färber wrote:

> Hi,
>
> Am 19.02.2014 22:39, schrieb Mark Cave-Ayland:
>> On 19/02/14 13:35, Leandro Dorileo wrote:
>>
>> Hi Leandro,
>>
>>>> +static void cg3_realizefn(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>> +    CG3State *s = CG3(dev);
>>>> +    int ret;
>>>> +    char *fcode_filename;
>>>> +
>>>> +    /* FCode ROM */
>>>> +    memory_region_init_ram(&s->rom, NULL, "cg3.prom",
>>>> FCODE_MAX_ROM_SIZE);
>>>> +    vmstate_register_ram_global(&s->rom);
>>>> +    memory_region_set_readonly(&s->rom, true);
>>>> +    sysbus_init_mmio(sbd,&s->rom);
>>>> +
>>>
>>>
>>> I think this initialization code could be done in a SysBusDeviceClass
>>> init operation,
>>> don't you think?
>>
>> I think it's possible since these MemoryRegions don't depend upon
>> properties, but I leave that to Andres who seems reasonably happy with
>> the patchset in its current form.
>
> Just seeing this...
>
> memory_region_init_ram() and sysbus_init_mmio() could indeed be moved to
> an .instance_init function, given that FCODE_MAX_ROM_SIZE is constant.
> The others no. It makes a difference when considering reentrancy of the
> property code via qom-set (just posted a patchset that makes playing
> with that easier), although there's probably more corner cases to
> consider. Could either of you follow up with a cleanup?

Is something like this correct? It also seems that the register I/O 
region initialisation can get moved into the initfn since that doesn't 
depend on any properties either.

If it looks good, I'll spin up a proper patchset that does the same to 
TCX too.


--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -274,19 +274,30 @@ static const GraphicHwOps cg3_ops = {
      .gfx_update = cg3_update_display,
  };

-static void cg3_realizefn(DeviceState *dev, Error **errp)
+static void cg3_initfn(Object *obj)
  {
-    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-    CG3State *s = CG3(dev);
-    int ret;
-    char *fcode_filename;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    CG3State *s = CG3(obj);

      /* FCode ROM */
      memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE);
      vmstate_register_ram_global(&s->rom);
      memory_region_set_readonly(&s->rom, true);
      sysbus_init_mmio(sbd, &s->rom);
+
+    memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
+                          CG3_REG_SIZE);
+    sysbus_init_mmio(sbd, &s->reg);
+}

+static void cg3_realizefn(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    CG3State *s = CG3(dev);
+    int ret;
+    char *fcode_filename;
+
+    /* FCode ROM */
      fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
      if (fcode_filename) {
          ret = load_image_targphys(fcode_filename, s->prom_addr,
@@ -296,10 +307,6 @@ static void cg3_realizefn(DeviceState *dev, Error 
**errp)
          }
      }

-    memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
-                          CG3_REG_SIZE);
-    sysbus_init_mmio(sbd, &s->reg);
-
      memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size);
      vmstate_register_ram_global(&s->vram_mem);
      sysbus_init_mmio(sbd, &s->vram_mem);
@@ -374,6 +381,7 @@ static const TypeInfo cg3_info = {
      .name          = TYPE_CG3,
      .parent        = TYPE_SYS_BUS_DEVICE,
      .instance_size = sizeof(CG3State),
+    .instance_init = cg3_initfn,
      .class_init    = cg3_class_init,
  };



ATB,

Mark.

Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM

[Andreas Färber]

Am 19.05.2014 15:03, schrieb Mark Cave-Ayland:
> On 08/05/14 15:34, Andreas Färber wrote:
>> Am 19.02.2014 22:39, schrieb Mark Cave-Ayland:
>>> On 19/02/14 13:35, Leandro Dorileo wrote:
>>>>> +static void cg3_realizefn(DeviceState *dev, Error **errp)
>>>>> +{
>>>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>>> +    CG3State *s = CG3(dev);
>>>>> +    int ret;
>>>>> +    char *fcode_filename;
>>>>> +
>>>>> +    /* FCode ROM */
>>>>> +    memory_region_init_ram(&s->rom, NULL, "cg3.prom",
>>>>> FCODE_MAX_ROM_SIZE);
>>>>> +    vmstate_register_ram_global(&s->rom);
>>>>> +    memory_region_set_readonly(&s->rom, true);
>>>>> +    sysbus_init_mmio(sbd,&s->rom);
>>>>> +
>>>>
>>>>
>>>> I think this initialization code could be done in a SysBusDeviceClass
>>>> init operation,
>>>> don't you think?
>>>
>>> I think it's possible since these MemoryRegions don't depend upon
>>> properties, but I leave that to Andres who seems reasonably happy with
>>> the patchset in its current form.
>>
>> memory_region_init_ram() and sysbus_init_mmio() could indeed be moved to
>> an .instance_init function, given that FCODE_MAX_ROM_SIZE is constant.
>> The others no. It makes a difference when considering reentrancy of the
>> property code via qom-set (just posted a patchset that makes playing
>> with that easier), although there's probably more corner cases to
>> consider. Could either of you follow up with a cleanup?
> 
> Is something like this correct? It also seems that the register I/O
> region initialisation can get moved into the initfn since that doesn't
> depend on any properties either.
> 
> If it looks good, I'll spin up a proper patchset that does the same to
> TCX too.
> 
> 
> --- a/hw/display/cg3.c
> +++ b/hw/display/cg3.c
> @@ -274,19 +274,30 @@ static const GraphicHwOps cg3_ops = {
>      .gfx_update = cg3_update_display,
>  };
> 
> -static void cg3_realizefn(DeviceState *dev, Error **errp)
> +static void cg3_initfn(Object *obj)
>  {
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -    CG3State *s = CG3(dev);
> -    int ret;
> -    char *fcode_filename;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    CG3State *s = CG3(obj);
> 
>      /* FCode ROM */
>      memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE);

>      vmstate_register_ram_global(&s->rom);

This has global effects, so must stay in realize. The rest looks good.

Cheers,
Andreas

>      memory_region_set_readonly(&s->rom, true);
>      sysbus_init_mmio(sbd, &s->rom);
> +
> +    memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
> +                          CG3_REG_SIZE);
> +    sysbus_init_mmio(sbd, &s->reg);
> +}
> 
> +static void cg3_realizefn(DeviceState *dev, Error **errp)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    CG3State *s = CG3(dev);
> +    int ret;
> +    char *fcode_filename;
> +
> +    /* FCode ROM */
>      fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
>      if (fcode_filename) {
>          ret = load_image_targphys(fcode_filename, s->prom_addr,
> @@ -296,10 +307,6 @@ static void cg3_realizefn(DeviceState *dev, Error
> **errp)
>          }
>      }
> 
> -    memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
> -                          CG3_REG_SIZE);
> -    sysbus_init_mmio(sbd, &s->reg);
> -
>      memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size);
>      vmstate_register_ram_global(&s->vram_mem);
>      sysbus_init_mmio(sbd, &s->vram_mem);
> @@ -374,6 +381,7 @@ static const TypeInfo cg3_info = {
>      .name          = TYPE_CG3,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(CG3State),
> +    .instance_init = cg3_initfn,
>      .class_init    = cg3_class_init,
>  };

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg