[Intel-gfx] [PATCH v2] agp/intel: Serialise after GTT updates

[Chris Wilson]

An interesting bug occurs on Pineview through which the root cause is
that the writes of the PTE values into the GTT is not serialised with
subsequent memory access through the GTT (when using WC updates of the
PTE values). This is despite there being a posting read after the GTT
update. However, by changing the address of the posting read, the memory
access is indeed serialised correctly.

Whilst we are manipulating the memory barriers, we can remove the
compiler :memory restraint on the intermediate PTE writes knowing that
we explicitly perform a posting read afterwards.

v2: Replace posting reads with explicit write memory barriers - in
particular this is advantages in case of single page objects. Update
comments to mention this issue is only with WC writes.

Testcase: igt/gem_exec_big #pnv
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88191
Tested-by: huax.lu at intel.com (v1)
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/char/agp/intel-gtt.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 92aa43fa8d70..0b4188b9af7c 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -225,7 +225,7 @@ static int i810_insert_dcache_entries(struct agp_memory *mem, off_t pg_start,
 		intel_private.driver->write_entry(addr,
 						  i, type);
 	}
-	readl(intel_private.gtt+i-1);
+	wmb();
 
 	return 0;
 }
@@ -329,7 +329,7 @@ static void i810_write_entry(dma_addr_t addr, unsigned int entry,
 		break;
 	}
 
-	writel(addr | pte_flags, intel_private.gtt + entry);
+	writel_relaxed(addr | pte_flags, intel_private.gtt + entry);
 }
 
 static const struct aper_size_info_fixed intel_fake_agp_sizes[] = {
@@ -735,7 +735,7 @@ static void i830_write_entry(dma_addr_t addr, unsigned int entry,
 	if (flags ==  AGP_USER_CACHED_MEMORY)
 		pte_flags |= I830_PTE_SYSTEM_CACHED;
 
-	writel(addr | pte_flags, intel_private.gtt + entry);
+	writel_relaxed(addr | pte_flags, intel_private.gtt + entry);
 }
 
 bool intel_enable_gtt(void)
@@ -858,7 +858,7 @@ void intel_gtt_insert_sg_entries(struct sg_table *st,
 			j++;
 		}
 	}
-	readl(intel_private.gtt+j-1);
+	wmb();
 }
 EXPORT_SYMBOL(intel_gtt_insert_sg_entries);
 
@@ -875,7 +875,7 @@ static void intel_gtt_insert_pages(unsigned int first_entry,
 		intel_private.driver->write_entry(addr,
 						  j, flags);
 	}
-	readl(intel_private.gtt+j-1);
+	wmb();
 }
 
 static int intel_fake_agp_insert_entries(struct agp_memory *mem,
@@ -938,7 +938,7 @@ void intel_gtt_clear_range(unsigned int first_entry, unsigned int num_entries)
 		intel_private.driver->write_entry(intel_private.scratch_page_dma,
 						  i, 0);
 	}
-	readl(intel_private.gtt+i-1);
+	wmb();
 }
 EXPORT_SYMBOL(intel_gtt_clear_range);
 
@@ -1106,7 +1106,7 @@ static void i965_write_entry(dma_addr_t addr,
 
 	/* Shift high bits down */
 	addr |= (addr >> 28) & 0xf0;
-	writel(addr | pte_flags, intel_private.gtt + entry);
+	writel_relaxed(addr | pte_flags, intel_private.gtt + entry);
 }
 
 static int i9xx_setup(void)
-- 
2.1.4