[05/14] dma: support NEC PC-9821 family

This patch is to add NEC PC-9821 family i/o to dma.

On 09.09.2009, at 17:41, 武田 俊也 wrote:

> This patch is to add NEC PC-9821 family i/o to dma.
>
> diff -ur a/hw/dma.c b/hw/dma.c
> --- a/hw/dma.c	Tue Sep  8 21:26:50 2009
> +++ b/hw/dma.c	Wed Sep  9 21:51:15 2009
> @@ -41,6 +41,7 @@
>     uint8_t mode;
>     uint8_t page;
>     uint8_t pageh;
> +    uint8_t bound;
>     uint8_t dack;
>     uint8_t eop;
>     DMA_transfer_handler transfer_handler;
> @@ -55,8 +56,11 @@
>     uint8_t command;
>     uint8_t mask;
>     uint8_t flip_flop;
> +    uint8_t access_ctrl;
>     int dshift;
>     struct dma_regs regs[4];
> +    /* NEC PC-98x1 quirks? */
> +    int pc98;
> } dma_controllers[2];
>
> enum {
> @@ -329,7 +333,8 @@
> static void channel_run (int ncont, int ichan)
> {
>     int n;
> -    struct dma_regs *r = &dma_controllers[ncont].regs[ichan];
> +    struct dma_cont *d = &dma_controllers[ncont];
> +    struct dma_regs *r = &d->regs[ichan];
> #ifdef DEBUG_DMA
>     int dir, opmode;
>
> @@ -344,11 +349,26 @@
>     }
> #endif
>
> -    r = dma_controllers[ncont].regs + ichan;
>     n = r->transfer_handler (r->opaque, ichan + (ncont << 2),
>                              r->now[COUNT], (r->base[COUNT] + 1) <<  
> ncont);
>     r->now[COUNT] = n;
>     ldebug ("dma_pos %d size %d\n", n, (r->base[COUNT] + 1) << ncont);
> +
> +    /* increment page register */
> +    if (d->pc98 && r->bound) {
> +        uint32_t last_addr = r->base[ADDR];
> +        if ((r->mode >> 5) & 1) {

magic number

Please add #define's for magic numbers that make you understand the  
code when you read it.

> +            last_addr -= n >> ncont;
> +            if (last_addr & 0xffff0000) {
> +                r->page = ((r->page - 1) & r->bound) | (r->page &  
> (~r->bound));
> +            }
> +        } else {
> +            last_addr += n >> ncont;
> +            if (last_addr & 0xffff0000) {
> +                r->page = ((r->page + 1) & r->bound) | (r->page &  
> (~r->bound));
> +            }
> +        }
> +    }
> }
>
> static QEMUBH *dma_bh;
> @@ -400,9 +420,13 @@
>
> int DMA_read_memory (int nchan, void *buf, int pos, int len)
> {
> -    struct dma_regs *r = &dma_controllers[nchan > 3].regs[nchan & 3];
> +    struct dma_cont *d = &dma_controllers[nchan > 3];
> +    struct dma_regs *r = &d->regs[nchan & 3];
>     target_phys_addr_t addr = ((r->pageh & 0x7f) << 24) | (r->page  
> << 16) | r->now[ADDR];
>
> +    if (d->pc98 && (d->access_ctrl & 4)) {

magic number

> +        addr &= 0xfffff;
> +    }
>     if (r->mode & 0x20) {
>         int i;
>         uint8_t *p = buf;
> @@ -422,9 +446,13 @@
>
> int DMA_write_memory (int nchan, void *buf, int pos, int len)
> {
> -    struct dma_regs *r = &dma_controllers[nchan > 3].regs[nchan & 3];
> +    struct dma_cont *d = &dma_controllers[nchan > 3];
> +    struct dma_regs *r = &d->regs[nchan & 3];
>     target_phys_addr_t addr = ((r->pageh & 0x7f) << 24) | (r->page  
> << 16) | r->now[ADDR];
>
> +    if (d->pc98 && (d->access_ctrl & 4)) {
> +        addr &= 0xfffff;
> +    }
>     if (r->mode & 0x20) {
>         int i;
>         uint8_t *p = buf;
> @@ -453,6 +481,14 @@
> static void dma_reset(void *opaque)
> {
>     struct dma_cont *d = opaque;
> +
> +    if (d->pc98) {
> +        int i;
> +        for (i = 0; i < ARRAY_SIZE (d->regs); ++i) {
> +            d->regs[i].bound = 0;
> +        }
> +        d->access_ctrl = 0xb4;

even more magic number...
This is apparently a bitfield that ORs quite some different values  
together. Please use #define's for the separate values and OR them  
here or #define a constant that is created by ORing them.

> +    }
>     write_cont (d, (0x0d << d->dshift), 0);
> }
>
> @@ -470,6 +506,7 @@
>     static const int page_port_list[] = { 0x1, 0x2, 0x3, 0x7 };
>     int i;
>
> +    d->pc98 = 0;

Normally qemu structures are allocated with qemu_mallocz, so they  
contain 0s everywhere already. Are you sure you need this?

>     d->dshift = dshift;
>     for (i = 0; i < 8; i++) {
>         register_ioport_write (base + (i << dshift), 1, 1,  
> write_chan, d);
> @@ -509,6 +546,9 @@
>     qemu_put_8s (f, &d->command);
>     qemu_put_8s (f, &d->mask);
>     qemu_put_8s (f, &d->flip_flop);
> +    if (d->pc98) {
> +        qemu_put_8s (f, &d->access_ctrl);
> +    }

New format version? This also breaks the versioning in general,  
because the same version number would mean completely different  
things, depending on which -M you use.

I guess we can leave it as is for now, because all the save/restore  
code is supposed to be changed anyways, right?

>     qemu_put_be32 (f, d->dshift);
>
>     for (i = 0; i < 4; ++i) {
> @@ -520,6 +560,9 @@
>         qemu_put_8s (f, &r->mode);
>         qemu_put_8s (f, &r->page);
>         qemu_put_8s (f, &r->pageh);
> +        if (d->pc98) {
> +            qemu_put_8s (f, &r->bound);
> +        }
>         qemu_put_8s (f, &r->dack);
>         qemu_put_8s (f, &r->eop);
>     }
> @@ -537,6 +580,9 @@
>     qemu_get_8s (f, &d->command);
>     qemu_get_8s (f, &d->mask);
>     qemu_get_8s (f, &d->flip_flop);
> +    if (d->pc98) {
> +        qemu_get_8s (f, &d->access_ctrl);
> +    }
>     d->dshift=qemu_get_be32 (f);
>
>     for (i = 0; i < 4; ++i) {
> @@ -548,6 +594,9 @@
>         qemu_get_8s (f, &r->mode);
>         qemu_get_8s (f, &r->page);
>         qemu_get_8s (f, &r->pageh);
> +        if (d->pc98) {
> +            qemu_get_8s (f, &r->bound);
> +        }
>         qemu_get_8s (f, &r->dack);
>         qemu_get_8s (f, &r->eop);
>     }
> @@ -565,6 +614,106 @@
>               high_page_enable ? 0x488 : -1);
>     register_savevm ("dma", 0, 1, dma_save, dma_load,  
> &dma_controllers[0]);
>     register_savevm ("dma", 1, 1, dma_save, dma_load,  
> &dma_controllers[1]);
> +
> +    dma_bh = qemu_bh_new(DMA_run_bh, NULL);
> +}
> +
> +/* NEC PC-98x1 */
> +
> +static const uint8_t bounds[4] = {0, 0x0f, 0, 0xff};
> +
> +static void pc98_write_chan(void *opaque, uint32_t nport, uint32_t  
> data)
> +{
> +    write_chan (opaque, nport >> 1, data);
> +}
> +
> +static uint32_t pc98_read_chan(void *opaque, uint32_t nport)
> +{
> +    return read_chan (opaque, nport >> 1);
> +}
> +
> +static void pc98_write_cont (void *opaque, uint32_t nport, uint32_t  
> data)
> +{
> +    write_cont (opaque, nport >> 1, data);
> +}
> +
> +static uint32_t pc98_read_cont (void *opaque, uint32_t nport)
> +{
> +    return read_cont (opaque, nport >> 1);
> +}
> +
> +static void pc98_write_page (void *opaque, uint32_t nport, uint32_t  
> data)
> +{
> +    struct dma_cont *d = opaque;
> +    int ichan;
> +
> +    ichan = ((nport >> 1) + 1) & 3;
> +    d->regs[ichan].page = data;
> +}
> +
> +static void pc98_write_bound (void *opaque, uint32_t nport,  
> uint32_t data)
> +{
> +    struct dma_cont *d = opaque;
> +    int ichan;
> +
> +    ichan = data & 3;
> +    d->regs[ichan].bound = bounds[(data >> 2) & 3];
> +}
> +
> +static void pc98_write_access_ctrl (void *opaque, uint32_t nport,  
> uint32_t data)
> +{
> +    struct dma_cont *d = opaque;
> +
> +    d->access_ctrl = data;
> +}
> +
> +static uint32_t pc98_read_access_ctrl (void *opaque, uint32_t nport)
> +{
> +    struct dma_cont *d = opaque;
> +
> +    return d->access_ctrl;
> +}
> +
> +static void pc98_write_pageh (void *opaque, uint32_t nport,  
> uint32_t data)
> +{
> +    struct dma_cont *d = opaque;
> +    int ichan;
> +
> +    ichan = ((nport >> 1) - 2) & 3;
> +    d->regs[ichan].pageh = data;
> +}
> +
> +void pc98_DMA_init (int high_page_enable)
> +{
> +    struct dma_cont *d = &dma_controllers[0];
> +    int i;
> +
> +    d->pc98 = 1;
> +    d->dshift = 0;
> +    for (i = 0; i < 8; i++) {
> +        register_ioport_write (0x01 + (i << 1), 1, 1,  
> pc98_write_chan, d);
> +        register_ioport_read (0x01 + (i << 1), 1, 1,  
> pc98_read_chan, d);
> +        register_ioport_write (0x11 + (i << 1), 1, 1,  
> pc98_write_cont, d);
> +        register_ioport_read (0x11 + (i << 1), 1, 1,  
> pc98_read_cont, d);
> +    }
> +    for (i = 0; i < 4; i++) {
> +        register_ioport_write (0x21 + (i << 1), 1, 1,  
> pc98_write_page, d);
> +    }
> +    register_ioport_write (0x29, 1, 1, pc98_write_bound, d);
> +    register_ioport_write (0x439, 1, 1, pc98_write_access_ctrl, d);
> +    register_ioport_read (0x439, 1, 1, pc98_read_access_ctrl, d);

I'd be inclined to say that the ioport handlers and registrations  
belong to pc98.c, not dma.c.

Alex