[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/4] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 3/4] hw/display/vga-mmio: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO |
Date: |
Mon, 13 Dec 2021 12:05:37 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 |
On 12/13/21 11:48, Thomas Huth wrote:
> On 06/12/2021 23.45, Philippe Mathieu-Daudé wrote:
>> Introduce TYPE_VGA_MMIO, a sysbus device.
>>
>> While there is no change in the vga_mmio_init()
>> interface, this is a migration compatibility break
>> of the MIPS Acer Pica 61 Jazz machine (pica61).
>
> It's unfortunate, but as far as I know, it would be pretty difficult or
> even impossible to get this done without versioned machine types? So
> IMHO it's ok to break this in this case here.
Hervé and Zoltan were already Cc'ed, adding Mark and Finn;
I am not sure who else would be annoyed by that change.
>> Suggested-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> include/hw/display/vga.h | 2 +
>> hw/display/vga-mmio.c | 132 +++++++++++++++++++++++++++------------
>> 2 files changed, 94 insertions(+), 40 deletions(-)
>>
>> diff --git a/include/hw/display/vga.h b/include/hw/display/vga.h
>> index c16a5c26dae..98b2e560f9b 100644
>> --- a/include/hw/display/vga.h
>> +++ b/include/hw/display/vga.h
>> @@ -24,6 +24,8 @@ enum vga_retrace_method {
>> extern enum vga_retrace_method vga_retrace_method;
>> +#define TYPE_VGA_MMIO "vga-mmio"
>> +
>> int vga_mmio_init(hwaddr vram_base, hwaddr ctrl_base,
>> int it_shift, MemoryRegion *address_space);
>> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
>> index 5671fdb920f..10bde32af5c 100644
>> --- a/hw/display/vga-mmio.c
>> +++ b/hw/display/vga-mmio.c
>> @@ -23,21 +23,34 @@
>> */
>> #include "qemu/osdep.h"
>> -#include "qemu/bitops.h"
>> -#include "qemu/units.h"
>> -#include "migration/vmstate.h"
>> +#include "qapi/error.h"
>> #include "hw/display/vga.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/display/vga.h"
>> +#include "hw/qdev-properties.h"
>> #include "vga_int.h"
>> -#include "ui/pixel_ops.h"
>> -#define VGA_RAM_SIZE (8 * MiB)
>> +/*
>> + * QEMU interface:
>> + * + sysbus MMIO region 0: VGA I/O registers
>> + * + sysbus MMIO region 1: VGA MMIO registers
>> + * + sysbus MMIO region 2: VGA memory
>> + */
>> -typedef struct VGAMmioState {
>> +OBJECT_DECLARE_SIMPLE_TYPE(VGAMmioState, VGA_MMIO)
>> +
>> +struct VGAMmioState {
>> + /*< private >*/
>> + SysBusDevice parent_obj;
>> +
>> + /*< public >*/
>> VGACommonState vga;
>> - int it_shift;
>> -} VGAMmioState;
>> + MemoryRegion iomem;
>> + MemoryRegion lowmem;
>> +
>> + uint8_t it_shift;
>> +};
>> -/* Memory mapped interface */
>> static uint64_t vga_mm_read(void *opaque, hwaddr addr, unsigned size)
>> {
>> VGAMmioState *s = opaque;
>> @@ -65,42 +78,81 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>> +static void vga_mmio_reset(DeviceState *dev)
>> +{
>> + VGAMmioState *s = VGA_MMIO(dev);
>> +
>> + vga_common_reset(&s->vga);
>> +}
>> +
>> int vga_mmio_init(hwaddr vram_base, hwaddr ctrl_base,
>> int it_shift, MemoryRegion *address_space)
>> {
>> - VGAMmioState *s;
>> - MemoryRegion *s_ioport_ctrl, *vga_io_memory;
>> + DeviceState *dev;
>> + SysBusDevice *s;
>> - s = g_malloc0(sizeof(*s));
>> + dev = qdev_new(TYPE_VGA_MMIO);
>> + qdev_prop_set_uint8(dev, "it_shift", it_shift);
>> + s = SYS_BUS_DEVICE(dev);
>> + sysbus_realize_and_unref(s, &error_fatal);
>> - s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> - s->vga.global_vmstate = true;
>> - vga_common_init(&s->vga, NULL);
>> -
>> - s->it_shift = it_shift;
>> - s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
>> - memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
>> - "vga-mm-ctrl", 0x100000);
>> - memory_region_set_flush_coalesced(s_ioport_ctrl);
>> -
>> - vga_io_memory = g_malloc(sizeof(*vga_io_memory));
>> - /* XXX: endianness? */
>> - memory_region_init_io(vga_io_memory, NULL, &vga_mem_ops, &s->vga,
>> - "vga-mem", 0x20000);
>> -
>> - vmstate_register(NULL, 0, &vmstate_vga_common, s);
>> -
>> - memory_region_add_subregion(address_space, ctrl_base,
>> s_ioport_ctrl);
>> - s->vga.bank_offset = 0;
>> - memory_region_add_subregion(address_space,
>> - vram_base + 0x000a0000, vga_io_memory);
>> - memory_region_set_coalescing(vga_io_memory);
>> -
>> - s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>> -
>> - memory_region_add_subregion(address_space,
>> - VBE_DISPI_LFB_PHYSICAL_ADDRESS,
>> - &s->vga.vram);
>> + sysbus_mmio_map(s, 0, ctrl_base);
>> + sysbus_mmio_map(s, 1, vram_base + 0x000a0000);
>> + sysbus_mmio_map(s, 2, VBE_DISPI_LFB_PHYSICAL_ADDRESS);
>> return 0;
>> }
>> +
>> +static void vga_mmio_realizefn(DeviceState *dev, Error **errp)
>> +{
>> + VGAMmioState *s = VGA_MMIO(dev);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +
>> + memory_region_init_io(&s->iomem, OBJECT(dev), &vga_mm_ctrl_ops, s,
>> + "vga-mmio", 0x100000);
>> + memory_region_set_flush_coalesced(&s->iomem);
>> + sysbus_init_mmio(sbd, &s->iomem);
>> +
>> + /* XXX: endianness? */
>> + memory_region_init_io(&s->lowmem, OBJECT(dev), &vga_mem_ops,
>> &s->vga,
>> + "vga-lowmem", 0x20000);
>> + memory_region_set_coalescing(&s->lowmem);
>> + sysbus_init_mmio(sbd, &s->lowmem);
>> +
>> + s->vga.bank_offset = 0;
>> + s->vga.global_vmstate = true;
>> + vga_common_init(&s->vga, OBJECT(dev));
>> + sysbus_init_mmio(sbd, &s->vga.vram);
>> + s->vga.con = graphic_console_init(dev, 0, s->vga.hw_ops, &s->vga);
>> +}
>> +
>> +static Property vga_mmio_properties[] = {
>> + DEFINE_PROP_UINT8("it_shift", VGAMmioState, it_shift, 0),
>> + DEFINE_PROP_UINT32("vgamem_mb", VGAMmioState, vga.vram_size_mb, 8),
>> + DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void vga_mmio_class_initfn(ObjectClass *klass, void *data)
>
> Just a matter of taste, but I'd prefer "oc" instead of "klass".
>
> Anyway:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
>
[PATCH v2 4/4] hw/mips/jazz: Inline vga_mmio_init() and remove it, Philippe Mathieu-Daudé, 2021/12/06
Re: [PATCH v2 0/4] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO, Philippe Mathieu-Daudé, 2021/12/13