qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 05/14] dma: support NEC PC-9821 family

From: Alexander Graf
Subject: Re: [Qemu-devel] [PATCH 05/14] dma: support NEC PC-9821 family
Date: Thu, 10 Sep 2009 09:16:29 +0200 

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
reply via email to
[Prev in Thread] Current Thread [Next in Thread]