| Message ID | 869a8c5b526be9eb1316c9e7c7442bdd1c88c4b2.1488068248.git.balaton@eik.bme.hu |
|---|---|
| State | New |
| Headers | show |
On 25 February 2017 at 23:53, BALATON Zoltan <balaton@eik.bme.hu> wrote: > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/display/sm501.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 105 insertions(+), 5 deletions(-) > > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index 32223f6..b682a95 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -65,6 +65,7 @@ > > #define MMIO_BASE_OFFSET 0x3e00000 > #define MMIO_SIZE 0x200000 > +#define DC_PALETTE_ENTRIES (0x400 * 3) > > /* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */ > > @@ -491,7 +492,7 @@ typedef struct SM501State { > uint32_t uart0_mcr; > uint32_t uart0_scr; > > - uint8_t dc_palette[0x400 * 3]; > + uint8_t dc_palette[DC_PALETTE_ENTRIES]; > > uint32_t dc_panel_control; > uint32_t dc_panel_panning_control; > @@ -1537,16 +1538,32 @@ static const GraphicHwOps sm501_ops = { > .gfx_update = sm501_update_display, > }; > > +static void sm501_reset(void *p) > +{ > + SM501State *s = p; > + > + s->system_control = 0x00100000; /* 2D engine FIFO empty */ > + s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */ Don't forget to update this patch when you fix this reset value in patch 1. > + s->gpio_31_0_control = 0; > + s->gpio_63_32_control = 0; > + s->dram_control = 0; > + s->arbitration_control = 0x05146732; > + s->irq_mask = 0; > + s->misc_timing = 0; > + s->power_mode_control = 0; > + s->dc_panel_control = 0x00010000; /* FIFO level 3 */ > + s->dc_video_control = 0; > + s->dc_crt_control = 0x00010000; > + s->twoD_control = 0; > +} > + > static void sm501_init(SM501State *s, DeviceState *dev, > uint32_t local_mem_bytes) > { > s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes); > SM501_DPRINTF("sm501 local mem size=%x. index=%d\n", get_local_mem_size(s), > s->local_mem_size_index); > - s->system_control = 0x00100000; /* 2D engine FIFO empty */ > - s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */ > - s->dc_panel_control = 0x00010000; /* FIFO level 3 */ > - s->dc_crt_control = 0x00010000; > + qemu_register_reset(sm501_reset, s); Don't use qemu_register_reset(). Set the appropriate dc->reset function pointers instead. > > /* local memory */ > memory_region_init_ram(&s->local_mem_region, OBJECT(dev), "sm501.local", > @@ -1577,6 +1594,77 @@ static void sm501_init(SM501State *s, DeviceState *dev, > s->con = graphic_console_init(DEVICE(dev), 0, &sm501_ops, s); > } > > +static const VMStateDescription vmstate_sm501_regs = { > + .name = "sm501-regs", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(system_control, SM501State), > + VMSTATE_UINT32(misc_control, SM501State), > + VMSTATE_UINT32(gpio_31_0_control, SM501State), > + VMSTATE_UINT32(gpio_63_32_control, SM501State), > + VMSTATE_UINT32(dram_control, SM501State), > + VMSTATE_UINT32(arbitration_control, SM501State), > + VMSTATE_UINT32(irq_mask, SM501State), > + VMSTATE_UINT32(misc_timing, SM501State), > + VMSTATE_UINT32(power_mode_control, SM501State), > + VMSTATE_UINT32(uart0_ier, SM501State), > + VMSTATE_UINT32(uart0_lcr, SM501State), > + VMSTATE_UINT32(uart0_mcr, SM501State), > + VMSTATE_UINT32(uart0_scr, SM501State), > + VMSTATE_UINT8_ARRAY(dc_palette, SM501State, DC_PALETTE_ENTRIES), > + VMSTATE_UINT32(dc_panel_control, SM501State), > + VMSTATE_UINT32(dc_panel_panning_control, SM501State), > + VMSTATE_UINT32(dc_panel_fb_addr, SM501State), > + VMSTATE_UINT32(dc_panel_fb_offset, SM501State), > + VMSTATE_UINT32(dc_panel_fb_width, SM501State), > + VMSTATE_UINT32(dc_panel_fb_height, SM501State), > + VMSTATE_UINT32(dc_panel_tl_location, SM501State), > + VMSTATE_UINT32(dc_panel_br_location, SM501State), > + VMSTATE_UINT32(dc_panel_h_total, SM501State), > + VMSTATE_UINT32(dc_panel_h_sync, SM501State), > + VMSTATE_UINT32(dc_panel_v_total, SM501State), > + VMSTATE_UINT32(dc_panel_v_sync, SM501State), > + VMSTATE_UINT32(dc_panel_hwc_addr, SM501State), > + VMSTATE_UINT32(dc_panel_hwc_location, SM501State), > + VMSTATE_UINT32(dc_panel_hwc_color_1_2, SM501State), > + VMSTATE_UINT32(dc_panel_hwc_color_3, SM501State), > + VMSTATE_UINT32(dc_video_control, SM501State), > + VMSTATE_UINT32(dc_crt_control, SM501State), > + VMSTATE_UINT32(dc_crt_fb_addr, SM501State), > + VMSTATE_UINT32(dc_crt_fb_offset, SM501State), > + VMSTATE_UINT32(dc_crt_h_total, SM501State), > + VMSTATE_UINT32(dc_crt_h_sync, SM501State), > + VMSTATE_UINT32(dc_crt_v_total, SM501State), > + VMSTATE_UINT32(dc_crt_v_sync, SM501State), > + VMSTATE_UINT32(dc_crt_hwc_addr, SM501State), > + VMSTATE_UINT32(dc_crt_hwc_location, SM501State), > + VMSTATE_UINT32(dc_crt_hwc_color_1_2, SM501State), > + VMSTATE_UINT32(dc_crt_hwc_color_3, SM501State), > + VMSTATE_UINT32(twoD_source, SM501State), > + VMSTATE_UINT32(twoD_destination, SM501State), > + VMSTATE_UINT32(twoD_dimension, SM501State), > + VMSTATE_UINT32(twoD_control, SM501State), > + VMSTATE_UINT32(twoD_pitch, SM501State), > + VMSTATE_UINT32(twoD_foreground, SM501State), > + VMSTATE_UINT32(twoD_background, SM501State), > + VMSTATE_UINT32(twoD_stretch, SM501State), > + VMSTATE_UINT32(twoD_color_compare, SM501State), > + VMSTATE_UINT32(twoD_color_compare_mask, SM501State), > + VMSTATE_UINT32(twoD_mask, SM501State), > + VMSTATE_UINT32(twoD_clip_tl, SM501State), > + VMSTATE_UINT32(twoD_clip_br, SM501State), > + VMSTATE_UINT32(twoD_mono_pattern_low, SM501State), > + VMSTATE_UINT32(twoD_mono_pattern_high, SM501State), > + VMSTATE_UINT32(twoD_window_width, SM501State), > + VMSTATE_UINT32(twoD_source_base, SM501State), > + VMSTATE_UINT32(twoD_destination_base, SM501State), > + VMSTATE_UINT32(twoD_alpha, SM501State), > + VMSTATE_UINT32(twoD_wrap, SM501State), > + VMSTATE_END_OF_LIST() > + } > +}; local_mem_size_index is also guest updatable state (it's some of the bits from the SM501_DRAM_CONTROL register), so we need to migrate it as well. > + > #define TYPE_SYSBUS_SM501 "sysbus-sm501" > #define SYSBUS_SM501(obj) \ > OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501) > @@ -1673,6 +1761,17 @@ static Property sm501_pci_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +static const VMStateDescription vmstate_sm501 = { > + .name = "sm501", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_PCI_DEVICE(parent_obj, SM501PCIState), > + VMSTATE_STRUCT(state, SM501PCIState, 1, vmstate_sm501_regs, SM501State), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static void sm501_pci_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -1686,6 +1785,7 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data) > dc->desc = "SM501 Display Controller"; > dc->props = sm501_pci_properties; > dc->hotpluggable = false; > + dc->vmsd = &vmstate_sm501; > } > > static const TypeInfo sm501_pci_info = { > -- > 2.7.4 What about reset and vmstate for the sysbus device? Consider putting reset and vmstate in separate patches. thanks -- PMM
On Thu, 2 Mar 2017, Peter Maydell wrote: > On 25 February 2017 at 23:53, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/display/sm501.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 105 insertions(+), 5 deletions(-) >> >> diff --git a/hw/display/sm501.c b/hw/display/sm501.c >> index 32223f6..b682a95 100644 >> --- a/hw/display/sm501.c >> +++ b/hw/display/sm501.c >> @@ -65,6 +65,7 @@ >> >> #define MMIO_BASE_OFFSET 0x3e00000 >> #define MMIO_SIZE 0x200000 >> +#define DC_PALETTE_ENTRIES (0x400 * 3) >> >> /* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */ >> >> @@ -491,7 +492,7 @@ typedef struct SM501State { >> uint32_t uart0_mcr; >> uint32_t uart0_scr; >> >> - uint8_t dc_palette[0x400 * 3]; >> + uint8_t dc_palette[DC_PALETTE_ENTRIES]; >> >> uint32_t dc_panel_control; >> uint32_t dc_panel_panning_control; >> @@ -1537,16 +1538,32 @@ static const GraphicHwOps sm501_ops = { >> .gfx_update = sm501_update_display, >> }; >> >> +static void sm501_reset(void *p) >> +{ >> + SM501State *s = p; >> + >> + s->system_control = 0x00100000; /* 2D engine FIFO empty */ >> + s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */ > > Don't forget to update this patch when you fix this reset value in patch 1. > >> + s->gpio_31_0_control = 0; >> + s->gpio_63_32_control = 0; >> + s->dram_control = 0; >> + s->arbitration_control = 0x05146732; >> + s->irq_mask = 0; >> + s->misc_timing = 0; >> + s->power_mode_control = 0; >> + s->dc_panel_control = 0x00010000; /* FIFO level 3 */ >> + s->dc_video_control = 0; >> + s->dc_crt_control = 0x00010000; >> + s->twoD_control = 0; >> +} >> + >> static void sm501_init(SM501State *s, DeviceState *dev, >> uint32_t local_mem_bytes) >> { >> s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes); >> SM501_DPRINTF("sm501 local mem size=%x. index=%d\n", get_local_mem_size(s), >> s->local_mem_size_index); >> - s->system_control = 0x00100000; /* 2D engine FIFO empty */ >> - s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */ >> - s->dc_panel_control = 0x00010000; /* FIFO level 3 */ >> - s->dc_crt_control = 0x00010000; >> + qemu_register_reset(sm501_reset, s); > > Don't use qemu_register_reset(). Set the appropriate dc->reset > function pointers instead. Any reason for that? This way I could save two more boilerplate functions because I could define reset function once, otherwise I'd need two versions taking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead? >> /* local memory */ >> memory_region_init_ram(&s->local_mem_region, OBJECT(dev), "sm501.local", >> @@ -1577,6 +1594,77 @@ static void sm501_init(SM501State *s, DeviceState *dev, >> s->con = graphic_console_init(DEVICE(dev), 0, &sm501_ops, s); >> } >> >> +static const VMStateDescription vmstate_sm501_regs = { >> + .name = "sm501-regs", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT32(system_control, SM501State), >> + VMSTATE_UINT32(misc_control, SM501State), >> + VMSTATE_UINT32(gpio_31_0_control, SM501State), >> + VMSTATE_UINT32(gpio_63_32_control, SM501State), >> + VMSTATE_UINT32(dram_control, SM501State), >> + VMSTATE_UINT32(arbitration_control, SM501State), >> + VMSTATE_UINT32(irq_mask, SM501State), >> + VMSTATE_UINT32(misc_timing, SM501State), >> + VMSTATE_UINT32(power_mode_control, SM501State), >> + VMSTATE_UINT32(uart0_ier, SM501State), >> + VMSTATE_UINT32(uart0_lcr, SM501State), >> + VMSTATE_UINT32(uart0_mcr, SM501State), >> + VMSTATE_UINT32(uart0_scr, SM501State), >> + VMSTATE_UINT8_ARRAY(dc_palette, SM501State, DC_PALETTE_ENTRIES), >> + VMSTATE_UINT32(dc_panel_control, SM501State), >> + VMSTATE_UINT32(dc_panel_panning_control, SM501State), >> + VMSTATE_UINT32(dc_panel_fb_addr, SM501State), >> + VMSTATE_UINT32(dc_panel_fb_offset, SM501State), >> + VMSTATE_UINT32(dc_panel_fb_width, SM501State), >> + VMSTATE_UINT32(dc_panel_fb_height, SM501State), >> + VMSTATE_UINT32(dc_panel_tl_location, SM501State), >> + VMSTATE_UINT32(dc_panel_br_location, SM501State), >> + VMSTATE_UINT32(dc_panel_h_total, SM501State), >> + VMSTATE_UINT32(dc_panel_h_sync, SM501State), >> + VMSTATE_UINT32(dc_panel_v_total, SM501State), >> + VMSTATE_UINT32(dc_panel_v_sync, SM501State), >> + VMSTATE_UINT32(dc_panel_hwc_addr, SM501State), >> + VMSTATE_UINT32(dc_panel_hwc_location, SM501State), >> + VMSTATE_UINT32(dc_panel_hwc_color_1_2, SM501State), >> + VMSTATE_UINT32(dc_panel_hwc_color_3, SM501State), >> + VMSTATE_UINT32(dc_video_control, SM501State), >> + VMSTATE_UINT32(dc_crt_control, SM501State), >> + VMSTATE_UINT32(dc_crt_fb_addr, SM501State), >> + VMSTATE_UINT32(dc_crt_fb_offset, SM501State), >> + VMSTATE_UINT32(dc_crt_h_total, SM501State), >> + VMSTATE_UINT32(dc_crt_h_sync, SM501State), >> + VMSTATE_UINT32(dc_crt_v_total, SM501State), >> + VMSTATE_UINT32(dc_crt_v_sync, SM501State), >> + VMSTATE_UINT32(dc_crt_hwc_addr, SM501State), >> + VMSTATE_UINT32(dc_crt_hwc_location, SM501State), >> + VMSTATE_UINT32(dc_crt_hwc_color_1_2, SM501State), >> + VMSTATE_UINT32(dc_crt_hwc_color_3, SM501State), >> + VMSTATE_UINT32(twoD_source, SM501State), >> + VMSTATE_UINT32(twoD_destination, SM501State), >> + VMSTATE_UINT32(twoD_dimension, SM501State), >> + VMSTATE_UINT32(twoD_control, SM501State), >> + VMSTATE_UINT32(twoD_pitch, SM501State), >> + VMSTATE_UINT32(twoD_foreground, SM501State), >> + VMSTATE_UINT32(twoD_background, SM501State), >> + VMSTATE_UINT32(twoD_stretch, SM501State), >> + VMSTATE_UINT32(twoD_color_compare, SM501State), >> + VMSTATE_UINT32(twoD_color_compare_mask, SM501State), >> + VMSTATE_UINT32(twoD_mask, SM501State), >> + VMSTATE_UINT32(twoD_clip_tl, SM501State), >> + VMSTATE_UINT32(twoD_clip_br, SM501State), >> + VMSTATE_UINT32(twoD_mono_pattern_low, SM501State), >> + VMSTATE_UINT32(twoD_mono_pattern_high, SM501State), >> + VMSTATE_UINT32(twoD_window_width, SM501State), >> + VMSTATE_UINT32(twoD_source_base, SM501State), >> + VMSTATE_UINT32(twoD_destination_base, SM501State), >> + VMSTATE_UINT32(twoD_alpha, SM501State), >> + VMSTATE_UINT32(twoD_wrap, SM501State), >> + VMSTATE_END_OF_LIST() >> + } >> +}; > > local_mem_size_index is also guest updatable state (it's some > of the bits from the SM501_DRAM_CONTROL register), so we need > to migrate it as well. OK, thanks for catching this. >> + >> #define TYPE_SYSBUS_SM501 "sysbus-sm501" >> #define SYSBUS_SM501(obj) \ >> OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501) >> @@ -1673,6 +1761,17 @@ static Property sm501_pci_properties[] = { >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> +static const VMStateDescription vmstate_sm501 = { >> + .name = "sm501", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_PCI_DEVICE(parent_obj, SM501PCIState), >> + VMSTATE_STRUCT(state, SM501PCIState, 1, vmstate_sm501_regs, SM501State), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> static void sm501_pci_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> @@ -1686,6 +1785,7 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data) >> dc->desc = "SM501 Display Controller"; >> dc->props = sm501_pci_properties; >> dc->hotpluggable = false; >> + dc->vmsd = &vmstate_sm501; >> } >> >> static const TypeInfo sm501_pci_info = { >> -- >> 2.7.4 > > What about reset and vmstate for the sysbus device? > > Consider putting reset and vmstate in separate patches. So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, vmstate for sysbus or 2 patches: reset for both, vmstate for both? > > thanks > -- PMM > >
On 2 March 2017 at 20:18, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Thu, 2 Mar 2017, Peter Maydell wrote: >> >> Don't use qemu_register_reset(). Set the appropriate dc->reset >> function pointers instead. > > > Any reason for that? This way I could save two more boilerplate functions > because I could define reset function once, otherwise I'd need two versions > taking sysbus and pci states just to extract the SM501State function and > call this function. Do you still think I should do that instead? qemu_register_reset is a pre-QOM method for doing reset; the standard QOM way of saying "my device has some reset behaviour" is to set its reset method pointer. Code calling qemu_register_reset() is generally either (a) doing something kind of weird or (b) old device code that hasn't yet been converted to QOM. > So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, > vmstate for sysbus or 2 patches: reset for both, vmstate for both? I think I'd go with 2 patches, since the two are going to share the bulk of the implementation in each case. thanks -- PMM
On Thu, 2 Mar 2017, Peter Maydell wrote: > On 2 March 2017 at 20:18, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> On Thu, 2 Mar 2017, Peter Maydell wrote: >>> >>> Don't use qemu_register_reset(). Set the appropriate dc->reset >>> function pointers instead. >> >> >> Any reason for that? This way I could save two more boilerplate functions >> because I could define reset function once, otherwise I'd need two versions >> taking sysbus and pci states just to extract the SM501State function and >> call this function. Do you still think I should do that instead? > > qemu_register_reset is a pre-QOM method for doing reset; > the standard QOM way of saying "my device has some reset > behaviour" is to set its reset method pointer. > Code calling qemu_register_reset() is generally either (a) doing > something kind of weird or (b) old device code that hasn't yet > been converted to QOM. Hmm, does having a device with both sysbus and pci versions qualify as weird? Does adding a comment saying that this way we don't need two more reset functions just to call the registered one justifies using qemu_register_reset() or this function is deprecated and should not be used and should go with the separate reset functions instead? >> So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, >> vmstate for sysbus or 2 patches: reset for both, vmstate for both? > > I think I'd go with 2 patches, since the two are going to > share the bulk of the implementation in each case. OK, thanks for the explanation in previous reply as well, I'll do this then.
On Thu, 2 Mar 2017, BALATON Zoltan wrote: > On Thu, 2 Mar 2017, Peter Maydell wrote: >> On 2 March 2017 at 20:18, BALATON Zoltan <balaton@eik.bme.hu> wrote: >>> On Thu, 2 Mar 2017, Peter Maydell wrote: >>>> >>>> Don't use qemu_register_reset(). Set the appropriate dc->reset >>>> function pointers instead. >>> >>> >>> Any reason for that? This way I could save two more boilerplate functions >>> because I could define reset function once, otherwise I'd need two >>> versions >>> taking sysbus and pci states just to extract the SM501State function and >>> call this function. Do you still think I should do that instead? >> >> qemu_register_reset is a pre-QOM method for doing reset; >> the standard QOM way of saying "my device has some reset >> behaviour" is to set its reset method pointer. >> Code calling qemu_register_reset() is generally either (a) doing >> something kind of weird or (b) old device code that hasn't yet >> been converted to QOM. > > Hmm, does having a device with both sysbus and pci versions qualify as weird? > Does adding a comment saying that this way we don't need two more reset > functions just to call the registered one justifies using > qemu_register_reset() or this function is deprecated and should not be used > and should go with the separate reset functions instead? On second thought I may go with the separate reset functions because then I can also update the bus type in the misc_control (not that anything I know depends on it but we can get it right). >>> So should it be 4 patches: reset for pci, reset for sysbus, vmstate for >>> pci, >>> vmstate for sysbus or 2 patches: reset for both, vmstate for both? >> >> I think I'd go with 2 patches, since the two are going to >> share the bulk of the implementation in each case. > > OK, thanks for the explanation in previous reply as well, I'll do this then. So I may also add the reset function already in the QOMify and Add PCI version patches then only one more patch is needed for the vmstate.
On 2 March 2017 at 20:55, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Thu, 2 Mar 2017, Peter Maydell wrote: >> >> On 2 March 2017 at 20:18, BALATON Zoltan <balaton@eik.bme.hu> wrote: >>> >>> On Thu, 2 Mar 2017, Peter Maydell wrote: >>>> >>>> >>>> Don't use qemu_register_reset(). Set the appropriate dc->reset >>>> function pointers instead. >>> >>> >>> >>> Any reason for that? This way I could save two more boilerplate functions >>> because I could define reset function once, otherwise I'd need two >>> versions >>> taking sysbus and pci states just to extract the SM501State function and >>> call this function. Do you still think I should do that instead? >> >> >> qemu_register_reset is a pre-QOM method for doing reset; >> the standard QOM way of saying "my device has some reset >> behaviour" is to set its reset method pointer. >> Code calling qemu_register_reset() is generally either (a) doing >> something kind of weird or (b) old device code that hasn't yet >> been converted to QOM. > > > Hmm, does having a device with both sysbus and pci versions qualify as > weird? Does adding a comment saying that this way we don't need two more > reset functions just to call the registered one justifies using > qemu_register_reset() or this function is deprecated and should not be used > and should go with the separate reset functions instead? Just use the QOM reset method pointers. Every QOM device should have a reset method, and that should be how it gets reset. (PCI is special anyway, it needs to get reset when the guest does a PCI bus reset, and that results in your device's reset method getting called. Functions registered with qemu_register_reset() only get called for full system reset.) thanks -- PMM
diff --git a/hw/display/sm501.c b/hw/display/sm501.c index 32223f6..b682a95 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -65,6 +65,7 @@ #define MMIO_BASE_OFFSET 0x3e00000 #define MMIO_SIZE 0x200000 +#define DC_PALETTE_ENTRIES (0x400 * 3) /* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */ @@ -491,7 +492,7 @@ typedef struct SM501State { uint32_t uart0_mcr; uint32_t uart0_scr; - uint8_t dc_palette[0x400 * 3]; + uint8_t dc_palette[DC_PALETTE_ENTRIES]; uint32_t dc_panel_control; uint32_t dc_panel_panning_control; @@ -1537,16 +1538,32 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; +static void sm501_reset(void *p) +{ + SM501State *s = p; + + s->system_control = 0x00100000; /* 2D engine FIFO empty */ + s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */ + s->gpio_31_0_control = 0; + s->gpio_63_32_control = 0; + s->dram_control = 0; + s->arbitration_control = 0x05146732; + s->irq_mask = 0; + s->misc_timing = 0; + s->power_mode_control = 0; + s->dc_panel_control = 0x00010000; /* FIFO level 3 */ + s->dc_video_control = 0; + s->dc_crt_control = 0x00010000; + s->twoD_control = 0; +} + static void sm501_init(SM501State *s, DeviceState *dev, uint32_t local_mem_bytes) { s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes); SM501_DPRINTF("sm501 local mem size=%x. index=%d\n", get_local_mem_size(s), s->local_mem_size_index); - s->system_control = 0x00100000; /* 2D engine FIFO empty */ - s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */ - s->dc_panel_control = 0x00010000; /* FIFO level 3 */ - s->dc_crt_control = 0x00010000; + qemu_register_reset(sm501_reset, s); /* local memory */ memory_region_init_ram(&s->local_mem_region, OBJECT(dev), "sm501.local", @@ -1577,6 +1594,77 @@ static void sm501_init(SM501State *s, DeviceState *dev, s->con = graphic_console_init(DEVICE(dev), 0, &sm501_ops, s); } +static const VMStateDescription vmstate_sm501_regs = { + .name = "sm501-regs", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(system_control, SM501State), + VMSTATE_UINT32(misc_control, SM501State), + VMSTATE_UINT32(gpio_31_0_control, SM501State), + VMSTATE_UINT32(gpio_63_32_control, SM501State), + VMSTATE_UINT32(dram_control, SM501State), + VMSTATE_UINT32(arbitration_control, SM501State), + VMSTATE_UINT32(irq_mask, SM501State), + VMSTATE_UINT32(misc_timing, SM501State), + VMSTATE_UINT32(power_mode_control, SM501State), + VMSTATE_UINT32(uart0_ier, SM501State), + VMSTATE_UINT32(uart0_lcr, SM501State), + VMSTATE_UINT32(uart0_mcr, SM501State), + VMSTATE_UINT32(uart0_scr, SM501State), + VMSTATE_UINT8_ARRAY(dc_palette, SM501State, DC_PALETTE_ENTRIES), + VMSTATE_UINT32(dc_panel_control, SM501State), + VMSTATE_UINT32(dc_panel_panning_control, SM501State), + VMSTATE_UINT32(dc_panel_fb_addr, SM501State), + VMSTATE_UINT32(dc_panel_fb_offset, SM501State), + VMSTATE_UINT32(dc_panel_fb_width, SM501State), + VMSTATE_UINT32(dc_panel_fb_height, SM501State), + VMSTATE_UINT32(dc_panel_tl_location, SM501State), + VMSTATE_UINT32(dc_panel_br_location, SM501State), + VMSTATE_UINT32(dc_panel_h_total, SM501State), + VMSTATE_UINT32(dc_panel_h_sync, SM501State), + VMSTATE_UINT32(dc_panel_v_total, SM501State), + VMSTATE_UINT32(dc_panel_v_sync, SM501State), + VMSTATE_UINT32(dc_panel_hwc_addr, SM501State), + VMSTATE_UINT32(dc_panel_hwc_location, SM501State), + VMSTATE_UINT32(dc_panel_hwc_color_1_2, SM501State), + VMSTATE_UINT32(dc_panel_hwc_color_3, SM501State), + VMSTATE_UINT32(dc_video_control, SM501State), + VMSTATE_UINT32(dc_crt_control, SM501State), + VMSTATE_UINT32(dc_crt_fb_addr, SM501State), + VMSTATE_UINT32(dc_crt_fb_offset, SM501State), + VMSTATE_UINT32(dc_crt_h_total, SM501State), + VMSTATE_UINT32(dc_crt_h_sync, SM501State), + VMSTATE_UINT32(dc_crt_v_total, SM501State), + VMSTATE_UINT32(dc_crt_v_sync, SM501State), + VMSTATE_UINT32(dc_crt_hwc_addr, SM501State), + VMSTATE_UINT32(dc_crt_hwc_location, SM501State), + VMSTATE_UINT32(dc_crt_hwc_color_1_2, SM501State), + VMSTATE_UINT32(dc_crt_hwc_color_3, SM501State), + VMSTATE_UINT32(twoD_source, SM501State), + VMSTATE_UINT32(twoD_destination, SM501State), + VMSTATE_UINT32(twoD_dimension, SM501State), + VMSTATE_UINT32(twoD_control, SM501State), + VMSTATE_UINT32(twoD_pitch, SM501State), + VMSTATE_UINT32(twoD_foreground, SM501State), + VMSTATE_UINT32(twoD_background, SM501State), + VMSTATE_UINT32(twoD_stretch, SM501State), + VMSTATE_UINT32(twoD_color_compare, SM501State), + VMSTATE_UINT32(twoD_color_compare_mask, SM501State), + VMSTATE_UINT32(twoD_mask, SM501State), + VMSTATE_UINT32(twoD_clip_tl, SM501State), + VMSTATE_UINT32(twoD_clip_br, SM501State), + VMSTATE_UINT32(twoD_mono_pattern_low, SM501State), + VMSTATE_UINT32(twoD_mono_pattern_high, SM501State), + VMSTATE_UINT32(twoD_window_width, SM501State), + VMSTATE_UINT32(twoD_source_base, SM501State), + VMSTATE_UINT32(twoD_destination_base, SM501State), + VMSTATE_UINT32(twoD_alpha, SM501State), + VMSTATE_UINT32(twoD_wrap, SM501State), + VMSTATE_END_OF_LIST() + } +}; + #define TYPE_SYSBUS_SM501 "sysbus-sm501" #define SYSBUS_SM501(obj) \ OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501) @@ -1673,6 +1761,17 @@ static Property sm501_pci_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const VMStateDescription vmstate_sm501 = { + .name = "sm501", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_PCI_DEVICE(parent_obj, SM501PCIState), + VMSTATE_STRUCT(state, SM501PCIState, 1, vmstate_sm501_regs, SM501State), + VMSTATE_END_OF_LIST() + } +}; + static void sm501_pci_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1686,6 +1785,7 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data) dc->desc = "SM501 Display Controller"; dc->props = sm501_pci_properties; dc->hotpluggable = false; + dc->vmsd = &vmstate_sm501; } static const TypeInfo sm501_pci_info = {
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/sm501.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 105 insertions(+), 5 deletions(-)