[patch 2/2] agpgart - Remove unnecessary flushes.

[patch 2/2] agpgart - Remove unnecessary flushes.

[Thomas Hellström]

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

This patch is to speed up flipping of pages in and out of the AGP 
aperture as needed by the new drm memory manager.

A number of global cache flushes are removed as well as some PCI posting 
flushes.
The following guidelines have been used:

1) Memory that is only mapped uncached and that has been subject to a 
global cache flush after the mapping was changed to uncached does not 
need any more cache flushes. Neither before binding to the aperture nor 
after unbinding.

2) Only do one PCI posting flush after a sequence of writes modifying 
page entries in the GATT.

Patch against davej's agpgart.git

/Thomas Hellström





[-- Attachment #2: patch2.diff --]
[-- Type: text/x-patch, Size: 4656 bytes --]

diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 7a6107f..75557de 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -1076,8 +1076,8 @@ int agp_generic_insert_memory(struct agp
 
 	for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
 		writel(bridge->driver->mask_memory(bridge, mem->memory[i], mask_type), bridge->gatt_table+j);
-		readl(bridge->gatt_table+j);	/* PCI Posting. */
 	}
+	readl(bridge->gatt_table+j-1);	/* PCI Posting. */
 
 	bridge->driver->tlb_flush(mem);
 	return 0;
@@ -1111,10 +1111,9 @@ int agp_generic_remove_memory(struct agp
 	/* AK: bogus, should encode addresses > 4GB */
 	for (i = pg_start; i < (mem->page_count + pg_start); i++) {
 		writel(bridge->scratch_page, bridge->gatt_table+i);
-		readl(bridge->gatt_table+i);	/* PCI Posting. */
 	}
+	readl(bridge->gatt_table+i-1);	/* PCI Posting. */
 
-	global_cache_flush();
 	bridge->driver->tlb_flush(mem);
 	return 0;
 }
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 677581a..aaf9b30 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -256,9 +256,8 @@ static int intel_i810_insert_entries(str
 		for (i = pg_start; i < (pg_start + mem->page_count); i++) {
 			writel((i*4096)|I810_PTE_LOCAL|I810_PTE_VALID, 
 			       intel_i810_private.registers+I810_PTE_BASE+(i*4));
-			readl(intel_i810_private.registers+I810_PTE_BASE+(i*4));
-			
 		}
+		readl(intel_i810_private.registers+I810_PTE_BASE+((i-1)*4));
 		break;
 	case AGP_PHYS_MEMORY:
 		if (!mem->is_flushed) 
@@ -268,13 +267,13 @@ static int intel_i810_insert_entries(str
 							       mem->memory[i],
 							       mask_type),
 			       intel_i810_private.registers+I810_PTE_BASE+(j*4));
-			readl(intel_i810_private.registers+I810_PTE_BASE+(j*4));
 		}
+		readl(intel_i810_private.registers+I810_PTE_BASE+((j-1)*4));
 		break;
 	default:
 		goto out_err;
 	}
-	global_cache_flush();
+	
 	agp_bridge->driver->tlb_flush(mem);
 out:
 	ret = 0;
@@ -293,10 +292,9 @@ static int intel_i810_remove_entries(str
 
 	for (i = pg_start; i < (mem->page_count + pg_start); i++) {
 		writel(agp_bridge->scratch_page, intel_i810_private.registers+I810_PTE_BASE+(i*4));
-		readl(intel_i810_private.registers+I810_PTE_BASE+(i*4))	;
 	}
-	
-	global_cache_flush();
+	readl(intel_i810_private.registers+I810_PTE_BASE+((i-1)*4));	
+
 	agp_bridge->driver->tlb_flush(mem);
 	return 0;
 }
@@ -646,7 +644,7 @@ static int intel_i830_insert_entries(str
 	if (mask_type != 0 && mask_type != AGP_PHYS_MEMORY && 
 	    mask_type != INTEL_AGP_CACHED_MEMORY)
 		goto out_err;
-
+	
 	if (!mem->is_flushed)
 		global_cache_flush();
 
@@ -654,9 +652,8 @@ static int intel_i830_insert_entries(str
 		writel(agp_bridge->driver->mask_memory(agp_bridge,
 						       mem->memory[i], mask_type),
 		       intel_i830_private.registers+I810_PTE_BASE+(j*4));
-		readl(intel_i830_private.registers+I810_PTE_BASE+(j*4));
 	}
-	global_cache_flush();
+	readl(intel_i830_private.registers+I810_PTE_BASE+((j-1)*4));
 	agp_bridge->driver->tlb_flush(mem);
 
 out:
@@ -671,8 +668,6 @@ static int intel_i830_remove_entries(str
 {
 	int i;
 
-	global_cache_flush();
-
 	if (pg_start < intel_i830_private.gtt_entries) {
 		printk (KERN_INFO PFX "Trying to disable local/stolen memory\n");
 		return -EINVAL;
@@ -680,10 +675,9 @@ static int intel_i830_remove_entries(str
 
 	for (i = pg_start; i < (mem->page_count + pg_start); i++) {
 		writel(agp_bridge->scratch_page, intel_i830_private.registers+I810_PTE_BASE+(i*4));
-		readl(intel_i830_private.registers+I810_PTE_BASE+(i*4));
 	}
-	
-	global_cache_flush();
+	readl(intel_i830_private.registers+I810_PTE_BASE+((i-1)*4));
+
 	agp_bridge->driver->tlb_flush(mem);
 	return 0;
 }
@@ -777,10 +771,9 @@ static int intel_i915_insert_entries(str
 	for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
 		writel(agp_bridge->driver->mask_memory(agp_bridge,
 			mem->memory[i], mask_type), intel_i830_private.gtt+j);
-		readl(intel_i830_private.gtt+j);
 	}
 	
-	global_cache_flush();
+	readl(intel_i830_private.gtt+j-1);
 	agp_bridge->driver->tlb_flush(mem);
 
  out:
@@ -795,19 +788,16 @@ static int intel_i915_remove_entries(str
 {
 	int i;
 
-	global_cache_flush();
-
 	if (pg_start < intel_i830_private.gtt_entries) {
 		printk (KERN_INFO PFX "Trying to disable local/stolen memory\n");
 		return -EINVAL;
 	}
-
+	
 	for (i = pg_start; i < (mem->page_count + pg_start); i++) {
 		writel(agp_bridge->scratch_page, intel_i830_private.gtt+i);
-		readl(intel_i830_private.gtt+i);
 	}
-
-	global_cache_flush();
+	readl(intel_i830_private.gtt+i-1);
+	
 	agp_bridge->driver->tlb_flush(mem);
 	return 0;
 }

Re: [patch 2/2] agpgart - Remove unnecessary flushes.

[Dave Jones]

On Fri, Dec 08, 2006 at 07:24:37PM +0100, Thomas Hellström wrote:
 > This patch is to speed up flipping of pages in and out of the AGP 
 > aperture as needed by the new drm memory manager.
 > 
 > A number of global cache flushes are removed as well as some PCI posting 
 > flushes.
 > The following guidelines have been used:
 > 
 > 1) Memory that is only mapped uncached and that has been subject to a 
 > global cache flush after the mapping was changed to uncached does not 
 > need any more cache flushes. Neither before binding to the aperture nor 
 > after unbinding.
 > 
 > 2) Only do one PCI posting flush after a sequence of writes modifying 
 > page entries in the GATT.
 > 
 > Patch against davej's agpgart.git

I looked at applying this one to agpgart.git, as it's less controversial
than the other patch. However,..

- MIME : just say no. I had to hand fix up a few things before git would
  even see that I was feeding it a diff.
- No Signed-off-by: line.
- The diff adds trailing whitespace. This makes git sad also.
  (This problem also affects the other diff, which is possibly why...)
- Finally..
   error: patch failed: drivers/char/agp/generic.c:1076
   error: drivers/char/agp/generic.c: patch does not apply
   error: patch failed: drivers/char/agp/intel-agp.c:256
   error: drivers/char/agp/intel-agp.c: patch does not apply

  Perhaps this diff should have have been [1/2] instead.

Can you fix those up, and resend this one?

The other diff I want to chew over some more before applying, especially
after Arjan's comments.

		Dave

-- 
http://www.codemonkey.org.uk