Re: PROBLEM: memory corruption with i815 chipset variant

Re: PROBLEM: memory corruption with i815 chipset variant

[Nicolas Aspert]

Alan Cox wrote:

> 
> That means its actually using the same GART code as the 440BX and friends
> if I remember rightly (the i815 special stuff is for on board video)


Alessandro reported the problem to me also. I went through the i815 
specs and found 2 'strange' things (maybe they are not but...)
1) No 'ERRSTS' register (well... a bus that does no error should be a 
feature ;-)
2) The ATTBASE register to which the *_configure functions write is 
different from other Intel chipsets. In the i815, the ATT base adress 
should be written between bits 12 and 28, whereas in all other Intel 
chipsets, it should be written between bits 12 and 31 (don't ask me why 
Intel feels like changing the adresses/specs for registers at each new 
chipsets....) .
Alan, do you think this could cause all those troubles ?

> 
>>Without agpgart module, kernel seems stable.  A naive (totally naive,
>>I admit it) interpretation suggests a problem in setting the AGP aperture.
> 
> 
> Does the ram survive memtest86 overnight with no errors logged if you boot
> memtest86 and just leave it ?

 From what Alessandro reported, it seems clear that the 'insmod agpgart' 
triggers the mayhem, including memtest failures.

Best regards

Nicolas.
-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)

Re: PROBLEM: memory corruption with i815 chipset variant

[Nicolas Aspert]

Alan Cox wrote:

> 
> It certainly could be. If bits 29-31 maybe control things like memory
> timings then it could do quite horrible things. Fixing it to leave the
> ERRSTS register alone and keep bits 29-31 is definitely worth trying. If
> that fixes it then its going to be easy enough to drop a fix into the
> mainstream code
> 

OK, I have a patch almost ready to do that except, I am not sure about 
what to do for those 3 bits...

The *usual* call is :
	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
			       agp_bridge.gatt_bus_addr);

Where 'gatt_bus_addr' is returned from a 'virt_to_phys' on 
'gatt_table_real'.

Should I mask those three bits out when writing or write
'gatt_bus_addr >> 3' instead ? I am not too sure about the assumptions 
that can be made about what returns 'virt_to_phys' ...

Thanks in advance.

Nicolas.
-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)

[PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

[Nicolas Aspert]

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

Hi all

Thanks Alan's advice, here is a patch that aims at fixing the mem. 
corruption problems encountered by some users, when trying to use 
agpgart with intel 815 chipsets.
The patch is against 2.4.19-pre8-ac5

Alessandro, could you please test whether this patch improves things or 
not ?

Best regards
-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)


[-- Attachment #2: patch-intel_815-2.4.19-pre8-ac5 --]
[-- Type: text/plain, Size: 3768 bytes --]

Only in agp: Makefile
diff -u agp.orig/agp.h agp/agp.h
--- agp.orig/agp.h	Fri May 24 15:08:37 2002
+++ agp/agp.h	Mon May 27 11:26:55 2002
@@ -293,6 +293,10 @@
 /* This one is for I830MP w. an external graphic card */
 #define INTEL_I830_ERRSTS          0x92
 
+/* intel 815 register */
+#define INTEL_815_APCONT        0x51
+#define INTEL_815_ATTBASE_MASK  0x1FFFFFFF
+
 /* intel i820 registers */
 #define INTEL_I820_RDCR     0x51
 #define INTEL_I820_ERRSTS   0xc8
diff -u agp.orig/agpgart_be.c agp/agpgart_be.c
--- agp.orig/agpgart_be.c	Fri May 24 15:08:44 2002
+++ agp/agpgart_be.c	Mon May 27 11:28:07 2002
@@ -1490,6 +1490,42 @@
 	return 0;
 }
 
+static int intel_815_configure(void)
+{
+	u32 temp, addr;
+	u8 temp2;
+	aper_size_info_8 *current_size;
+
+	current_size = A_SIZE_8(agp_bridge.current_size);
+
+	/* aperture size */
+	pci_write_config_byte(agp_bridge.dev, INTEL_APSIZE,
+			      current_size->size_value); 
+
+	/* address to map to */
+	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
+	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
+
+	/* attbase - aperture base */
+        /* the Intel 815 chipset spec. says that bits 29-31 in the
+         * ATTBASE register are reserved -> try not to write them */
+        if (agp_bridge.gatt_bus_addr & (~ INTEL_815_ATTBASE_MASK))
+		panic("gatt bus addr too high");
+	addr = agp_bridge.gatt_bus_addr & INTEL_815_ATTBASE_MASK;
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);
+
+	/* agpctrl */
+	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
+
+	/* apcont */
+	pci_read_config_byte(agp_bridge.dev, INTEL_I815_APCONT, &temp2);
+	pci_write_config_byte(agp_bridge.dev, INTEL_I815_APCONT,
+			      temp2 | (1 << 1));
+	/* clear any possible error conditions */
+        /* Oddness : this chipset seems to have no ERRSTS register ! */
+	return 0;
+}
+
 static void intel_820_tlbflush(agp_memory * mem)
 {
   return;
@@ -1724,6 +1760,12 @@
 	{0x00000017, 0}
 };
 
+static aper_size_info_8 intel_815_sizes[2] =
+{
+	{64, 16384, 4, 0},
+	{32, 8192, 3, 8},
+};
+
 static aper_size_info_8 intel_8xx_sizes[7] =
 {
 	{256, 65536, 6, 0},
@@ -1787,7 +1829,38 @@
 	(void) pdev; /* unused */
 }
 
+static int __init intel_815_setup (struct pci_dev *pdev)
+{
+	agp_bridge.masks = intel_generic_masks;
+	agp_bridge.num_of_masks = 1;
+	agp_bridge.aperture_sizes = (void *) intel_815_sizes;
+	agp_bridge.size_type = U8_APER_SIZE;
+	agp_bridge.num_aperture_sizes = 2;
+	agp_bridge.dev_private_data = NULL;
+	agp_bridge.needs_scratch_page = FALSE;
+	agp_bridge.configure = intel_815_configure;
+	agp_bridge.fetch_size = intel_8xx_fetch_size;
+	agp_bridge.cleanup = intel_8xx_cleanup;
+	agp_bridge.tlb_flush = intel_8xx_tlbflush;
+	agp_bridge.mask_memory = intel_mask_memory;
+	agp_bridge.agp_enable = agp_generic_agp_enable;
+	agp_bridge.cache_flush = global_cache_flush;
+	agp_bridge.create_gatt_table = agp_generic_create_gatt_table;
+	agp_bridge.free_gatt_table = agp_generic_free_gatt_table;
+	agp_bridge.insert_memory = agp_generic_insert_memory;
+	agp_bridge.remove_memory = agp_generic_remove_memory;
+	agp_bridge.alloc_by_type = agp_generic_alloc_by_type;
+	agp_bridge.free_by_type = agp_generic_free_by_type;
+	agp_bridge.agp_alloc_page = agp_generic_alloc_page;
+	agp_bridge.agp_destroy_page = agp_generic_destroy_page;
+	agp_bridge.suspend = agp_generic_suspend;
+	agp_bridge.resume = agp_generic_resume;
+	agp_bridge.cant_use_aperture = 0;
 
+	return 0;
+	
+	(void) pdev; /* unused */
+}
 
 static int __init intel_820_setup (struct pci_dev *pdev)
 {
@@ -3929,7 +4002,7 @@
 		INTEL_I815,
 		"Intel",
 		"i815",
-		intel_generic_setup },
+		intel_815_setup },
 	{ PCI_DEVICE_ID_INTEL_820_0,
 		PCI_VENDOR_ID_INTEL,
 		INTEL_I820,
Only in agp: agpgart_fe.c

Re: PROBLEM: memory corruption with i815 chipset variant

[Alan Cox]

On Mon, 2002-05-27 at 09:11, Nicolas Aspert wrote:
> Alessandro reported the problem to me also. I went through the i815 
> specs and found 2 'strange' things (maybe they are not but...)
> 1) No 'ERRSTS' register (well... a bus that does no error should be a 
> feature ;-)
> 2) The ATTBASE register to which the *_configure functions write is 
> different from other Intel chipsets. In the i815, the ATT base adress 
> should be written between bits 12 and 28, whereas in all other Intel 
> chipsets, it should be written between bits 12 and 31 (don't ask me why 
> Intel feels like changing the adresses/specs for registers at each new 
> chipsets....) .
> Alan, do you think this could cause all those troubles ?

It certainly could be. If bits 29-31 maybe control things like memory
timings then it could do quite horrible things. Fixing it to leave the
ERRSTS register alone and keep bits 29-31 is definitely worth trying. If
that fixes it then its going to be easy enough to drop a fix into the
mainstream code

Alan

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

[Nicolas Aspert]

Alan Cox wrote:
> On Mon, 2002-05-27 at 10:32, Nicolas Aspert wrote:
> p to */
> 
>>+	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
>>+	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
>>+
>>+	/* attbase - aperture base */
>>+        /* the Intel 815 chipset spec. says that bits 29-31 in the
>>+         * ATTBASE register are reserved -> try not to write them */
>>+        if (agp_bridge.gatt_bus_addr & (~ INTEL_815_ATTBASE_MASK))
>>+		panic("gatt bus addr too high");
>>+	addr = agp_bridge.gatt_bus_addr & INTEL_815_ATTBASE_MASK;
> 
> 
> You need to add  + temp&~INTEL_815_ATTBASE_MASK ..
> 
> 

I am not sure to understand... Do you really mean mixing 'APBASE' which 
is the AGP base aperture adress along with the *gatt* which is the 
translation table adress ? If yes, I think I need a supplementary 
explanation...

Best regards.
-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)

Re: PROBLEM: memory corruption with i815 chipset variant

[Alan Cox]

On Mon, 2002-05-27 at 09:56, Nicolas Aspert wrote:
> OK, I have a patch almost ready to do that except, I am not sure about 
> what to do for those 3 bits...
> 
> The *usual* call is :
> 	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
> 			       agp_bridge.gatt_bus_addr);
> 
> Where 'gatt_bus_addr' is returned from a 'virt_to_phys' on 
> 'gatt_table_real'.
> 
> Should I mask those three bits out when writing or write
> 'gatt_bus_addr >> 3' instead ? I am not too sure about the assumptions 
> that can be made about what returns 'virt_to_phys' ...

virt_to_phys returns you the CPU physical (after the MMU) address of the
memory in question. You'd want to check the documentation but assuming
the base is still written as an address in bytes then you'd want to do 
something like

	if(agp_bridge.gatt_bus_addr & ~0x1FFFFFFF)
		panic("gatt bus addr too high");
	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);
	addr&=~0x1FFFFFFF;
	addr|=agp_bridge.gatt_bus_addr;
	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

[Alan Cox]

On Mon, 2002-05-27 at 10:32, Nicolas Aspert wrote:
p to */
> +	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
> +	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
> +
> +	/* attbase - aperture base */
> +        /* the Intel 815 chipset spec. says that bits 29-31 in the
> +         * ATTBASE register are reserved -> try not to write them */
> +        if (agp_bridge.gatt_bus_addr & (~ INTEL_815_ATTBASE_MASK))
> +		panic("gatt bus addr too high");
> +	addr = agp_bridge.gatt_bus_addr & INTEL_815_ATTBASE_MASK;

You need to add  + temp&~INTEL_815_ATTBASE_MASK ..

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

[Nicolas Aspert]

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

Alan Cox wrote:

> Your code isnt reading the top bits from the register and anding them
> back into it with the address.
> 
> 

OK I got it now... I was confused by the 'temp' thing but I should have 
read more carefully your initial suggestion (doing 2 things at the same 
time seems beyond my possibilities today ;-)
Anyway, here is take 2 of the patch, hopefully correct this time...

Just another quick thought... in all intel chipsets datasheets, the bits 
0-11 of the ATTBASE register are also marked as 'reserved'. So far, all 
the intel_*_configure routines are writing shamelessly on these bits. 
Shouldn't we mask those bits out too (though it seems this has not 
caused any trouble so far) ?

Best regards

-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)

[-- Attachment #2: patch-intel_815-2.4.19-pre8-ac5 --]
[-- Type: text/plain, Size: 3791 bytes --]

diff -u agp.orig/agp.h agp/agp.h
--- agp.orig/agp.h	Fri May 24 15:08:37 2002
+++ agp/agp.h	Mon May 27 13:00:27 2002
@@ -293,6 +293,10 @@
 /* This one is for I830MP w. an external graphic card */
 #define INTEL_I830_ERRSTS          0x92
 
+/* intel 815 register */
+#define INTEL_815_APCONT        0x51
+#define INTEL_815_ATTBASE_MASK  ~0x1FFFFFFF
+
 /* intel i820 registers */
 #define INTEL_I820_RDCR     0x51
 #define INTEL_I820_ERRSTS   0xc8
diff -u agp.orig/agpgart_be.c agp/agpgart_be.c
--- agp.orig/agpgart_be.c	Fri May 24 15:08:44 2002
+++ agp/agpgart_be.c	Mon May 27 13:00:10 2002
@@ -1490,6 +1490,44 @@
 	return 0;
 }
 
+static int intel_815_configure(void)
+{
+	u32 temp, addr;
+	u8 temp2;
+	aper_size_info_8 *current_size;
+
+	current_size = A_SIZE_8(agp_bridge.current_size);
+
+	/* aperture size */
+	pci_write_config_byte(agp_bridge.dev, INTEL_APSIZE,
+			      current_size->size_value); 
+
+	/* address to map to */
+	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
+	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
+
+	/* attbase - aperture base */
+        /* the Intel 815 chipset spec. says that bits 29-31 in the
+         * ATTBASE register are reserved -> try not to write them */
+        if (agp_bridge.gatt_bus_addr &  INTEL_815_ATTBASE_MASK)
+		panic("gatt bus addr too high");
+	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);
+	addr &= INTEL_815_ATTBASE_MASK;
+	addr |= agp_bridge.gatt_bus_addr;
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);
+
+	/* agpctrl */
+	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
+
+	/* apcont */
+	pci_read_config_byte(agp_bridge.dev, INTEL_I815_APCONT, &temp2);
+	pci_write_config_byte(agp_bridge.dev, INTEL_I815_APCONT,
+			      temp2 | (1 << 1));
+	/* clear any possible error conditions */
+        /* Oddness : this chipset seems to have no ERRSTS register ! */
+	return 0;
+}
+
 static void intel_820_tlbflush(agp_memory * mem)
 {
   return;
@@ -1724,6 +1762,12 @@
 	{0x00000017, 0}
 };
 
+static aper_size_info_8 intel_815_sizes[2] =
+{
+	{64, 16384, 4, 0},
+	{32, 8192, 3, 8},
+};
+
 static aper_size_info_8 intel_8xx_sizes[7] =
 {
 	{256, 65536, 6, 0},
@@ -1787,7 +1831,38 @@
 	(void) pdev; /* unused */
 }
 
+static int __init intel_815_setup (struct pci_dev *pdev)
+{
+	agp_bridge.masks = intel_generic_masks;
+	agp_bridge.num_of_masks = 1;
+	agp_bridge.aperture_sizes = (void *) intel_815_sizes;
+	agp_bridge.size_type = U8_APER_SIZE;
+	agp_bridge.num_aperture_sizes = 2;
+	agp_bridge.dev_private_data = NULL;
+	agp_bridge.needs_scratch_page = FALSE;
+	agp_bridge.configure = intel_815_configure;
+	agp_bridge.fetch_size = intel_8xx_fetch_size;
+	agp_bridge.cleanup = intel_8xx_cleanup;
+	agp_bridge.tlb_flush = intel_8xx_tlbflush;
+	agp_bridge.mask_memory = intel_mask_memory;
+	agp_bridge.agp_enable = agp_generic_agp_enable;
+	agp_bridge.cache_flush = global_cache_flush;
+	agp_bridge.create_gatt_table = agp_generic_create_gatt_table;
+	agp_bridge.free_gatt_table = agp_generic_free_gatt_table;
+	agp_bridge.insert_memory = agp_generic_insert_memory;
+	agp_bridge.remove_memory = agp_generic_remove_memory;
+	agp_bridge.alloc_by_type = agp_generic_alloc_by_type;
+	agp_bridge.free_by_type = agp_generic_free_by_type;
+	agp_bridge.agp_alloc_page = agp_generic_alloc_page;
+	agp_bridge.agp_destroy_page = agp_generic_destroy_page;
+	agp_bridge.suspend = agp_generic_suspend;
+	agp_bridge.resume = agp_generic_resume;
+	agp_bridge.cant_use_aperture = 0;
 
+	return 0;
+	
+	(void) pdev; /* unused */
+}
 
 static int __init intel_820_setup (struct pci_dev *pdev)
 {
@@ -3929,7 +4004,7 @@
 		INTEL_I815,
 		"Intel",
 		"i815",
-		intel_generic_setup },
+		intel_815_setup },
 	{ PCI_DEVICE_ID_INTEL_820_0,
 		PCI_VENDOR_ID_INTEL,
 		INTEL_I820,

[PATCH,CFT] Tentative fix for agpgart (writing on 'reserved' bits)

[Nicolas Aspert]

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

Alan Cox wrote:
> 
> We ought to mask and copy the original yes. The number of times we've
> had Linux driver breakages by not masking and avoiding writes to
> reserved bits is small but it does happen occasionally.

That sounds reasonable. I attached a patch that should do this. 
According to what was done before (i.e. writing on the reserved bits), 
the 12 LSB of ATTBASE should be discarded...
The patch is against 2.4.19-pre8-ac5

Please test

Best regards

Nicolas.
-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)

[-- Attachment #2: patch-agp_fix_attbase-2.4.19-pre8-ac5 --]
[-- Type: text/plain, Size: 8592 bytes --]

diff -u agp.orig/agp.h agp/agp.h
--- agp.orig/agp.h	Fri May 24 15:08:37 2002
+++ agp/agp.h	Mon May 27 16:06:09 2002
@@ -274,6 +274,9 @@
 #define INTEL_NBXCFG    0x50
 #define INTEL_ERRSTS    0x91
 
+/* mask to preserve the 12 LSB of the ATTBASE register */
+#define INTEL_ATTBASE_MASK 0x00000FFF
+
 /* intel i830 registers */
 #define I830_GMCH_CTRL             0x52
 #define I830_GMCH_ENABLED          0x4
@@ -293,6 +296,10 @@
 /* This one is for I830MP w. an external graphic card */
 #define INTEL_I830_ERRSTS          0x92
 
+/* intel 815 register */
+#define INTEL_815_APCONT        0x51
+#define INTEL_815_ATTBASE_MASK  0xE0000FFF
+
 /* intel i820 registers */
 #define INTEL_I820_RDCR     0x51
 #define INTEL_I820_ERRSTS   0xc8
diff -u agp.orig/agpgart_be.c agp/agpgart_be.c
--- agp.orig/agpgart_be.c	Fri May 24 15:08:44 2002
+++ agp/agpgart_be.c	Mon May 27 16:04:05 2002
@@ -1475,8 +1475,10 @@
 	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
 	/* attbase - aperture base */
-	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-			       agp_bridge.gatt_bus_addr);
+	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp);
+        temp &= INTEL_ATTBASE_MASK;
+        temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
 	/* agpctrl */
 	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x2280);
@@ -1490,6 +1492,46 @@
 	return 0;
 }
 
+static int intel_815_configure(void)
+{
+	u32 temp;
+	u8 temp2;
+	aper_size_info_8 *current_size;
+
+	current_size = A_SIZE_8(agp_bridge.current_size);
+
+	/* aperture size */
+	pci_write_config_byte(agp_bridge.dev, INTEL_APSIZE,
+			      current_size->size_value); 
+
+	/* address to map to */
+	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
+	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
+
+	/* attbase - aperture base */
+        /* the Intel 815 chipset spec. says that bits 29-31 in the
+         * ATTBASE register are reserved -> try not to write them */
+        if ((agp_bridge.gatt_bus_addr & INTEL_815_ATTBASE_MASK) & 
+            ~ INTEL_ATTBASE_MASK)
+          panic("gatt bus addr too high");
+	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp);
+	temp &= INTEL_815_ATTBASE_MASK;
+	temp |= agp_bridge.gatt_bus_addr & (~ INTEL_815_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
+
+	/* agpctrl */
+	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
+
+	/* apcont */
+	pci_read_config_byte(agp_bridge.dev, INTEL_I815_APCONT, &temp2);
+	pci_write_config_byte(agp_bridge.dev, INTEL_I815_APCONT,
+			      temp2 | (1 << 1));
+
+	/* clear any possible error conditions */
+        /* Oddness : this chipset seems to have no ERRSTS register ! */
+	return 0;
+}
+
 static void intel_820_tlbflush(agp_memory * mem)
 {
   return;
@@ -1526,8 +1568,10 @@
 	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
 	/* attbase - aperture base */
-	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-			       agp_bridge.gatt_bus_addr); 
+	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp); 
+        temp &= INTEL_ATTBASE_MASK;
+        temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
 	/* agpctrl */
 	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
@@ -1560,8 +1604,10 @@
        agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
        /* attbase - aperture base */
-       pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-                              agp_bridge.gatt_bus_addr);
+       pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp);
+       temp &= INTEL_ATTBASE_MASK;
+       temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+       pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
        /* agpctrl */
        pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000);
@@ -1570,6 +1616,7 @@
        pci_read_config_word(agp_bridge.dev, INTEL_NBXCFG, &temp2);
        pci_write_config_word(agp_bridge.dev, INTEL_NBXCFG,
                              temp2 | (1 << 9));
+
        /* clear any possible AGP-related error conditions */
        pci_write_config_word(agp_bridge.dev, INTEL_I830_ERRSTS, 0x1c);
        return 0;
@@ -1594,8 +1641,10 @@
 	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
 	/* attbase - aperture base */
-	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-			       agp_bridge.gatt_bus_addr); 
+	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp); 
+        temp &= INTEL_ATTBASE_MASK;
+        temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
 	/* agpctrl */
 	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
@@ -1604,6 +1653,7 @@
 	pci_read_config_word(agp_bridge.dev, INTEL_I840_MCHCFG, &temp2);
 	pci_write_config_word(agp_bridge.dev, INTEL_I840_MCHCFG,
 			      temp2 | (1 << 9));
+
 	/* clear any possible error conditions */
 	pci_write_config_word(agp_bridge.dev, INTEL_I840_ERRSTS, 0xc000); 
 	return 0;
@@ -1626,8 +1676,10 @@
 	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
 	/* attbase - aperture base */
-	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-			       agp_bridge.gatt_bus_addr); 
+	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp); 
+        temp &= INTEL_ATTBASE_MASK;
+        temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
 	/* agpctrl */
 	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
@@ -1658,8 +1710,10 @@
 	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
 	/* attbase - aperture base */
-	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-			       agp_bridge.gatt_bus_addr); 
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp); 
+        temp &= INTEL_ATTBASE_MASK;
+        temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
 	/* agpctrl */
 	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000); 
@@ -1690,8 +1744,10 @@
 	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
 
 	/* attbase - aperture base */
-	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
-			       agp_bridge.gatt_bus_addr);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, &temp);
+        temp &= INTEL_ATTBASE_MASK;
+        temp |= agp_bridge.gatt_bus_addr & (~ INTEL_ATTBASE_MASK);
+	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, temp);
 
 	/* agpctrl */
 	pci_write_config_dword(agp_bridge.dev, INTEL_AGPCTRL, 0x0000);
@@ -1724,6 +1780,12 @@
 	{0x00000017, 0}
 };
 
+static aper_size_info_8 intel_815_sizes[2] =
+{
+	{64, 16384, 4, 0},
+	{32, 8192, 3, 8},
+};
+
 static aper_size_info_8 intel_8xx_sizes[7] =
 {
 	{256, 65536, 6, 0},
@@ -1787,7 +1849,38 @@
 	(void) pdev; /* unused */
 }
 
+static int __init intel_815_setup (struct pci_dev *pdev)
+{
+	agp_bridge.masks = intel_generic_masks;
+	agp_bridge.num_of_masks = 1;
+	agp_bridge.aperture_sizes = (void *) intel_815_sizes;
+	agp_bridge.size_type = U8_APER_SIZE;
+	agp_bridge.num_aperture_sizes = 2;
+	agp_bridge.dev_private_data = NULL;
+	agp_bridge.needs_scratch_page = FALSE;
+	agp_bridge.configure = intel_815_configure;
+	agp_bridge.fetch_size = intel_8xx_fetch_size;
+	agp_bridge.cleanup = intel_8xx_cleanup;
+	agp_bridge.tlb_flush = intel_8xx_tlbflush;
+	agp_bridge.mask_memory = intel_mask_memory;
+	agp_bridge.agp_enable = agp_generic_agp_enable;
+	agp_bridge.cache_flush = global_cache_flush;
+	agp_bridge.create_gatt_table = agp_generic_create_gatt_table;
+	agp_bridge.free_gatt_table = agp_generic_free_gatt_table;
+	agp_bridge.insert_memory = agp_generic_insert_memory;
+	agp_bridge.remove_memory = agp_generic_remove_memory;
+	agp_bridge.alloc_by_type = agp_generic_alloc_by_type;
+	agp_bridge.free_by_type = agp_generic_free_by_type;
+	agp_bridge.agp_alloc_page = agp_generic_alloc_page;
+	agp_bridge.agp_destroy_page = agp_generic_destroy_page;
+	agp_bridge.suspend = agp_generic_suspend;
+	agp_bridge.resume = agp_generic_resume;
+	agp_bridge.cant_use_aperture = 0;
 
+	return 0;
+	
+	(void) pdev; /* unused */
+}
 
 static int __init intel_820_setup (struct pci_dev *pdev)
 {
@@ -3929,7 +4022,7 @@
 		INTEL_I815,
 		"Intel",
 		"i815",
-		intel_generic_setup },
+		intel_815_setup },
 	{ PCI_DEVICE_ID_INTEL_820_0,
 		PCI_VENDOR_ID_INTEL,
 		INTEL_I820,

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

[Alan Cox]

On Mon, 2002-05-27 at 12:11, Nicolas Aspert wrote:
> Just another quick thought... in all intel chipsets datasheets, the bits 
> 0-11 of the ATTBASE register are also marked as 'reserved'. So far, all 
> the intel_*_configure routines are writing shamelessly on these bits. 
> Shouldn't we mask those bits out too (though it seems this has not 
> caused any trouble so far) ?

We ought to mask and copy the original yes. The number of times we've
had Linux driver breakages by not masking and avoiding writes to
reserved bits is small but it does happen occasionally.

Alan

Re: [PATCH,CFT] Tentative fix for agpgart (writing on 'reserved' bits)

[Nicolas Aspert]

Alessandro Morelli wrote:

> 
> Hello
> I managed to get rid of customers, in order to test the patches....
> Bad news...no change...

At least it is not worse (I hope ...) ;-)


> I'm starting to believe I'm mad...my PC works like charm without AGP and 
> gets all mixed when I enabled it...

Does it start messing right after you enable it, or only after you write 
to it (starting X...) ?

> What kind of information can I provide (/proc readings, tests, etc.) to 
> shed some light?
 >
> Could this be a case of interaction between AGP and VM?
> 
> tlb-flush weirdness?
> 

Apart from this kinda usual write-on-reserved-bits in the AGP 
initialization, which has been done before on various chipsets and 
almost never caused any trouble, I saw no blatant errors that may lead 
to such things... In fact, we have several PCs with i815 chipsets in the 
lab. People are using AGP without any troubles. The only difference is 
that we run an old kernel version (2.4.3-13, thanks the sysadmin for 
never upgrading....), and we have a different graphic card (Matrox).

Alan, do you have any idea about this one ?

Best regards
-- 
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)