[PATCH] add boot option to control Intel combined mode behavior (to allow DMA in combined mode configs!)

[PATCH] add boot option to control Intel combined mode behavior (to allow DMA in combined mode configs!)

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

Combined mode sucks.  Especially when both libata and the legacy IDE 
drivers try to drive ports on the same device, since that makes DMA 
rather difficult.

This patch addresses the problem by allowing the user to control which 
driver binds to the ports in a combined mode configuration.  In many 
cases, they'll probably want the libata driver to control both ports 
since it can use DMA for talking with ATAPI devices (when 
libata.atapi_enabled=1 of course).  It also allows the user to get old 
school behavior by letting the legacy IDE driver bind to both ports.  
But neither is forced, the patch doesn't change current behavior unless 
one of intel_combined_mode=ide or intel_combined_mode=libata is passed 
on the boot line.  Either of those options may require you to access 
your devices via different device nodes (/dev/hd* in the ide case 
and /dev/sd* in the libata case), though of course if you have udev 
installed nicely you may not notice anything. :)

Let me know if the documentation is too cryptic, I'd be happy to expand 
on it if necessary.  I think most users will want to boot with 
'intel_combined_mode=libata' and add 'options libata atapi_enabled=1' 
to their modules.conf to get good DVD playing and disk behavior 
(haven't tested CD or DVD writing though).

 Documentation/kernel-parameters.txt |    7 +++++++
 drivers/pci/quirks.c                |   30 ++++++++++++++++++++++++++++

I'd much rather things behave sanely by default (i.e. DMA for devices on 
both ports), but apparently that's difficult given the various chip 
bugs and hardware configs out there (not to mention that people's 
drives may suddenly change from /dev/hdc to /dev/sdb), so this boot 
option may be the correct long term fix.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Thanks,
Jesse

[-- Attachment #2: intel-combined-mode-boot-option.patch --]
[-- Type: text/x-diff, Size: 2202 bytes --]

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5dffcfe..5f8104f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -633,6 +633,13 @@ running once the system is up.
 	inport.irq=	[HW] Inport (ATI XL and Microsoft) busmouse driver
 			Format: <irq>
 
+	intel_combined_mode=
+			[HW] control which driver uses IDE ports in combined
+			mode, legacy IDE driver, both or libata exclusively
+			(in the latter case, libata.atapi_enabled=1 may be
+			useful as well)
+			Format: ide,combined,libata
+
 	inttest=	[IA64]
 
 	io7=		[HW] IO7 for Marvel based alpha systems
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 3a4f49f..9be5ae0 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1099,6 +1099,23 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_I
 #endif
 
 #ifdef CONFIG_SCSI_SATA_INTEL_COMBINED
+enum ide_combined_type { COMBINED = 0, IDE = 1, LIBATA = 2 };
+/* Defaults to combined */
+static enum ide_combined_type intel_combined_mode_type;
+
+static int __init intel_ide_combined_setup(char *str)
+{
+	if (!strncmp(str, "ide", 3))
+		intel_combined_mode_type = IDE;
+	else if (!strncmp(str, "libata", 6))
+		intel_combined_mode_type = LIBATA;
+	else /* "combined" or anything else defaults to old behavior */
+		intel_combined_mode_type = COMBINED;
+
+	return 1;
+}
+__setup("intel_combined_mode=", intel_ide_combined_setup);
+
 static void __devinit quirk_intel_ide_combined(struct pci_dev *pdev)
 {
 	u8 prog, comb, tmp;
@@ -1164,6 +1181,19 @@ static void __devinit quirk_intel_ide_co
 	if (prog & comb)
 		return;
 
+	/* Don't reserve any so the IDE driver can get them (but only if
+	 * intel_combined_mode=ide).
+	 */
+	if (intel_combined_mode_type == IDE)
+		return;
+
+	/* Grab them both for libata if intel_combined_mode=libata. */
+	if (intel_combined_mode_type == LIBATA) {
+		request_region(0x1f0, 8, "libata");	/* port 0 */
+		request_region(0x170, 8, "libata");	/* port 1 */
+		return;
+	}
+
 	/* SATA port is in legacy mode.  Reserve port so that
 	 * IDE driver does not attempt to use it.  If request_region
 	 * fails, it will be obvious at boot time, so we don't bother

Re: [PATCH] add boot option to control Intel combined mode behavior (to allow DMA in combined mode configs!)

[Jeff Garzik]

Jesse Barnes wrote:
> Combined mode sucks.  Especially when both libata and the legacy IDE 
> drivers try to drive ports on the same device, since that makes DMA 
> rather difficult.
> 
> This patch addresses the problem by allowing the user to control which 
> driver binds to the ports in a combined mode configuration.  In many 
> cases, they'll probably want the libata driver to control both ports 
> since it can use DMA for talking with ATAPI devices (when 
> libata.atapi_enabled=1 of course).  It also allows the user to get old 
> school behavior by letting the legacy IDE driver bind to both ports.  
> But neither is forced, the patch doesn't change current behavior unless 
> one of intel_combined_mode=ide or intel_combined_mode=libata is passed 
> on the boot line.  Either of those options may require you to access 
> your devices via different device nodes (/dev/hd* in the ide case 
> and /dev/sd* in the libata case), though of course if you have udev 
> installed nicely you may not notice anything. :)
> 
> Let me know if the documentation is too cryptic, I'd be happy to expand 
> on it if necessary.  I think most users will want to boot with 
> 'intel_combined_mode=libata' and add 'options libata atapi_enabled=1' 
> to their modules.conf to get good DVD playing and disk behavior 
> (haven't tested CD or DVD writing though).
> 
>  Documentation/kernel-parameters.txt |    7 +++++++
>  drivers/pci/quirks.c                |   30 ++++++++++++++++++++++++++++
> 
> I'd much rather things behave sanely by default (i.e. DMA for devices on 
> both ports), but apparently that's difficult given the various chip 
> bugs and hardware configs out there (not to mention that people's 
> drives may suddenly change from /dev/hdc to /dev/sdb), so this boot 
> option may be the correct long term fix.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Seems like it should work.  I presume you tested this?

Remove the 'intel_' prefix from the kernel parameter, since this concept 
applies to other controllers as well.  Otherwise, seems OK.

	Jeff

Re: [PATCH] add boot option to control Intel combined mode behavior (to allow DMA in combined mode configs!)

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2144 bytes --]

On Tuesday, November 29, 2005 1:54 pm, Jeff Garzik wrote:
> Seems like it should work.  I presume you tested this?

Yep, worked like a charm.  As it stands, I can't play DVDs on my laptop, 
but with this patch I can set combined_mode=libata and 
libata.atapi_enabled=1 and DVDs are no longer choppy.  It's great!

> Remove the 'intel_' prefix from the kernel parameter, since this
> concept applies to other controllers as well.  Otherwise, seems OK.

Ok, how does this one look?

Thanks,
Jesse

Combined mode sucks.  Especially when both libata and the legacy IDE 
drivers try to drive ports on the same device, since that makes DMA 
rather difficult.

This patch addresses the problem by allowing the user to control which 
driver binds to the ports in a combined mode configuration.  In many 
cases, they'll probably want the libata driver to control both ports 
since it can use DMA for talking with ATAPI devices (when 
libata.atapi_enabled=1 of course).  It also allows the user to get old 
school behavior by letting the legacy IDE driver bind to both ports.  
But neither is forced, the patch doesn't change current behavior unless 
one of combined_mode=ide or combined_mode=libata is passed 
on the boot line.  Either of those options may require you to access 
your devices via different device nodes (/dev/hd* in the ide case 
and /dev/sd* in the libata case), though of course if you have udev 
installed nicely you may not notice anything. :)

Let me know if the documentation is too cryptic, I'd be happy to expand 
on it if necessary.  I think most users will want to boot with 
'combined_mode=libata' and add 'options libata atapi_enabled=1' 
to their modules.conf to get good DVD playing and disk behavior 
(haven't tested CD or DVD writing though).

I'd much rather things behave sanely by default (i.e. DMA for devices on 
both ports), but apparently that's difficult given the various chip 
bugs and hardware configs out there (not to mention that people's 
drives may suddenly change from /dev/hdc to /dev/sdb), so this boot 
option may be the correct long term fix.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

[-- Attachment #2: combined-mode-boot-option.patch --]
[-- Type: text/x-diff, Size: 2279 bytes --]

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5dffcfe..61a56b1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -633,6 +633,14 @@ running once the system is up.
 	inport.irq=	[HW] Inport (ATI XL and Microsoft) busmouse driver
 			Format: <irq>
 
+	combined_mode=	[HW] control which driver uses IDE ports in combined
+			mode: legacy IDE driver, libata, or both
+			(in the libata case, libata.atapi_enabled=1 may be
+			useful as well).  Note that using the ide or libata
+			options may affect your device naming (e.g. by
+			changing hdc to sdb).
+			Format: combined (default), ide, or libata
+
 	inttest=	[IA64]
 
 	io7=		[HW] IO7 for Marvel based alpha systems
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 3a4f49f..f28ebdd 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1098,6 +1098,23 @@ static void __init quirk_alder_ioapic(st
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_EESSC,	quirk_alder_ioapic );
 #endif
 
+enum ide_combined_type { COMBINED = 0, IDE = 1, LIBATA = 2 };
+/* Defaults to combined */
+static enum ide_combined_type combined_mode;
+
+static int __init combined_setup(char *str)
+{
+	if (!strncmp(str, "ide", 3))
+		combined_mode = IDE;
+	else if (!strncmp(str, "libata", 6))
+		combined_mode = LIBATA;
+	else /* "combined" or anything else defaults to old behavior */
+		combined_mode = COMBINED;
+
+	return 1;
+}
+__setup("combined_mode=", combined_setup);
+
 #ifdef CONFIG_SCSI_SATA_INTEL_COMBINED
 static void __devinit quirk_intel_ide_combined(struct pci_dev *pdev)
 {
@@ -1164,6 +1181,19 @@ static void __devinit quirk_intel_ide_co
 	if (prog & comb)
 		return;
 
+	/* Don't reserve any so the IDE driver can get them (but only if
+	 * combined_mode=ide).
+	 */
+	if (combined_mode == IDE)
+		return;
+
+	/* Grab them both for libata if combined_mode=libata. */
+	if (combined_mode == LIBATA) {
+		request_region(0x1f0, 8, "libata");	/* port 0 */
+		request_region(0x170, 8, "libata");	/* port 1 */
+		return;
+	}
+
 	/* SATA port is in legacy mode.  Reserve port so that
 	 * IDE driver does not attempt to use it.  If request_region
 	 * fails, it will be obvious at boot time, so we don't bother

Re: [PATCH] add boot option to control Intel combined mode behavior (to allow DMA in combined mode configs!)

[Jesse Barnes]

Just FYI, I tested this new patch (the one that changes the option from 
intel_combined_mode= to combined_mode=) and it works great.  Please 
push it upstream after 2.6.15 comes out; I think lots of people will 
benefit from being able to turn on DMA.

Thanks,
Jesse

Re: [PATCH] add boot option to control Intel combined mode behavior (to allow DMA in combined mode configs!)

[Jeff Garzik]

applied