[PATCH 00/18] ppgtt cleanups / scratch merge (v3)

[PATCH 00/18] ppgtt cleanups / scratch merge (v3)

[Mika Kuoppala]

V3 of the ppgtt cleanup series. I dropped the preallocation
of pdps as Michel has patch with updating pdps through ring LRIs
in his 48bit series.

All except 3/18 and 12/18 have Reviewed-bys.

Tomas Elf had concern with 12/18 that the gen9+ hw
cmd parser will NOOP out the commands. Even if it so,
we gain timely hangs and regonizable pattern with older gens.

-Mika

Mika Kuoppala (18):
  drm/i915/gtt: Mark TLBS dirty for gen8+
  drm/i915/gtt: Check va range against vm size
  drm/i915/gtt: Allow >= 4GB sizes for vm.
  drm/i915/gtt: Introduce i915_page_dir_dma_addr
  drm/i915/gtt: Introduce struct i915_page_dma
  drm/i915/gtt: Rename unmap_and_free_px to free_px
  drm/i915/gtt: Remove superfluous free_pd with gen6/7
  drm/i915/gtt: Introduce fill_page_dma()
  drm/i915/gtt: Introduce kmap|kunmap for dma page
  drm/i915/gtt: Use macros to access dma mapped pages
  drm/i915/gtt: Make scratch page i915_page_dma compatible
  drm/i915/gtt: Fill scratch page
  drm/i915/gtt: Pin vma during virtual address allocation
  drm/i915/gtt: Cleanup page directory encoding
  drm/i915/gtt: Move scratch_pd and scratch_pt into vm area
  drm/i915/gtt: One instance of scratch page table/directory
  drm/i915/gtt: Use nonatomic bitmap ops
  drm/i915/gtt: Reorder page alloc/free/init functions

 drivers/char/agp/intel-gtt.c        |   4 +-
 drivers/gpu/drm/i915/i915_debugfs.c |  44 +--
 drivers/gpu/drm/i915/i915_gem.c     |   6 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 659 ++++++++++++++++++++----------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  59 ++--
 drivers/gpu/drm/i915/intel_lrc.c    |   4 +-
 include/drm/intel-gtt.h             |   4 +-
 7 files changed, 435 insertions(+), 345 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/18] drm/i915/gtt: Mark TLBS dirty for gen8+

[Mika Kuoppala]

When we touch gen8+ page maps, mark them dirty like we
do with previous gens.

v2: Update comment (Joonas)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2279e03..bc41063 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -831,6 +831,16 @@ err_out:
 	return -ENOMEM;
 }
 
+/* PDE TLBs are a pain to invalidate on GEN8+. When we modify
+ * the page table structures, we mark them dirty so that
+ * context switching/execlist queuing code takes extra steps
+ * to ensure that tlbs are flushed.
+ */
+static void mark_tlbs_dirty(struct i915_hw_ppgtt *ppgtt)
+{
+	ppgtt->pd_dirty_rings = INTEL_INFO(ppgtt->base.dev)->ring_mask;
+}
+
 static int gen8_alloc_va_range(struct i915_address_space *vm,
 			       uint64_t start,
 			       uint64_t length)
@@ -916,6 +926,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 	}
 
 	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
+	mark_tlbs_dirty(ppgtt);
 	return 0;
 
 err_out:
@@ -928,6 +939,7 @@ err_out:
 		unmap_and_free_pd(ppgtt->pdp.page_directory[pdpe], vm->dev);
 
 	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
+	mark_tlbs_dirty(ppgtt);
 	return ret;
 }
 
@@ -1272,16 +1284,6 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 		kunmap_atomic(pt_vaddr);
 }
 
-/* PDE TLBs are a pain invalidate pre GEN8. It requires a context reload. If we
- * are switching between contexts with the same LRCA, we also must do a force
- * restore.
- */
-static void mark_tlbs_dirty(struct i915_hw_ppgtt *ppgtt)
-{
-	/* If current vm != vm, */
-	ppgtt->pd_dirty_rings = INTEL_INFO(ppgtt->base.dev)->ring_mask;
-}
-
 static void gen6_initialize_pt(struct i915_address_space *vm,
 		struct i915_page_table *pt)
 {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/18] drm/i915/gtt: Check va range against vm size

[Mika Kuoppala]

Check the allocation area against the known end
of address space instead of against fixed value.

v2: Return ENODEV on internal bugs (Chris)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bc41063..68705e3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -757,9 +757,6 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
 
 	WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
 
-	/* FIXME: upper bound must not overflow 32 bits  */
-	WARN_ON((start + length) > (1ULL << 32));
-
 	gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
 		if (pd)
 			continue;
@@ -859,7 +856,10 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 	 * actually use the other side of the canonical address space.
 	 */
 	if (WARN_ON(start + length < start))
-		return -ERANGE;
+		return -ENODEV;
+
+	if (WARN_ON(start + length > ppgtt->base.total))
+		return -ENODEV;
 
 	ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables);
 	if (ret)
@@ -1304,7 +1304,7 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
 }
 
 static int gen6_alloc_va_range(struct i915_address_space *vm,
-			       uint64_t start, uint64_t length)
+			       uint64_t start_in, uint64_t length_in)
 {
 	DECLARE_BITMAP(new_page_tables, I915_PDES);
 	struct drm_device *dev = vm->dev;
@@ -1312,11 +1312,15 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	struct i915_hw_ppgtt *ppgtt =
 				container_of(vm, struct i915_hw_ppgtt, base);
 	struct i915_page_table *pt;
-	const uint32_t start_save = start, length_save = length;
+	uint32_t start, length, start_save, length_save;
 	uint32_t pde, temp;
 	int ret;
 
-	WARN_ON(upper_32_bits(start));
+	if (WARN_ON(start_in + length_in > ppgtt->base.total))
+		return -ENODEV;
+
+	start = start_save = start_in;
+	length = length_save = length_in;
 
 	bitmap_zero(new_page_tables, I915_PDES);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/18] drm/i915/gtt: Allow >= 4GB sizes for vm.

[Mika Kuoppala]

We can have exactly 4GB sized ppgtt with 32bit system.
size_t is inadequate for this.

v2: Convert a lot more places (Daniel)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/char/agp/intel-gtt.c        |  4 ++--
 drivers/gpu/drm/i915/i915_debugfs.c | 42 ++++++++++++++++++-------------------
 drivers/gpu/drm/i915/i915_gem.c     |  6 +++---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +++++++++----------
 drivers/gpu/drm/i915/i915_gem_gtt.h | 12 +++++------
 include/drm/intel-gtt.h             |  4 ++--
 6 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 0b4188b..4734d02 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1408,8 +1408,8 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 }
 EXPORT_SYMBOL(intel_gmch_probe);
 
-void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
-		   phys_addr_t *mappable_base, unsigned long *mappable_end)
+void intel_gtt_get(u64 *gtt_total, size_t *stolen_size,
+		   phys_addr_t *mappable_base, u64 *mappable_end)
 {
 	*gtt_total = intel_private.gtt_total_entries << PAGE_SHIFT;
 	*stolen_size = intel_private.stolen_size;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f3b8062..c654b7d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -198,7 +198,7 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_address_space *vm = &dev_priv->gtt.base;
 	struct i915_vma *vma;
-	size_t total_obj_size, total_gtt_size;
+	u64 total_obj_size, total_gtt_size;
 	int count, ret;
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -231,7 +231,7 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
 	}
 	mutex_unlock(&dev->struct_mutex);
 
-	seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
+	seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
 		   count, total_obj_size, total_gtt_size);
 	return 0;
 }
@@ -253,7 +253,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj;
-	size_t total_obj_size, total_gtt_size;
+	u64 total_obj_size, total_gtt_size;
 	LIST_HEAD(stolen);
 	int count, ret;
 
@@ -292,7 +292,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 	}
 	mutex_unlock(&dev->struct_mutex);
 
-	seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
+	seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
 		   count, total_obj_size, total_gtt_size);
 	return 0;
 }
@@ -310,10 +310,10 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 
 struct file_stats {
 	struct drm_i915_file_private *file_priv;
-	int count;
-	size_t total, unbound;
-	size_t global, shared;
-	size_t active, inactive;
+	unsigned long count;
+	u64 total, unbound;
+	u64 global, shared;
+	u64 active, inactive;
 };
 
 static int per_file_stats(int id, void *ptr, void *data)
@@ -370,7 +370,7 @@ static int per_file_stats(int id, void *ptr, void *data)
 
 #define print_file_stats(m, name, stats) do { \
 	if (stats.count) \
-		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu shared, %zu unbound)\n", \
+		seq_printf(m, "%s: %lu objects, %llu bytes (%llu active, %llu inactive, %llu global, %llu shared, %llu unbound)\n", \
 			   name, \
 			   stats.count, \
 			   stats.total, \
@@ -420,7 +420,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 count, mappable_count, purgeable_count;
-	size_t size, mappable_size, purgeable_size;
+	u64 size, mappable_size, purgeable_size;
 	struct drm_i915_gem_object *obj;
 	struct i915_address_space *vm = &dev_priv->gtt.base;
 	struct drm_file *file;
@@ -437,17 +437,17 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 
 	size = count = mappable_size = mappable_count = 0;
 	count_objects(&dev_priv->mm.bound_list, global_list);
-	seq_printf(m, "%u [%u] objects, %zu [%zu] bytes in gtt\n",
+	seq_printf(m, "%u [%u] objects, %llu [%llu] bytes in gtt\n",
 		   count, mappable_count, size, mappable_size);
 
 	size = count = mappable_size = mappable_count = 0;
 	count_vmas(&vm->active_list, mm_list);
-	seq_printf(m, "  %u [%u] active objects, %zu [%zu] bytes\n",
+	seq_printf(m, "  %u [%u] active objects, %llu [%llu] bytes\n",
 		   count, mappable_count, size, mappable_size);
 
 	size = count = mappable_size = mappable_count = 0;
 	count_vmas(&vm->inactive_list, mm_list);
-	seq_printf(m, "  %u [%u] inactive objects, %zu [%zu] bytes\n",
+	seq_printf(m, "  %u [%u] inactive objects, %llu [%llu] bytes\n",
 		   count, mappable_count, size, mappable_size);
 
 	size = count = purgeable_size = purgeable_count = 0;
@@ -456,7 +456,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 		if (obj->madv == I915_MADV_DONTNEED)
 			purgeable_size += obj->base.size, ++purgeable_count;
 	}
-	seq_printf(m, "%u unbound objects, %zu bytes\n", count, size);
+	seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
 
 	size = count = mappable_size = mappable_count = 0;
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
@@ -473,16 +473,16 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 			++purgeable_count;
 		}
 	}
-	seq_printf(m, "%u purgeable objects, %zu bytes\n",
+	seq_printf(m, "%u purgeable objects, %llu bytes\n",
 		   purgeable_count, purgeable_size);
-	seq_printf(m, "%u pinned mappable objects, %zu bytes\n",
+	seq_printf(m, "%u pinned mappable objects, %llu bytes\n",
 		   mappable_count, mappable_size);
-	seq_printf(m, "%u fault mappable objects, %zu bytes\n",
+	seq_printf(m, "%u fault mappable objects, %llu bytes\n",
 		   count, size);
 
-	seq_printf(m, "%zu [%lu] gtt total\n",
+	seq_printf(m, "%llu [%llu] gtt total\n",
 		   dev_priv->gtt.base.total,
-		   dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
+		   (u64)dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
 
 	seq_putc(m, '\n');
 	print_batch_pool_stats(m, dev_priv);
@@ -519,7 +519,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
 	uintptr_t list = (uintptr_t) node->info_ent->data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj;
-	size_t total_obj_size, total_gtt_size;
+	u64 total_obj_size, total_gtt_size;
 	int count, ret;
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -541,7 +541,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
 
 	mutex_unlock(&dev->struct_mutex);
 
-	seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
+	seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
 		   count, total_obj_size, total_gtt_size);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 52efe43..cc5bf93 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3712,9 +3712,9 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 size, fence_size, fence_alignment, unfenced_alignment;
-	unsigned long start =
+	u64 start =
 		flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
-	unsigned long end =
+	u64 end =
 		flags & PIN_MAPPABLE ? dev_priv->gtt.mappable_end : vm->total;
 	struct i915_vma *vma;
 	int ret;
@@ -3770,7 +3770,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
 	 * attempt to find space.
 	 */
 	if (size > end) {
-		DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%lu\n",
+		DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n",
 			  ggtt_view ? ggtt_view->type : 0,
 			  size,
 			  flags & PIN_MAPPABLE ? "mappable" : "total",
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 68705e3..7a7789e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2112,7 +2112,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
 void i915_gem_init_global_gtt(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned long gtt_size, mappable_size;
+	u64 gtt_size, mappable_size;
 
 	gtt_size = dev_priv->gtt.base.total;
 	mappable_size = dev_priv->gtt.mappable_end;
@@ -2369,13 +2369,13 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv)
 }
 
 static int gen8_gmch_probe(struct drm_device *dev,
-			   size_t *gtt_total,
+			   u64 *gtt_total,
 			   size_t *stolen,
 			   phys_addr_t *mappable_base,
-			   unsigned long *mappable_end)
+			   u64 *mappable_end)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned int gtt_size;
+	u64 gtt_size;
 	u16 snb_gmch_ctl;
 	int ret;
 
@@ -2417,10 +2417,10 @@ static int gen8_gmch_probe(struct drm_device *dev,
 }
 
 static int gen6_gmch_probe(struct drm_device *dev,
-			   size_t *gtt_total,
+			   u64 *gtt_total,
 			   size_t *stolen,
 			   phys_addr_t *mappable_base,
-			   unsigned long *mappable_end)
+			   u64 *mappable_end)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned int gtt_size;
@@ -2434,7 +2434,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
 	 * a coarse sanity check.
 	 */
 	if ((*mappable_end < (64<<20) || (*mappable_end > (512<<20)))) {
-		DRM_ERROR("Unknown GMADR size (%lx)\n",
+		DRM_ERROR("Unknown GMADR size (%llx)\n",
 			  dev_priv->gtt.mappable_end);
 		return -ENXIO;
 	}
@@ -2468,10 +2468,10 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
 }
 
 static int i915_gmch_probe(struct drm_device *dev,
-			   size_t *gtt_total,
+			   u64 *gtt_total,
 			   size_t *stolen,
 			   phys_addr_t *mappable_base,
-			   unsigned long *mappable_end)
+			   u64 *mappable_end)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
@@ -2536,9 +2536,9 @@ int i915_gem_gtt_init(struct drm_device *dev)
 	gtt->base.dev = dev;
 
 	/* GMADR is the PCI mmio aperture into the global GTT. */
-	DRM_INFO("Memory usable by graphics device = %zdM\n",
+	DRM_INFO("Memory usable by graphics device = %lluM\n",
 		 gtt->base.total >> 20);
-	DRM_DEBUG_DRIVER("GMADR size = %ldM\n", gtt->mappable_end >> 20);
+	DRM_DEBUG_DRIVER("GMADR size = %lldM\n", gtt->mappable_end >> 20);
 	DRM_DEBUG_DRIVER("GTT stolen size = %zdM\n", gtt->stolen_size >> 20);
 #ifdef CONFIG_INTEL_IOMMU
 	if (intel_iommu_gfx_mapped)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 017ea30..600eec0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -235,8 +235,8 @@ struct i915_address_space {
 	struct drm_mm mm;
 	struct drm_device *dev;
 	struct list_head global_link;
-	unsigned long start;		/* Start offset always 0 for dri2 */
-	size_t total;		/* size addr space maps (ex. 2GB for ggtt) */
+	u64 start;		/* Start offset always 0 for dri2 */
+	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
 
 	struct {
 		dma_addr_t addr;
@@ -302,9 +302,9 @@ struct i915_address_space {
  */
 struct i915_gtt {
 	struct i915_address_space base;
-	size_t stolen_size;		/* Total size of stolen memory */
 
-	unsigned long mappable_end;	/* End offset that we can CPU map */
+	size_t stolen_size;		/* Total size of stolen memory */
+	u64 mappable_end;		/* End offset that we can CPU map */
 	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
 	phys_addr_t mappable_base;	/* PA of our GMADR */
 
@@ -316,9 +316,9 @@ struct i915_gtt {
 	int mtrr;
 
 	/* global gtt ops */
-	int (*gtt_probe)(struct drm_device *dev, size_t *gtt_total,
+	int (*gtt_probe)(struct drm_device *dev, u64 *gtt_total,
 			  size_t *stolen, phys_addr_t *mappable_base,
-			  unsigned long *mappable_end);
+			  u64 *mappable_end);
 };
 
 struct i915_hw_ppgtt {
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index b08bdad..9e9bddaa5 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -3,8 +3,8 @@
 #ifndef _DRM_INTEL_GTT_H
 #define	_DRM_INTEL_GTT_H
 
-void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
-		   phys_addr_t *mappable_base, unsigned long *mappable_end);
+void intel_gtt_get(u64 *gtt_total, size_t *stolen_size,
+		   phys_addr_t *mappable_base, u64 *mappable_end);
 
 int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 		     struct agp_bridge_data *bridge);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/18] drm/i915/gtt: Introduce i915_page_dir_dma_addr

[Mika Kuoppala]

The legacy mode mm switch and the execlist context assignment
needs dma address for the page directories.

Introduce a function that encapsulates the scratch_pd dma
fallback if no pd is found.

v2: Rebase, s/ring/req

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> (v1)
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++----
 drivers/gpu/drm/i915/i915_gem_gtt.h | 8 ++++++++
 drivers/gpu/drm/i915/intel_lrc.c    | 4 +---
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7a7789e..47e8e2e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -482,10 +482,8 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
 	int i, ret;
 
 	for (i = GEN8_LEGACY_PDPES - 1; i >= 0; i--) {
-		struct i915_page_directory *pd = ppgtt->pdp.page_directory[i];
-		dma_addr_t pd_daddr = pd ? pd->daddr : ppgtt->scratch_pd->daddr;
-		/* The page directory might be NULL, but we need to clear out
-		 * whatever the previous context might have used. */
+		const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
+
 		ret = gen8_write_pdp(req, i, pd_daddr);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 600eec0..f368c71 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -470,6 +470,14 @@ static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
 	return i915_pte_count(address, length, GEN8_PDE_SHIFT);
 }
 
+static inline dma_addr_t
+i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
+{
+	return test_bit(n, ppgtt->pdp.used_pdpes) ?
+		ppgtt->pdp.page_directory[n]->daddr :
+		ppgtt->scratch_pd->daddr;
+}
+
 int i915_gem_gtt_init(struct drm_device *dev);
 void i915_gem_init_global_gtt(struct drm_device *dev);
 void i915_global_gtt_cleanup(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5298103..d527b7b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -190,9 +190,7 @@
 #define GEN8_CTX_PRIVILEGE (1<<8)
 
 #define ASSIGN_CTX_PDP(ppgtt, reg_state, n) { \
-	const u64 _addr = test_bit(n, ppgtt->pdp.used_pdpes) ? \
-		ppgtt->pdp.page_directory[n]->daddr : \
-		ppgtt->scratch_pd->daddr; \
+	const u64 _addr = i915_page_dir_dma_addr((ppgtt), (n));	\
 	reg_state[CTX_PDP ## n ## _UDW+1] = upper_32_bits(_addr); \
 	reg_state[CTX_PDP ## n ## _LDW+1] = lower_32_bits(_addr); \
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/18] drm/i915/gtt: Introduce struct i915_page_dma

[Mika Kuoppala]

All our paging structures have struct page and dma address
for that page.

Add struct for page/dma address pairs and use it to make
the setup and teardown for different paging structures
identical.

Include the page directory offset also in the struct for legacy
gens. Rename it to clearly point out that it is offset into the
ggtt.

v2: Add comment about ggtt_offset (Michel)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 120 ++++++++++++++----------------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  25 +++++---
 3 files changed, 64 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c654b7d..e40e479 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2230,7 +2230,7 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
 		struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
 
 		seq_puts(m, "aliasing PPGTT:\n");
-		seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd.pd_offset);
+		seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd.base.ggtt_offset);
 
 		ppgtt->debug_dump(ppgtt, m);
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 47e8e2e..a7bdbeb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -301,52 +301,39 @@ static gen6_pte_t iris_pte_encode(dma_addr_t addr,
 	return pte;
 }
 
-#define i915_dma_unmap_single(px, dev) \
-	__i915_dma_unmap_single((px)->daddr, dev)
-
-static void __i915_dma_unmap_single(dma_addr_t daddr,
-				    struct drm_device *dev)
+static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 {
 	struct device *device = &dev->pdev->dev;
 
-	dma_unmap_page(device, daddr, 4096, PCI_DMA_BIDIRECTIONAL);
-}
-
-/**
- * i915_dma_map_single() - Create a dma mapping for a page table/dir/etc.
- * @px:	Page table/dir/etc to get a DMA map for
- * @dev:	drm device
- *
- * Page table allocations are unified across all gens. They always require a
- * single 4k allocation, as well as a DMA mapping. If we keep the structs
- * symmetric here, the simple macro covers us for every page table type.
- *
- * Return: 0 if success.
- */
-#define i915_dma_map_single(px, dev) \
-	i915_dma_map_page_single((px)->page, (dev), &(px)->daddr)
+	p->page = alloc_page(GFP_KERNEL);
+	if (!p->page)
+		return -ENOMEM;
 
-static int i915_dma_map_page_single(struct page *page,
-				    struct drm_device *dev,
-				    dma_addr_t *daddr)
-{
-	struct device *device = &dev->pdev->dev;
+	p->daddr = dma_map_page(device,
+				p->page, 0, 4096, PCI_DMA_BIDIRECTIONAL);
 
-	*daddr = dma_map_page(device, page, 0, 4096, PCI_DMA_BIDIRECTIONAL);
-	if (dma_mapping_error(device, *daddr))
-		return -ENOMEM;
+	if (dma_mapping_error(device, p->daddr)) {
+		__free_page(p->page);
+		return -EINVAL;
+	}
 
 	return 0;
 }
 
-static void unmap_and_free_pt(struct i915_page_table *pt,
-			       struct drm_device *dev)
+static void cleanup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 {
-	if (WARN_ON(!pt->page))
+	if (WARN_ON(!p->page))
 		return;
 
-	i915_dma_unmap_single(pt, dev);
-	__free_page(pt->page);
+	dma_unmap_page(&dev->pdev->dev, p->daddr, 4096, PCI_DMA_BIDIRECTIONAL);
+	__free_page(p->page);
+	memset(p, 0, sizeof(*p));
+}
+
+static void unmap_and_free_pt(struct i915_page_table *pt,
+			       struct drm_device *dev)
+{
+	cleanup_page_dma(dev, &pt->base);
 	kfree(pt->used_ptes);
 	kfree(pt);
 }
@@ -357,7 +344,7 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
 	gen8_pte_t *pt_vaddr, scratch_pte;
 	int i;
 
-	pt_vaddr = kmap_atomic(pt->page);
+	pt_vaddr = kmap_atomic(pt->base.page);
 	scratch_pte = gen8_pte_encode(vm->scratch.addr,
 				      I915_CACHE_LLC, true);
 
@@ -386,19 +373,13 @@ static struct i915_page_table *alloc_pt(struct drm_device *dev)
 	if (!pt->used_ptes)
 		goto fail_bitmap;
 
-	pt->page = alloc_page(GFP_KERNEL);
-	if (!pt->page)
-		goto fail_page;
-
-	ret = i915_dma_map_single(pt, dev);
+	ret = setup_page_dma(dev, &pt->base);
 	if (ret)
-		goto fail_dma;
+		goto fail_page_m;
 
 	return pt;
 
-fail_dma:
-	__free_page(pt->page);
-fail_page:
+fail_page_m:
 	kfree(pt->used_ptes);
 fail_bitmap:
 	kfree(pt);
@@ -409,9 +390,8 @@ fail_bitmap:
 static void unmap_and_free_pd(struct i915_page_directory *pd,
 			      struct drm_device *dev)
 {
-	if (pd->page) {
-		i915_dma_unmap_single(pd, dev);
-		__free_page(pd->page);
+	if (pd->base.page) {
+		cleanup_page_dma(dev, &pd->base);
 		kfree(pd->used_pdes);
 		kfree(pd);
 	}
@@ -431,18 +411,12 @@ static struct i915_page_directory *alloc_pd(struct drm_device *dev)
 	if (!pd->used_pdes)
 		goto free_pd;
 
-	pd->page = alloc_page(GFP_KERNEL);
-	if (!pd->page)
-		goto free_bitmap;
-
-	ret = i915_dma_map_single(pd, dev);
+	ret = setup_page_dma(dev, &pd->base);
 	if (ret)
-		goto free_page;
+		goto free_bitmap;
 
 	return pd;
 
-free_page:
-	__free_page(pd->page);
 free_bitmap:
 	kfree(pd->used_pdes);
 free_pd:
@@ -524,10 +498,10 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 
 		pt = pd->page_table[pde];
 
-		if (WARN_ON(!pt->page))
+		if (WARN_ON(!pt->base.page))
 			continue;
 
-		page_table = pt->page;
+		page_table = pt->base.page;
 
 		last_pte = pte + num_entries;
 		if (last_pte > GEN8_PTES)
@@ -574,7 +548,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 		if (pt_vaddr == NULL) {
 			struct i915_page_directory *pd = ppgtt->pdp.page_directory[pdpe];
 			struct i915_page_table *pt = pd->page_table[pde];
-			struct page *page_table = pt->page;
+			struct page *page_table = pt->base.page;
 
 			pt_vaddr = kmap_atomic(page_table);
 		}
@@ -606,7 +580,7 @@ static void __gen8_do_map_pt(gen8_pde_t * const pde,
 			     struct drm_device *dev)
 {
 	gen8_pde_t entry =
-		gen8_pde_encode(dev, pt->daddr, I915_CACHE_LLC);
+		gen8_pde_encode(dev, pt->base.daddr, I915_CACHE_LLC);
 	*pde = entry;
 }
 
@@ -619,7 +593,7 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
 	struct i915_page_table *pt;
 	int i;
 
-	page_directory = kmap_atomic(pd->page);
+	page_directory = kmap_atomic(pd->base.page);
 	pt = ppgtt->scratch_pt;
 	for (i = 0; i < I915_PDES; i++)
 		/* Map the PDE to the page table */
@@ -634,7 +608,7 @@ static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_dev
 {
 	int i;
 
-	if (!pd->page)
+	if (!pd->base.page)
 		return;
 
 	for_each_set_bit(i, pd->used_pdes, I915_PDES) {
@@ -885,7 +859,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 	/* Allocations have completed successfully, so set the bitmaps, and do
 	 * the mappings. */
 	gen8_for_each_pdpe(pd, &ppgtt->pdp, start, length, temp, pdpe) {
-		gen8_pde_t *const page_directory = kmap_atomic(pd->page);
+		gen8_pde_t *const page_directory = kmap_atomic(pd->base.page);
 		struct i915_page_table *pt;
 		uint64_t pd_len = gen8_clamp_pd(start, length);
 		uint64_t pd_start = start;
@@ -996,7 +970,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde) {
 		u32 expected;
 		gen6_pte_t *pt_vaddr;
-		dma_addr_t pt_addr = ppgtt->pd.page_table[pde]->daddr;
+		dma_addr_t pt_addr = ppgtt->pd.page_table[pde]->base.daddr;
 		pd_entry = readl(ppgtt->pd_addr + pde);
 		expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
 
@@ -1007,7 +981,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 				   expected);
 		seq_printf(m, "\tPDE: %x\n", pd_entry);
 
-		pt_vaddr = kmap_atomic(ppgtt->pd.page_table[pde]->page);
+		pt_vaddr = kmap_atomic(ppgtt->pd.page_table[pde]->base.page);
 		for (pte = 0; pte < GEN6_PTES; pte+=4) {
 			unsigned long va =
 				(pde * PAGE_SIZE * GEN6_PTES) +
@@ -1042,7 +1016,7 @@ static void gen6_write_pde(struct i915_page_directory *pd,
 		container_of(pd, struct i915_hw_ppgtt, pd);
 	u32 pd_entry;
 
-	pd_entry = GEN6_PDE_ADDR_ENCODE(pt->daddr);
+	pd_entry = GEN6_PDE_ADDR_ENCODE(pt->base.daddr);
 	pd_entry |= GEN6_PDE_VALID;
 
 	writel(pd_entry, ppgtt->pd_addr + pde);
@@ -1067,9 +1041,9 @@ static void gen6_write_page_range(struct drm_i915_private *dev_priv,
 
 static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
 {
-	BUG_ON(ppgtt->pd.pd_offset & 0x3f);
+	BUG_ON(ppgtt->pd.base.ggtt_offset & 0x3f);
 
-	return (ppgtt->pd.pd_offset / 64) << 16;
+	return (ppgtt->pd.base.ggtt_offset / 64) << 16;
 }
 
 static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
@@ -1236,7 +1210,7 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 		if (last_pte > GEN6_PTES)
 			last_pte = GEN6_PTES;
 
-		pt_vaddr = kmap_atomic(ppgtt->pd.page_table[act_pt]->page);
+		pt_vaddr = kmap_atomic(ppgtt->pd.page_table[act_pt]->base.page);
 
 		for (i = first_pte; i < last_pte; i++)
 			pt_vaddr[i] = scratch_pte;
@@ -1265,7 +1239,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 	pt_vaddr = NULL;
 	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
 		if (pt_vaddr == NULL)
-			pt_vaddr = kmap_atomic(ppgtt->pd.page_table[act_pt]->page);
+			pt_vaddr = kmap_atomic(ppgtt->pd.page_table[act_pt]->base.page);
 
 		pt_vaddr[act_pte] =
 			vm->pte_encode(sg_page_iter_dma_address(&sg_iter),
@@ -1293,7 +1267,7 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
 	scratch_pte = vm->pte_encode(vm->scratch.addr,
 			I915_CACHE_LLC, true, 0);
 
-	pt_vaddr = kmap_atomic(pt->page);
+	pt_vaddr = kmap_atomic(pt->base.page);
 
 	for (i = 0; i < GEN6_PTES; i++)
 		pt_vaddr[i] = scratch_pte;
@@ -1509,11 +1483,11 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	ppgtt->base.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
 	ppgtt->debug_dump = gen6_dump_ppgtt;
 
-	ppgtt->pd.pd_offset =
+	ppgtt->pd.base.ggtt_offset =
 		ppgtt->node.start / PAGE_SIZE * sizeof(gen6_pte_t);
 
 	ppgtt->pd_addr = (gen6_pte_t __iomem *)dev_priv->gtt.gsm +
-		ppgtt->pd.pd_offset / sizeof(gen6_pte_t);
+		ppgtt->pd.base.ggtt_offset / sizeof(gen6_pte_t);
 
 	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.total);
 
@@ -1524,7 +1498,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 			 ppgtt->node.start / PAGE_SIZE);
 
 	DRM_DEBUG("Adding PPGTT at offset %x\n",
-		  ppgtt->pd.pd_offset << 10);
+		  ppgtt->pd.base.ggtt_offset << 10);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f368c71..c681573b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -207,19 +207,26 @@ struct i915_vma {
 #define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf
 };
 
-struct i915_page_table {
+struct i915_page_dma {
 	struct page *page;
-	dma_addr_t daddr;
+	union {
+		dma_addr_t daddr;
+
+		/* For gen6/gen7 only. This is the offset in the GGTT
+		 * where the page directory entries for PPGTT begin
+		 */
+		uint32_t ggtt_offset;
+	};
+};
+
+struct i915_page_table {
+	struct i915_page_dma base;
 
 	unsigned long *used_ptes;
 };
 
 struct i915_page_directory {
-	struct page *page; /* NULL for GEN6-GEN7 */
-	union {
-		uint32_t pd_offset;
-		dma_addr_t daddr;
-	};
+	struct i915_page_dma base;
 
 	unsigned long *used_pdes;
 	struct i915_page_table *page_table[I915_PDES]; /* PDEs */
@@ -474,8 +481,8 @@ static inline dma_addr_t
 i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
 {
 	return test_bit(n, ppgtt->pdp.used_pdpes) ?
-		ppgtt->pdp.page_directory[n]->daddr :
-		ppgtt->scratch_pd->daddr;
+		ppgtt->pdp.page_directory[n]->base.daddr :
+		ppgtt->scratch_pd->base.daddr;
 }
 
 int i915_gem_gtt_init(struct drm_device *dev);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/18] drm/i915/gtt: Rename unmap_and_free_px to free_px

[Mika Kuoppala]

All the paging structures are now similar and mapped for
dma. The unmapping is taken care of by common accessors, so
don't overload the reader with such details.

v2: Be consistent with goto labels (Michel)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 40 ++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a7bdbeb..82ccc69 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -330,8 +330,7 @@ static void cleanup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 	memset(p, 0, sizeof(*p));
 }
 
-static void unmap_and_free_pt(struct i915_page_table *pt,
-			       struct drm_device *dev)
+static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
 {
 	cleanup_page_dma(dev, &pt->base);
 	kfree(pt->used_ptes);
@@ -387,8 +386,7 @@ fail_bitmap:
 	return ERR_PTR(ret);
 }
 
-static void unmap_and_free_pd(struct i915_page_directory *pd,
-			      struct drm_device *dev)
+static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
 {
 	if (pd->base.page) {
 		cleanup_page_dma(dev, &pd->base);
@@ -409,17 +407,17 @@ static struct i915_page_directory *alloc_pd(struct drm_device *dev)
 	pd->used_pdes = kcalloc(BITS_TO_LONGS(I915_PDES),
 				sizeof(*pd->used_pdes), GFP_KERNEL);
 	if (!pd->used_pdes)
-		goto free_pd;
+		goto fail_bitmap;
 
 	ret = setup_page_dma(dev, &pd->base);
 	if (ret)
-		goto free_bitmap;
+		goto fail_page_m;
 
 	return pd;
 
-free_bitmap:
+fail_page_m:
 	kfree(pd->used_pdes);
-free_pd:
+fail_bitmap:
 	kfree(pd);
 
 	return ERR_PTR(ret);
@@ -615,7 +613,7 @@ static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_dev
 		if (WARN_ON(!pd->page_table[i]))
 			continue;
 
-		unmap_and_free_pt(pd->page_table[i], dev);
+		free_pt(dev, pd->page_table[i]);
 		pd->page_table[i] = NULL;
 	}
 }
@@ -631,11 +629,11 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 			continue;
 
 		gen8_free_page_tables(ppgtt->pdp.page_directory[i], ppgtt->base.dev);
-		unmap_and_free_pd(ppgtt->pdp.page_directory[i], ppgtt->base.dev);
+		free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
 	}
 
-	unmap_and_free_pd(ppgtt->scratch_pd, ppgtt->base.dev);
-	unmap_and_free_pt(ppgtt->scratch_pt, ppgtt->base.dev);
+	free_pd(ppgtt->base.dev, ppgtt->scratch_pd);
+	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
 }
 
 /**
@@ -688,7 +686,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
 
 unwind_out:
 	for_each_set_bit(pde, new_pts, I915_PDES)
-		unmap_and_free_pt(pd->page_table[pde], dev);
+		free_pt(dev, pd->page_table[pde]);
 
 	return -ENOMEM;
 }
@@ -746,7 +744,7 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
 
 unwind_out:
 	for_each_set_bit(pdpe, new_pds, GEN8_LEGACY_PDPES)
-		unmap_and_free_pd(pdp->page_directory[pdpe], dev);
+		free_pd(dev, pdp->page_directory[pdpe]);
 
 	return -ENOMEM;
 }
@@ -904,11 +902,11 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 err_out:
 	while (pdpe--) {
 		for_each_set_bit(temp, new_page_tables[pdpe], I915_PDES)
-			unmap_and_free_pt(ppgtt->pdp.page_directory[pdpe]->page_table[temp], vm->dev);
+			free_pt(vm->dev, ppgtt->pdp.page_directory[pdpe]->page_table[temp]);
 	}
 
 	for_each_set_bit(pdpe, new_page_dirs, GEN8_LEGACY_PDPES)
-		unmap_and_free_pd(ppgtt->pdp.page_directory[pdpe], vm->dev);
+		free_pd(vm->dev, ppgtt->pdp.page_directory[pdpe]);
 
 	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
 	mark_tlbs_dirty(ppgtt);
@@ -1358,7 +1356,7 @@ unwind_out:
 		struct i915_page_table *pt = ppgtt->pd.page_table[pde];
 
 		ppgtt->pd.page_table[pde] = ppgtt->scratch_pt;
-		unmap_and_free_pt(pt, vm->dev);
+		free_pt(vm->dev, pt);
 	}
 
 	mark_tlbs_dirty(ppgtt);
@@ -1377,11 +1375,11 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 
 	gen6_for_all_pdes(pt, ppgtt, pde) {
 		if (pt != ppgtt->scratch_pt)
-			unmap_and_free_pt(pt, ppgtt->base.dev);
+			free_pt(ppgtt->base.dev, pt);
 	}
 
-	unmap_and_free_pt(ppgtt->scratch_pt, ppgtt->base.dev);
-	unmap_and_free_pd(&ppgtt->pd, ppgtt->base.dev);
+	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
+	free_pd(ppgtt->base.dev, &ppgtt->pd);
 }
 
 static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
@@ -1431,7 +1429,7 @@ alloc:
 	return 0;
 
 err_out:
-	unmap_and_free_pt(ppgtt->scratch_pt, ppgtt->base.dev);
+	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
 	return ret;
 }
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/18] drm/i915/gtt: Remove superfluous free_pd with gen6/7

[Mika Kuoppala]

This has slipped in somewhere but it was harmless
as we check the page pointer before teardown.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 82ccc69..21fba03 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1379,7 +1379,6 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 	}
 
 	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
-	free_pd(ppgtt->base.dev, &ppgtt->pd);
 }
 
 static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/18] drm/i915/gtt: Introduce fill_page_dma()

[Mika Kuoppala]

When we setup page directories and tables, we point the entries
to a to the next level scratch structure. Make this generic
by introducing a fill_page_dma which maps and flushes. We also
need 32 bit variant for legacy gens.

v2: Fix flushes and handle valleyview (Ville)
v3: Now really fix flushes (Michel, Ville)

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 74 ++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 21fba03..734ffd2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -330,6 +330,34 @@ static void cleanup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 	memset(p, 0, sizeof(*p));
 }
 
+static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
+			  const uint64_t val)
+{
+	int i;
+	uint64_t * const vaddr = kmap_atomic(p->page);
+
+	for (i = 0; i < 512; i++)
+		vaddr[i] = val;
+
+	/* There are only few exceptions for gen >=6. chv and bxt.
+	 * And we are not sure about the latter so play safe for now.
+	 */
+	if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
+		drm_clflush_virt_range(vaddr, PAGE_SIZE);
+
+	kunmap_atomic(vaddr);
+}
+
+static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
+			     const uint32_t val32)
+{
+	uint64_t v = val32;
+
+	v = v << 32 | val32;
+
+	fill_page_dma(dev, p, v);
+}
+
 static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
 {
 	cleanup_page_dma(dev, &pt->base);
@@ -340,19 +368,11 @@ static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
 static void gen8_initialize_pt(struct i915_address_space *vm,
 			       struct i915_page_table *pt)
 {
-	gen8_pte_t *pt_vaddr, scratch_pte;
-	int i;
-
-	pt_vaddr = kmap_atomic(pt->base.page);
-	scratch_pte = gen8_pte_encode(vm->scratch.addr,
-				      I915_CACHE_LLC, true);
+	gen8_pte_t scratch_pte;
 
-	for (i = 0; i < GEN8_PTES; i++)
-		pt_vaddr[i] = scratch_pte;
+	scratch_pte = gen8_pte_encode(vm->scratch.addr, I915_CACHE_LLC, true);
 
-	if (!HAS_LLC(vm->dev))
-		drm_clflush_virt_range(pt_vaddr, PAGE_SIZE);
-	kunmap_atomic(pt_vaddr);
+	fill_page_dma(vm->dev, &pt->base, scratch_pte);
 }
 
 static struct i915_page_table *alloc_pt(struct drm_device *dev)
@@ -586,20 +606,13 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
 			       struct i915_page_directory *pd)
 {
 	struct i915_hw_ppgtt *ppgtt =
-			container_of(vm, struct i915_hw_ppgtt, base);
-	gen8_pde_t *page_directory;
-	struct i915_page_table *pt;
-	int i;
+		container_of(vm, struct i915_hw_ppgtt, base);
+	gen8_pde_t scratch_pde;
 
-	page_directory = kmap_atomic(pd->base.page);
-	pt = ppgtt->scratch_pt;
-	for (i = 0; i < I915_PDES; i++)
-		/* Map the PDE to the page table */
-		__gen8_do_map_pt(page_directory + i, pt, vm->dev);
+	scratch_pde = gen8_pde_encode(vm->dev, ppgtt->scratch_pt->base.daddr,
+				      I915_CACHE_LLC);
 
-	if (!HAS_LLC(vm->dev))
-		drm_clflush_virt_range(page_directory, PAGE_SIZE);
-	kunmap_atomic(page_directory);
+	fill_page_dma(vm->dev, &pd->base, scratch_pde);
 }
 
 static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_device *dev)
@@ -1255,22 +1268,15 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 }
 
 static void gen6_initialize_pt(struct i915_address_space *vm,
-		struct i915_page_table *pt)
+			       struct i915_page_table *pt)
 {
-	gen6_pte_t *pt_vaddr, scratch_pte;
-	int i;
+	gen6_pte_t scratch_pte;
 
 	WARN_ON(vm->scratch.addr == 0);
 
-	scratch_pte = vm->pte_encode(vm->scratch.addr,
-			I915_CACHE_LLC, true, 0);
-
-	pt_vaddr = kmap_atomic(pt->base.page);
-
-	for (i = 0; i < GEN6_PTES; i++)
-		pt_vaddr[i] = scratch_pte;
+	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0);
 
-	kunmap_atomic(pt_vaddr);
+	fill_page_dma_32(vm->dev, &pt->base, scratch_pte);
 }
 
 static int gen6_alloc_va_range(struct i915_address_space *vm,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/18] drm/i915/gtt: Introduce kmap|kunmap for dma page

[Mika Kuoppala]

As there is flushing involved when we have done the cpu
write, make functions for mapping for cpu space. Make macros
to map any type of paging structure.

v2: Make it clear tha flushing kunmap is only for ppgtt (Ville)
v3: Flushing fixed (Ville, Michel). Removed superfluous semicolon

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 77 +++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 734ffd2..6abcf32 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -330,15 +330,16 @@ static void cleanup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 	memset(p, 0, sizeof(*p));
 }
 
-static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
-			  const uint64_t val)
+static void *kmap_page_dma(struct i915_page_dma *p)
 {
-	int i;
-	uint64_t * const vaddr = kmap_atomic(p->page);
-
-	for (i = 0; i < 512; i++)
-		vaddr[i] = val;
+	return kmap_atomic(p->page);
+}
 
+/* We use the flushing unmap only with ppgtt structures:
+ * page directories, page tables and scratch pages.
+ */
+static void kunmap_page_dma(struct drm_device *dev, void *vaddr)
+{
 	/* There are only few exceptions for gen >=6. chv and bxt.
 	 * And we are not sure about the latter so play safe for now.
 	 */
@@ -348,6 +349,21 @@ static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
 	kunmap_atomic(vaddr);
 }
 
+#define kmap_px(px) kmap_page_dma(&(px)->base)
+#define kunmap_px(ppgtt, vaddr) kunmap_page_dma((ppgtt)->base.dev, (vaddr))
+
+static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
+			  const uint64_t val)
+{
+	int i;
+	uint64_t * const vaddr = kmap_page_dma(p);
+
+	for (i = 0; i < 512; i++)
+		vaddr[i] = val;
+
+	kunmap_page_dma(dev, vaddr);
+}
+
 static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
 			     const uint32_t val32)
 {
@@ -504,7 +520,6 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 	while (num_entries) {
 		struct i915_page_directory *pd;
 		struct i915_page_table *pt;
-		struct page *page_table;
 
 		if (WARN_ON(!ppgtt->pdp.page_directory[pdpe]))
 			continue;
@@ -519,22 +534,18 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 		if (WARN_ON(!pt->base.page))
 			continue;
 
-		page_table = pt->base.page;
-
 		last_pte = pte + num_entries;
 		if (last_pte > GEN8_PTES)
 			last_pte = GEN8_PTES;
 
-		pt_vaddr = kmap_atomic(page_table);
+		pt_vaddr = kmap_px(pt);
 
 		for (i = pte; i < last_pte; i++) {
 			pt_vaddr[i] = scratch_pte;
 			num_entries--;
 		}
 
-		if (!HAS_LLC(ppgtt->base.dev))
-			drm_clflush_virt_range(pt_vaddr, PAGE_SIZE);
-		kunmap_atomic(pt_vaddr);
+		kunmap_px(ppgtt, pt);
 
 		pte = 0;
 		if (++pde == I915_PDES) {
@@ -566,18 +577,14 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 		if (pt_vaddr == NULL) {
 			struct i915_page_directory *pd = ppgtt->pdp.page_directory[pdpe];
 			struct i915_page_table *pt = pd->page_table[pde];
-			struct page *page_table = pt->base.page;
-
-			pt_vaddr = kmap_atomic(page_table);
+			pt_vaddr = kmap_px(pt);
 		}
 
 		pt_vaddr[pte] =
 			gen8_pte_encode(sg_page_iter_dma_address(&sg_iter),
 					cache_level, true);
 		if (++pte == GEN8_PTES) {
-			if (!HAS_LLC(ppgtt->base.dev))
-				drm_clflush_virt_range(pt_vaddr, PAGE_SIZE);
-			kunmap_atomic(pt_vaddr);
+			kunmap_px(ppgtt, pt_vaddr);
 			pt_vaddr = NULL;
 			if (++pde == I915_PDES) {
 				pdpe++;
@@ -586,11 +593,9 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 			pte = 0;
 		}
 	}
-	if (pt_vaddr) {
-		if (!HAS_LLC(ppgtt->base.dev))
-			drm_clflush_virt_range(pt_vaddr, PAGE_SIZE);
-		kunmap_atomic(pt_vaddr);
-	}
+
+	if (pt_vaddr)
+		kunmap_px(ppgtt, pt_vaddr);
 }
 
 static void __gen8_do_map_pt(gen8_pde_t * const pde,
@@ -870,7 +875,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 	/* Allocations have completed successfully, so set the bitmaps, and do
 	 * the mappings. */
 	gen8_for_each_pdpe(pd, &ppgtt->pdp, start, length, temp, pdpe) {
-		gen8_pde_t *const page_directory = kmap_atomic(pd->base.page);
+		gen8_pde_t *const page_directory = kmap_px(pd);
 		struct i915_page_table *pt;
 		uint64_t pd_len = gen8_clamp_pd(start, length);
 		uint64_t pd_start = start;
@@ -900,10 +905,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 			 * point we're still relying on insert_entries() */
 		}
 
-		if (!HAS_LLC(vm->dev))
-			drm_clflush_virt_range(page_directory, PAGE_SIZE);
-
-		kunmap_atomic(page_directory);
+		kunmap_px(ppgtt, page_directory);
 
 		set_bit(pdpe, ppgtt->pdp.used_pdpes);
 	}
@@ -992,7 +994,8 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 				   expected);
 		seq_printf(m, "\tPDE: %x\n", pd_entry);
 
-		pt_vaddr = kmap_atomic(ppgtt->pd.page_table[pde]->base.page);
+		pt_vaddr = kmap_px(ppgtt->pd.page_table[pde]);
+
 		for (pte = 0; pte < GEN6_PTES; pte+=4) {
 			unsigned long va =
 				(pde * PAGE_SIZE * GEN6_PTES) +
@@ -1014,7 +1017,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 			}
 			seq_puts(m, "\n");
 		}
-		kunmap_atomic(pt_vaddr);
+		kunmap_px(ppgtt, pt_vaddr);
 	}
 }
 
@@ -1221,12 +1224,12 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 		if (last_pte > GEN6_PTES)
 			last_pte = GEN6_PTES;
 
-		pt_vaddr = kmap_atomic(ppgtt->pd.page_table[act_pt]->base.page);
+		pt_vaddr = kmap_px(ppgtt->pd.page_table[act_pt]);
 
 		for (i = first_pte; i < last_pte; i++)
 			pt_vaddr[i] = scratch_pte;
 
-		kunmap_atomic(pt_vaddr);
+		kunmap_px(ppgtt, pt_vaddr);
 
 		num_entries -= last_pte - first_pte;
 		first_pte = 0;
@@ -1250,21 +1253,21 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 	pt_vaddr = NULL;
 	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
 		if (pt_vaddr == NULL)
-			pt_vaddr = kmap_atomic(ppgtt->pd.page_table[act_pt]->base.page);
+			pt_vaddr = kmap_px(ppgtt->pd.page_table[act_pt]);
 
 		pt_vaddr[act_pte] =
 			vm->pte_encode(sg_page_iter_dma_address(&sg_iter),
 				       cache_level, true, flags);
 
 		if (++act_pte == GEN6_PTES) {
-			kunmap_atomic(pt_vaddr);
+			kunmap_px(ppgtt, pt_vaddr);
 			pt_vaddr = NULL;
 			act_pt++;
 			act_pte = 0;
 		}
 	}
 	if (pt_vaddr)
-		kunmap_atomic(pt_vaddr);
+		kunmap_px(ppgtt, pt_vaddr);
 }
 
 static void gen6_initialize_pt(struct i915_address_space *vm,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/18] drm/i915/gtt: Use macros to access dma mapped pages

[Mika Kuoppala]

Make paging structure type agnostic *_px macros to access
page dma struct, the backing page and the dma address.

This makes the code less cluttered on internals of
i915_page_dma.

v2: Superfluous const -> nonconst removed
v3: Rebased

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> (v2)
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++---------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  8 ++++++--
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6abcf32..e85676e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -349,9 +349,14 @@ static void kunmap_page_dma(struct drm_device *dev, void *vaddr)
 	kunmap_atomic(vaddr);
 }
 
-#define kmap_px(px) kmap_page_dma(&(px)->base)
+#define kmap_px(px) kmap_page_dma(px_base(px))
 #define kunmap_px(ppgtt, vaddr) kunmap_page_dma((ppgtt)->base.dev, (vaddr))
 
+#define setup_px(dev, px) setup_page_dma((dev), px_base(px))
+#define cleanup_px(dev, px) cleanup_page_dma((dev), px_base(px))
+#define fill_px(dev, px, v) fill_page_dma((dev), px_base(px), (v))
+#define fill32_px(dev, px, v) fill_page_dma_32((dev), px_base(px), (v))
+
 static void fill_page_dma(struct drm_device *dev, struct i915_page_dma *p,
 			  const uint64_t val)
 {
@@ -376,7 +381,7 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
 
 static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
 {
-	cleanup_page_dma(dev, &pt->base);
+	cleanup_px(dev, pt);
 	kfree(pt->used_ptes);
 	kfree(pt);
 }
@@ -388,7 +393,7 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
 
 	scratch_pte = gen8_pte_encode(vm->scratch.addr, I915_CACHE_LLC, true);
 
-	fill_page_dma(vm->dev, &pt->base, scratch_pte);
+	fill_px(vm->dev, pt, scratch_pte);
 }
 
 static struct i915_page_table *alloc_pt(struct drm_device *dev)
@@ -408,7 +413,7 @@ static struct i915_page_table *alloc_pt(struct drm_device *dev)
 	if (!pt->used_ptes)
 		goto fail_bitmap;
 
-	ret = setup_page_dma(dev, &pt->base);
+	ret = setup_px(dev, pt);
 	if (ret)
 		goto fail_page_m;
 
@@ -424,8 +429,8 @@ fail_bitmap:
 
 static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
 {
-	if (pd->base.page) {
-		cleanup_page_dma(dev, &pd->base);
+	if (px_page(pd)) {
+		cleanup_px(dev, pd);
 		kfree(pd->used_pdes);
 		kfree(pd);
 	}
@@ -445,7 +450,7 @@ static struct i915_page_directory *alloc_pd(struct drm_device *dev)
 	if (!pd->used_pdes)
 		goto fail_bitmap;
 
-	ret = setup_page_dma(dev, &pd->base);
+	ret = setup_px(dev, pd);
 	if (ret)
 		goto fail_page_m;
 
@@ -531,7 +536,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 
 		pt = pd->page_table[pde];
 
-		if (WARN_ON(!pt->base.page))
+		if (WARN_ON(!px_page(pt)))
 			continue;
 
 		last_pte = pte + num_entries;
@@ -603,7 +608,7 @@ static void __gen8_do_map_pt(gen8_pde_t * const pde,
 			     struct drm_device *dev)
 {
 	gen8_pde_t entry =
-		gen8_pde_encode(dev, pt->base.daddr, I915_CACHE_LLC);
+		gen8_pde_encode(dev, px_dma(pt), I915_CACHE_LLC);
 	*pde = entry;
 }
 
@@ -614,17 +619,17 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
 		container_of(vm, struct i915_hw_ppgtt, base);
 	gen8_pde_t scratch_pde;
 
-	scratch_pde = gen8_pde_encode(vm->dev, ppgtt->scratch_pt->base.daddr,
+	scratch_pde = gen8_pde_encode(vm->dev, px_dma(ppgtt->scratch_pt),
 				      I915_CACHE_LLC);
 
-	fill_page_dma(vm->dev, &pd->base, scratch_pde);
+	fill_px(vm->dev, pd, scratch_pde);
 }
 
 static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_device *dev)
 {
 	int i;
 
-	if (!pd->base.page)
+	if (!px_page(pd))
 		return;
 
 	for_each_set_bit(i, pd->used_pdes, I915_PDES) {
@@ -983,7 +988,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde) {
 		u32 expected;
 		gen6_pte_t *pt_vaddr;
-		dma_addr_t pt_addr = ppgtt->pd.page_table[pde]->base.daddr;
+		const dma_addr_t pt_addr = px_dma(ppgtt->pd.page_table[pde]);
 		pd_entry = readl(ppgtt->pd_addr + pde);
 		expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
 
@@ -1030,7 +1035,7 @@ static void gen6_write_pde(struct i915_page_directory *pd,
 		container_of(pd, struct i915_hw_ppgtt, pd);
 	u32 pd_entry;
 
-	pd_entry = GEN6_PDE_ADDR_ENCODE(pt->base.daddr);
+	pd_entry = GEN6_PDE_ADDR_ENCODE(px_dma(pt));
 	pd_entry |= GEN6_PDE_VALID;
 
 	writel(pd_entry, ppgtt->pd_addr + pde);
@@ -1279,7 +1284,7 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
 
 	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0);
 
-	fill_page_dma_32(vm->dev, &pt->base, scratch_pte);
+	fill32_px(vm->dev, pt, scratch_pte);
 }
 
 static int gen6_alloc_va_range(struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c681573b..f4bcec2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -219,6 +219,10 @@ struct i915_page_dma {
 	};
 };
 
+#define px_base(px) (&(px)->base)
+#define px_page(px) (px_base(px)->page)
+#define px_dma(px) (px_base(px)->daddr)
+
 struct i915_page_table {
 	struct i915_page_dma base;
 
@@ -481,8 +485,8 @@ static inline dma_addr_t
 i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
 {
 	return test_bit(n, ppgtt->pdp.used_pdpes) ?
-		ppgtt->pdp.page_directory[n]->base.daddr :
-		ppgtt->scratch_pd->base.daddr;
+		px_dma(ppgtt->pdp.page_directory[n]) :
+		px_dma(ppgtt->scratch_pd);
 }
 
 int i915_gem_gtt_init(struct drm_device *dev);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/18] drm/i915/gtt: Make scratch page i915_page_dma compatible

[Mika Kuoppala]

Lay out scratch page structure in similar manner than other
paging structures. This allows us to use the same tools for
setup and teardown.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 89 ++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  9 ++--
 2 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e85676e..0cc0cf4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -301,11 +301,12 @@ static gen6_pte_t iris_pte_encode(dma_addr_t addr,
 	return pte;
 }
 
-static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
+static int __setup_page_dma(struct drm_device *dev,
+			    struct i915_page_dma *p, gfp_t flags)
 {
 	struct device *device = &dev->pdev->dev;
 
-	p->page = alloc_page(GFP_KERNEL);
+	p->page = alloc_page(flags);
 	if (!p->page)
 		return -ENOMEM;
 
@@ -320,6 +321,11 @@ static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 	return 0;
 }
 
+static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
+{
+	return __setup_page_dma(dev, p, GFP_KERNEL);
+}
+
 static void cleanup_page_dma(struct drm_device *dev, struct i915_page_dma *p)
 {
 	if (WARN_ON(!p->page))
@@ -391,7 +397,8 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
 {
 	gen8_pte_t scratch_pte;
 
-	scratch_pte = gen8_pte_encode(vm->scratch.addr, I915_CACHE_LLC, true);
+	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
+				      I915_CACHE_LLC, true);
 
 	fill_px(vm->dev, pt, scratch_pte);
 }
@@ -519,7 +526,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 	unsigned num_entries = length >> PAGE_SHIFT;
 	unsigned last_pte, i;
 
-	scratch_pte = gen8_pte_encode(ppgtt->base.scratch.addr,
+	scratch_pte = gen8_pte_encode(px_dma(ppgtt->base.scratch_page),
 				      I915_CACHE_LLC, use_scratch);
 
 	while (num_entries) {
@@ -983,7 +990,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 	uint32_t  pte, pde, temp;
 	uint32_t start = ppgtt->base.start, length = ppgtt->base.total;
 
-	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0);
+	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), I915_CACHE_LLC, true, 0);
 
 	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde) {
 		u32 expected;
@@ -1222,7 +1229,8 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 	unsigned first_pte = first_entry % GEN6_PTES;
 	unsigned last_pte, i;
 
-	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0);
+	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
+				     I915_CACHE_LLC, true, 0);
 
 	while (num_entries) {
 		last_pte = first_pte + num_entries;
@@ -1280,9 +1288,10 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
 {
 	gen6_pte_t scratch_pte;
 
-	WARN_ON(vm->scratch.addr == 0);
+	WARN_ON(px_dma(vm->scratch_page) == 0);
 
-	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0);
+	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
+				     I915_CACHE_LLC, true, 0);
 
 	fill32_px(vm->dev, pt, scratch_pte);
 }
@@ -1519,13 +1528,14 @@ static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	ppgtt->base.dev = dev;
-	ppgtt->base.scratch = dev_priv->gtt.base.scratch;
+	ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page;
 
 	if (INTEL_INFO(dev)->gen < 8)
 		return gen6_ppgtt_init(ppgtt);
 	else
 		return gen8_ppgtt_init(ppgtt);
 }
+
 int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1842,7 +1852,7 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 		 first_entry, num_entries, max_entries))
 		num_entries = max_entries;
 
-	scratch_pte = gen8_pte_encode(vm->scratch.addr,
+	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
 				      I915_CACHE_LLC,
 				      use_scratch);
 	for (i = 0; i < num_entries; i++)
@@ -1868,7 +1878,8 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm,
 		 first_entry, num_entries, max_entries))
 		num_entries = max_entries;
 
-	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, use_scratch, 0);
+	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
+				     I915_CACHE_LLC, use_scratch, 0);
 
 	for (i = 0; i < num_entries; i++)
 		iowrite32(scratch_pte, &gtt_base[i]);
@@ -2125,42 +2136,40 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
 	vm->cleanup(vm);
 }
 
-static int setup_scratch_page(struct drm_device *dev)
+static int alloc_scratch_page(struct i915_address_space *vm)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct page *page;
-	dma_addr_t dma_addr;
+	struct i915_page_scratch *sp;
+	int ret;
+
+	WARN_ON(vm->scratch_page);
 
-	page = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
-	if (page == NULL)
+	sp = kzalloc(sizeof(*sp), GFP_KERNEL);
+	if (sp == NULL)
 		return -ENOMEM;
-	set_pages_uc(page, 1);
 
-#ifdef CONFIG_INTEL_IOMMU
-	dma_addr = pci_map_page(dev->pdev, page, 0, PAGE_SIZE,
-				PCI_DMA_BIDIRECTIONAL);
-	if (pci_dma_mapping_error(dev->pdev, dma_addr)) {
-		__free_page(page);
-		return -EINVAL;
+	ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
+	if (ret) {
+		kfree(sp);
+		return ret;
 	}
-#else
-	dma_addr = page_to_phys(page);
-#endif
-	dev_priv->gtt.base.scratch.page = page;
-	dev_priv->gtt.base.scratch.addr = dma_addr;
+
+	set_pages_uc(px_page(sp), 1);
+
+	vm->scratch_page = sp;
 
 	return 0;
 }
 
-static void teardown_scratch_page(struct drm_device *dev)
+static void free_scratch_page(struct i915_address_space *vm)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct page *page = dev_priv->gtt.base.scratch.page;
+	struct i915_page_scratch *sp = vm->scratch_page;
 
-	set_pages_wb(page, 1);
-	pci_unmap_page(dev->pdev, dev_priv->gtt.base.scratch.addr,
-		       PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
-	__free_page(page);
+	set_pages_wb(px_page(sp), 1);
+
+	cleanup_px(vm->dev, sp);
+	kfree(sp);
+
+	vm->scratch_page = NULL;
 }
 
 static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
@@ -2268,7 +2277,7 @@ static int ggtt_probe_common(struct drm_device *dev,
 		return -ENOMEM;
 	}
 
-	ret = setup_scratch_page(dev);
+	ret = alloc_scratch_page(&dev_priv->gtt.base);
 	if (ret) {
 		DRM_ERROR("Scratch setup failed\n");
 		/* iounmap will also get called at remove, but meh */
@@ -2447,7 +2456,7 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
 	struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base);
 
 	iounmap(gtt->gsm);
-	teardown_scratch_page(vm->dev);
+	free_scratch_page(vm);
 }
 
 static int i915_gmch_probe(struct drm_device *dev,
@@ -2511,13 +2520,13 @@ int i915_gem_gtt_init(struct drm_device *dev)
 		dev_priv->gtt.base.cleanup = gen6_gmch_remove;
 	}
 
+	gtt->base.dev = dev;
+
 	ret = gtt->gtt_probe(dev, &gtt->base.total, &gtt->stolen_size,
 			     &gtt->mappable_base, &gtt->mappable_end);
 	if (ret)
 		return ret;
 
-	gtt->base.dev = dev;
-
 	/* GMADR is the PCI mmio aperture into the global GTT. */
 	DRM_INFO("Memory usable by graphics device = %lluM\n",
 		 gtt->base.total >> 20);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f4bcec2..216d949 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -223,6 +223,10 @@ struct i915_page_dma {
 #define px_page(px) (px_base(px)->page)
 #define px_dma(px) (px_base(px)->daddr)
 
+struct i915_page_scratch {
+	struct i915_page_dma base;
+};
+
 struct i915_page_table {
 	struct i915_page_dma base;
 
@@ -249,10 +253,7 @@ struct i915_address_space {
 	u64 start;		/* Start offset always 0 for dri2 */
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
 
-	struct {
-		dma_addr_t addr;
-		struct page *page;
-	} scratch;
+	struct i915_page_scratch *scratch_page;
 
 	/**
 	 * List of objects currently involved in rendering.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/18] drm/i915/gtt: Fill scratch page

[Mika Kuoppala]

During review of dynamic page tables series, I was able
to hit a lite restore bug with execlists. I assume that
due to incorrect pd, the batch run out of legit address space
and into the scratch page area. The ACTHD was increasing
due to scratch being all zeroes (MI_NOOPs). And as gen8
address space is quite large, the hangcheck happily waited
for a long long time, keeping the process effectively stuck.

According to Chris Wilson any modern gpu will grind to halt
if it encounters commands of all ones. This seemed to do the
trick and hang was declared promptly when the gpu wandered into
the scratch land.

v2: Use 0xffff00ff pattern (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0cc0cf4..be6521f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2136,6 +2136,8 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
 	vm->cleanup(vm);
 }
 
+#define SCRATCH_PAGE_MAGIC 0xffff00ffffff00ffULL
+
 static int alloc_scratch_page(struct i915_address_space *vm)
 {
 	struct i915_page_scratch *sp;
@@ -2153,6 +2155,7 @@ static int alloc_scratch_page(struct i915_address_space *vm)
 		return ret;
 	}
 
+	fill_px(vm->dev, sp, SCRATCH_PAGE_MAGIC);
 	set_pages_uc(px_page(sp), 1);
 
 	vm->scratch_page = sp;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 13/18] drm/i915/gtt: Pin vma during virtual address allocation

[Mika Kuoppala]

Dynamic page table allocation might wake the shrinker
when memory is requested for page table structures.
As this happens when we try to allocate the virtual address
during binding, our vma might be among the targets for eviction.
We should do i915_vma_pin() and do pin early in there like Chris
suggests but this is interim solution.

Shield our vma from shrinker by incrementing pin count before
the virtual address is allocated.

The proper place to fix this would be in gem, inside of
i915_vma_pin(). But we don't have that yet so take the short
cut as a intermediate solution.

Testcase: igt/gem_ctx_thrash
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index be6521f..a3eade1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2873,9 +2873,12 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 				    vma->node.size,
 				    VM_TO_TRACE_NAME(vma->vm));
 
+		/* XXX: i915_vma_pin() will fix this +- hack */
+		vma->pin_count++;
 		ret = vma->vm->allocate_va_range(vma->vm,
 						 vma->node.start,
 						 vma->node.size);
+		vma->pin_count--;
 		if (ret)
 			return ret;
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 14/18] drm/i915/gtt: Cleanup page directory encoding

[Mika Kuoppala]

Write page directory entry without using superfluous
indirect function. Also remove unused device parameter
from the encode function.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a3eade1..03f86ce 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -192,9 +192,8 @@ static gen8_pte_t gen8_pte_encode(dma_addr_t addr,
 	return pte;
 }
 
-static gen8_pde_t gen8_pde_encode(struct drm_device *dev,
-				  dma_addr_t addr,
-				  enum i915_cache_level level)
+static gen8_pde_t gen8_pde_encode(const dma_addr_t addr,
+				  const enum i915_cache_level level)
 {
 	gen8_pde_t pde = _PAGE_PRESENT | _PAGE_RW;
 	pde |= addr;
@@ -610,15 +609,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 		kunmap_px(ppgtt, pt_vaddr);
 }
 
-static void __gen8_do_map_pt(gen8_pde_t * const pde,
-			     struct i915_page_table *pt,
-			     struct drm_device *dev)
-{
-	gen8_pde_t entry =
-		gen8_pde_encode(dev, px_dma(pt), I915_CACHE_LLC);
-	*pde = entry;
-}
-
 static void gen8_initialize_pd(struct i915_address_space *vm,
 			       struct i915_page_directory *pd)
 {
@@ -626,7 +616,7 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
 		container_of(vm, struct i915_hw_ppgtt, base);
 	gen8_pde_t scratch_pde;
 
-	scratch_pde = gen8_pde_encode(vm->dev, px_dma(ppgtt->scratch_pt),
+	scratch_pde = gen8_pde_encode(px_dma(ppgtt->scratch_pt),
 				      I915_CACHE_LLC);
 
 	fill_px(vm->dev, pd, scratch_pde);
@@ -911,7 +901,8 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 			set_bit(pde, pd->used_pdes);
 
 			/* Map the PDE to the page table */
-			__gen8_do_map_pt(page_directory + pde, pt, vm->dev);
+			page_directory[pde] = gen8_pde_encode(px_dma(pt),
+							      I915_CACHE_LLC);
 
 			/* NB: We haven't yet mapped ptes to pages. At this
 			 * point we're still relying on insert_entries() */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 15/18] drm/i915/gtt: Move scratch_pd and scratch_pt into vm area

[Mika Kuoppala]

Scratch page is part of struct i915_address_space. Move other
scratch entities into the same struct. This is a preparatory patch
for having only one instance of each scratch_pt/pd.

v2: make commit msg more readable

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> (v1)
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 51 +++++++++++++++++--------------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  7 +++--
 2 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 03f86ce..9b5e813 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -612,12 +612,9 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 static void gen8_initialize_pd(struct i915_address_space *vm,
 			       struct i915_page_directory *pd)
 {
-	struct i915_hw_ppgtt *ppgtt =
-		container_of(vm, struct i915_hw_ppgtt, base);
 	gen8_pde_t scratch_pde;
 
-	scratch_pde = gen8_pde_encode(px_dma(ppgtt->scratch_pt),
-				      I915_CACHE_LLC);
+	scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
 
 	fill_px(vm->dev, pd, scratch_pde);
 }
@@ -652,8 +649,8 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 		free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
 	}
 
-	free_pd(ppgtt->base.dev, ppgtt->scratch_pd);
-	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
+	free_pd(vm->dev, vm->scratch_pd);
+	free_pt(vm->dev, vm->scratch_pt);
 }
 
 /**
@@ -689,7 +686,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
 		/* Don't reallocate page tables */
 		if (pt) {
 			/* Scratch is never allocated this way */
-			WARN_ON(pt == ppgtt->scratch_pt);
+			WARN_ON(pt == ppgtt->base.scratch_pt);
 			continue;
 		}
 
@@ -940,16 +937,16 @@ err_out:
  */
 static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 {
-	ppgtt->scratch_pt = alloc_pt(ppgtt->base.dev);
-	if (IS_ERR(ppgtt->scratch_pt))
-		return PTR_ERR(ppgtt->scratch_pt);
+	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
+	if (IS_ERR(ppgtt->base.scratch_pt))
+		return PTR_ERR(ppgtt->base.scratch_pt);
 
-	ppgtt->scratch_pd = alloc_pd(ppgtt->base.dev);
-	if (IS_ERR(ppgtt->scratch_pd))
-		return PTR_ERR(ppgtt->scratch_pd);
+	ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
+	if (IS_ERR(ppgtt->base.scratch_pd))
+		return PTR_ERR(ppgtt->base.scratch_pd);
 
-	gen8_initialize_pt(&ppgtt->base, ppgtt->scratch_pt);
-	gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
+	gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
+	gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
 
 	ppgtt->base.start = 0;
 	ppgtt->base.total = 1ULL << 32;
@@ -981,7 +978,8 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 	uint32_t  pte, pde, temp;
 	uint32_t start = ppgtt->base.start, length = ppgtt->base.total;
 
-	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), I915_CACHE_LLC, true, 0);
+	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
+				     I915_CACHE_LLC, true, 0);
 
 	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde) {
 		u32 expected;
@@ -1314,7 +1312,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 	 * tables.
 	 */
 	gen6_for_each_pde(pt, &ppgtt->pd, start, length, temp, pde) {
-		if (pt != ppgtt->scratch_pt) {
+		if (pt != vm->scratch_pt) {
 			WARN_ON(bitmap_empty(pt->used_ptes, GEN6_PTES));
 			continue;
 		}
@@ -1369,7 +1367,7 @@ unwind_out:
 	for_each_set_bit(pde, new_page_tables, I915_PDES) {
 		struct i915_page_table *pt = ppgtt->pd.page_table[pde];
 
-		ppgtt->pd.page_table[pde] = ppgtt->scratch_pt;
+		ppgtt->pd.page_table[pde] = vm->scratch_pt;
 		free_pt(vm->dev, pt);
 	}
 
@@ -1384,15 +1382,14 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 	struct i915_page_table *pt;
 	uint32_t pde;
 
-
 	drm_mm_remove_node(&ppgtt->node);
 
 	gen6_for_all_pdes(pt, ppgtt, pde) {
-		if (pt != ppgtt->scratch_pt)
+		if (pt != vm->scratch_pt)
 			free_pt(ppgtt->base.dev, pt);
 	}
 
-	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
+	free_pt(vm->dev, vm->scratch_pt);
 }
 
 static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
@@ -1407,11 +1404,11 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
 	 * size. We allocate at the top of the GTT to avoid fragmentation.
 	 */
 	BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
-	ppgtt->scratch_pt = alloc_pt(ppgtt->base.dev);
-	if (IS_ERR(ppgtt->scratch_pt))
-		return PTR_ERR(ppgtt->scratch_pt);
+	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
+	if (IS_ERR(ppgtt->base.scratch_pt))
+		return PTR_ERR(ppgtt->base.scratch_pt);
 
-	gen6_initialize_pt(&ppgtt->base, ppgtt->scratch_pt);
+	gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
 
 alloc:
 	ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
@@ -1442,7 +1439,7 @@ alloc:
 	return 0;
 
 err_out:
-	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
+	free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
 	return ret;
 }
 
@@ -1458,7 +1455,7 @@ static void gen6_scratch_va_range(struct i915_hw_ppgtt *ppgtt,
 	uint32_t pde, temp;
 
 	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde)
-		ppgtt->pd.page_table[pde] = ppgtt->scratch_pt;
+		ppgtt->pd.page_table[pde] = ppgtt->base.scratch_pt;
 }
 
 static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 216d949..e1cfa29 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -254,6 +254,8 @@ struct i915_address_space {
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
 
 	struct i915_page_scratch *scratch_page;
+	struct i915_page_table *scratch_pt;
+	struct i915_page_directory *scratch_pd;
 
 	/**
 	 * List of objects currently involved in rendering.
@@ -343,9 +345,6 @@ struct i915_hw_ppgtt {
 		struct i915_page_directory pd;
 	};
 
-	struct i915_page_table *scratch_pt;
-	struct i915_page_directory *scratch_pd;
-
 	struct drm_i915_file_private *file_priv;
 
 	gen6_pte_t __iomem *pd_addr;
@@ -487,7 +486,7 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
 {
 	return test_bit(n, ppgtt->pdp.used_pdpes) ?
 		px_dma(ppgtt->pdp.page_directory[n]) :
-		px_dma(ppgtt->scratch_pd);
+		px_dma(ppgtt->base.scratch_pd);
 }
 
 int i915_gem_gtt_init(struct drm_device *dev);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 16/18] drm/i915/gtt: One instance of scratch page table/directory

[Mika Kuoppala]

As we use one scratch page for all ppgtt instances, we can
use one scratch page table and scratch directory across
all ppgtt instances, saving 2 pages + structs per ppgtt.

v2: Rebase
v3: Rebase

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> (v2)
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 269 +++++++++++++++++++++++-------------
 1 file changed, 175 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9b5e813..c53c934 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -433,6 +433,17 @@ fail_bitmap:
 	return ERR_PTR(ret);
 }
 
+static void gen6_initialize_pt(struct i915_address_space *vm,
+			       struct i915_page_table *pt)
+{
+	gen6_pte_t scratch_pte;
+
+	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
+				     I915_CACHE_LLC, true, 0);
+
+	fill32_px(vm->dev, pt, scratch_pte);
+}
+
 static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
 {
 	if (px_page(pd)) {
@@ -470,6 +481,156 @@ fail_bitmap:
 	return ERR_PTR(ret);
 }
 
+static void gen8_initialize_pd(struct i915_address_space *vm,
+			       struct i915_page_directory *pd)
+{
+	gen8_pde_t scratch_pde;
+
+	scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
+
+	fill_px(vm->dev, pd, scratch_pde);
+}
+
+#define SCRATCH_PAGE_MAGIC 0xffff00ffffff00ffULL
+
+static int alloc_scratch_page(struct i915_address_space *vm)
+{
+	struct i915_page_scratch *sp;
+	int ret;
+
+	WARN_ON(vm->scratch_page);
+
+	sp = kzalloc(sizeof(*sp), GFP_KERNEL);
+	if (sp == NULL)
+		return -ENOMEM;
+
+	ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
+	if (ret) {
+		kfree(sp);
+		return ret;
+	}
+
+	fill_px(vm->dev, sp, SCRATCH_PAGE_MAGIC);
+	set_pages_uc(px_page(sp), 1);
+
+	vm->scratch_page = sp;
+
+	return 0;
+}
+
+static void free_scratch_page(struct i915_address_space *vm)
+{
+	struct i915_page_scratch *sp = vm->scratch_page;
+
+	set_pages_wb(px_page(sp), 1);
+
+	cleanup_px(vm->dev, sp);
+	kfree(sp);
+
+	vm->scratch_page = NULL;
+}
+
+static int setup_scratch_ggtt(struct i915_address_space *vm)
+{
+	int ret;
+
+	ret = alloc_scratch_page(vm);
+	if (ret)
+		return ret;
+
+	WARN_ON(vm->scratch_pt);
+
+	if (INTEL_INFO(vm->dev)->gen < 6)
+		return 0;
+
+	vm->scratch_pt = alloc_pt(vm->dev);
+	if (IS_ERR(vm->scratch_pt))
+		return PTR_ERR(vm->scratch_pt);
+
+	WARN_ON(px_dma(vm->scratch_page) == 0);
+
+	if (INTEL_INFO(vm->dev)->gen >= 8) {
+		gen8_initialize_pt(vm, vm->scratch_pt);
+
+		WARN_ON(vm->scratch_pd);
+
+		vm->scratch_pd = alloc_pd(vm->dev);
+		if (IS_ERR(vm->scratch_pd)) {
+			ret = PTR_ERR(vm->scratch_pd);
+			goto err_pd;
+		}
+
+		WARN_ON(px_dma(vm->scratch_pt) == 0);
+		gen8_initialize_pd(vm, vm->scratch_pd);
+	} else {
+		gen6_initialize_pt(vm, vm->scratch_pt);
+	}
+
+	return 0;
+
+err_pd:
+	free_pt(vm->dev, vm->scratch_pt);
+	return ret;
+}
+
+static int setup_scratch(struct i915_address_space *vm)
+{
+	struct i915_address_space *ggtt_vm = &to_i915(vm->dev)->gtt.base;
+
+	if (i915_is_ggtt(vm))
+		return setup_scratch_ggtt(vm);
+
+	vm->scratch_page = ggtt_vm->scratch_page;
+	vm->scratch_pt = ggtt_vm->scratch_pt;
+	vm->scratch_pd = ggtt_vm->scratch_pd;
+
+	return 0;
+}
+
+static void check_scratch_page(struct i915_address_space *vm)
+{
+	struct i915_hw_ppgtt *ppgtt =
+		container_of(vm, struct i915_hw_ppgtt, base);
+	int i;
+	u64 *vaddr;
+
+	vaddr = kmap_px(vm->scratch_page);
+
+	for (i = 0; i < PAGE_SIZE / sizeof(u64); i++) {
+		if (vaddr[i] == SCRATCH_PAGE_MAGIC)
+			continue;
+
+		DRM_ERROR("%p scratch[%d] = 0x%08llx\n", vm, i, vaddr[i]);
+		break;
+	}
+
+	kunmap_px(ppgtt, vaddr);
+}
+
+static void cleanup_scratch_ggtt(struct i915_address_space *vm)
+{
+	check_scratch_page(vm);
+	free_scratch_page(vm);
+
+	if (INTEL_INFO(vm->dev)->gen < 6)
+		return;
+
+	free_pt(vm->dev, vm->scratch_pt);
+
+	if (INTEL_INFO(vm->dev)->gen >= 8)
+		free_pd(vm->dev, vm->scratch_pd);
+}
+
+static void cleanup_scratch(struct i915_address_space *vm)
+{
+	if (i915_is_ggtt(vm))
+		cleanup_scratch_ggtt(vm);
+
+	vm->scratch_page = NULL;
+	vm->scratch_pt = NULL;
+	vm->scratch_pd = NULL;
+}
+
 /* Broadwell Page Directory Pointer Descriptors */
 static int gen8_write_pdp(struct drm_i915_gem_request *req,
 			  unsigned entry,
@@ -525,7 +686,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 	unsigned num_entries = length >> PAGE_SHIFT;
 	unsigned last_pte, i;
 
-	scratch_pte = gen8_pte_encode(px_dma(ppgtt->base.scratch_page),
+	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
 				      I915_CACHE_LLC, use_scratch);
 
 	while (num_entries) {
@@ -609,16 +770,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 		kunmap_px(ppgtt, pt_vaddr);
 }
 
-static void gen8_initialize_pd(struct i915_address_space *vm,
-			       struct i915_page_directory *pd)
-{
-	gen8_pde_t scratch_pde;
-
-	scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
-
-	fill_px(vm->dev, pd, scratch_pde);
-}
-
 static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_device *dev)
 {
 	int i;
@@ -649,8 +800,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 		free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
 	}
 
-	free_pd(vm->dev, vm->scratch_pd);
-	free_pt(vm->dev, vm->scratch_pt);
+	cleanup_scratch(vm);
 }
 
 /**
@@ -937,16 +1087,7 @@ err_out:
  */
 static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 {
-	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
-	if (IS_ERR(ppgtt->base.scratch_pt))
-		return PTR_ERR(ppgtt->base.scratch_pt);
-
-	ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
-	if (IS_ERR(ppgtt->base.scratch_pd))
-		return PTR_ERR(ppgtt->base.scratch_pd);
-
-	gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
-	gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
+	int ret;
 
 	ppgtt->base.start = 0;
 	ppgtt->base.total = 1ULL << 32;
@@ -966,6 +1107,10 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 
 	ppgtt->switch_mm = gen8_mm_switch;
 
+	ret = setup_scratch(&ppgtt->base);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -1272,19 +1417,6 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 		kunmap_px(ppgtt, pt_vaddr);
 }
 
-static void gen6_initialize_pt(struct i915_address_space *vm,
-			       struct i915_page_table *pt)
-{
-	gen6_pte_t scratch_pte;
-
-	WARN_ON(px_dma(vm->scratch_page) == 0);
-
-	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
-				     I915_CACHE_LLC, true, 0);
-
-	fill32_px(vm->dev, pt, scratch_pte);
-}
-
 static int gen6_alloc_va_range(struct i915_address_space *vm,
 			       uint64_t start_in, uint64_t length_in)
 {
@@ -1389,7 +1521,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 			free_pt(ppgtt->base.dev, pt);
 	}
 
-	free_pt(vm->dev, vm->scratch_pt);
+	cleanup_scratch(vm);
 }
 
 static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
@@ -1404,11 +1536,10 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
 	 * size. We allocate at the top of the GTT to avoid fragmentation.
 	 */
 	BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
-	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
-	if (IS_ERR(ppgtt->base.scratch_pt))
-		return PTR_ERR(ppgtt->base.scratch_pt);
 
-	gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
+	ret = setup_scratch(&ppgtt->base);
+	if (ret)
+		return ret;
 
 alloc:
 	ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
@@ -1439,7 +1570,7 @@ alloc:
 	return 0;
 
 err_out:
-	free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
+	cleanup_scratch(&ppgtt->base);
 	return ret;
 }
 
@@ -1513,10 +1644,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 
 static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
 	ppgtt->base.dev = dev;
-	ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page;
 
 	if (INTEL_INFO(dev)->gen < 8)
 		return gen6_ppgtt_init(ppgtt);
@@ -2124,45 +2252,6 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
 	vm->cleanup(vm);
 }
 
-#define SCRATCH_PAGE_MAGIC 0xffff00ffffff00ffULL
-
-static int alloc_scratch_page(struct i915_address_space *vm)
-{
-	struct i915_page_scratch *sp;
-	int ret;
-
-	WARN_ON(vm->scratch_page);
-
-	sp = kzalloc(sizeof(*sp), GFP_KERNEL);
-	if (sp == NULL)
-		return -ENOMEM;
-
-	ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
-	if (ret) {
-		kfree(sp);
-		return ret;
-	}
-
-	fill_px(vm->dev, sp, SCRATCH_PAGE_MAGIC);
-	set_pages_uc(px_page(sp), 1);
-
-	vm->scratch_page = sp;
-
-	return 0;
-}
-
-static void free_scratch_page(struct i915_address_space *vm)
-{
-	struct i915_page_scratch *sp = vm->scratch_page;
-
-	set_pages_wb(px_page(sp), 1);
-
-	cleanup_px(vm->dev, sp);
-	kfree(sp);
-
-	vm->scratch_page = NULL;
-}
-
 static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
 {
 	snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
@@ -2246,7 +2335,6 @@ static int ggtt_probe_common(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	phys_addr_t gtt_phys_addr;
-	int ret;
 
 	/* For Modern GENs the PTEs and register space are split in the BAR */
 	gtt_phys_addr = pci_resource_start(dev->pdev, 0) +
@@ -2268,14 +2356,7 @@ static int ggtt_probe_common(struct drm_device *dev,
 		return -ENOMEM;
 	}
 
-	ret = alloc_scratch_page(&dev_priv->gtt.base);
-	if (ret) {
-		DRM_ERROR("Scratch setup failed\n");
-		/* iounmap will also get called at remove, but meh */
-		iounmap(dev_priv->gtt.gsm);
-	}
-
-	return ret;
+	return setup_scratch(&dev_priv->gtt.base);
 }
 
 /* The GGTT and PPGTT need a private PPAT setup in order to handle cacheability
@@ -2447,7 +2528,7 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
 	struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base);
 
 	iounmap(gtt->gsm);
-	free_scratch_page(vm);
+	cleanup_scratch(vm);
 }
 
 static int i915_gmch_probe(struct drm_device *dev,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 17/18] drm/i915/gtt: Use nonatomic bitmap ops

[Mika Kuoppala]

There is no need for atomicity here. Convert all bitmap
operations to nonatomic variants.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c53c934..dee25ad 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -846,7 +846,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
 
 		gen8_initialize_pt(&ppgtt->base, pt);
 		pd->page_table[pde] = pt;
-		set_bit(pde, new_pts);
+		__set_bit(pde, new_pts);
 	}
 
 	return 0;
@@ -904,7 +904,7 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
 
 		gen8_initialize_pd(&ppgtt->base, pd);
 		pdp->page_directory[pdpe] = pd;
-		set_bit(pdpe, new_pds);
+		__set_bit(pdpe, new_pds);
 	}
 
 	return 0;
@@ -1045,7 +1045,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 				   gen8_pte_count(pd_start, pd_len));
 
 			/* Our pde is now pointing to the pagetable, pt */
-			set_bit(pde, pd->used_pdes);
+			__set_bit(pde, pd->used_pdes);
 
 			/* Map the PDE to the page table */
 			page_directory[pde] = gen8_pde_encode(px_dma(pt),
@@ -1057,7 +1057,7 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
 
 		kunmap_px(ppgtt, page_directory);
 
-		set_bit(pdpe, ppgtt->pdp.used_pdpes);
+		__set_bit(pdpe, ppgtt->pdp.used_pdpes);
 	}
 
 	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
@@ -1461,7 +1461,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 		gen6_initialize_pt(vm, pt);
 
 		ppgtt->pd.page_table[pde] = pt;
-		set_bit(pde, new_page_tables);
+		__set_bit(pde, new_page_tables);
 		trace_i915_page_table_entry_alloc(vm, pde, start, GEN6_PDE_SHIFT);
 	}
 
@@ -1475,7 +1475,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 		bitmap_set(tmp_bitmap, gen6_pte_index(start),
 			   gen6_pte_count(start, length));
 
-		if (test_and_clear_bit(pde, new_page_tables))
+		if (__test_and_clear_bit(pde, new_page_tables))
 			gen6_write_pde(&ppgtt->pd, pde, pt);
 
 		trace_i915_page_table_entry_map(vm, pde, pt,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 18/18] drm/i915/gtt: Reorder page alloc/free/init functions

[Mika Kuoppala]

Maintain base page handling functions in order of
alloc, free, init. No functional changes.

v2: s/Introduce/Maintain (Michel)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 54 ++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index dee25ad..b08f623 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -384,24 +384,6 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
 	fill_page_dma(dev, p, v);
 }
 
-static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
-{
-	cleanup_px(dev, pt);
-	kfree(pt->used_ptes);
-	kfree(pt);
-}
-
-static void gen8_initialize_pt(struct i915_address_space *vm,
-			       struct i915_page_table *pt)
-{
-	gen8_pte_t scratch_pte;
-
-	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
-				      I915_CACHE_LLC, true);
-
-	fill_px(vm->dev, pt, scratch_pte);
-}
-
 static struct i915_page_table *alloc_pt(struct drm_device *dev)
 {
 	struct i915_page_table *pt;
@@ -433,6 +415,24 @@ fail_bitmap:
 	return ERR_PTR(ret);
 }
 
+static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
+{
+	cleanup_px(dev, pt);
+	kfree(pt->used_ptes);
+	kfree(pt);
+}
+
+static void gen8_initialize_pt(struct i915_address_space *vm,
+			       struct i915_page_table *pt)
+{
+	gen8_pte_t scratch_pte;
+
+	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
+				      I915_CACHE_LLC, true);
+
+	fill_px(vm->dev, pt, scratch_pte);
+}
+
 static void gen6_initialize_pt(struct i915_address_space *vm,
 			       struct i915_page_table *pt)
 {
@@ -444,15 +444,6 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
 	fill32_px(vm->dev, pt, scratch_pte);
 }
 
-static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
-{
-	if (px_page(pd)) {
-		cleanup_px(dev, pd);
-		kfree(pd->used_pdes);
-		kfree(pd);
-	}
-}
-
 static struct i915_page_directory *alloc_pd(struct drm_device *dev)
 {
 	struct i915_page_directory *pd;
@@ -481,6 +472,15 @@ fail_bitmap:
 	return ERR_PTR(ret);
 }
 
+static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
+{
+	if (px_page(pd)) {
+		cleanup_px(dev, pd);
+		kfree(pd->used_pdes);
+		kfree(pd);
+	}
+}
+
 static void gen8_initialize_pd(struct i915_address_space *vm,
 			       struct i915_page_directory *pd)
 {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/18] drm/i915/gtt: Allow >= 4GB sizes for vm.

[Michel Thierry]

On 6/25/2015 4:35 PM, Mika Kuoppala wrote:
> We can have exactly 4GB sized ppgtt with 32bit system.
> size_t is inadequate for this.
>
> v2: Convert a lot more places (Daniel)

Looks good to me.

The only possible remaining size_t are the return values in the 
*_pte_count functions in i915_gem_gtt.h, but these won't return anything 
larger than '512' (so they could be u32 if we want to get rid of size_t 
completely).

I also tried reverting commit 501fd70f ("drm/i915: limit PPGTT size to 
2GB in 32-bit platforms") in a bdw running a 32-bit kernel and it worked 
fine.

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>   drivers/char/agp/intel-gtt.c        |  4 ++--
>   drivers/gpu/drm/i915/i915_debugfs.c | 42 ++++++++++++++++++-------------------
>   drivers/gpu/drm/i915/i915_gem.c     |  6 +++---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +++++++++----------
>   drivers/gpu/drm/i915/i915_gem_gtt.h | 12 +++++------
>   include/drm/intel-gtt.h             |  4 ++--
>   6 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 0b4188b..4734d02 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -1408,8 +1408,8 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
>   }
>   EXPORT_SYMBOL(intel_gmch_probe);
>
> -void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
> -                  phys_addr_t *mappable_base, unsigned long *mappable_end)
> +void intel_gtt_get(u64 *gtt_total, size_t *stolen_size,
> +                  phys_addr_t *mappable_base, u64 *mappable_end)
>   {
>          *gtt_total = intel_private.gtt_total_entries << PAGE_SHIFT;
>          *stolen_size = intel_private.stolen_size;
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f3b8062..c654b7d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -198,7 +198,7 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          struct i915_address_space *vm = &dev_priv->gtt.base;
>          struct i915_vma *vma;
> -       size_t total_obj_size, total_gtt_size;
> +       u64 total_obj_size, total_gtt_size;
>          int count, ret;
>
>          ret = mutex_lock_interruptible(&dev->struct_mutex);
> @@ -231,7 +231,7 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
>          }
>          mutex_unlock(&dev->struct_mutex);
>
> -       seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
> +       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
>                     count, total_obj_size, total_gtt_size);
>          return 0;
>   }
> @@ -253,7 +253,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>          struct drm_device *dev = node->minor->dev;
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          struct drm_i915_gem_object *obj;
> -       size_t total_obj_size, total_gtt_size;
> +       u64 total_obj_size, total_gtt_size;
>          LIST_HEAD(stolen);
>          int count, ret;
>
> @@ -292,7 +292,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>          }
>          mutex_unlock(&dev->struct_mutex);
>
> -       seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
> +       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
>                     count, total_obj_size, total_gtt_size);
>          return 0;
>   }
> @@ -310,10 +310,10 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>
>   struct file_stats {
>          struct drm_i915_file_private *file_priv;
> -       int count;
> -       size_t total, unbound;
> -       size_t global, shared;
> -       size_t active, inactive;
> +       unsigned long count;
> +       u64 total, unbound;
> +       u64 global, shared;
> +       u64 active, inactive;
>   };
>
>   static int per_file_stats(int id, void *ptr, void *data)
> @@ -370,7 +370,7 @@ static int per_file_stats(int id, void *ptr, void *data)
>
>   #define print_file_stats(m, name, stats) do { \
>          if (stats.count) \
> -               seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu shared, %zu unbound)\n", \
> +               seq_printf(m, "%s: %lu objects, %llu bytes (%llu active, %llu inactive, %llu global, %llu shared, %llu unbound)\n", \
>                             name, \
>                             stats.count, \
>                             stats.total, \
> @@ -420,7 +420,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>          struct drm_device *dev = node->minor->dev;
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          u32 count, mappable_count, purgeable_count;
> -       size_t size, mappable_size, purgeable_size;
> +       u64 size, mappable_size, purgeable_size;
>          struct drm_i915_gem_object *obj;
>          struct i915_address_space *vm = &dev_priv->gtt.base;
>          struct drm_file *file;
> @@ -437,17 +437,17 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>
>          size = count = mappable_size = mappable_count = 0;
>          count_objects(&dev_priv->mm.bound_list, global_list);
> -       seq_printf(m, "%u [%u] objects, %zu [%zu] bytes in gtt\n",
> +       seq_printf(m, "%u [%u] objects, %llu [%llu] bytes in gtt\n",
>                     count, mappable_count, size, mappable_size);
>
>          size = count = mappable_size = mappable_count = 0;
>          count_vmas(&vm->active_list, mm_list);
> -       seq_printf(m, "  %u [%u] active objects, %zu [%zu] bytes\n",
> +       seq_printf(m, "  %u [%u] active objects, %llu [%llu] bytes\n",
>                     count, mappable_count, size, mappable_size);
>
>          size = count = mappable_size = mappable_count = 0;
>          count_vmas(&vm->inactive_list, mm_list);
> -       seq_printf(m, "  %u [%u] inactive objects, %zu [%zu] bytes\n",
> +       seq_printf(m, "  %u [%u] inactive objects, %llu [%llu] bytes\n",
>                     count, mappable_count, size, mappable_size);
>
>          size = count = purgeable_size = purgeable_count = 0;
> @@ -456,7 +456,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>                  if (obj->madv == I915_MADV_DONTNEED)
>                          purgeable_size += obj->base.size, ++purgeable_count;
>          }
> -       seq_printf(m, "%u unbound objects, %zu bytes\n", count, size);
> +       seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
>
>          size = count = mappable_size = mappable_count = 0;
>          list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> @@ -473,16 +473,16 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>                          ++purgeable_count;
>                  }
>          }
> -       seq_printf(m, "%u purgeable objects, %zu bytes\n",
> +       seq_printf(m, "%u purgeable objects, %llu bytes\n",
>                     purgeable_count, purgeable_size);
> -       seq_printf(m, "%u pinned mappable objects, %zu bytes\n",
> +       seq_printf(m, "%u pinned mappable objects, %llu bytes\n",
>                     mappable_count, mappable_size);
> -       seq_printf(m, "%u fault mappable objects, %zu bytes\n",
> +       seq_printf(m, "%u fault mappable objects, %llu bytes\n",
>                     count, size);
>
> -       seq_printf(m, "%zu [%lu] gtt total\n",
> +       seq_printf(m, "%llu [%llu] gtt total\n",
>                     dev_priv->gtt.base.total,
> -                  dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
> +                  (u64)dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
>
>          seq_putc(m, '\n');
>          print_batch_pool_stats(m, dev_priv);
> @@ -519,7 +519,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
>          uintptr_t list = (uintptr_t) node->info_ent->data;
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          struct drm_i915_gem_object *obj;
> -       size_t total_obj_size, total_gtt_size;
> +       u64 total_obj_size, total_gtt_size;
>          int count, ret;
>
>          ret = mutex_lock_interruptible(&dev->struct_mutex);
> @@ -541,7 +541,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
>
>          mutex_unlock(&dev->struct_mutex);
>
> -       seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
> +       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
>                     count, total_obj_size, total_gtt_size);
>
>          return 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 52efe43..cc5bf93 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3712,9 +3712,9 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>          struct drm_device *dev = obj->base.dev;
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          u32 size, fence_size, fence_alignment, unfenced_alignment;
> -       unsigned long start =
> +       u64 start =
>                  flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
> -       unsigned long end =
> +       u64 end =
>                  flags & PIN_MAPPABLE ? dev_priv->gtt.mappable_end : vm->total;
>          struct i915_vma *vma;
>          int ret;
> @@ -3770,7 +3770,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
>           * attempt to find space.
>           */
>          if (size > end) {
> -               DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%lu\n",
> +               DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n",
>                            ggtt_view ? ggtt_view->type : 0,
>                            size,
>                            flags & PIN_MAPPABLE ? "mappable" : "total",
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 68705e3..7a7789e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2112,7 +2112,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>   void i915_gem_init_global_gtt(struct drm_device *dev)
>   {
>          struct drm_i915_private *dev_priv = dev->dev_private;
> -       unsigned long gtt_size, mappable_size;
> +       u64 gtt_size, mappable_size;
>
>          gtt_size = dev_priv->gtt.base.total;
>          mappable_size = dev_priv->gtt.mappable_end;
> @@ -2369,13 +2369,13 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv)
>   }
>
>   static int gen8_gmch_probe(struct drm_device *dev,
> -                          size_t *gtt_total,
> +                          u64 *gtt_total,
>                             size_t *stolen,
>                             phys_addr_t *mappable_base,
> -                          unsigned long *mappable_end)
> +                          u64 *mappable_end)
>   {
>          struct drm_i915_private *dev_priv = dev->dev_private;
> -       unsigned int gtt_size;
> +       u64 gtt_size;
>          u16 snb_gmch_ctl;
>          int ret;
>
> @@ -2417,10 +2417,10 @@ static int gen8_gmch_probe(struct drm_device *dev,
>   }
>
>   static int gen6_gmch_probe(struct drm_device *dev,
> -                          size_t *gtt_total,
> +                          u64 *gtt_total,
>                             size_t *stolen,
>                             phys_addr_t *mappable_base,
> -                          unsigned long *mappable_end)
> +                          u64 *mappable_end)
>   {
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          unsigned int gtt_size;
> @@ -2434,7 +2434,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
>           * a coarse sanity check.
>           */
>          if ((*mappable_end < (64<<20) || (*mappable_end > (512<<20)))) {
> -               DRM_ERROR("Unknown GMADR size (%lx)\n",
> +               DRM_ERROR("Unknown GMADR size (%llx)\n",
>                            dev_priv->gtt.mappable_end);
>                  return -ENXIO;
>          }
> @@ -2468,10 +2468,10 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
>   }
>
>   static int i915_gmch_probe(struct drm_device *dev,
> -                          size_t *gtt_total,
> +                          u64 *gtt_total,
>                             size_t *stolen,
>                             phys_addr_t *mappable_base,
> -                          unsigned long *mappable_end)
> +                          u64 *mappable_end)
>   {
>          struct drm_i915_private *dev_priv = dev->dev_private;
>          int ret;
> @@ -2536,9 +2536,9 @@ int i915_gem_gtt_init(struct drm_device *dev)
>          gtt->base.dev = dev;
>
>          /* GMADR is the PCI mmio aperture into the global GTT. */
> -       DRM_INFO("Memory usable by graphics device = %zdM\n",
> +       DRM_INFO("Memory usable by graphics device = %lluM\n",
>                   gtt->base.total >> 20);
> -       DRM_DEBUG_DRIVER("GMADR size = %ldM\n", gtt->mappable_end >> 20);
> +       DRM_DEBUG_DRIVER("GMADR size = %lldM\n", gtt->mappable_end >> 20);
>          DRM_DEBUG_DRIVER("GTT stolen size = %zdM\n", gtt->stolen_size >> 20);
>   #ifdef CONFIG_INTEL_IOMMU
>          if (intel_iommu_gfx_mapped)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 017ea30..600eec0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -235,8 +235,8 @@ struct i915_address_space {
>          struct drm_mm mm;
>          struct drm_device *dev;
>          struct list_head global_link;
> -       unsigned long start;            /* Start offset always 0 for dri2 */
> -       size_t total;           /* size addr space maps (ex. 2GB for ggtt) */
> +       u64 start;              /* Start offset always 0 for dri2 */
> +       u64 total;              /* size addr space maps (ex. 2GB for ggtt) */
>
>          struct {
>                  dma_addr_t addr;
> @@ -302,9 +302,9 @@ struct i915_address_space {
>    */
>   struct i915_gtt {
>          struct i915_address_space base;
> -       size_t stolen_size;             /* Total size of stolen memory */
>
> -       unsigned long mappable_end;     /* End offset that we can CPU map */
> +       size_t stolen_size;             /* Total size of stolen memory */
> +       u64 mappable_end;               /* End offset that we can CPU map */
>          struct io_mapping *mappable;    /* Mapping to our CPU mappable region */
>          phys_addr_t mappable_base;      /* PA of our GMADR */
>
> @@ -316,9 +316,9 @@ struct i915_gtt {
>          int mtrr;
>
>          /* global gtt ops */
> -       int (*gtt_probe)(struct drm_device *dev, size_t *gtt_total,
> +       int (*gtt_probe)(struct drm_device *dev, u64 *gtt_total,
>                            size_t *stolen, phys_addr_t *mappable_base,
> -                         unsigned long *mappable_end);
> +                         u64 *mappable_end);
>   };
>
>   struct i915_hw_ppgtt {
> diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> index b08bdad..9e9bddaa5 100644
> --- a/include/drm/intel-gtt.h
> +++ b/include/drm/intel-gtt.h
> @@ -3,8 +3,8 @@
>   #ifndef _DRM_INTEL_GTT_H
>   #define        _DRM_INTEL_GTT_H
>
> -void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
> -                  phys_addr_t *mappable_base, unsigned long *mappable_end);
> +void intel_gtt_get(u64 *gtt_total, size_t *stolen_size,
> +                  phys_addr_t *mappable_base, u64 *mappable_end);
>
>   int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
>                       struct agp_bridge_data *bridge);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/18] drm/i915/gtt: Fill scratch page

[Chris Wilson]

On Thu, Jun 25, 2015 at 06:35:14PM +0300, Mika Kuoppala wrote:
> During review of dynamic page tables series, I was able
> to hit a lite restore bug with execlists. I assume that
> due to incorrect pd, the batch run out of legit address space
> and into the scratch page area. The ACTHD was increasing
> due to scratch being all zeroes (MI_NOOPs). And as gen8
> address space is quite large, the hangcheck happily waited
> for a long long time, keeping the process effectively stuck.
> 
> According to Chris Wilson any modern gpu will grind to halt
> if it encounters commands of all ones. This seemed to do the
> trick and hang was declared promptly when the gpu wandered into
> the scratch land.
> 
> v2: Use 0xffff00ff pattern (Chris)

Thinking about this, could we add a scratch page checker to hangcheck?
Just check the first/last u64 perhaps? Or random offset_in_page?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/18] drm/i915/gtt: Allow >= 4GB sizes for vm.

[Daniel Vetter]

On Thu, Jun 25, 2015 at 06:46:06PM +0100, Michel Thierry wrote:
> On 6/25/2015 4:35 PM, Mika Kuoppala wrote:
> >We can have exactly 4GB sized ppgtt with 32bit system.
> >size_t is inadequate for this.
> >
> >v2: Convert a lot more places (Daniel)
> 
> Looks good to me.
> 
> The only possible remaining size_t are the return values in the *_pte_count
> functions in i915_gem_gtt.h, but these won't return anything larger than
> '512' (so they could be u32 if we want to get rid of size_t completely).
> 
> I also tried reverting commit 501fd70f ("drm/i915: limit PPGTT size to 2GB
> in 32-bit platforms") in a bdw running a 32-bit kernel and it worked fine.
> 
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>

i915_gem_obj_offset is the first one I've checked and still unsigned long.
Essentially everything dealing with a g/ppgtt offset needs to be an u64,
starting from drm_mm down to gtt pte writing on the other end.

I merged up to this patch meanwhile.
-Daniel

> 
> >
> >Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >---
> >  drivers/char/agp/intel-gtt.c        |  4 ++--
> >  drivers/gpu/drm/i915/i915_debugfs.c | 42 ++++++++++++++++++-------------------
> >  drivers/gpu/drm/i915/i915_gem.c     |  6 +++---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +++++++++----------
> >  drivers/gpu/drm/i915/i915_gem_gtt.h | 12 +++++------
> >  include/drm/intel-gtt.h             |  4 ++--
> >  6 files changed, 45 insertions(+), 45 deletions(-)
> >
> >diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> >index 0b4188b..4734d02 100644
> >--- a/drivers/char/agp/intel-gtt.c
> >+++ b/drivers/char/agp/intel-gtt.c
> >@@ -1408,8 +1408,8 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> >  }
> >  EXPORT_SYMBOL(intel_gmch_probe);
> >
> >-void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
> >-                  phys_addr_t *mappable_base, unsigned long *mappable_end)
> >+void intel_gtt_get(u64 *gtt_total, size_t *stolen_size,
> >+                  phys_addr_t *mappable_base, u64 *mappable_end)
> >  {
> >         *gtt_total = intel_private.gtt_total_entries << PAGE_SHIFT;
> >         *stolen_size = intel_private.stolen_size;
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index f3b8062..c654b7d 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -198,7 +198,7 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         struct i915_address_space *vm = &dev_priv->gtt.base;
> >         struct i915_vma *vma;
> >-       size_t total_obj_size, total_gtt_size;
> >+       u64 total_obj_size, total_gtt_size;
> >         int count, ret;
> >
> >         ret = mutex_lock_interruptible(&dev->struct_mutex);
> >@@ -231,7 +231,7 @@ static int i915_gem_object_list_info(struct seq_file *m, void *data)
> >         }
> >         mutex_unlock(&dev->struct_mutex);
> >
> >-       seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
> >+       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
> >                    count, total_obj_size, total_gtt_size);
> >         return 0;
> >  }
> >@@ -253,7 +253,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> >         struct drm_device *dev = node->minor->dev;
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         struct drm_i915_gem_object *obj;
> >-       size_t total_obj_size, total_gtt_size;
> >+       u64 total_obj_size, total_gtt_size;
> >         LIST_HEAD(stolen);
> >         int count, ret;
> >
> >@@ -292,7 +292,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> >         }
> >         mutex_unlock(&dev->struct_mutex);
> >
> >-       seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
> >+       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
> >                    count, total_obj_size, total_gtt_size);
> >         return 0;
> >  }
> >@@ -310,10 +310,10 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> >
> >  struct file_stats {
> >         struct drm_i915_file_private *file_priv;
> >-       int count;
> >-       size_t total, unbound;
> >-       size_t global, shared;
> >-       size_t active, inactive;
> >+       unsigned long count;
> >+       u64 total, unbound;
> >+       u64 global, shared;
> >+       u64 active, inactive;
> >  };
> >
> >  static int per_file_stats(int id, void *ptr, void *data)
> >@@ -370,7 +370,7 @@ static int per_file_stats(int id, void *ptr, void *data)
> >
> >  #define print_file_stats(m, name, stats) do { \
> >         if (stats.count) \
> >-               seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu shared, %zu unbound)\n", \
> >+               seq_printf(m, "%s: %lu objects, %llu bytes (%llu active, %llu inactive, %llu global, %llu shared, %llu unbound)\n", \
> >                            name, \
> >                            stats.count, \
> >                            stats.total, \
> >@@ -420,7 +420,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> >         struct drm_device *dev = node->minor->dev;
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         u32 count, mappable_count, purgeable_count;
> >-       size_t size, mappable_size, purgeable_size;
> >+       u64 size, mappable_size, purgeable_size;
> >         struct drm_i915_gem_object *obj;
> >         struct i915_address_space *vm = &dev_priv->gtt.base;
> >         struct drm_file *file;
> >@@ -437,17 +437,17 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> >
> >         size = count = mappable_size = mappable_count = 0;
> >         count_objects(&dev_priv->mm.bound_list, global_list);
> >-       seq_printf(m, "%u [%u] objects, %zu [%zu] bytes in gtt\n",
> >+       seq_printf(m, "%u [%u] objects, %llu [%llu] bytes in gtt\n",
> >                    count, mappable_count, size, mappable_size);
> >
> >         size = count = mappable_size = mappable_count = 0;
> >         count_vmas(&vm->active_list, mm_list);
> >-       seq_printf(m, "  %u [%u] active objects, %zu [%zu] bytes\n",
> >+       seq_printf(m, "  %u [%u] active objects, %llu [%llu] bytes\n",
> >                    count, mappable_count, size, mappable_size);
> >
> >         size = count = mappable_size = mappable_count = 0;
> >         count_vmas(&vm->inactive_list, mm_list);
> >-       seq_printf(m, "  %u [%u] inactive objects, %zu [%zu] bytes\n",
> >+       seq_printf(m, "  %u [%u] inactive objects, %llu [%llu] bytes\n",
> >                    count, mappable_count, size, mappable_size);
> >
> >         size = count = purgeable_size = purgeable_count = 0;
> >@@ -456,7 +456,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> >                 if (obj->madv == I915_MADV_DONTNEED)
> >                         purgeable_size += obj->base.size, ++purgeable_count;
> >         }
> >-       seq_printf(m, "%u unbound objects, %zu bytes\n", count, size);
> >+       seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
> >
> >         size = count = mappable_size = mappable_count = 0;
> >         list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> >@@ -473,16 +473,16 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> >                         ++purgeable_count;
> >                 }
> >         }
> >-       seq_printf(m, "%u purgeable objects, %zu bytes\n",
> >+       seq_printf(m, "%u purgeable objects, %llu bytes\n",
> >                    purgeable_count, purgeable_size);
> >-       seq_printf(m, "%u pinned mappable objects, %zu bytes\n",
> >+       seq_printf(m, "%u pinned mappable objects, %llu bytes\n",
> >                    mappable_count, mappable_size);
> >-       seq_printf(m, "%u fault mappable objects, %zu bytes\n",
> >+       seq_printf(m, "%u fault mappable objects, %llu bytes\n",
> >                    count, size);
> >
> >-       seq_printf(m, "%zu [%lu] gtt total\n",
> >+       seq_printf(m, "%llu [%llu] gtt total\n",
> >                    dev_priv->gtt.base.total,
> >-                  dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
> >+                  (u64)dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
> >
> >         seq_putc(m, '\n');
> >         print_batch_pool_stats(m, dev_priv);
> >@@ -519,7 +519,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
> >         uintptr_t list = (uintptr_t) node->info_ent->data;
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         struct drm_i915_gem_object *obj;
> >-       size_t total_obj_size, total_gtt_size;
> >+       u64 total_obj_size, total_gtt_size;
> >         int count, ret;
> >
> >         ret = mutex_lock_interruptible(&dev->struct_mutex);
> >@@ -541,7 +541,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
> >
> >         mutex_unlock(&dev->struct_mutex);
> >
> >-       seq_printf(m, "Total %d objects, %zu bytes, %zu GTT size\n",
> >+       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
> >                    count, total_obj_size, total_gtt_size);
> >
> >         return 0;
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 52efe43..cc5bf93 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -3712,9 +3712,9 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
> >         struct drm_device *dev = obj->base.dev;
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         u32 size, fence_size, fence_alignment, unfenced_alignment;
> >-       unsigned long start =
> >+       u64 start =
> >                 flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
> >-       unsigned long end =
> >+       u64 end =
> >                 flags & PIN_MAPPABLE ? dev_priv->gtt.mappable_end : vm->total;
> >         struct i915_vma *vma;
> >         int ret;
> >@@ -3770,7 +3770,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj,
> >          * attempt to find space.
> >          */
> >         if (size > end) {
> >-               DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%lu\n",
> >+               DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n",
> >                           ggtt_view ? ggtt_view->type : 0,
> >                           size,
> >                           flags & PIN_MAPPABLE ? "mappable" : "total",
> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >index 68705e3..7a7789e 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >@@ -2112,7 +2112,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
> >  void i915_gem_init_global_gtt(struct drm_device *dev)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >-       unsigned long gtt_size, mappable_size;
> >+       u64 gtt_size, mappable_size;
> >
> >         gtt_size = dev_priv->gtt.base.total;
> >         mappable_size = dev_priv->gtt.mappable_end;
> >@@ -2369,13 +2369,13 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv)
> >  }
> >
> >  static int gen8_gmch_probe(struct drm_device *dev,
> >-                          size_t *gtt_total,
> >+                          u64 *gtt_total,
> >                            size_t *stolen,
> >                            phys_addr_t *mappable_base,
> >-                          unsigned long *mappable_end)
> >+                          u64 *mappable_end)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >-       unsigned int gtt_size;
> >+       u64 gtt_size;
> >         u16 snb_gmch_ctl;
> >         int ret;
> >
> >@@ -2417,10 +2417,10 @@ static int gen8_gmch_probe(struct drm_device *dev,
> >  }
> >
> >  static int gen6_gmch_probe(struct drm_device *dev,
> >-                          size_t *gtt_total,
> >+                          u64 *gtt_total,
> >                            size_t *stolen,
> >                            phys_addr_t *mappable_base,
> >-                          unsigned long *mappable_end)
> >+                          u64 *mappable_end)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         unsigned int gtt_size;
> >@@ -2434,7 +2434,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
> >          * a coarse sanity check.
> >          */
> >         if ((*mappable_end < (64<<20) || (*mappable_end > (512<<20)))) {
> >-               DRM_ERROR("Unknown GMADR size (%lx)\n",
> >+               DRM_ERROR("Unknown GMADR size (%llx)\n",
> >                           dev_priv->gtt.mappable_end);
> >                 return -ENXIO;
> >         }
> >@@ -2468,10 +2468,10 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
> >  }
> >
> >  static int i915_gmch_probe(struct drm_device *dev,
> >-                          size_t *gtt_total,
> >+                          u64 *gtt_total,
> >                            size_t *stolen,
> >                            phys_addr_t *mappable_base,
> >-                          unsigned long *mappable_end)
> >+                          u64 *mappable_end)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         int ret;
> >@@ -2536,9 +2536,9 @@ int i915_gem_gtt_init(struct drm_device *dev)
> >         gtt->base.dev = dev;
> >
> >         /* GMADR is the PCI mmio aperture into the global GTT. */
> >-       DRM_INFO("Memory usable by graphics device = %zdM\n",
> >+       DRM_INFO("Memory usable by graphics device = %lluM\n",
> >                  gtt->base.total >> 20);
> >-       DRM_DEBUG_DRIVER("GMADR size = %ldM\n", gtt->mappable_end >> 20);
> >+       DRM_DEBUG_DRIVER("GMADR size = %lldM\n", gtt->mappable_end >> 20);
> >         DRM_DEBUG_DRIVER("GTT stolen size = %zdM\n", gtt->stolen_size >> 20);
> >  #ifdef CONFIG_INTEL_IOMMU
> >         if (intel_iommu_gfx_mapped)
> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >index 017ea30..600eec0 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >@@ -235,8 +235,8 @@ struct i915_address_space {
> >         struct drm_mm mm;
> >         struct drm_device *dev;
> >         struct list_head global_link;
> >-       unsigned long start;            /* Start offset always 0 for dri2 */
> >-       size_t total;           /* size addr space maps (ex. 2GB for ggtt) */
> >+       u64 start;              /* Start offset always 0 for dri2 */
> >+       u64 total;              /* size addr space maps (ex. 2GB for ggtt) */
> >
> >         struct {
> >                 dma_addr_t addr;
> >@@ -302,9 +302,9 @@ struct i915_address_space {
> >   */
> >  struct i915_gtt {
> >         struct i915_address_space base;
> >-       size_t stolen_size;             /* Total size of stolen memory */
> >
> >-       unsigned long mappable_end;     /* End offset that we can CPU map */
> >+       size_t stolen_size;             /* Total size of stolen memory */
> >+       u64 mappable_end;               /* End offset that we can CPU map */
> >         struct io_mapping *mappable;    /* Mapping to our CPU mappable region */
> >         phys_addr_t mappable_base;      /* PA of our GMADR */
> >
> >@@ -316,9 +316,9 @@ struct i915_gtt {
> >         int mtrr;
> >
> >         /* global gtt ops */
> >-       int (*gtt_probe)(struct drm_device *dev, size_t *gtt_total,
> >+       int (*gtt_probe)(struct drm_device *dev, u64 *gtt_total,
> >                           size_t *stolen, phys_addr_t *mappable_base,
> >-                         unsigned long *mappable_end);
> >+                         u64 *mappable_end);
> >  };
> >
> >  struct i915_hw_ppgtt {
> >diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> >index b08bdad..9e9bddaa5 100644
> >--- a/include/drm/intel-gtt.h
> >+++ b/include/drm/intel-gtt.h
> >@@ -3,8 +3,8 @@
> >  #ifndef _DRM_INTEL_GTT_H
> >  #define        _DRM_INTEL_GTT_H
> >
> >-void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
> >-                  phys_addr_t *mappable_base, unsigned long *mappable_end);
> >+void intel_gtt_get(u64 *gtt_total, size_t *stolen_size,
> >+                  phys_addr_t *mappable_base, u64 *mappable_end);
> >
> >  int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> >                      struct agp_bridge_data *bridge);
> >--
> >1.9.1
> >
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 15/18] drm/i915/gtt: Move scratch_pd and scratch_pt into vm area

[Daniel Vetter]

On Thu, Jun 25, 2015 at 06:35:17PM +0300, Mika Kuoppala wrote:
> Scratch page is part of struct i915_address_space. Move other
> scratch entities into the same struct. This is a preparatory patch
> for having only one instance of each scratch_pt/pd.
> 
> v2: make commit msg more readable
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com> (v1)

I bikesheded the summary from "vm area" to vm struct. Since vm area is the
vma we already have to track obj bindings into a vm ;-)
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 51 +++++++++++++++++--------------------
>  drivers/gpu/drm/i915/i915_gem_gtt.h |  7 +++--
>  2 files changed, 27 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 03f86ce..9b5e813 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -612,12 +612,9 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>  static void gen8_initialize_pd(struct i915_address_space *vm,
>  			       struct i915_page_directory *pd)
>  {
> -	struct i915_hw_ppgtt *ppgtt =
> -		container_of(vm, struct i915_hw_ppgtt, base);
>  	gen8_pde_t scratch_pde;
>  
> -	scratch_pde = gen8_pde_encode(px_dma(ppgtt->scratch_pt),
> -				      I915_CACHE_LLC);
> +	scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
>  
>  	fill_px(vm->dev, pd, scratch_pde);
>  }
> @@ -652,8 +649,8 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
>  		free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
>  	}
>  
> -	free_pd(ppgtt->base.dev, ppgtt->scratch_pd);
> -	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
> +	free_pd(vm->dev, vm->scratch_pd);
> +	free_pt(vm->dev, vm->scratch_pt);
>  }
>  
>  /**
> @@ -689,7 +686,7 @@ static int gen8_ppgtt_alloc_pagetabs(struct i915_hw_ppgtt *ppgtt,
>  		/* Don't reallocate page tables */
>  		if (pt) {
>  			/* Scratch is never allocated this way */
> -			WARN_ON(pt == ppgtt->scratch_pt);
> +			WARN_ON(pt == ppgtt->base.scratch_pt);
>  			continue;
>  		}
>  
> @@ -940,16 +937,16 @@ err_out:
>   */
>  static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  {
> -	ppgtt->scratch_pt = alloc_pt(ppgtt->base.dev);
> -	if (IS_ERR(ppgtt->scratch_pt))
> -		return PTR_ERR(ppgtt->scratch_pt);
> +	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
> +	if (IS_ERR(ppgtt->base.scratch_pt))
> +		return PTR_ERR(ppgtt->base.scratch_pt);
>  
> -	ppgtt->scratch_pd = alloc_pd(ppgtt->base.dev);
> -	if (IS_ERR(ppgtt->scratch_pd))
> -		return PTR_ERR(ppgtt->scratch_pd);
> +	ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
> +	if (IS_ERR(ppgtt->base.scratch_pd))
> +		return PTR_ERR(ppgtt->base.scratch_pd);
>  
> -	gen8_initialize_pt(&ppgtt->base, ppgtt->scratch_pt);
> -	gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
> +	gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
> +	gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
>  
>  	ppgtt->base.start = 0;
>  	ppgtt->base.total = 1ULL << 32;
> @@ -981,7 +978,8 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
>  	uint32_t  pte, pde, temp;
>  	uint32_t start = ppgtt->base.start, length = ppgtt->base.total;
>  
> -	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), I915_CACHE_LLC, true, 0);
> +	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
> +				     I915_CACHE_LLC, true, 0);
>  
>  	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde) {
>  		u32 expected;
> @@ -1314,7 +1312,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
>  	 * tables.
>  	 */
>  	gen6_for_each_pde(pt, &ppgtt->pd, start, length, temp, pde) {
> -		if (pt != ppgtt->scratch_pt) {
> +		if (pt != vm->scratch_pt) {
>  			WARN_ON(bitmap_empty(pt->used_ptes, GEN6_PTES));
>  			continue;
>  		}
> @@ -1369,7 +1367,7 @@ unwind_out:
>  	for_each_set_bit(pde, new_page_tables, I915_PDES) {
>  		struct i915_page_table *pt = ppgtt->pd.page_table[pde];
>  
> -		ppgtt->pd.page_table[pde] = ppgtt->scratch_pt;
> +		ppgtt->pd.page_table[pde] = vm->scratch_pt;
>  		free_pt(vm->dev, pt);
>  	}
>  
> @@ -1384,15 +1382,14 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>  	struct i915_page_table *pt;
>  	uint32_t pde;
>  
> -
>  	drm_mm_remove_node(&ppgtt->node);
>  
>  	gen6_for_all_pdes(pt, ppgtt, pde) {
> -		if (pt != ppgtt->scratch_pt)
> +		if (pt != vm->scratch_pt)
>  			free_pt(ppgtt->base.dev, pt);
>  	}
>  
> -	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
> +	free_pt(vm->dev, vm->scratch_pt);
>  }
>  
>  static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
> @@ -1407,11 +1404,11 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
>  	 * size. We allocate at the top of the GTT to avoid fragmentation.
>  	 */
>  	BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
> -	ppgtt->scratch_pt = alloc_pt(ppgtt->base.dev);
> -	if (IS_ERR(ppgtt->scratch_pt))
> -		return PTR_ERR(ppgtt->scratch_pt);
> +	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
> +	if (IS_ERR(ppgtt->base.scratch_pt))
> +		return PTR_ERR(ppgtt->base.scratch_pt);
>  
> -	gen6_initialize_pt(&ppgtt->base, ppgtt->scratch_pt);
> +	gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
>  
>  alloc:
>  	ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
> @@ -1442,7 +1439,7 @@ alloc:
>  	return 0;
>  
>  err_out:
> -	free_pt(ppgtt->base.dev, ppgtt->scratch_pt);
> +	free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
>  	return ret;
>  }
>  
> @@ -1458,7 +1455,7 @@ static void gen6_scratch_va_range(struct i915_hw_ppgtt *ppgtt,
>  	uint32_t pde, temp;
>  
>  	gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde)
> -		ppgtt->pd.page_table[pde] = ppgtt->scratch_pt;
> +		ppgtt->pd.page_table[pde] = ppgtt->base.scratch_pt;
>  }
>  
>  static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 216d949..e1cfa29 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -254,6 +254,8 @@ struct i915_address_space {
>  	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
>  
>  	struct i915_page_scratch *scratch_page;
> +	struct i915_page_table *scratch_pt;
> +	struct i915_page_directory *scratch_pd;
>  
>  	/**
>  	 * List of objects currently involved in rendering.
> @@ -343,9 +345,6 @@ struct i915_hw_ppgtt {
>  		struct i915_page_directory pd;
>  	};
>  
> -	struct i915_page_table *scratch_pt;
> -	struct i915_page_directory *scratch_pd;
> -
>  	struct drm_i915_file_private *file_priv;
>  
>  	gen6_pte_t __iomem *pd_addr;
> @@ -487,7 +486,7 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
>  {
>  	return test_bit(n, ppgtt->pdp.used_pdpes) ?
>  		px_dma(ppgtt->pdp.page_directory[n]) :
> -		px_dma(ppgtt->scratch_pd);
> +		px_dma(ppgtt->base.scratch_pd);
>  }
>  
>  int i915_gem_gtt_init(struct drm_device *dev);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 16/18] drm/i915/gtt: One instance of scratch page table/directory

[Daniel Vetter]

On Thu, Jun 25, 2015 at 06:35:18PM +0300, Mika Kuoppala wrote:
> +static int setup_scratch(struct i915_address_space *vm)
> +{
> +	struct i915_address_space *ggtt_vm = &to_i915(vm->dev)->gtt.base;
> +
> +	if (i915_is_ggtt(vm))
> +		return setup_scratch_ggtt(vm);
> +
> +	vm->scratch_page = ggtt_vm->scratch_page;
> +	vm->scratch_pt = ggtt_vm->scratch_pt;
> +	vm->scratch_pd = ggtt_vm->scratch_pd;
> +
> +	return 0;
> +}

The point of a ppgtt is full isolation, sharing the scratch page destroys
that. Hence nack. If you want a bit of polish, renaming initialize_pd/pt
to initialize_scratch_pd/pt would make sense though I think.
-Daniel

> +
> +static void check_scratch_page(struct i915_address_space *vm)
> +{
> +	struct i915_hw_ppgtt *ppgtt =
> +		container_of(vm, struct i915_hw_ppgtt, base);
> +	int i;
> +	u64 *vaddr;
> +
> +	vaddr = kmap_px(vm->scratch_page);
> +
> +	for (i = 0; i < PAGE_SIZE / sizeof(u64); i++) {
> +		if (vaddr[i] == SCRATCH_PAGE_MAGIC)
> +			continue;
> +
> +		DRM_ERROR("%p scratch[%d] = 0x%08llx\n", vm, i, vaddr[i]);
> +		break;
> +	}
> +
> +	kunmap_px(ppgtt, vaddr);
> +}
> +
> +static void cleanup_scratch_ggtt(struct i915_address_space *vm)
> +{
> +	check_scratch_page(vm);
> +	free_scratch_page(vm);
> +
> +	if (INTEL_INFO(vm->dev)->gen < 6)
> +		return;
> +
> +	free_pt(vm->dev, vm->scratch_pt);
> +
> +	if (INTEL_INFO(vm->dev)->gen >= 8)
> +		free_pd(vm->dev, vm->scratch_pd);
> +}
> +
> +static void cleanup_scratch(struct i915_address_space *vm)
> +{
> +	if (i915_is_ggtt(vm))
> +		cleanup_scratch_ggtt(vm);
> +
> +	vm->scratch_page = NULL;
> +	vm->scratch_pt = NULL;
> +	vm->scratch_pd = NULL;
> +}
> +
>  /* Broadwell Page Directory Pointer Descriptors */
>  static int gen8_write_pdp(struct drm_i915_gem_request *req,
>  			  unsigned entry,
> @@ -525,7 +686,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
>  	unsigned num_entries = length >> PAGE_SHIFT;
>  	unsigned last_pte, i;
>  
> -	scratch_pte = gen8_pte_encode(px_dma(ppgtt->base.scratch_page),
> +	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
>  				      I915_CACHE_LLC, use_scratch);
>  
>  	while (num_entries) {
> @@ -609,16 +770,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>  		kunmap_px(ppgtt, pt_vaddr);
>  }
>  
> -static void gen8_initialize_pd(struct i915_address_space *vm,
> -			       struct i915_page_directory *pd)
> -{
> -	gen8_pde_t scratch_pde;
> -
> -	scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
> -
> -	fill_px(vm->dev, pd, scratch_pde);
> -}
> -
>  static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_device *dev)
>  {
>  	int i;
> @@ -649,8 +800,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
>  		free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
>  	}
>  
> -	free_pd(vm->dev, vm->scratch_pd);
> -	free_pt(vm->dev, vm->scratch_pt);
> +	cleanup_scratch(vm);
>  }
>  
>  /**
> @@ -937,16 +1087,7 @@ err_out:
>   */
>  static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  {
> -	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
> -	if (IS_ERR(ppgtt->base.scratch_pt))
> -		return PTR_ERR(ppgtt->base.scratch_pt);
> -
> -	ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
> -	if (IS_ERR(ppgtt->base.scratch_pd))
> -		return PTR_ERR(ppgtt->base.scratch_pd);
> -
> -	gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
> -	gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
> +	int ret;
>  
>  	ppgtt->base.start = 0;
>  	ppgtt->base.total = 1ULL << 32;
> @@ -966,6 +1107,10 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  
>  	ppgtt->switch_mm = gen8_mm_switch;
>  
> +	ret = setup_scratch(&ppgtt->base);
> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }
>  
> @@ -1272,19 +1417,6 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
>  		kunmap_px(ppgtt, pt_vaddr);
>  }
>  
> -static void gen6_initialize_pt(struct i915_address_space *vm,
> -			       struct i915_page_table *pt)
> -{
> -	gen6_pte_t scratch_pte;
> -
> -	WARN_ON(px_dma(vm->scratch_page) == 0);
> -
> -	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
> -				     I915_CACHE_LLC, true, 0);
> -
> -	fill32_px(vm->dev, pt, scratch_pte);
> -}
> -
>  static int gen6_alloc_va_range(struct i915_address_space *vm,
>  			       uint64_t start_in, uint64_t length_in)
>  {
> @@ -1389,7 +1521,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>  			free_pt(ppgtt->base.dev, pt);
>  	}
>  
> -	free_pt(vm->dev, vm->scratch_pt);
> +	cleanup_scratch(vm);
>  }
>  
>  static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
> @@ -1404,11 +1536,10 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
>  	 * size. We allocate at the top of the GTT to avoid fragmentation.
>  	 */
>  	BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
> -	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
> -	if (IS_ERR(ppgtt->base.scratch_pt))
> -		return PTR_ERR(ppgtt->base.scratch_pt);
>  
> -	gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
> +	ret = setup_scratch(&ppgtt->base);
> +	if (ret)
> +		return ret;
>  
>  alloc:
>  	ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
> @@ -1439,7 +1570,7 @@ alloc:
>  	return 0;
>  
>  err_out:
> -	free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
> +	cleanup_scratch(&ppgtt->base);
>  	return ret;
>  }
>  
> @@ -1513,10 +1644,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>  
>  static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
>  	ppgtt->base.dev = dev;
> -	ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page;
>  
>  	if (INTEL_INFO(dev)->gen < 8)
>  		return gen6_ppgtt_init(ppgtt);
> @@ -2124,45 +2252,6 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
>  	vm->cleanup(vm);
>  }
>  
> -#define SCRATCH_PAGE_MAGIC 0xffff00ffffff00ffULL
> -
> -static int alloc_scratch_page(struct i915_address_space *vm)
> -{
> -	struct i915_page_scratch *sp;
> -	int ret;
> -
> -	WARN_ON(vm->scratch_page);
> -
> -	sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> -	if (sp == NULL)
> -		return -ENOMEM;
> -
> -	ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
> -	if (ret) {
> -		kfree(sp);
> -		return ret;
> -	}
> -
> -	fill_px(vm->dev, sp, SCRATCH_PAGE_MAGIC);
> -	set_pages_uc(px_page(sp), 1);
> -
> -	vm->scratch_page = sp;
> -
> -	return 0;
> -}
> -
> -static void free_scratch_page(struct i915_address_space *vm)
> -{
> -	struct i915_page_scratch *sp = vm->scratch_page;
> -
> -	set_pages_wb(px_page(sp), 1);
> -
> -	cleanup_px(vm->dev, sp);
> -	kfree(sp);
> -
> -	vm->scratch_page = NULL;
> -}
> -
>  static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
>  {
>  	snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
> @@ -2246,7 +2335,6 @@ static int ggtt_probe_common(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	phys_addr_t gtt_phys_addr;
> -	int ret;
>  
>  	/* For Modern GENs the PTEs and register space are split in the BAR */
>  	gtt_phys_addr = pci_resource_start(dev->pdev, 0) +
> @@ -2268,14 +2356,7 @@ static int ggtt_probe_common(struct drm_device *dev,
>  		return -ENOMEM;
>  	}
>  
> -	ret = alloc_scratch_page(&dev_priv->gtt.base);
> -	if (ret) {
> -		DRM_ERROR("Scratch setup failed\n");
> -		/* iounmap will also get called at remove, but meh */
> -		iounmap(dev_priv->gtt.gsm);
> -	}
> -
> -	return ret;
> +	return setup_scratch(&dev_priv->gtt.base);
>  }
>  
>  /* The GGTT and PPGTT need a private PPAT setup in order to handle cacheability
> @@ -2447,7 +2528,7 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
>  	struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base);
>  
>  	iounmap(gtt->gsm);
> -	free_scratch_page(vm);
> +	cleanup_scratch(vm);
>  }
>  
>  static int i915_gmch_probe(struct drm_device *dev,
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 18/18] drm/i915/gtt: Reorder page alloc/free/init functions

[Daniel Vetter]

On Thu, Jun 25, 2015 at 06:35:20PM +0300, Mika Kuoppala wrote:
> Maintain base page handling functions in order of
> alloc, free, init. No functional changes.
> 
> v2: s/Introduce/Maintain (Michel)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>

Conflicts since I didn't apply the scratch merging. Same comment about
adding _scratch to these functions (where it makes sense ofc). I merged
all the other reviewed patches to dinq.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 54 ++++++++++++++++++-------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index dee25ad..b08f623 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -384,24 +384,6 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
>  	fill_page_dma(dev, p, v);
>  }
>  
> -static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
> -{
> -	cleanup_px(dev, pt);
> -	kfree(pt->used_ptes);
> -	kfree(pt);
> -}
> -
> -static void gen8_initialize_pt(struct i915_address_space *vm,
> -			       struct i915_page_table *pt)
> -{
> -	gen8_pte_t scratch_pte;
> -
> -	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> -				      I915_CACHE_LLC, true);
> -
> -	fill_px(vm->dev, pt, scratch_pte);
> -}
> -
>  static struct i915_page_table *alloc_pt(struct drm_device *dev)
>  {
>  	struct i915_page_table *pt;
> @@ -433,6 +415,24 @@ fail_bitmap:
>  	return ERR_PTR(ret);
>  }
>  
> +static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
> +{
> +	cleanup_px(dev, pt);
> +	kfree(pt->used_ptes);
> +	kfree(pt);
> +}
> +
> +static void gen8_initialize_pt(struct i915_address_space *vm,
> +			       struct i915_page_table *pt)
> +{
> +	gen8_pte_t scratch_pte;
> +
> +	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> +				      I915_CACHE_LLC, true);
> +
> +	fill_px(vm->dev, pt, scratch_pte);
> +}
> +
>  static void gen6_initialize_pt(struct i915_address_space *vm,
>  			       struct i915_page_table *pt)
>  {
> @@ -444,15 +444,6 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
>  	fill32_px(vm->dev, pt, scratch_pte);
>  }
>  
> -static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
> -{
> -	if (px_page(pd)) {
> -		cleanup_px(dev, pd);
> -		kfree(pd->used_pdes);
> -		kfree(pd);
> -	}
> -}
> -
>  static struct i915_page_directory *alloc_pd(struct drm_device *dev)
>  {
>  	struct i915_page_directory *pd;
> @@ -481,6 +472,15 @@ fail_bitmap:
>  	return ERR_PTR(ret);
>  }
>  
> +static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
> +{
> +	if (px_page(pd)) {
> +		cleanup_px(dev, pd);
> +		kfree(pd->used_pdes);
> +		kfree(pd);
> +	}
> +}
> +
>  static void gen8_initialize_pd(struct i915_address_space *vm,
>  			       struct i915_page_directory *pd)
>  {
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 16/18] drm/i915/gtt: One instance of scratch page table/directory

[Mika Kuoppala]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Thu, Jun 25, 2015 at 06:35:18PM +0300, Mika Kuoppala wrote:
>> +static int setup_scratch(struct i915_address_space *vm)
>> +{
>> +	struct i915_address_space *ggtt_vm = &to_i915(vm->dev)->gtt.base;
>> +
>> +	if (i915_is_ggtt(vm))
>> +		return setup_scratch_ggtt(vm);
>> +
>> +	vm->scratch_page = ggtt_vm->scratch_page;
>> +	vm->scratch_pt = ggtt_vm->scratch_pt;
>> +	vm->scratch_pd = ggtt_vm->scratch_pd;
>> +
>> +	return 0;
>> +}
>
> The point of a ppgtt is full isolation, sharing the scratch page destroys
> that. Hence nack. If you want a bit of polish, renaming initialize_pd/pt
> to initialize_scratch_pd/pt would make sense though I think.
> -Daniel

We already have a shared scratch. This just makes the upper layer
structures also shared as there is no point of having identical
scratch pt and scratch pd pointing to the same scratch.

Do we want per ppgtt scratch? I have patches for that also.

-Mika

>> +
>> +static void check_scratch_page(struct i915_address_space *vm)
>> +{
>> +	struct i915_hw_ppgtt *ppgtt =
>> +		container_of(vm, struct i915_hw_ppgtt, base);
>> +	int i;
>> +	u64 *vaddr;
>> +
>> +	vaddr = kmap_px(vm->scratch_page);
>> +
>> +	for (i = 0; i < PAGE_SIZE / sizeof(u64); i++) {
>> +		if (vaddr[i] == SCRATCH_PAGE_MAGIC)
>> +			continue;
>> +
>> +		DRM_ERROR("%p scratch[%d] = 0x%08llx\n", vm, i, vaddr[i]);
>> +		break;
>> +	}
>> +
>> +	kunmap_px(ppgtt, vaddr);
>> +}
>> +
>> +static void cleanup_scratch_ggtt(struct i915_address_space *vm)
>> +{
>> +	check_scratch_page(vm);
>> +	free_scratch_page(vm);
>> +
>> +	if (INTEL_INFO(vm->dev)->gen < 6)
>> +		return;
>> +
>> +	free_pt(vm->dev, vm->scratch_pt);
>> +
>> +	if (INTEL_INFO(vm->dev)->gen >= 8)
>> +		free_pd(vm->dev, vm->scratch_pd);
>> +}
>> +
>> +static void cleanup_scratch(struct i915_address_space *vm)
>> +{
>> +	if (i915_is_ggtt(vm))
>> +		cleanup_scratch_ggtt(vm);
>> +
>> +	vm->scratch_page = NULL;
>> +	vm->scratch_pt = NULL;
>> +	vm->scratch_pd = NULL;
>> +}
>> +
>>  /* Broadwell Page Directory Pointer Descriptors */
>>  static int gen8_write_pdp(struct drm_i915_gem_request *req,
>>  			  unsigned entry,
>> @@ -525,7 +686,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
>>  	unsigned num_entries = length >> PAGE_SHIFT;
>>  	unsigned last_pte, i;
>>  
>> -	scratch_pte = gen8_pte_encode(px_dma(ppgtt->base.scratch_page),
>> +	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
>>  				      I915_CACHE_LLC, use_scratch);
>>  
>>  	while (num_entries) {
>> @@ -609,16 +770,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>>  		kunmap_px(ppgtt, pt_vaddr);
>>  }
>>  
>> -static void gen8_initialize_pd(struct i915_address_space *vm,
>> -			       struct i915_page_directory *pd)
>> -{
>> -	gen8_pde_t scratch_pde;
>> -
>> -	scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
>> -
>> -	fill_px(vm->dev, pd, scratch_pde);
>> -}
>> -
>>  static void gen8_free_page_tables(struct i915_page_directory *pd, struct drm_device *dev)
>>  {
>>  	int i;
>> @@ -649,8 +800,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
>>  		free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
>>  	}
>>  
>> -	free_pd(vm->dev, vm->scratch_pd);
>> -	free_pt(vm->dev, vm->scratch_pt);
>> +	cleanup_scratch(vm);
>>  }
>>  
>>  /**
>> @@ -937,16 +1087,7 @@ err_out:
>>   */
>>  static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>>  {
>> -	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
>> -	if (IS_ERR(ppgtt->base.scratch_pt))
>> -		return PTR_ERR(ppgtt->base.scratch_pt);
>> -
>> -	ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
>> -	if (IS_ERR(ppgtt->base.scratch_pd))
>> -		return PTR_ERR(ppgtt->base.scratch_pd);
>> -
>> -	gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
>> -	gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
>> +	int ret;
>>  
>>  	ppgtt->base.start = 0;
>>  	ppgtt->base.total = 1ULL << 32;
>> @@ -966,6 +1107,10 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>>  
>>  	ppgtt->switch_mm = gen8_mm_switch;
>>  
>> +	ret = setup_scratch(&ppgtt->base);
>> +	if (ret)
>> +		return ret;
>> +
>>  	return 0;
>>  }
>>  
>> @@ -1272,19 +1417,6 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
>>  		kunmap_px(ppgtt, pt_vaddr);
>>  }
>>  
>> -static void gen6_initialize_pt(struct i915_address_space *vm,
>> -			       struct i915_page_table *pt)
>> -{
>> -	gen6_pte_t scratch_pte;
>> -
>> -	WARN_ON(px_dma(vm->scratch_page) == 0);
>> -
>> -	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
>> -				     I915_CACHE_LLC, true, 0);
>> -
>> -	fill32_px(vm->dev, pt, scratch_pte);
>> -}
>> -
>>  static int gen6_alloc_va_range(struct i915_address_space *vm,
>>  			       uint64_t start_in, uint64_t length_in)
>>  {
>> @@ -1389,7 +1521,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>  			free_pt(ppgtt->base.dev, pt);
>>  	}
>>  
>> -	free_pt(vm->dev, vm->scratch_pt);
>> +	cleanup_scratch(vm);
>>  }
>>  
>>  static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
>> @@ -1404,11 +1536,10 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
>>  	 * size. We allocate at the top of the GTT to avoid fragmentation.
>>  	 */
>>  	BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
>> -	ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
>> -	if (IS_ERR(ppgtt->base.scratch_pt))
>> -		return PTR_ERR(ppgtt->base.scratch_pt);
>>  
>> -	gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
>> +	ret = setup_scratch(&ppgtt->base);
>> +	if (ret)
>> +		return ret;
>>  
>>  alloc:
>>  	ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
>> @@ -1439,7 +1570,7 @@ alloc:
>>  	return 0;
>>  
>>  err_out:
>> -	free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
>> +	cleanup_scratch(&ppgtt->base);
>>  	return ret;
>>  }
>>  
>> @@ -1513,10 +1644,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>>  
>>  static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>>  {
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -
>>  	ppgtt->base.dev = dev;
>> -	ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page;
>>  
>>  	if (INTEL_INFO(dev)->gen < 8)
>>  		return gen6_ppgtt_init(ppgtt);
>> @@ -2124,45 +2252,6 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
>>  	vm->cleanup(vm);
>>  }
>>  
>> -#define SCRATCH_PAGE_MAGIC 0xffff00ffffff00ffULL
>> -
>> -static int alloc_scratch_page(struct i915_address_space *vm)
>> -{
>> -	struct i915_page_scratch *sp;
>> -	int ret;
>> -
>> -	WARN_ON(vm->scratch_page);
>> -
>> -	sp = kzalloc(sizeof(*sp), GFP_KERNEL);
>> -	if (sp == NULL)
>> -		return -ENOMEM;
>> -
>> -	ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
>> -	if (ret) {
>> -		kfree(sp);
>> -		return ret;
>> -	}
>> -
>> -	fill_px(vm->dev, sp, SCRATCH_PAGE_MAGIC);
>> -	set_pages_uc(px_page(sp), 1);
>> -
>> -	vm->scratch_page = sp;
>> -
>> -	return 0;
>> -}
>> -
>> -static void free_scratch_page(struct i915_address_space *vm)
>> -{
>> -	struct i915_page_scratch *sp = vm->scratch_page;
>> -
>> -	set_pages_wb(px_page(sp), 1);
>> -
>> -	cleanup_px(vm->dev, sp);
>> -	kfree(sp);
>> -
>> -	vm->scratch_page = NULL;
>> -}
>> -
>>  static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
>>  {
>>  	snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
>> @@ -2246,7 +2335,6 @@ static int ggtt_probe_common(struct drm_device *dev,
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	phys_addr_t gtt_phys_addr;
>> -	int ret;
>>  
>>  	/* For Modern GENs the PTEs and register space are split in the BAR */
>>  	gtt_phys_addr = pci_resource_start(dev->pdev, 0) +
>> @@ -2268,14 +2356,7 @@ static int ggtt_probe_common(struct drm_device *dev,
>>  		return -ENOMEM;
>>  	}
>>  
>> -	ret = alloc_scratch_page(&dev_priv->gtt.base);
>> -	if (ret) {
>> -		DRM_ERROR("Scratch setup failed\n");
>> -		/* iounmap will also get called at remove, but meh */
>> -		iounmap(dev_priv->gtt.gsm);
>> -	}
>> -
>> -	return ret;
>> +	return setup_scratch(&dev_priv->gtt.base);
>>  }
>>  
>>  /* The GGTT and PPGTT need a private PPAT setup in order to handle cacheability
>> @@ -2447,7 +2528,7 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
>>  	struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base);
>>  
>>  	iounmap(gtt->gsm);
>> -	free_scratch_page(vm);
>> +	cleanup_scratch(vm);
>>  }
>>  
>>  static int i915_gmch_probe(struct drm_device *dev,
>> -- 
>> 1.9.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 16/18] drm/i915/gtt: One instance of scratch page table/directory

[Daniel Vetter]

On Fri, Jun 26, 2015 at 03:05:29PM +0300, Mika Kuoppala wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> > On Thu, Jun 25, 2015 at 06:35:18PM +0300, Mika Kuoppala wrote:
> >> +static int setup_scratch(struct i915_address_space *vm)
> >> +{
> >> +	struct i915_address_space *ggtt_vm = &to_i915(vm->dev)->gtt.base;
> >> +
> >> +	if (i915_is_ggtt(vm))
> >> +		return setup_scratch_ggtt(vm);
> >> +
> >> +	vm->scratch_page = ggtt_vm->scratch_page;
> >> +	vm->scratch_pt = ggtt_vm->scratch_pt;
> >> +	vm->scratch_pd = ggtt_vm->scratch_pd;
> >> +
> >> +	return 0;
> >> +}
> >
> > The point of a ppgtt is full isolation, sharing the scratch page destroys
> > that. Hence nack. If you want a bit of polish, renaming initialize_pd/pt
> > to initialize_scratch_pd/pt would make sense though I think.
> > -Daniel
> 
> We already have a shared scratch. This just makes the upper layer
> structures also shared as there is no point of having identical
> scratch pt and scratch pd pointing to the same scratch.
> 
> Do we want per ppgtt scratch? I have patches for that also.

Summary of our irc discussion: Yes I think we want a per-ppgtt scratch,
and I even thought we once had such a thing. But didn't digg around in
history tbh.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/18] drm/i915/gtt: Fill scratch page

[Dave Gordon]

On 25/06/15 18:51, Chris Wilson wrote:
> On Thu, Jun 25, 2015 at 06:35:14PM +0300, Mika Kuoppala wrote:
>> During review of dynamic page tables series, I was able
>> to hit a lite restore bug with execlists. I assume that
>> due to incorrect pd, the batch run out of legit address space
>> and into the scratch page area. The ACTHD was increasing
>> due to scratch being all zeroes (MI_NOOPs). And as gen8
>> address space is quite large, the hangcheck happily waited
>> for a long long time, keeping the process effectively stuck.
>>
>> According to Chris Wilson any modern gpu will grind to halt
>> if it encounters commands of all ones. This seemed to do the
>> trick and hang was declared promptly when the gpu wandered into
>> the scratch land.
>>
>> v2: Use 0xffff00ff pattern (Chris)
> 
> Thinking about this, could we add a scratch page checker to hangcheck?
> Just check the first/last u64 perhaps? Or random offset_in_page?
> -Chris

I've suggested to Tomas that when running in 32-bit PPGTT mode, if ACTHD
is >4G then it's definitely broken. Doesn't help much once PPGTT space
expands to 48b though.

.Dave.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx