Linus,
Please accept the attached for 2.5.46.
It introduces a new boot parameter (eg. ide0=biostimings) that forces the
IDE driver to honour BIOS DMA/PIO timings. Sometimes the BIOS has a better
overview of how the IDE devices are connected/setup and some chipsets doesn't
support >ata66 speed detection.
The patch has been tested for quite a while on both PIIX and serverworks.
Torben
On Thu, 2002-11-07 at 16:40, Torben Mathiasen wrote:
> Linus,
>
> Please accept the attached for 2.5.46.
>
> It introduces a new boot parameter (eg. ide0=biostimings) that forces the
> IDE driver to honour BIOS DMA/PIO timings. Sometimes the BIOS has a better
> overview of how the IDE devices are connected/setup and some chipsets doesn't
> support >ata66 speed detection.
>
> The patch has been tested for quite a while on both PIIX and serverworks.
Linus please drop this patch for now. Its not been tested on enough
controllers, its making things unneccessarily ugly and its also just
going to make updates hard.
On Fri, Nov 08 2002, Alan Cox wrote:
> On Thu, 2002-11-07 at 16:40, Torben Mathiasen wrote:
> > Linus,
> >
> > Please accept the attached for 2.5.46.
> >
> > It introduces a new boot parameter (eg. ide0=biostimings) that forces the
> > IDE driver to honour BIOS DMA/PIO timings. Sometimes the BIOS has a better
> > overview of how the IDE devices are connected/setup and some chipsets doesn't
> > support >ata66 speed detection.
> >
> > The patch has been tested for quite a while on both PIIX and serverworks.
>
> Linus please drop this patch for now. Its not been tested on enough
> controllers, its making things unneccessarily ugly and its also just
> going to make updates hard.
Alan, the patch is pretty much straight forward. Cleaning up the magic
numbers and ->autotune consistencies is a good thing, imo.
I'm attaching Torben's modified version which adds symbolic names
instead of using more weird autotune magics.
--- linux-2.5.46-clean/include/linux/ide.h 2002-11-08 10:17:50.000000000 +0100
+++ linux-2.5.46/include/linux/ide.h 2002-11-08 10:51:36.000000000 +0100
@@ -96,6 +96,14 @@
#define ERROR_RECAL 1 /* Recalibrate every 2nd retry */
/*
+ * Tune flags
+ */
+#define IDE_TUNE_BIOS 3
+#define IDE_TUNE_NOAUTO 2
+#define IDE_TUNE_AUTO 1
+#define IDE_TUNE_DEFAULT 0
+
+/*
* state flags
*/
#define DMA_PIO_RETRY 1 /* retrying in PIO */
@@ -743,7 +751,8 @@
unsigned nice0 : 1; /* give obvious excess bandwidth */
unsigned nice2 : 1; /* give a share in our own bandwidth */
unsigned doorlocking : 1; /* for removable only: door lock/unlock works */
- unsigned autotune : 2; /* 1=autotune, 2=noautotune, 0=default */
+ unsigned autotune : 3; /* 1=autotune, 2=noautotune,
+ 3=biostimings, 0=default */
unsigned remap_0_to_1 : 2; /* 0=remap if ezdrive, 1=remap, 2=noremap */
unsigned ata_flash : 1; /* 1=present, 0=default */
unsigned blocked : 1; /* 1=powermanagment told us not to do anything, so sleep nicely */
diff -ur linux-2.5.46-clean/drivers/ide/ide-dma.c linux-2.5.46/drivers/ide/ide-dma.c
--- linux-2.5.46-clean/drivers/ide/ide-dma.c 2002-11-08 10:16:58.000000000 +0100
+++ linux-2.5.46/drivers/ide/ide-dma.c 2002-11-08 10:35:41.000000000 +0100
@@ -1028,9 +1028,18 @@
if (hwif->chipset != ide_trm290) {
u8 dma_stat = hwif->INB(hwif->dma_status);
- printk(", BIOS settings: %s:%s, %s:%s",
+ printk(", BIOS settings: %s:%s%s, %s:%s%s",
hwif->drives[0].name, (dma_stat & 0x20) ? "DMA" : "pio",
- hwif->drives[1].name, (dma_stat & 0x40) ? "DMA" : "pio");
+ hwif->drives[0].autotune == IDE_TUNE_BIOS ?
+ " (used)" : "",
+ hwif->drives[1].name, (dma_stat & 0x40) ? "DMA" : "pio",
+ hwif->drives[1].autotune == IDE_TUNE_BIOS ?
+ " (used)" : "");
+
+ if (hwif->drives[0].autotune == IDE_TUNE_BIOS)
+ hwif->drives[0].using_dma = (dma_stat & 0x20);
+ if (hwif->drives[1].autotune == IDE_TUNE_BIOS)
+ hwif->drives[1].using_dma = (dma_stat & 0x40);
}
printk("\n");
diff -ur linux-2.5.46-clean/drivers/ide/ide-probe.c linux-2.5.46/drivers/ide/ide-probe.c
--- linux-2.5.46-clean/drivers/ide/ide-probe.c 2002-11-08 10:16:58.000000000 +0100
+++ linux-2.5.46/drivers/ide/ide-probe.c 2002-11-08 10:34:17.000000000 +0100
@@ -434,7 +434,9 @@
if (hwif->INB(IDE_STATUS_REG) == (BUSY_STAT|READY_STAT))
return 4;
- if (rc == 1 && cmd == WIN_PIDENTIFY && drive->autotune != 2) {
+ if ((rc == 1 && cmd == WIN_PIDENTIFY) &&
+ ((drive->autotune == IDE_TUNE_DEFAULT) ||
+ (drive->autotune == IDE_TUNE_AUTO))) {
unsigned long timeout;
printk("%s: no response (status = 0x%02x), "
"resetting drive\n", drive->name,
@@ -699,7 +701,8 @@
for (unit = 0; unit < MAX_DRIVES; ++unit) {
ide_drive_t *drive = &hwif->drives[unit];
if (drive->present) {
- if (hwif->tuneproc != NULL && drive->autotune == 1)
+ if (hwif->tuneproc != NULL &&
+ drive->autotune == IDE_TUNE_AUTO)
/* auto-tune PIO mode */
hwif->tuneproc(drive, 255);
/*
@@ -711,7 +714,9 @@
* Move here to prevent module loading clashing.
*/
// drive->autodma = hwif->autodma;
- if ((drive->autotune != 2) && (hwif->ide_dma_check)) {
+ if ((hwif->ide_dma_check) &&
+ ((drive->autotune == IDE_TUNE_DEFAULT) ||
+ (drive->autotune == IDE_TUNE_AUTO))) {
/*
* Force DMAing for the beginning of the check.
* Some chipsets appear to do interesting
diff -ur linux-2.5.46-clean/drivers/ide/ide.c linux-2.5.46/drivers/ide/ide.c
--- linux-2.5.46-clean/drivers/ide/ide.c 2002-11-08 10:16:58.000000000 +0100
+++ linux-2.5.46/drivers/ide/ide.c 2002-11-08 10:28:39.000000000 +0100
@@ -2621,7 +2621,8 @@
* Not fully supported by all chipset types,
* and quite likely to cause trouble with
* older/odd IDE drives.
- *
+ * "hdx=biostimings" : driver will NOT attempt to tune interface speed
+ * (DMA/PIO) but always honour BIOS timings.
* "hdx=slow" : insert a huge pause after each access to the data
* port. Should be used only as a last resort.
*
@@ -2658,6 +2659,8 @@
* "idex=noautotune" : driver will NOT attempt to tune interface speed
* This is the default for most chipsets,
* except the cmd640.
+ * "idex=biostimings" : driver will NOT attempt to tune interface speed
+ * (DMA/PIO) but always honour BIOS timings.
* "idex=serialize" : do not overlap operations on idex and ide(x^1)
* "idex=four" : four drives on idex and ide(x^1) share same ports
* "idex=reset" : reset interface before first use
@@ -2733,7 +2736,8 @@
const char *hd_words[] = {"none", "noprobe", "nowerr", "cdrom",
"serialize", "autotune", "noautotune",
"slow", "swapdata", "bswap", "flash",
- "remap", "noremap", "scsi", NULL};
+ "remap", "noremap", "scsi", "biostimings",
+ NULL};
unit = s[2] - 'a';
hw = unit / MAX_DRIVES;
unit = unit % MAX_DRIVES;
@@ -2775,10 +2779,10 @@
printk(" -- USE \"ide%d=serialize\" INSTEAD", hw);
goto do_serialize;
case -6: /* "autotune" */
- drive->autotune = 1;
+ drive->autotune = IDE_TUNE_AUTO;
goto done;
case -7: /* "noautotune" */
- drive->autotune = 2;
+ drive->autotune = IDE_TUNE_NOAUTO;
goto done;
case -8: /* "slow" */
drive->slow = 1;
@@ -2804,6 +2808,9 @@
drive->scsi = 0;
goto bad_option;
#endif /* defined(CONFIG_BLK_DEV_IDESCSI) && defined(CONFIG_SCSI) */
+ case -15: /* "biostimings" */
+ drive->autotune = IDE_TUNE_BIOS;
+ goto done;
case 3: /* cyl,head,sect */
drive->media = ide_disk;
drive->cyl = drive->bios_cyl = vals[0];
@@ -2841,9 +2848,10 @@
* -8,-9,-10 : are reserved for future idex calls to ease the hardcoding.
*/
const char *ide_words[] = {
- "noprobe", "serialize", "autotune", "noautotune", "reset", "dma", "ata66",
- "minus8", "minus9", "minus10",
- "four", "qd65xx", "ht6560b", "cmd640_vlb", "dtc2278", "umc8672", "ali14xx", "dc4030", NULL };
+ "noprobe", "serialize", "autotune", "noautotune",
+ "reset", "dma", "ata66", "minus8", "minus9", "minus10",
+ "four", "qd65xx", "ht6560b", "cmd640_vlb", "dtc2278",
+ "umc8672", "ali14xx", "dc4030", "biostimings", NULL };
hw = s[3] - '0';
hwif = &ide_hwifs[hw];
i = match_parm(&s[4], ide_words, vals, 3);
@@ -2862,6 +2870,10 @@
}
switch (i) {
+ case -19: /* "biostimings" */
+ hwif->drives[0].autotune = IDE_TUNE_BIOS;
+ hwif->drives[1].autotune = IDE_TUNE_BIOS;
+ goto done;
#ifdef CONFIG_BLK_DEV_PDC4030
case -18: /* "dc4030" */
{
@@ -2949,12 +2961,12 @@
hwif->reset = 1;
goto done;
case -4: /* "noautotune" */
- hwif->drives[0].autotune = 2;
- hwif->drives[1].autotune = 2;
+ hwif->drives[0].autotune = IDE_TUNE_NOAUTO;
+ hwif->drives[1].autotune = IDE_TUNE_NOAUTO;
goto done;
case -3: /* "autotune" */
- hwif->drives[0].autotune = 1;
- hwif->drives[1].autotune = 1;
+ hwif->drives[0].autotune = IDE_TUNE_AUTO;
+ hwif->drives[1].autotune = IDE_TUNE_AUTO;
goto done;
case -2: /* "serialize" */
do_serialize:
@@ -3211,7 +3223,8 @@
spin_lock(&drives_lock);
list_add(&drive->list, &driver->drives);
spin_unlock(&drives_lock);
- if (drive->autotune != 2) {
+ if ((drive->autotune == IDE_TUNE_DEFAULT) ||
+ (drive->autotune == IDE_TUNE_AUTO)) {
/* DMA timings and setup moved to ide-probe.c */
if (!driver->supports_dma && HWIF(drive)->ide_dma_off_quietly)
// HWIF(drive)->ide_dma_off_quietly(drive);
diff -ur linux-2.5.46-clean/drivers/ide/setup-pci.c linux-2.5.46/drivers/ide/setup-pci.c
--- linux-2.5.46-clean/drivers/ide/setup-pci.c 2002-11-08 10:16:58.000000000 +0100
+++ linux-2.5.46/drivers/ide/setup-pci.c 2002-11-08 10:46:57.000000000 +0100
@@ -590,6 +590,7 @@
u32 port, at_least_one_hwif_enabled = 0, autodma = 0;
int pciirq = 0;
int tried_config = 0;
+ int drive0_tune, drive1_tune;
ata_index_t index;
u8 tmp = 0;
ide_hwif_t *hwif, *mate = NULL;
@@ -698,11 +699,20 @@
ide_hwif_setup_dma(dev, d, hwif);
bypass_legacy_dma:
#endif /* CONFIG_BLK_DEV_IDEDMA */
+
+ drive0_tune = hwif->drives[0].autotune;
+ drive1_tune = hwif->drives[1].autotune;
+
if (d->init_hwif)
/* Call chipset-specific routine
* for each enabled hwif
*/
d->init_hwif(hwif);
+
+ if (drive0_tune == IDE_TUNE_BIOS) /* biostimings */
+ hwif->drives[0].autotune = IDE_TUNE_BIOS;
+ if (drive1_tune == IDE_TUNE_BIOS)
+ hwif->drives[1].autotune = IDE_TUNE_BIOS;
mate = hwif;
at_least_one_hwif_enabled = 1;
--
Jens Axboe
On Fri, 2002-11-08 at 16:56, Jens Axboe wrote:
> > Linus please drop this patch for now. Its not been tested on enough
> > controllers, its making things unneccessarily ugly and its also just
> > going to make updates hard.
>
> Alan, the patch is pretty much straight forward. Cleaning up the magic
> numbers and ->autotune consistencies is a good thing, imo.
You can clean up the naming but it still hasn't been tested, not all
bioses neccessarily give us timings we can trust either. I'm not
opposed to the concept but after the previous IDE mess in 2.5 merging
something that isnt tested on lots of controllers and might have weird
effects does both me a bit
Jens Axboe <[email protected]> writes:
>- unsigned autotune : 2; /* 1=autotune, 2=noautotune, 0=default */
>+ unsigned autotune : 3; /* 1=autotune, 2=noautotune,
>+ 3=biostimings, 0=default */
Why are three bits reserved for four states? (or am I reading this
wrong? Sorry, my C has started to become a little rusty).
Regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH [email protected]
Am Schwabachgrund 22 Fon.: 09131 / 50654-0 [email protected]
D-91054 Buckenhof Fax.: 09131 / 50654-20
On 8 Nov 2002, Alan Cox wrote:
> On Fri, 2002-11-08 at 16:56, Jens Axboe wrote:
> > > Linus please drop this patch for now. Its not been tested on enough
> > > controllers, its making things unneccessarily ugly and its also just
> > > going to make updates hard.
> >
> > Alan, the patch is pretty much straight forward. Cleaning up the magic
> > numbers and ->autotune consistencies is a good thing, imo.
>
> You can clean up the naming but it still hasn't been tested, not all
> bioses neccessarily give us timings we can trust either. I'm not
> opposed to the concept but after the previous IDE mess in 2.5 merging
> something that isnt tested on lots of controllers and might have weird
> effects does both me a bit
This is one of those things which we should allow at user risk. After all,
you can shoot yourself in the foot with hdparm as well, there are many
unwise things allowed.
Having seen all the warnings from bad setup of MPS and ACPI in dmesg, I
would say it's more likely that the BIOS get these settings right, since
they may be used by that other operating system.
--
bill davidsen <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.
On Mon, Nov 11, 2002 at 12:10:13PM -0500, Bill Davidsen wrote:
> On 8 Nov 2002, Alan Cox wrote:
>
> > On Fri, 2002-11-08 at 16:56, Jens Axboe wrote:
> > > > Linus please drop this patch for now. Its not been tested on enough
> > > > controllers, its making things unneccessarily ugly and its also just
> > > > going to make updates hard.
> > >
> > > Alan, the patch is pretty much straight forward. Cleaning up the magic
> > > numbers and ->autotune consistencies is a good thing, imo.
> >
> > You can clean up the naming but it still hasn't been tested, not all
> > bioses neccessarily give us timings we can trust either. I'm not
> > opposed to the concept but after the previous IDE mess in 2.5 merging
> > something that isnt tested on lots of controllers and might have weird
> > effects does both me a bit
>
> This is one of those things which we should allow at user risk. After all,
> you can shoot yourself in the foot with hdparm as well, there are many
> unwise things allowed.
>
> Having seen all the warnings from bad setup of MPS and ACPI in dmesg, I
> would say it's more likely that the BIOS get these settings right, since
> they may be used by that other operating system.
I can tell you that many VIA (namely older) boards simply crash after
the first DMA access when you don't fix the timings/fifo settings of the
chip after what mess the BIOS left there.
--
Vojtech Pavlik
SuSE Labs
On Tue, 12 Nov 2002, Vojtech Pavlik wrote:
> On Mon, Nov 11, 2002 at 12:10:13PM -0500, Bill Davidsen wrote:
> > This is one of those things which we should allow at user risk. After all,
> > you can shoot yourself in the foot with hdparm as well, there are many
> > unwise things allowed.
> >
> > Having seen all the warnings from bad setup of MPS and ACPI in dmesg, I
> > would say it's more likely that the BIOS get these settings right, since
> > they may be used by that other operating system.
>
> I can tell you that many VIA (namely older) boards simply crash after
> the first DMA access when you don't fix the timings/fifo settings of the
> chip after what mess the BIOS left there.
Then those users shouldn't use the option. The standard behaviour would
remain the same, it only affects those who choose to use it. I don't doubt
it could cause problems on some boards, but there are many things which do
(like compiling for P4 on a 486 which someone local tried).
It should be marked experimental initially, and probably DANGEROUS
permanently.
--
bill davidsen <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.