<< Newer Article #153 Older >>

Save State Fundamentals, part 4

Warning: this is a long one!

In this final article in the series on adding save state support to MAME drivers, I will walk through the process of adding support to a basic driver. Although there were a number of great suggestions for which driver to look at, all the proposed drivers had a large number of games and some hidden complexities that would hide the fundamentals I am trying to illustrate here. So for this example, we will add save state support to the Return of the Jedi driver. To follow along, you will need MAME 0.104u2.

The first step in adding support to a driver is to identify what files are relevant. For Return of the Jedi we have the obvious file drivers/jedi.c. There is also a matching jedi.c in the vidhrdw directory, so we will need to look at that as well. These are our starting points.

From there, we should determine what the dependencies are. If a driver depends on code elsewhere in the MAME system, we need to make sure that that code also has save state support. The easiest way to do this is to look at the #includes at the top of each file. In drivers/jedi.c, we see that it references the m6502 CPU core (cpu/m6502/m6502.h), the TMS5220 sound core (sound/5220intf.h), the POKEY sound core (sound/pokey.h), and some generic video hardware functions (vidhrdw/generic.h).

Doing my homework, I searched the CPU and sound cores to see if there were any state_save functions being called. The 6502 and TMS5220 already had support built-in. I did this before 0.104u2 was released and found that the POKEY emulator was lacking save state support, so I added it quickly to make sure we would be able to make this work. We'll have to wait until we look more closely at the video system before we can determine what functions in vidhrdw/generic.c were used.

The next step is to give the game a try and see if there are any MAME-detected issues. We run "mame jedi -log" to start the game, and then hit Shift+F7 to do a save. After a second, you will discover that it fails due to "pending anonymous timers". It also says check out error.log for details. If you open up error.log, you will see a whole bunch of logging messages listing anonymous timers. The ones in cpunum_empty_event_queue are expected and don't happen in every case. The one that is causing us issues is that one at drivers/jedi.c line 150 (generate_interrupt). Let's look at the code:

static void generate_interrupt(int scanline)
{
    ...
    timer_set(cpu_getscanlinetime(scanline), scanline, generate_interrupt);
}

So, this is the real problem. Every time the generate_interrupt call is made, it requests another anonymous timer (via timer_set), meaning that there is always at least one anonymous timer running in the system. How does the first call to generate_interrupt happen? A little farther down the code we see:

static MACHINE_INIT( jedi )
{
    ...
    /* set a timer to run the interrupts */
    timer_set(cpu_getscanlinetime(32), 32, generate_interrupt);
}

So at MACHINE_INIT time, we set up our first timer to go off, and then a bit later, when the timer fires, its callback sets another timer, etc. This pattern isn't fundamentally flawed, but we need to avoid the use of anonymous timers to make it work. To solve this, we add the following declaration to the local variables at the top of driver/jedi.c:

static mame_timer *interrupt_timer;

Then we modify the code in MACHINE_INIT to allocate a timer explicitly, and then prime the first callback in a separate step:

static MACHINE_INIT( jedi )
{
    ...
    /* set a timer to run the interrupts */
    interrupt_timer = timer_alloc(generate_interrupt);
    timer_adjust(interrupt_timer, cpu_getscanlinetime(32), 32, 0);
}

Finally, we modify the code in the callback to also use timer_adjust to change the firing time of the timer:

static void generate_interrupt(int scanline)
{
    ...
    timer_adjust(interrupt_timer, cpu_getscanlinetime(scanline), scanline, 0);
}

At this point, we recompile, try saving again, and find that we get a different behavior. It now says it was able to successfully save the game, but warns that it might not work because save states are not officially supported. Time to fix that!

Once you can actually try saving the game, the next thing to look for is memory banking. Scan the code for calls to memory_set_bankptr. If that functon is called only at init time, you probably don't need to worry about it. But if it is called in any kind of write handler or other callback, you probably need to convert the memory banking system over to the new system I described in the previous part of this series.

In this case, driver/jedi.c does indeed make some calls to memory_set_bankptr in its rom_banksel_w callback:

static WRITE8_HANDLER( rom_banksel_w )
{
    UINT8 *RAM = memory_region(REGION_CPU1);

    if (data & 0x01) memory_set_bankptr(1, &RAM[0x10000]);
    if (data & 0x02) memory_set_bankptr(1, &RAM[0x14000]);
    if (data & 0x04) memory_set_bankptr(1, &RAM[0x18000]);
}

To fix this, we need to pre-configure all the banks with the memory system at init time. Since this driver doesn't have a DRIVER_INIT call, we'll add it to the MACHINE_INIT function:

static MACHINE_INIT( jedi )
{
    ...
    /* configure the banks */
    memory_configure_bank(1, 0, 3, memory_region(REGION_CPU1) + 0x10000, 0x4000);
}

This configures 3 separate banks starting at offset $10000 of the CPU1 memory region, each bank being $4000 bytes away from the previous one. Then we re-write rom_banksel_w to look like this:

static WRITE8_HANDLER( rom_banksel_w )
{
    if (data & 0x01) memory_set_bank(1, 0);
    if (data & 0x02) memory_set_bank(1, 1);
    if (data & 0x04) memory_set_bank(1, 2);
}

There -- memory banking issues solved!

Now on to global variables. The best place to look for global variables is at the top of each module. However, you need to be careful. Often people have added global variables down below in between functions, or even disguised as static variables declared within a function. In this case, the driver source is pretty clean, so all the variables we care about in drivers/jedi.c are at the top, neatly organized and identified:

/* local variables */
static UINT8 control_num;
static UINT8 sound_latch;
static UINT8 sound_ack_latch;
static UINT8 sound_comm_stat;
static UINT8 speech_write_buffer;
static UINT8 speech_strobe_state;
static UINT8 nvram_enabled;

The first thing we need to do is make sure that the save state functions are included and available to us, so at the top of drivers/jedi.c we add:

#include "state.h"

And then at the end of our MACHINE_INIT function, we register our global variables:

static MACHINE_INIT( jedi )
{
    ...
    /* set up save state */
    state_save_register_global(control_num);
    state_save_register_global(sound_latch);
    state_save_register_global(sound_ack_latch);
    state_save_register_global(sound_comm_stat);
    state_save_register_global(speech_write_buffer);
    state_save_register_global(speech_strobe_state);
    state_save_register_global(nvram_enabled);
}

Easy enough. And that pretty much covers it for drivers/jedi.c. Now onto the video side of things....

At the top of vidhrdw/jedi.c, we see all the global variable declarations:

/* globals */
UINT8 *jedi_backgroundram;
size_t jedi_backgroundram_size;
UINT8 *jedi_PIXIRAM;

/* local variables */
static UINT32 jedi_vscroll;
static UINT32 jedi_hscroll;
static UINT32 jedi_alpha_bank;
static int video_off, smooth_table;
static UINT8 *fgdirty, *bgdirty;
static mame_bitmap *fgbitmap, *mobitmap, *bgbitmap, *bgexbitmap;

We'll have to examine each of these and understand whether or not they need to be saved. I glossed over this with the drivers/jedi.c file because all the globals happened to be relevant, but for the most part you should look through the code and understand whether or not the data needs to be saved.

Looking at the first items here, we see that jedi_backgroundram, jedi_backgroundram_size, and jedi_PIXIRAM are all referenced in the address map for the CPUs in drivers/jedi.c. When a pointer is filled in by the memory system (i.e., its address is passed into the AM_BASE or AM_SIZE macro), then generally this means that the memory system allocated the memory for it and told us where that memory lives. Since the memory system is responsible for saving all of its memory regions, it means that we don't need to worry about the memory pointed to by these variables -- it's all taken care of for us automatically.

The next three variables -- jedi_vscroll, jedi_hscroll, and jedi_alpha_bank -- are all written to directly by the memory handler code, and later used in the VIDEO_UPDATE routine, so they definitely need to be saved.

The next two variables (video_off and smooth_table) follow the exact same pattern: they are written to directly by the memory handler code and are later used in the VIDEO_UPDATE call. The difference here is that both of these variables are ints. Ints are a problem in save states because the size of an int variable is not guaranteed to be anything consistent across all platforms (this is the reason we have explicitly sized types in MAME like UINT8 or INT32.) Although you can save these variables as-is, doing so makes the save states less portable. So before we can save them, we should really take the time to convert them to something explicitly-sized.

Scanning for where these variables are used reveals that neither of them are used to store anything larger than 8 bits, so they can both be converted to UINT8s. If you're ever in doubt, however, changing an int to an INT32 for the purposes of adding save state support is the safe thing to do.

Saving these first five variables is easy. Just add the following code to VIDEO_START:

VIDEO_START( jedi )
{
    ...
    /* register for saving */
    state_save_register_global(jedi_vscroll);
    state_save_register_global(jedi_hscroll);
    state_save_register_global(jedi_alpha_bank);
    state_save_register_global(video_off);
    state_save_register_global(smooth_table);
}

All the remaining variables in vidhrdw/jedi.c are allocated in the VIDEO_START routine and represent the bitmap data of the various layers. This is an example of where you need to take the time to really understand what's going on in order to properly perform a save state. Here you have two options. One is just to blindly save everything. That is, you could save the full fgdirty and bgdirty arrays as well as the contents of all four bitmaps. And that would work just fine if you added code like this:

VIDEO_START( jedi )
{
    ...
    state_save_register_global_pointer(fgdirty, videoram_size);
    state_save_register_global_pointer(bgdirty, jedi_backgroundram_size);
    state_save_register_global_bitmap(fgbitmap);
    state_save_register_global_bitmap(mobitmap);
    state_save_register_global_bitmap(bgbitmap);
    state_save_register_global_bitmap(bgexbitmap);
}

(Note the use of the new macro state_save_register_global_bitmap which was added in 0.104u2.) So this approach is the "heavyweight" but dumb approach. It is generally frowned upon because it is wasteful to store all that data when with a little bit of code examination, we can avoid saving any of that stuff at all! How, you ask?

Well, first let's look at fgbitmap. If you look in the VIDEO_UPDATE callback farther down in the file, you will see that fgbitmap is redrawn section by section depending on whether or not a corresponding entry in fgdirty is set. Which means that if all the entries in fgdirty were set after a restore, the entirety of the fgbitmap would be re-generated on the first VIDEO_UPDATE call. You can see a very similar pattern for bgbitmap and bgdirty. So, rather than saving any of these variables, we could register ourselves a postload callback:

static void jedi_postload(void)
{
    memset(fgdirty, 1, videoram_size);
    memset(bgdirty, 1, jedi_backgroundram_size);
}

VIDEO_START( jedi )
{
    ...
    state_save_register_func_postload(jedi_postload);
}

Which means that on a restore, we can mark all of the foreground and background bitmaps "dirty" so that they get completely regenerated on the next VIDEO_UPDATE. Nifty.

This leaves us with bgexbitmap and mobitmap to consider. If you examine the code, you will see that as the bgbitmap gets updated, parts of the bgexbitmap are tagged dirty. Logically, if all of bgbitmap gets updated, all of bgexbitmap will be tagged dirty. Since we are already forcing bgbitmap to be fully updated, we don't need to worry about bgexbitmap at all -- it will just work.

Similarly, if you look at the logic for handling mobitmap, you will see that everything that is drawn into it during VIDEO_UPDATE is erased at the end, meaning that mobitmap never contains any useful data across refreshes; hence it does not need to be saved either.

So, you now have two approaches to saving the video state. I recommend giving them both a try, checking out the difference in save state file sizes, seeing what happens when you try to load a state saved one way after changing the code to work another way. Once you've made the changes, I also recommend playing the game to various points, saving, restoring back to different points, jumping around, and trying to break it. Hopefully everything will work solidly! Once you're sure things are looking good, then and only then can we add the flag to the GAME entry in drivers/jedi.c:

GAME( 1984, jedi, 0, jedi, jedi, 0, ROT0, "Atari", "Return of the Jedi", GAME_SUPPORTS_SAVE )

Whew!