--5mCyUwZo2JvN/JJP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sun, Jan 08, 2006 at 01:32:05AM +1100, James Haggerty wrote:
> > > No panic, but it fails (with line 128 etc. removed):
> >
> > What lines did you remove ?
> >
> 
> 124-136 or so - just the for loop, obviously something to do with the DMA
> init (why it isn't working). 'chan' is not declared - just try compiling
> it :-)

Ha OK, sorry for that. Well, it' can't work without this loop.
Please try the attached patch instead.

> > > ...
> > > wd0 at atabus0 drive 0: <TOSHIBA MK6411MAT>
> > > wd0: drive supports 16-sector PIO transfers, LBA addressing
> > > wd0: 6194 MB, 13424 cyl, 15 head, 63 sec, 512 bytes/sect x 12685680
> > sectors
> > > wd0: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 2
> > (Ultra/33)
> >
> > Isn't there something between this line and the first lost interrupt
> > message ?
> >
> 
> Umm, pretty sure there wasn't, but unfortunately I've lost the output now
> and would have to recompile. Is it important?

Not now that you tried without DMA

> 
> > > pciide0:0:0: lost interrupt
> > >         type: ata tc_bcount: 512 tc_skip: 0
> > > pciide0:0:0: bus-master DMA error: missing interrupt, status=0x1
> >
> > In pciide_pnpbios.c, could you remove the line:
> > 	sc->sc_wdcdev.sc_atac.atac_cap |= ATAC_CAP_DMA;
> >
> > and see if it works (it'll disable IDE DMA, but will show if it's an
> > issue with DMA only, or with all IDE commands).
> 
> Yes, it works fine without that! I also just tried it with ATAC_CAP_DMA set

OK, so interrupts works.

> and just the reference to chan expunged (essentially setting to chan = 0)
> and the ordering slightly changed to get it to compile, and then it
> reported 'status=0x61' on the lost interrupt error, but after a retry
> there was only something about a soft error on wd0d and the system seemed
> to run fine (though there was little to no performance improvement -
> what's the easiest way to measure this?).

You can try the attached program:
./tst /dev/rwd0d 5000
this will give the bandwith between the host memory and the disk's cache.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--5mCyUwZo2JvN/JJP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: pciide_pnpbios.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/pnpbios/pciide_pnpbios.c,v
retrieving revision 1.20
diff -u -r1.20 pciide_pnpbios.c
--- pciide_pnpbios.c	11 Dec 2005 12:17:47 -0000	1.20
+++ pciide_pnpbios.c	7 Jan 2006 15:45:43 -0000
@@ -54,7 +54,6 @@
 
 static int	pciide_pnpbios_match(struct device *, struct cfdata *, void *);
 static void	pciide_pnpbios_attach(struct device *, struct device *, void *);
-void		pciide_pnpbios_setup_channel(struct ata_channel *);
 
 extern void	pciide_channel_dma_setup(struct pciide_channel *);
 extern int	pciide_dma_init(void *, int, int, void *, size_t, int);
@@ -91,61 +90,76 @@
 	struct wdc_regs *wdr;
 	bus_space_tag_t compat_iot;
 	bus_space_handle_t cmd_baseioh, ctl_ioh;
-	int i;
+	int i, drive, size;
+	u_int8_t idedma_ctl;
 
-	printf("\n");
+	aprint_naive(": disk controller\n");
+	aprint_normal("\n");
 	pnpbios_print_devres(self, aa);
 
-	printf("%s: Toshiba Extended IDE Controller\n", self->dv_xname);
+	aprint_normal("%s: Toshiba Extended IDE Controller\n", self->dv_xname);
 
 	if (pnpbios_io_map(aa->pbt, aa->resc, 2, &sc->sc_dma_iot,
 	    &sc->sc_dma_ioh) != 0) {
-		printf("%s: unable to map DMA registers\n", self->dv_xname);
+		aprint_error("%s: unable to map DMA registers\n",
+		    self->dv_xname);
 		return;
 	}
 	if (pnpbios_io_map(aa->pbt, aa->resc, 0, &compat_iot,
 	    &cmd_baseioh) != 0) {
-		printf("%s: unable to map command registers\n", self->dv_xname);
+		aprint_error("%s: unable to map command registers\n",
+		    self->dv_xname);
 		return;
 	}
 	if (pnpbios_io_map(aa->pbt, aa->resc, 1, &compat_iot,
 	    &ctl_ioh) != 0) {
-		printf("%s: unable to map control register\n", self->dv_xname);
+		aprint_error("%s: unable to map control register\n",
+		    self->dv_xname);
 		return;
 	}
 
 	sc->sc_dmat = &pci_bus_dma_tag;
 
+	cp = &sc->pciide_channels[0];
+	sc->wdc_chanarray[0] = &cp->ata_channel;
+	cp->ata_channel.ch_channel = 0;
+	cp->ata_channel.ch_atac = &sc->sc_wdcdev.sc_atac;
+	cp->ata_channel.ch_queue = malloc(sizeof(struct ata_queue),
+					  M_DEVBUF, M_NOWAIT);
+	if (cp->ata_channel.ch_queue == NULL) {
+		aprint_error("%s: unable to allocate memory for command "
+		    "queue\n", self->dv_xname);
+		return;
+	}
+
 	sc->sc_dma_ok = 1;
+	for (i = 0; i < IDEDMA_NREGS; i++) {
+		size = 4;
+		if (size > (IDEDMA_SCH_OFFSET - i))
+			size = IDEDMA_SCH_OFFSET - i;
+		if (bus_space_subregion(sc->sc_dma_iot, sc->sc_dma_ioh,
+		    i, size, &cp->dma_iohs[i]) != 0) {
+			aprint_error("%s: can't subregion offset %d "
+			    "size %lu", self->dv_xname, i, (u_long)size);
+			return;
+		}
+	}
+	sc->sc_dma_maxsegsz = IDEDMA_BYTE_COUNT_MAX;
+	sc->sc_dma_boundary = IDEDMA_BYTE_COUNT_ALIGN;
 	sc->sc_wdcdev.dma_arg = sc;
 	sc->sc_wdcdev.dma_init = pciide_dma_init;
 	sc->sc_wdcdev.dma_start = pciide_dma_start;
 	sc->sc_wdcdev.dma_finish = pciide_dma_finish;
+	sc->sc_wdcdev.sc_atac.atac_cap |= ATAC_CAP_DMA;
 	sc->sc_wdcdev.sc_atac.atac_channels = sc->wdc_chanarray;
 	sc->sc_wdcdev.sc_atac.atac_nchannels = 1;
 	sc->sc_wdcdev.sc_atac.atac_cap |= ATAC_CAP_DATA16 | ATAC_CAP_DATA32;
-	sc->sc_wdcdev.sc_atac.atac_cap |= ATAC_CAP_DMA | ATAC_CAP_UDMA;
-        sc->sc_wdcdev.sc_atac.atac_pio_cap = 4;
-        sc->sc_wdcdev.sc_atac.atac_dma_cap = 2;		/* XXX */
-        sc->sc_wdcdev.sc_atac.atac_udma_cap = 2;	/* XXX */
-#if 0 /* XXX */
-	sc->sc_wdcdev.set_modes = pciide_pnpbios_setup_channel;
-#endif
+        sc->sc_wdcdev.sc_atac.atac_pio_cap = 0;
+        sc->sc_wdcdev.sc_atac.atac_dma_cap = 0;		/* XXX */
+        sc->sc_wdcdev.sc_atac.atac_udma_cap = 0;	/* XXX */
 
 	wdc_allocate_regs(&sc->sc_wdcdev);
 
-	cp = &sc->pciide_channels[0];
-	sc->wdc_chanarray[0] = &cp->ata_channel;
-	cp->ata_channel.ch_channel = 0;
-	cp->ata_channel.ch_atac = &sc->sc_wdcdev.sc_atac;
-	cp->ata_channel.ch_queue = malloc(sizeof(struct ata_queue),
-					  M_DEVBUF, M_NOWAIT);
-	if (cp->ata_channel.ch_queue == NULL) {
-		printf("%s: unable to allocate memory for command queue\n",
-			self->dv_xname);
-		return;
-	}
-
 	wdc_cp = &cp->ata_channel;
 	wdr = CHAN_TO_WDC_REGS(wdc_cp);
 	wdr->cmd_iot = compat_iot;
@@ -154,8 +168,8 @@
 	for (i = 0; i < WDC_NREG; i++) {
 		if (bus_space_subregion(wdr->cmd_iot, wdr->cmd_baseioh, i,
 		    i == 0 ? 4 : 1, &wdr->cmd_iohs[i]) != 0) {
-			    printf("%s: unable to subregion control register\n",
-				self->dv_xname);
+			    aprint_error("%s: unable to subregion control "
+				"register\n", self->dv_xname);
 			    return;
 		}
 	}
@@ -170,13 +184,31 @@
 					pciide_compat_intr, cp);
 
 	wdcattach(wdc_cp);
-}
-
-void
-pciide_pnpbios_setup_channel(chp)
-	struct ata_channel *chp;
-{
-	struct pciide_channel *cp = CHAN_TO_PCHAN(chp);
 
-	pciide_channel_dma_setup(cp);
+	idedma_ctl = 0;
+	for (drive = 0; drive < cp->ata_channel.ch_ndrive; drive++) {
+		/*
+		 * we have not probed the drives yet,
+		 * allocate ressources for all of them.
+		 */
+		if (pciide_dma_table_setup(sc, 0, drive) != 0) {
+			/* Abort DMA setup */
+			aprint_error(
+			    "%s:%d:%d: can't allocate DMA maps, "
+			    "using PIO transfers\n",
+			    sc->sc_wdcdev.sc_atac.atac_dev.dv_xname,
+			    0, drive);
+			sc->sc_dma_ok = 0;
+			sc->sc_wdcdev.sc_atac.atac_cap &= ~ATAC_CAP_DMA;
+			sc->sc_wdcdev.irqack = NULL;
+			idedma_ctl = 0;
+			break;
+		}
+		idedma_ctl |= IDEDMA_CTL_DRV_DMA(drive);
+	}
+	if (idedma_ctl != 0) {
+		/* Add software bits in status register */
+		bus_space_write_1(sc->sc_dma_iot,
+		    cp->dma_iohs[IDEDMA_CTL], 0, idedma_ctl);
+	}
 }

--5mCyUwZo2JvN/JJP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="tst.c"

#include <fcntl.h>
#include <unistd.h>
#include <sys/time.h>
#include <stdio.h>
#include <stdlib.h>

int
main(int argc, char **argv)
{
	static char buf[64*1024];
	int fd, i;
	struct timeval tv0, tv1;
	int t;

	fd = open(argv[1], O_RDONLY, 0);
	if (fd < 0) {
		perror("open");
		exit(1);
	}
	if (gettimeofday(&tv0, NULL) < 0) {
		perror("gettimeofday");
		exit(1);
	}
	for (i = 0; i < atoi(argv[2]); i++) {
		if (read(fd, buf, sizeof(buf)) != sizeof(buf)) {
			perror("read");
			exit(1);
		}
		if (lseek(fd, 0, SEEK_SET) < 0) {
			perror("seek");
			exit(1);
		}
			
	}
	if (gettimeofday(&tv1, NULL) < 0) {
		perror("gettimeofday");
		exit(1);
	}
	t = (tv1.tv_sec - tv0.tv_sec) * 1000000;
	t = t + tv1.tv_usec - tv0.tv_usec;
	printf("%d us, %f MB/s\n", t,
	    ((double)64 * (double)i / 1024) / ((double)t / 1000000));
	exit(0);
}

--5mCyUwZo2JvN/JJP--