E7x05 chipset bug in 2.5 kernels' AGPGART driver.

Hello,

I noticed there is a bug in E7x05 (Granite Bay) chipset support of the AGPGART driver that does not seem to have been submitted yet.

There is a mistake in the Makefile of drivers/char/agp, the line concerning i7x05-agp support does not match the one in the Kconfig, thus e7x05 support is never compiled, be it as a module or in the kernel.

There are mistakes in the i7x05-agp source code. A few missings ";" and some undeclared functions prevent the module from properly compiling. Those mistakes are easily fixed though.

The last problem is more important and I have yet to find a solution. It seems like the driver stores device 0 in his agp_bridge->dev (0x255d for E7205, 0x2550 for E7505) but it uses registers from device 1 (0x2552) thus the chipset cannot be configured properly. The fetch_size function fails to determine aperture size.

Sorry if this is redundant and is already being looked at.

Gareth C.

P.S : include answers in C.C if you may, pretty please.

I'm really amazed. This has been broken for months, and no-one noticed. The day I fixed it in the agpgart bk tree, I got a half dozen reports, and now I'm getting one daily. Truly bizarre.

Yep, the other issues (compile problems) are fixed up and will be going to Linus real soon now, this problem though is something that is being looked at by Matt (i7x05 gart driver author) right now.
Hopefully that'll also get fixed up with the changes being readied for 2.5.67

The reason i7x05 does things differently is that the generic-3.0 code looks at the devices hanging of the device 0 (in that case, agp gfx cards). I realised last week that the generic code is broken, as every other agp chipset has the agp cards as secondaries of the agp bridge, not the host bridge. Rather than fudge things like this, it looks like a lot of the generic-3.0 stuff will be ripped out. It doesn't really work properly, and is of questionable use.

No problem 8-)

Dave

I've checked Intel E7205 and E7505 spec sheets. But I can not find
why we should use device #1 (virtual agp bridge) instaed of device #0 (MCH). I rewrite E7x05 agpgart driver to use device #0. It works fine for me (I tested it on E7505 motherboard with AGPx4 card and AGPx8 card). To use MCH registers (on device #0) is generic way for Intel agpgart driver, so the driver can merge into intel-agp.c easyly.

I put the patchset (against current agpgart BK tree)
on the URL. Please test them:

http://hanzubon.jp/agpgart/e7x05-agp-intel-agp-merge-4-1.diff
http://hanzubon.jp/agpgart/e7x05-agp-intel-agp-merge-4-2.diff
http://hanzubon.jp/agpgart/agp_enable_fallback_fix.diff

e7x05-agp-intel-agp-merge-4-1.diff:
main part of the patch to integrate e7x05 support into intel-agp.c

e7x05-agp-intel-agp-merge-4-2.diff:
drop i7505-agp.c and update Kconfig and Makefile.

agp_enable_fallback_fix.diff:
fix agp_generic_enable() function's fall back mehacnism to agp 2.0
mode. It is already sent and merged to dj's tree (thanks :-)
It is not directly related of E7x05 AGP issue, but it is needed to
test the E7x05 driver.

diff -urN linux-2.5.orig/drivers/char/agp/agp.h linux-2.5/drivers/char/agp/agp.h
@@ -273,14 +273,8 @@
 #define I810_DRAM_ROW_0		0x00000001
 #define I810_DRAM_ROW_0_SDRAM	0x00000001
 
-/* Intel 7505 registers */
-#define INTEL_I7505_NAPBASELO	0x10
-#define INTEL_I7505_APSIZE	0x74
-#define INTEL_I7505_NCAPID	0x60
-#define INTEL_I7505_NISTAT	0x6c
-#define INTEL_I7505_ATTBASE	0x78
-#define INTEL_I7505_ERRSTS	0x42
-#define INTEL_I7505_AGPCTRL	0x70
+/* Intel E7205/7505 register */
+#define INTEL_E7X05_MCHCFG	0x50
 
 /* VIA register */
 #define VIA_APBASE	0x10

diff -urN linux-2.5.orig/include/linux/agp_backend.h linux-2.5/include/linux/agp_backend.h
@@ -53,7 +53,7 @@
 	INTEL_I850,
 	INTEL_I860,
 	INTEL_460GX,
-	INTEL_I7505,
+	INTEL_E7X05,
 	VIA_GENERIC,
 	SIS_GENERIC,
 	AMD_GENERIC,

diff -urN linux-2.5.orig/drivers/char/agp/Kconfig linux-2.5/drivers/char/agp/Kconfig
@@ -34,13 +34,14 @@
 	depends on AGP
 
 config AGP_INTEL
-	tristate "Intel 440LX/BX/GX and I815/I820/830M/I830MP/I840/I845/845G/I850/852GM/855GM/I860/865G support"
+	tristate "Intel 440LX/BX/GX and I815/I820/830M/I830MP/I840/I845/845G/I850/852GM/855GM/I860/865G/E7205/E7505 support"
 	depends on AGP
 	help
 	  This option gives you AGP support for the GLX component of the
-	  XFree86 4.x on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850
-	  and 860 chipsets and full support for the 810, 815, 830M, 845G,
-	  852GM, 855GM and 865G integrated graphics chipsets.
+	  XFree86 4.x on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850,
+	  860, E7205 and E7505 chipsets and full support for the 810,
+	  815, 830M, 845G, 852GM, 855GM and 865G integrated graphics
+	  chipsets.
 
 	  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
 	  use GLX or DRI, or if you have any Intel integrated graphics

@@ -138,16 +139,3 @@
 	tristate
 	depends on AGP && (ALPHA_GENERIC || ALPHA_TITAN || ALPHA_MARVEL)
 	default AGP
-
-# Put AGP 3.0 entries below here.
-
-config AGP_I7x05
-	tristate "Intel 7205/7505 support (AGP 3.0)"
-	depends on AGP3
-	help
-	  This option gives you AGP support for the GLX component of the
-	  XFree86 4.x on Intel I7505 chipsets.
-
-	  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-	  use GLX or DRI.  If unsure, say N
-

diff -urN linux-2.5.orig/drivers/char/agp/Makefile linux-2.5/drivers/char/agp/Makefile
@@ -18,6 +18,3 @@
 obj-$(CONFIG_AGP_HP_ZX1)	+= hp-agp.o
 obj-$(CONFIG_AGP_AMD_8151)	+= amd-k8-agp.o
 obj-$(CONFIG_AGP_ALPHA_CORE)	+= alpha-agp.o
-
-obj-$(CONFIG_AGP_I7x05)		+= i7x05-agp.o
-

@@ -415,18 +415,15 @@
 		}
 	}
 #endif
-
-	if (major < 3) {
-		pci_read_config_dword(agp_bridge->dev,
+	pci_read_config_dword(agp_bridge->dev,
 			      agp_bridge->capndx + PCI_AGP_STATUS, &command);
 
-		command = agp_collect_device_status(mode, command);
-		command |= 0x100;
-
-		pci_write_config_dword(agp_bridge->dev,
+	command = agp_collect_device_status(mode, command);
+	command |= 0x100;
+	
+	pci_write_config_dword(agp_bridge->dev,
 			       agp_bridge->capndx + PCI_AGP_COMMAND, command);
-		agp_device_command(command, 0);
-	}
+	agp_device_command(command, 0);
 }
 
 int agp_generic_create_gatt_table(void)

ISHIKAWA Mutsumi
 <[email protected]>, <[email protected]>, <[email protected]>

Look at agp_3_0_node_enable()
In particular, the part that does a pci read of PCI_SECONDARY_BUS. device #1 (AGP bridge) has 1:0.0 (AGP graphic card) as its secondary bus here. Device #0 (Host bridge) doesn't have anything as a secondary bus, so it can't mate the card with the bridge. So i7x05 jumps through hoops using the wrong device in its agp_bridge->dev to get this right.

Its really fugly, and is against what every other gart driver is doing. In my working tree (not yet pushed to bkbits) I've ripped out that whole PCI_SECONDARY_BUS thing and replaced it with a simple bus scan that looks for 2 devices that are AGP host and AGP target and pairs them unconditionally.

This will probably break if ever someone does a >1 AGP Slot motherboard but with vendors preffering to just dual-head AGP cards, and with PCI Express in the pipeline, it's questionable whether or not we'll see them. I've heard from one vendor who says they're interested in doing such a board. If it ever sees light of day, it's worth worrying about, (and this system sounds so niche I doubt it'd ever be mainstream anyway).

I don't mind either way. If Matt wants to keep this as a seperate subdriver I'll keep it as it is. If he's ok with the merge, I'll apply your changes. The only questionable bit is the increase in size of the respective modules by a few KB.

Thanks for your work..

Dave

Jargon file have an entry about class of bugs which are latent (do not bite anybody) until discovered. When discovered, relevant pieces of code promptly stop working.

I thought that was a joke ;)
--
vda