Skip to contentSkip to conversation

IDE: Improving speed on reading data Part 1/1

PATCH New Stefan Althoefer 3 participants 4 messages Dec 4, 2008 – Jan 4, 2009

lore
Share & tools
lore

Stefan Althoefer

author
#0 lore raw

PATCH 1/1 IDE: Improving speed on reading data Part 1/1

[PATCH] IDE: Improving speed on reading data

This patch improves the speed when reading blocks from IDE devices by reading more than one block at a time. Up to 128 blocks are requested in one read command.

On my testplatform (Janz emPC-A400 with CompactFLASH card) this nearly doubled speed.

Also the ide_wait() code was rewritten to have lower latency by polling more frequently for status.

The patch is against "latest" u-boot git-repository

Please (still) be patient if style of submission or patches are offending.

Signed-off-by Stefan Althoefer <[email protected]>

----

common/cmd_ide.c renamed from /common/cmd_ide.c +30 -24
diff -uprN u-boot-orig//common/cmd_ide.c u-boot/common/cmd_ide.c
@@ -1302,6 +1361,7 @@ ulong ide_read (int device, lbaint_t blk
 	ulong n = 0;
 	unsigned char c;
 	unsigned char pwrsave=0; /* power save */
+	ulong scnt;
 #ifdef CONFIG_LBA48
 	unsigned char lba48 = 0;
 

@@ -1346,7 +1406,7 @@ ulong ide_read (int device, lbaint_t blk
 	}
 
 
-	while (blkcnt-- > 0) {
+	while (blkcnt > 0) {
 
 		c = ide_wait (device, IDE_TIME_OUT);
 

@@ -1368,7 +1428,8 @@ ulong ide_read (int device, lbaint_t blk
 #endif
 		}
 #endif
-		ide_outb (device, ATA_SECT_CNT, 1);
+		scnt = (blkcnt > 128) ? 128 : blkcnt;
+		ide_outb (device, ATA_SECT_CNT, scnt);
 		ide_outb (device, ATA_LBA_LOW,  (blknr >>  0) & 0xFF);
 		ide_outb (device, ATA_LBA_MID,  (blknr >>  8) & 0xFF);
 		ide_outb (device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);

@@ -1387,32 +1448,36 @@ ulong ide_read (int device, lbaint_t blk
 			ide_outb (device, ATA_COMMAND,  ATA_CMD_READ);
 		}
 
-		udelay (50);
+		while (scnt > 0) {
+			udelay (50);
 
-		if(pwrsave) {
-			c = ide_wait (device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */
-			pwrsave=0;
-		} else {
-			c = ide_wait (device, IDE_TIME_OUT);	/* can't take over 500 ms */
-		}
+			if(pwrsave) {
+				c = ide_wait (device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */
+				pwrsave=0;
+			} else {
+				c = ide_wait (device, IDE_TIME_OUT);	/* can't take over 500 ms */
+			}
 
-		if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) {
+			if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) {
 #if defined(CONFIG_SYS_64BIT_LBA) && defined(CONFIG_SYS_64BIT_VSPRINTF)
-			printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n",
-				device, blknr, c);
+				printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n",
+					device, blknr, c);
 #else
-			printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n",
-				device, (ulong)blknr, c);
+				printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n",
+					device, (ulong)blknr, c);
 #endif
-			break;
-		}
+				break;
+			}
 
-		input_data (device, buffer, ATA_SECTORWORDS);
-		(void) ide_inb (device, ATA_STATUS);	/* clear IRQ */
-
-		++n;
-		++blknr;
-		buffer += ATA_BLOCKSIZE;
+			input_data (device, buffer, ATA_SECTORWORDS);
+			(void) ide_inb (device, ATA_STATUS);	/* clear IRQ */
+
+			++n;
+			++blknr;
+			--blkcnt;
+			--scnt;
+			buffer += ATA_BLOCKSIZE;
+		}
 	}
 IDE_READ_E:
 	ide_led (DEVICE_LED(device), 0);	/* LED off	*/

@@ -1548,11 +1613,11 @@ OUT:
  */
 static uchar ide_wait (int dev, ulong t)
 {
-	ulong delay = 10 * t;		/* poll every 100 us */
+	ulong delay = (1000/5) * t;		/* poll every 5 us */
 	uchar c;
 
 	while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) {
-		udelay (100);
+		udelay (5);
 		if (delay-- == 0) {
 			break;
 		}
#1 lore raw

PATCH 1/1 IDE: Improving speed on reading data Part 1/1

Dear Stefan Althoefer,

Quoted 11 lines · In message <49384704.sPrSOzo/CYqyo4zk%[email protected]> you wrote:

[PATCH] IDE: Improving speed on reading data

This patch improves the speed when reading blocks from IDE devices by reading more than one block at a time. Up to 128 blocks are requested in one read command.

On my testplatform (Janz emPC-A400 with CompactFLASH card) this nearly doubled speed.

Also the ide_wait() code was rewritten to have lower latency by polling more frequently for status.

Can you please use git-format-patch to format the patch? I don't know how you created the diff, but it looks strange to me (and harldy readable).

Quoted 4 lines

The patch is against "latest" u-boot git-repository

Please (still) be patient if style of submission or patches are offending.

Such comments must go below the "---" line

Quoted 2 lines
Signed-off-by Stefan Althoefer <[email protected]>

----

Thsi is the place to add comments that are not supposed to become part of the commit message.

common/cmd_ide.c ulong ide_read (int device, lbaint_t blk · line 1432

Quoted 3 lines
common/cmd_ide.c renamed from /common/cmd_ide.c +2 -1
-		ide_outb (device, ATA_SECT_CNT, 1);
+		scnt = (blkcnt > 128) ? 128 : blkcnt;
+		ide_outb (device, ATA_SECT_CNT, scnt);

What happens if you try to read at or beyond the end of the device?

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Brain off-line, please wait.
author
#2 lore raw

PATCH 1/1 IDE: Improving speed on reading data Part 1/1

IDE: Improving speed on reading data

This patch improves the speed when reading blocks from IDE devices by reading more than one block at a time. Up to 128 blocks are requested in one read command.

The ide_wait() code was rewritten to have lower latency by polling more frequently for status.

On my testplatform (Janz emPC-A400 with CompactFLASH card) this nearly doubled speed.

Also, error handling has been improved, so that ide_read does not attempt to read beyond the last sector of the device.

Signed-off-by Stefan Althoefer <[email protected]>

---

Quoted 3 lines

Can you please use git-format-patch to format the patch? I don't know how you created the diff, but it looks strange to me (and harldy readable).

Sorry I wasn't fully aware of the wonderful world of git-* when I prepared the first patches ... ok I'm still not (fully).

Quoted 5 lines

Review

Quoted 3 lines
(quoted diff) +2 -1
-		ide_outb (device, ATA_SECT_CNT, 1);
+		scnt = (blkcnt > 128) ? 128 : blkcnt;
+		ide_outb (device, ATA_SECT_CNT, scnt);

What happens if you try to read at or beyond the end of the device?

Good point. The old code wasn't very smart with this either (it did not check for bounds but let the device decide). My code added a deadlock to this behaviour (break did not work in a inner loop).

I added some checks against block_dev_desc_t->lba to fix this.

 common/cmd_ide.c |   63 ++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 40 insertions(+), 23 deletions(-)
common/cmd_ide.c +40 -23
index dd62c87..ec64767 100644
@@ -1361,6 +1361,8 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 	ulong n = 0;
 	unsigned char c;
 	unsigned char pwrsave=0; /* power save */
+	ulong scnt;
+	lbaint_t blkrem;
 #ifdef CONFIG_LBA48
 	unsigned char lba48 = 0;
 

@@ -1372,6 +1374,12 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 	debug ("ide_read dev %d start %qX, blocks %lX buffer at %lX\n",
 		device, blknr, blkcnt, (ulong)buffer);
 
+	if( blknr >= ide_get_dev(device)->lba ){
+		printf ("IDE read: device %d read beyond last block\n", device);
+		goto IDE_READ_E;
+	}
+	blkrem = ide_get_dev(device)->lba - blknr;
+
 	ide_led (DEVICE_LED(device), 1);	/* LED on	*/
 
 	/* Select device

@@ -1405,7 +1413,7 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 	}
 
 
-	while (blkcnt-- > 0) {
+	while (blkcnt > 0) {
 
 		c = ide_wait (device, IDE_TIME_OUT);
 

@@ -1427,7 +1435,9 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 #endif
 		}
 #endif
-		ide_outb (device, ATA_SECT_CNT, 1);
+		scnt = (blkcnt > 128) ? 128 : blkcnt;
+		scnt = (scnt > blkrem) ? blkrem : scnt;
+		ide_outb (device, ATA_SECT_CNT, scnt);
 		ide_outb (device, ATA_LBA_LOW,  (blknr >>  0) & 0xFF);
 		ide_outb (device, ATA_LBA_MID,  (blknr >>  8) & 0xFF);
 		ide_outb (device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);

@@ -1446,32 +1456,39 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 			ide_outb (device, ATA_COMMAND,  ATA_CMD_READ);
 		}
 
-		udelay (50);
+		while (scnt > 0) {
+			udelay (50);
 
-		if(pwrsave) {
-			c = ide_wait (device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */
-			pwrsave=0;
-		} else {
-			c = ide_wait (device, IDE_TIME_OUT);	/* can't take over 500 ms */
-		}
+			if(pwrsave) {
+				c = ide_wait (device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */
+				pwrsave=0;
+			} else {
+				c = ide_wait (device, IDE_TIME_OUT);	/* can't take over 500 ms */
+			}
 
-		if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) {
+			if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) {
 #if defined(CONFIG_SYS_64BIT_LBA) && defined(CONFIG_SYS_64BIT_VSPRINTF)
-			printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n",
-				device, blknr, c);
+				printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n",
+					device, blknr, c);
 #else
-			printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n",
-				device, (ulong)blknr, c);
+				printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n",
+					device, (ulong)blknr, c);
 #endif
-			break;
-		}
+				goto IDE_READ_E;
+			}
 
-		input_data (device, buffer, ATA_SECTORWORDS);
-		(void) ide_inb (device, ATA_STATUS);	/* clear IRQ */
+			input_data (device, buffer, ATA_SECTORWORDS);
+			(void) ide_inb (device, ATA_STATUS);	/* clear IRQ */
 
-		++n;
-		++blknr;
-		buffer += ATA_BLOCKSIZE;
+			++n;
+			++blknr;
+			--blkcnt;
+			--blkrem;
+			--scnt;
+			buffer += ATA_BLOCKSIZE;
+		}
+		if (blkrem == 0)
+			break;
 	}
 IDE_READ_E:
 	ide_led (DEVICE_LED(device), 0);	/* LED off	*/

@@ -1607,11 +1624,11 @@ OUT:
  */
 static uchar ide_wait (int dev, ulong t)
 {
-	ulong delay = 10 * t;		/* poll every 100 us */
+	ulong delay = (1000/5) * t;		/* poll every 5 us */
 	uchar c;
 
 	while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) {
-		udelay (100);
+		udelay (5);
 		if (delay-- == 0) {
 			break;
 		}
Signature · 1 line
1.5.6
#3 lore raw

PATCH 1/1 IDE: Improving speed on reading data Part 1/1

Quoted 30 lines · Stefan Althoefer

IDE: Improving speed on reading data

This patch improves the speed when reading blocks from IDE devices by reading more than one block at a time. Up to 128 blocks are requested in one read command.

The ide_wait() code was rewritten to have lower latency by polling more frequently for status.

On my testplatform (Janz emPC-A400 with CompactFLASH card) this nearly doubled speed.

Also, error handling has been improved, so that ide_read does not attempt to read beyond the last sector of the device.

Signed-off-by Stefan Althoefer <[email protected]>

---

Quoted 3 lines

Can you please use git-format-patch to format the patch? I don't know how you created the diff, but it looks strange to me (and harldy readable).

Sorry I wasn't fully aware of the wonderful world of git-* when I prepared the first patches ... ok I'm still not (fully).

Quoted 5 lines

common/cmd_ide.c ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) · line 1440

Quoted 3 lines
common/cmd_ide.c +2 -1
-		ide_outb (device, ATA_SECT_CNT, 1);
+		scnt = (blkcnt > 128) ? 128 : blkcnt;
+		ide_outb (device, ATA_SECT_CNT, scnt);

What happens if you try to read at or beyond the end of the device?

Quoted 5 lines

Good point. The old code wasn't very smart with this either (it did not check for bounds but let the device decide). My code added a deadlock to this behaviour (break did not work in a inner loop).

I added some checks against block_dev_desc_t->lba to fix this.

please send patch as a separate e-mail or put the comment which will not appear in the commit message after the ---

Quoted 18 lines
 common/cmd_ide.c |   63 ++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 40 insertions(+), 23 deletions(-)
common/cmd_ide.c +2 -0
index dd62c87..ec64767 100644
@@ -1361,6 +1361,8 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 	ulong n = 0;
 	unsigned char c;
 	unsigned char pwrsave=0; /* power save */
+	ulong scnt;
+	lbaint_t blkrem;
 #ifdef CONFIG_LBA48
 	unsigned char lba48 = 0;

common/cmd_ide.c ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) · line 1377

Quoted 4 lines
common/cmd_ide.c +1 -0
@@ -1372,6 +1374,12 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 	debug ("ide_read dev %d start %qX, blocks %lX buffer at %lX\n",
 		device, blknr, blkcnt, (ulong)buffer);
 
+	if( blknr >= ide_get_dev(device)->lba ){

please replace with

	if (blknr >= ide_get_dev(device)->lba) {
Quoted 35 lines
common/cmd_ide.c +11 -3
+		printf ("IDE read: device %d read beyond last block\n", device);
+		goto IDE_READ_E;
+	}
+	blkrem = ide_get_dev(device)->lba - blknr;
+
 	ide_led (DEVICE_LED(device), 1);	/* LED on	*/
 
 	/* Select device

@@ -1405,7 +1413,7 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 	}
 
 
-	while (blkcnt-- > 0) {
+	while (blkcnt > 0) {
 
 		c = ide_wait (device, IDE_TIME_OUT);
 

@@ -1427,7 +1435,9 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 #endif
 		}
 #endif
-		ide_outb (device, ATA_SECT_CNT, 1);
+		scnt = (blkcnt > 128) ? 128 : blkcnt;
+		scnt = (scnt > blkrem) ? blkrem : scnt;
+		ide_outb (device, ATA_SECT_CNT, scnt);
 		ide_outb (device, ATA_LBA_LOW,  (blknr >>  0) & 0xFF);
 		ide_outb (device, ATA_LBA_MID,  (blknr >>  8) & 0xFF);
 		ide_outb (device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);

@@ -1446,32 +1456,39 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer)
 			ide_outb (device, ATA_COMMAND,  ATA_CMD_READ);
 		}
 
-		udelay (50);
+		while (scnt > 0) {
+			udelay (50);

common/cmd_ide.c ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) · line 1464

Quoted 10 lines
common/cmd_ide.c +3 -6
 
-		if(pwrsave) {
-			c = ide_wait (device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */
-			pwrsave=0;
-		} else {
-			c = ide_wait (device, IDE_TIME_OUT);	/* can't take over 500 ms */
-		}
+			if(pwrsave) {
+				c = ide_wait (device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */
+				pwrsave=0;

please add a space before and after the '='
add be carefull of the 80 chars limit

common/cmd_ide.c ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) · line 1469

Quoted 6 lines
common/cmd_ide.c +4 -1
+			} else {
+				c = ide_wait (device, IDE_TIME_OUT);	/* can't take over 500 ms */
+			}
 
-		if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) {
+			if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) {

please add a space before and after the '&' and '|'

Quoted 37 lines
common/cmd_ide.c +17 -11
 #if defined(CONFIG_SYS_64BIT_LBA) && defined(CONFIG_SYS_64BIT_VSPRINTF)
-			printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n",
-				device, blknr, c);
+				printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n",
+					device, blknr, c);
 #else
-			printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n",
-				device, (ulong)blknr, c);
+				printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n",
+					device, (ulong)blknr, c);
 #endif
-			break;
-		}
+				goto IDE_READ_E;
+			}
 
-		input_data (device, buffer, ATA_SECTORWORDS);
-		(void) ide_inb (device, ATA_STATUS);	/* clear IRQ */
+			input_data (device, buffer, ATA_SECTORWORDS);
+			(void) ide_inb (device, ATA_STATUS);	/* clear IRQ */
 
-		++n;
-		++blknr;
-		buffer += ATA_BLOCKSIZE;
+			++n;
+			++blknr;
+			--blkcnt;
+			--blkrem;
+			--scnt;
+			buffer += ATA_BLOCKSIZE;
+		}
+		if (blkrem == 0)
+			break;
 	}
 IDE_READ_E:
 	ide_led (DEVICE_LED(device), 0);	/* LED off	*/

common/cmd_ide.c OUT: · line 1627

Quoted 5 lines
common/cmd_ide.c +1 -1
@@ -1607,11 +1624,11 @@ OUT:
  */
 static uchar ide_wait (int dev, ulong t)
 {
-	ulong delay = 10 * t;		/* poll every 100 us */
+	ulong delay = (1000/5) * t;		/* poll every 5 us */

why not 200 instead?

common/cmd_ide.c OUT: · line 1634

Quoted 8 lines
common/cmd_ide.c +1 -1
 	uchar c;
 
 	while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) {
-		udelay (100);
+		udelay (5);
 		if (delay-- == 0) {
 			break;
 		}

Best Regards,
J.