diff mbox

[v2,2/2] drm/i915/gtt: Avoid using addresses in non-canonical form.

Message ID 1442340267-11240-1-git-send-email-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski Sept. 15, 2015, 6:04 p.m. UTC 
According to bspec, some parts of HW expect the addresses to be in
a canonical form where bits [63:48] == [47].
If we're using 32b addressing, we never need to handle such high
addresses, but since we've recently added 48b address space support,
lets satisfy the HW by converting the address prior to
alloc/insert/clear.

v2: Commit msg update,
    move WARN to gen8_alloc_va_range,
    also convert to canonical in insert_entries.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Micha? Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 21 ++++++++++++---------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  5 +++++
 2 files changed, 17 insertions(+), 9 deletions(-)

Comments

Chris Wilson Sept. 15, 2015, 8:03 p.m. UTC | #1 
On Tue, Sep 15, 2015 at 08:04:27PM +0200, Micha? Winiarski wrote:
> According to bspec, some parts of HW expect the addresses to be in
> a canonical form where bits [63:48] == [47].
> If we're using 32b addressing, we never need to handle such high
> addresses, but since we've recently added 48b address space support,
> lets satisfy the HW by converting the address prior to
> alloc/insert/clear.

Ah, that makes sense - it is only the PDE (et al) that you are setting,
right?
 
>  	if (USES_FULL_48BIT_PPGTT(vm->dev))
> -		return gen8_alloc_va_range_4lvl(vm, &ppgtt->pml4, start, length);
> +		return gen8_alloc_va_range_4lvl(vm, &ppgtt->pml4,
> +						gen8_canonical_addr(start),
> +						length);
>  	else
>  		return gen8_alloc_va_range_3lvl(vm, &ppgtt->pdp, start, length);

Whilst we cannot have the high bit set if !48b, if we just
	start = gen8_canonical_addr(start);
and used the same start for both paths here, that feels more organized -
or have I misunderstand where the high bits needs to be set? In which
case please add a comment.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 97c27da5..be66f75 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -766,6 +766,7 @@  static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 	} else {
 		uint64_t templ4, pml4e;
 		struct i915_page_directory_pointer *pdp;
+		start = gen8_canonical_addr(start);
 
 		gen8_for_each_pml4e(pdp, &ppgtt->pml4, start, length, templ4, pml4e) {
 			gen8_ppgtt_clear_pte_range(vm, pdp, start, length,
@@ -835,6 +836,7 @@  static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
 		struct i915_page_directory_pointer *pdp;
 		uint64_t templ4, pml4e;
 		uint64_t length = (uint64_t)pages->orig_nents << PAGE_SHIFT;
+		start = gen8_canonical_addr(start);
 
 		gen8_for_each_pml4e(pdp, &ppgtt->pml4, start, length, templ4, pml4e) {
 			gen8_ppgtt_insert_pte_entries(vm, pdp, &sg_iter,
@@ -1227,14 +1229,6 @@  static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm,
 	uint32_t pdpes = I915_PDPES_PER_PDP(dev);
 	int ret;
 
-	/* Wrap is never okay since we can only represent 48b, and we don't
-	 * actually use the other side of the canonical address space.
-	 */
-	if (WARN_ON(start + length < start))
-		return -ENODEV;
-
-	if (WARN_ON(start + length > vm->total))
-		return -ENODEV;
 
 	ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
 	if (ret)
@@ -1377,8 +1371,17 @@  static int gen8_alloc_va_range(struct i915_address_space *vm,
 	struct i915_hw_ppgtt *ppgtt =
 		container_of(vm, struct i915_hw_ppgtt, base);
 
+	/* Wrap is never okay */
+	if (WARN_ON(start + length < start))
+		return -ENODEV;
+
+	if (WARN_ON(start + length > vm->total))
+		return -ENODEV;
+
 	if (USES_FULL_48BIT_PPGTT(vm->dev))
-		return gen8_alloc_va_range_4lvl(vm, &ppgtt->pml4, start, length);
+		return gen8_alloc_va_range_4lvl(vm, &ppgtt->pml4,
+						gen8_canonical_addr(start),
+						length);
 	else
 		return gen8_alloc_va_range_3lvl(vm, &ppgtt->pdp, start, length);
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 2114d4e..0172590 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -503,6 +503,11 @@  static inline size_t gen8_pte_count(uint64_t address, uint64_t length)
 	return i915_pte_count(address, length, GEN8_PDE_SHIFT);
 }
 
+static inline uint64_t gen8_canonical_addr(uint64_t address)
+{
+	return ((int64_t)address << 16) >> 16;
+}
+
 static inline dma_addr_t
 i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n)
 {