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

Michał Winiarski
2015-09-16T09:50:56Z

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.
v3: Also convert for !48b (causes no harm, improves code stucture),
update wrap comment.

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 | 20 ++++++++++++--------
drivers/gpu/drm/i915/i915_gem_gtt.h | 5 +++++
2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 01f3521..feb499d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -759,6 +759,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
container_of(vm, struct i915_hw_ppgtt, base);
gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
I915_CACHE_LLC, use_scratch);
+ start = gen8_canonical_addr(start);

if (!USES_FULL_48BIT_PPGTT(vm->dev)) {
gen8_ppgtt_clear_pte_range(vm, &ppgtt->pdp, start, length,
@@ -827,6 +828,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
struct sg_page_iter sg_iter;

__sg_page_iter_start(&sg_iter, pages->sgl, sg_nents(pages->sgl), 0);
+ start = gen8_canonical_addr(start);

if (!USES_FULL_48BIT_PPGTT(vm->dev)) {
gen8_ppgtt_insert_pte_entries(vm, &ppgtt->pdp, &sg_iter, start,
@@ -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,6 +1371,16 @@ 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 not okay since we're dealing with addresses that have not
+ * been converted to canonical form yet */
+ if (WARN_ON(start + length < start))
+ return -ENODEV;
+
+ if (WARN_ON(start + length > vm->total))
+ return -ENODEV;
+
+ start = gen8_canonical_addr(start);
+
if (USES_FULL_48BIT_PPGTT(vm->dev))
return gen8_alloc_va_range_4lvl(vm, &ppgtt->pml4, start, length);
else
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8275007..9397387 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)
{
--
2.4.3