| Message ID | 1442397050-16268-1-git-send-email-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 16, 2015 at 11:50:50AM +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. > > 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> I didn't even complain about the surplus brackets :) Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On ?ro, 2015-09-16 at 11:50 +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. > > 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> Actually... Please ignore this patch. It's harmless, unfortunately it's also useless, and I need to address this in a different way. Sorry for the noise. -Micha?
On Thu, Sep 17, 2015 at 05:17:24PM +0000, Winiarski, Michal wrote: > On ?ro, 2015-09-16 at 11:50 +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. > > > > 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> > > Actually... Please ignore this patch. > It's harmless, unfortunately it's also useless, and I need to address > this in a different way. Please explain in more detail why this patch is void - it seems to do exactly what it says on the tin? > Sorry for the noise. Explaining why something doesn't work or not is never noise, but a great opportunity to learn something. -Daniel
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) {
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(-)